compare-method lint

src/librustc/infer/error_reporting.rs

@@ -646,13 +646,14 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
         };
 
         if let SubregionOrigin::CompareImplMethodObligation {
-            span, item_name, impl_item_def_id, trait_item_def_id
+            span, item_name, impl_item_def_id, trait_item_def_id, lint_id
         } = origin {
             self.report_extra_impl_obligation(span,
                                               item_name,
                                               impl_item_def_id,
                                               trait_item_def_id,
-                                              &format!("`{}: {}`", bound_kind, sub))
+                                              &format!("`{}: {}`", bound_kind, sub),
+                                              lint_id)
                 .emit();
             return;
         }
@@ -977,12 +978,14 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
             infer::CompareImplMethodObligation { span,
                                                  item_name,
                                                  impl_item_def_id,
-                                                 trait_item_def_id } => {
+                                                 trait_item_def_id,
+                                                 lint_id } => {
                 self.report_extra_impl_obligation(span,
                                                   item_name,
                                                   impl_item_def_id,
                                                   trait_item_def_id,
-                                                  &format!("`{}: {}`", sup, sub))
+                                                  &format!("`{}: {}`", sup, sub),
+                                                  lint_id)
             }
         }
     }

src/librustc/infer/mod.rs

@@ -368,6 +368,10 @@ pub enum SubregionOrigin<'tcx> {
         item_name: ast::Name,
         impl_item_def_id: DefId,
         trait_item_def_id: DefId,
+
+        // this is `Some(_)` if this error arises from the bug fix for
+        // #18937. This is a temporary measure.
+        lint_id: Option<ast::NodeId>,
     },
 }
 
@@ -1816,12 +1820,14 @@ impl<'tcx> SubregionOrigin<'tcx> {
 
             traits::ObligationCauseCode::CompareImplMethodObligation { item_name,
                                                                        impl_item_def_id,
-                                                                       trait_item_def_id } =>
+                                                                       trait_item_def_id,
+                                                                       lint_id } =>
                 SubregionOrigin::CompareImplMethodObligation {
                     span: cause.span,
                     item_name: item_name,
                     impl_item_def_id: impl_item_def_id,
                     trait_item_def_id: trait_item_def_id,
+                    lint_id: lint_id,
                 },
 
             _ => default(),

src/librustc/lint/builtin.rs

@@ -198,6 +198,12 @@ declare_lint! {
     "patterns in functions without body were erroneously allowed"
 }
 
+declare_lint! {
+    pub EXTRA_REQUIREMENT_IN_IMPL,
+    Warn,
+    "detects extra requirements in impls that were erroneously allowed"
+}
+
 /// Does nothing as a lint pass, but registers some `Lint`s
 /// which are used by other parts of the compiler.
 #[derive(Copy, Clone)]
@@ -235,7 +241,8 @@ impl LintPass for HardwiredLints {
             HR_LIFETIME_IN_ASSOC_TYPE,
             LIFETIME_UNDERSCORE,
             SAFE_EXTERN_STATICS,
-            PATTERNS_IN_FNS_WITHOUT_BODY
+            PATTERNS_IN_FNS_WITHOUT_BODY,
+            EXTRA_REQUIREMENT_IN_IMPL
         )
     }
 }

src/librustc/lint/context.rs

@@ -42,7 +42,7 @@ use std::fmt;
 use syntax::attr;
 use syntax::parse::token::InternedString;
 use syntax::ast;
-use syntax_pos::Span;
+use syntax_pos::{MultiSpan, Span};
 use errors::{self, Diagnostic, DiagnosticBuilder};
 use hir;
 use hir::intravisit as hir_visit;
@@ -107,9 +107,12 @@ impl fmt::Debug for EarlyLint {
 }
 
 impl EarlyLint {
-    pub fn new(id: LintId, span: Span, msg: String) -> Self {
+    pub fn new<M: EarlyLintMessage>(id: LintId, span: Span, msg: M) -> Self {
-        let mut diagnostic = Diagnostic::new(errors::Level::Warning, &msg);
+        let diagnostic = msg.into_diagnostic(span);
-        diagnostic.set_span(span);
+        EarlyLint { id: id, span: span, diagnostic: diagnostic }
+    }
+
+    pub fn with_diagnostic(id: LintId, span: Span, diagnostic: Diagnostic) -> Self {
         EarlyLint { id: id, span: span, diagnostic: diagnostic }
     }
 
@@ -120,7 +123,23 @@ impl EarlyLint {
     }
 }
 
+pub trait EarlyLintMessage {
+    fn into_diagnostic(self, span: Span) -> Diagnostic;
+}
 
+impl EarlyLintMessage for String {
+    fn into_diagnostic(self, span: Span) -> Diagnostic {
+        let mut diagnostic = Diagnostic::new(errors::Level::Warning, &self);
+        diagnostic.set_span(span);
+        diagnostic
+    }
+}
+
+impl EarlyLintMessage for Diagnostic {
+    fn into_diagnostic(self, _span: Span) -> Diagnostic {
+        self
+    }
+}
 
 /// Extra information for a future incompatibility lint. See the call
 /// to `register_future_incompatible` in `librustc_lint/lib.rs` for
@@ -439,13 +458,15 @@ pub fn raw_emit_lint(sess: &Session,
     raw_struct_lint(sess, lints, lint, lvlsrc, span, msg).emit();
 }
 
-pub fn raw_struct_lint<'a>(sess: &'a Session,
+pub fn raw_struct_lint<'a, S>(sess: &'a Session,
-                           lints: &LintStore,
+                              lints: &LintStore,
-                           lint: &'static Lint,
+                              lint: &'static Lint,
-                           lvlsrc: LevelSource,
+                              lvlsrc: LevelSource,
-                           span: Option<Span>,
+                              span: Option<S>,
-                           msg: &str)
+                              msg: &str)
-                           -> DiagnosticBuilder<'a> {
+                              -> DiagnosticBuilder<'a>
+    where S: Into<MultiSpan>
+{
     let (mut level, source) = lvlsrc;
     if level == Allow {
         return sess.diagnostic().struct_dummy();

src/librustc/lint/mod.rs

@@ -41,7 +41,7 @@ use hir;
 
 pub use lint::context::{LateContext, EarlyContext, LintContext, LintStore,
                         raw_emit_lint, check_crate, check_ast_crate, gather_attrs,
-                        raw_struct_lint, FutureIncompatibleInfo, EarlyLint};
+                        raw_struct_lint, FutureIncompatibleInfo, EarlyLint, EarlyLintMessage};
 
 /// Specification of a single lint.
 #[derive(Copy, Clone, Debug)]

src/librustc/session/mod.rs

@@ -258,11 +258,11 @@ impl Session {
     pub fn unimpl(&self, msg: &str) -> ! {
         self.diagnostic().unimpl(msg)
     }
-    pub fn add_lint(&self,
+    pub fn add_lint<M: lint::EarlyLintMessage>(&self,
-                    lint: &'static lint::Lint,
+                                               lint: &'static lint::Lint,
-                    id: ast::NodeId,
+                                               id: ast::NodeId,
-                    sp: Span,
+                                               sp: Span,
-                    msg: String) {
+                                               msg: M) {
         let lint_id = lint::LintId::of(lint);
         let mut lints = self.lints.borrow_mut();
         let early_lint = lint::EarlyLint::new(lint_id, sp, msg);

src/librustc/traits/error_reporting.rs

@@ -27,6 +27,7 @@ use super::{
 use fmt_macros::{Parser, Piece, Position};
 use hir::def_id::DefId;
 use infer::{self, InferCtxt, TypeOrigin};
+use rustc::lint::builtin::EXTRA_REQUIREMENT_IN_IMPL;
 use ty::{self, AdtKind, ToPredicate, ToPolyTraitRef, Ty, TyCtxt, TypeFoldable};
 use ty::error::ExpectedFound;
 use ty::fast_reject;
@@ -423,9 +424,15 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
                                         item_name: ast::Name,
                                         _impl_item_def_id: DefId,
                                         trait_item_def_id: DefId,
-                                        requirement: &fmt::Display)
+                                        requirement: &fmt::Display,
+                                        lint_id: Option<ast::NodeId>) // (*)
                                         -> DiagnosticBuilder<'tcx>
     {
+        // (*) This parameter is temporary and used only for phasing
+        // in the bug fix to #18937. If it is `Some`, it has a kind of
+        // weird effect -- the diagnostic is reported as a lint, and
+        // the builder which is returned is marked as canceled.
+
         let mut err =
             struct_span_err!(self.tcx.sess,
                              error_span,
@@ -441,6 +448,12 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
             error_span,
             &format!("impl has extra requirement {}", requirement));
 
+        if let Some(node_id) = lint_id {
+            let diagnostic = (*err).clone();
+            self.tcx.sess.add_lint(EXTRA_REQUIREMENT_IN_IMPL, node_id, error_span, diagnostic);
+            err.cancel();
+        }
+
         err
     }
 
@@ -452,14 +465,15 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
         let mut err = match *error {
             SelectionError::Unimplemented => {
                 if let ObligationCauseCode::CompareImplMethodObligation {
-                    item_name, impl_item_def_id, trait_item_def_id
+                    item_name, impl_item_def_id, trait_item_def_id, lint_id
                 } = obligation.cause.code {
                     self.report_extra_impl_obligation(
                         span,
                         item_name,
                         impl_item_def_id,
                         trait_item_def_id,
-                        &format!("`{}`", obligation.predicate))
+                        &format!("`{}`", obligation.predicate),
+                        lint_id)
                         .emit();
                     return;
                 } else {

src/librustc/traits/mod.rs

@@ -138,10 +138,12 @@ pub enum ObligationCauseCode<'tcx> {
 
     ImplDerivedObligation(DerivedObligationCause<'tcx>),
 
+    // error derived when matching traits/impls; see ObligationCause for more details
     CompareImplMethodObligation {
         item_name: ast::Name,
         impl_item_def_id: DefId,
-        trait_item_def_id: DefId
+        trait_item_def_id: DefId,
+        lint_id: Option<ast::NodeId>,
     },
 }
 

src/librustc/traits/structural_impls.rs

@@ -197,11 +197,13 @@ impl<'a, 'tcx> Lift<'tcx> for traits::ObligationCauseCode<'a> {
             }
             super::CompareImplMethodObligation { item_name,
                                                  impl_item_def_id,
-                                                 trait_item_def_id } => {
+                                                 trait_item_def_id,
+                                                 lint_id } => {
                 Some(super::CompareImplMethodObligation {
                     item_name: item_name,
                     impl_item_def_id: impl_item_def_id,
                     trait_item_def_id: trait_item_def_id,
+                    lint_id: lint_id,
                 })
             }
         }

src/librustc_errors/diagnostic.rs

@@ -6,7 +6,7 @@ use std::fmt;
 use syntax_pos::{MultiSpan, Span};
 
 #[must_use]
-#[derive(Clone)]
+#[derive(Clone, Debug)]
 pub struct Diagnostic {
     pub level: Level,
     pub message: String,
@@ -16,7 +16,7 @@ pub struct Diagnostic {
 }
 
 /// For example a note attached to an error.
-#[derive(Clone)]
+#[derive(Clone, Debug)]
 pub struct SubDiagnostic {
     pub level: Level,
     pub message: String,
@@ -190,9 +190,3 @@ impl Diagnostic {
         self.children.push(sub);
     }
 }
-
-impl fmt::Debug for Diagnostic {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        self.message.fmt(f)
-    }
-}

src/librustc_errors/diagnostic_builder.rs

@@ -70,6 +70,21 @@ impl<'a> DiagnosticBuilder<'a> {
             return;
         }
 
+        match self.level {
+            Level::Bug |
+            Level::Fatal |
+            Level::PhaseFatal |
+            Level::Error => {
+                self.handler.bump_err_count();
+            }
+
+            Level::Warning |
+            Level::Note |
+            Level::Help |
+            Level::Cancelled => {
+            }
+        }
+
         self.handler.emitter.borrow_mut().emit(&self);
         self.cancel();
         self.handler.panic_if_treat_err_as_bug();
@@ -140,6 +155,13 @@ impl<'a> DiagnosticBuilder<'a> {
             diagnostic: Diagnostic::new_with_code(level, code, message)
         }
     }
+
+    pub fn into_diagnostic(mut self) -> Diagnostic {
+        // annoyingly, the Drop impl means we can't actually move
+        let result = self.diagnostic.clone();
+        self.cancel();
+        result
+    }
 }
 
 impl<'a> Debug for DiagnosticBuilder<'a> {

src/librustc_errors/lib.rs

@@ -57,7 +57,7 @@ mod lock;
 use syntax_pos::{BytePos, Loc, FileLinesResult, FileName, MultiSpan, Span, NO_EXPANSION};
 use syntax_pos::MacroBacktrace;
 
-#[derive(Clone)]
+#[derive(Clone, Debug)]
 pub enum RenderSpan {
     /// A FullSpan renders with both with an initial line for the
     /// message, prefixed by file:linenum, followed by a summary of
@@ -71,7 +71,7 @@ pub enum RenderSpan {
     Suggestion(CodeSuggestion),
 }
 
-#[derive(Clone)]
+#[derive(Clone, Debug)]
 pub struct CodeSuggestion {
     pub msp: MultiSpan,
     pub substitutes: Vec<String>,
@@ -293,7 +293,6 @@ impl Handler {
                                                    sp: S,
                                                    msg: &str)
                                                    -> DiagnosticBuilder<'a> {
-        self.bump_err_count();
         let mut result = DiagnosticBuilder::new(self, Level::Error, msg);
         result.set_span(sp);
         result
@@ -303,21 +302,18 @@ impl Handler {
                                                              msg: &str,
                                                              code: &str)
                                                              -> DiagnosticBuilder<'a> {
-        self.bump_err_count();
         let mut result = DiagnosticBuilder::new(self, Level::Error, msg);
         result.set_span(sp);
         result.code(code.to_owned());
         result
     }
     pub fn struct_err<'a>(&'a self, msg: &str) -> DiagnosticBuilder<'a> {
-        self.bump_err_count();
         DiagnosticBuilder::new(self, Level::Error, msg)
     }
     pub fn struct_span_fatal<'a, S: Into<MultiSpan>>(&'a self,
                                                      sp: S,
                                                      msg: &str)
                                                      -> DiagnosticBuilder<'a> {
-        self.bump_err_count();
         let mut result = DiagnosticBuilder::new(self, Level::Fatal, msg);
         result.set_span(sp);
         result
@@ -327,24 +323,16 @@ impl Handler {
                                                                msg: &str,
                                                                code: &str)
                                                                -> DiagnosticBuilder<'a> {
-        self.bump_err_count();
         let mut result = DiagnosticBuilder::new(self, Level::Fatal, msg);
         result.set_span(sp);
         result.code(code.to_owned());
         result
     }
     pub fn struct_fatal<'a>(&'a self, msg: &str) -> DiagnosticBuilder<'a> {
-        self.bump_err_count();
         DiagnosticBuilder::new(self, Level::Fatal, msg)
     }
 
     pub fn cancel(&self, err: &mut DiagnosticBuilder) {
-        if err.level == Level::Error || err.level == Level::Fatal {
-            self.err_count.set(self.err_count
-                .get()
-                .checked_sub(1)
-                .expect("cancelled an error but err_count is 0"));
-        }
         err.cancel();
     }
 
@@ -356,7 +344,6 @@ impl Handler {
 
     pub fn span_fatal<S: Into<MultiSpan>>(&self, sp: S, msg: &str) -> FatalError {
         self.emit(&sp.into(), msg, Fatal);
-        self.bump_err_count();
         self.panic_if_treat_err_as_bug();
         return FatalError;
     }
@@ -366,13 +353,11 @@ impl Handler {
                                                     code: &str)
                                                     -> FatalError {
         self.emit_with_code(&sp.into(), msg, code, Fatal);
-        self.bump_err_count();
         self.panic_if_treat_err_as_bug();
         return FatalError;
     }
     pub fn span_err<S: Into<MultiSpan>>(&self, sp: S, msg: &str) {
         self.emit(&sp.into(), msg, Error);
-        self.bump_err_count();
         self.panic_if_treat_err_as_bug();
     }
     pub fn mut_span_err<'a, S: Into<MultiSpan>>(&'a self,
@@ -381,12 +366,10 @@ impl Handler {
                                                 -> DiagnosticBuilder<'a> {
         let mut result = DiagnosticBuilder::new(self, Level::Error, msg);
         result.set_span(sp);
-        self.bump_err_count();
         result
     }
     pub fn span_err_with_code<S: Into<MultiSpan>>(&self, sp: S, msg: &str, code: &str) {
         self.emit_with_code(&sp.into(), msg, code, Error);
-        self.bump_err_count();
         self.panic_if_treat_err_as_bug();
     }
     pub fn span_warn<S: Into<MultiSpan>>(&self, sp: S, msg: &str) {
@@ -405,7 +388,6 @@ impl Handler {
     }
     pub fn span_bug_no_panic<S: Into<MultiSpan>>(&self, sp: S, msg: &str) {
         self.emit(&sp.into(), msg, Bug);
-        self.bump_err_count();
     }
     pub fn span_note_without_error<S: Into<MultiSpan>>(&self, sp: S, msg: &str) {
         self.emit(&sp.into(), msg, Note);
@@ -419,7 +401,6 @@ impl Handler {
         }
         let mut db = DiagnosticBuilder::new(self, Fatal, msg);
         db.emit();
-        self.bump_err_count();
         FatalError
     }
     pub fn err(&self, msg: &str) {
@@ -428,7 +409,6 @@ impl Handler {
         }
         let mut db = DiagnosticBuilder::new(self, Error, msg);
         db.emit();
-        self.bump_err_count();
     }
     pub fn warn(&self, msg: &str) {
         let mut db = DiagnosticBuilder::new(self, Warning, msg);

src/librustc_lint/lib.rs

@@ -229,6 +229,10 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) {
             id: LintId::of(PATTERNS_IN_FNS_WITHOUT_BODY),
             reference: "issue #35203 <https://github.com/rust-lang/rust/issues/35203>",
         },
+        FutureIncompatibleInfo {
+            id: LintId::of(EXTRA_REQUIREMENT_IN_IMPL),
+            reference: "issue #18937 <https://github.com/rust-lang/rust/issues/18937>",
+        },
         ]);
 
     // Register renamed and removed lints

src/librustc_typeck/check/compare_method.rs

@@ -367,6 +367,7 @@ pub fn compare_impl_method<'a, 'tcx>(ccx: &CrateCtxt<'a, 'tcx>,
                     item_name: impl_m.name,
                     impl_item_def_id: impl_m.def_id,
                     trait_item_def_id: trait_m.def_id,
+                    lint_id: Some(impl_m_body_id),
                 },
             };
 

src/test/compile-fail/issue-18937.rs

@@ -10,6 +10,8 @@
 
 // Regression test for #18937.
 
+#![deny(extra_requirement_in_impl)]
+
 use std::fmt;
 
 #[derive(Debug)]

src/test/ui/compare-method/proj-outlives-region.rs

@@ -9,6 +9,7 @@
 // except according to those terms.
 
 #![allow(dead_code)]
+#![deny(extra_requirement_in_impl)]
 
 // Test that we elaborate `Type: 'region` constraints and infer various important things.
 

src/test/ui/compare-method/proj-outlives-region.stderr

@@ -1,11 +1,19 @@
 error[E0276]: impl has stricter requirements than trait
-  --> $DIR/proj-outlives-region.rs:21:5
+  --> $DIR/proj-outlives-region.rs:22:5
    |
-16 |     fn foo() where T: 'a;
+17 |     fn foo() where T: 'a;
    |     --------------------- definition of `foo` from trait
 ...
-21 |     fn foo() where U: 'a { } //~ ERROR E0276
+22 |     fn foo() where U: 'a { } //~ ERROR E0276
    |     ^^^^^^^^^^^^^^^^^^^^^^^^ impl has extra requirement `U: 'a`
+   |
+   = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
+   = note: for more information, see issue #18937 <https://github.com/rust-lang/rust/issues/18937>
+note: lint level defined here
+  --> $DIR/proj-outlives-region.rs:12:9
+   |
+12 | #![deny(extra_requirement_in_impl)]
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^
 
 error: aborting due to previous error
 

src/test/ui/compare-method/region-unrelated.rs

@@ -9,6 +9,7 @@
 // except according to those terms.
 
 #![allow(dead_code)]
+#![deny(extra_requirement_in_impl)]
 
 // Test that we elaborate `Type: 'region` constraints and infer various important things.
 

src/test/ui/compare-method/region-unrelated.stderr

@@ -1,11 +1,19 @@
 error[E0276]: impl has stricter requirements than trait
-  --> $DIR/region-unrelated.rs:21:5
+  --> $DIR/region-unrelated.rs:22:5
    |
-16 |     fn foo() where T: 'a;
+17 |     fn foo() where T: 'a;
    |     --------------------- definition of `foo` from trait
 ...
-21 |     fn foo() where V: 'a { }
+22 |     fn foo() where V: 'a { }
    |     ^^^^^^^^^^^^^^^^^^^^^^^^ impl has extra requirement `V: 'a`
+   |
+   = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
+   = note: for more information, see issue #18937 <https://github.com/rust-lang/rust/issues/18937>
+note: lint level defined here
+  --> $DIR/region-unrelated.rs:12:9
+   |
+12 | #![deny(extra_requirement_in_impl)]
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^
 
 error: aborting due to previous error
 

src/test/ui/compare-method/region.rs

@@ -9,6 +9,7 @@
 // except according to those terms.
 
 #![allow(dead_code)]
+#![deny(extra_requirement_in_impl)]
 
 // Test that we elaborate `Type: 'region` constraints and infer various important things.
 

src/test/ui/compare-method/region.stderr

@@ -1,11 +1,19 @@
 error[E0276]: impl has stricter requirements than trait
-  --> $DIR/region.rs:21:5
+  --> $DIR/region.rs:22:5
    |
-16 |     fn foo();
+17 |     fn foo();
    |     --------- definition of `foo` from trait
 ...
-21 |     fn foo() where 'a: 'b { }
+22 |     fn foo() where 'a: 'b { }
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^ impl has extra requirement `'a: 'b`
+   |
+   = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
+   = note: for more information, see issue #18937 <https://github.com/rust-lang/rust/issues/18937>
+note: lint level defined here
+  --> $DIR/region.rs:12:9
+   |
+12 | #![deny(extra_requirement_in_impl)]
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^
 
 error: aborting due to previous error