detect extra region requirements in impls

src/librustc/infer/error_reporting.rs

@@ -245,6 +245,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
         debug!("report_region_errors: {} errors after preprocessing", errors.len());
 
         for error in errors {
+            debug!("report_region_errors: error = {:?}", error);
             match error.clone() {
                 ConcreteFailure(origin, sub, sup) => {
                     self.report_concrete_failure(origin, sub, sup).emit();
@@ -299,44 +300,64 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
         let mut bound_failures = Vec::new();
 
         for error in errors {
+            // Check whether we can process this error into some other
+            // form; if not, fall through.
             match *error {
                 ConcreteFailure(ref origin, sub, sup) => {
                     debug!("processing ConcreteFailure");
-                    match free_regions_from_same_fn(self.tcx, sub, sup) {
-                        Some(ref same_frs) => {
-                            origins.push(
-                                ProcessedErrorOrigin::ConcreteFailure(
-                                    origin.clone(),
-                                    sub,
-                                    sup));
-                            append_to_same_regions(&mut same_regions, same_frs);
-                        }
-                        _ => {
-                            other_errors.push(error.clone());
-                        }
+                    if let SubregionOrigin::CompareImplMethodObligation { .. } = *origin {
+                        // When comparing an impl method against a
+                        // trait method, it is not helpful to suggest
+                        // changes to the impl method.  This is
+                        // because the impl method signature is being
+                        // checked using the trait's environment, so
+                        // usually the changes we suggest would
+                        // actually have to be applied to the *trait*
+                        // method (and it's not clear that the trait
+                        // method is even under the user's control).
+                    } else if let Some(same_frs) = free_regions_from_same_fn(self.tcx, sub, sup) {
+                        origins.push(
+                            ProcessedErrorOrigin::ConcreteFailure(
+                                origin.clone(),
+                                sub,
+                                sup));
+                        append_to_same_regions(&mut same_regions, &same_frs);
+                        continue;
                     }
                 }
-                SubSupConflict(ref var_origin, _, sub_r, _, sup_r) => {
-                    debug!("processing SubSupConflict sub: {:?} sup: {:?}", sub_r, sup_r);
-                    match free_regions_from_same_fn(self.tcx, sub_r, sup_r) {
-                        Some(ref same_frs) => {
-                            origins.push(
-                                ProcessedErrorOrigin::VariableFailure(
-                                    var_origin.clone()));
-                            append_to_same_regions(&mut same_regions, same_frs);
+                SubSupConflict(ref var_origin, ref sub_origin, sub, ref sup_origin, sup) => {
+                    debug!("processing SubSupConflict sub: {:?} sup: {:?}", sub, sup);
+                    match (sub_origin, sup_origin) {
+                        (&SubregionOrigin::CompareImplMethodObligation { .. }, _) => {
+                            // As above, when comparing an impl method
+                            // against a trait method, it is not helpful
+                            // to suggest changes to the impl method.
                         }
-                        None => {
-                            other_errors.push(error.clone());
+                        (_, &SubregionOrigin::CompareImplMethodObligation { .. }) => {
+                            // See above.
+                        }
+                        _ => {
+                            if let Some(same_frs) = free_regions_from_same_fn(self.tcx, sub, sup) {
+                                origins.push(
+                                    ProcessedErrorOrigin::VariableFailure(
+                                        var_origin.clone()));
+                                append_to_same_regions(&mut same_regions, &same_frs);
+                                continue;
+                            }
                         }
                     }
                 }
                 GenericBoundFailure(ref origin, ref kind, region) => {
                     bound_failures.push((origin.clone(), kind.clone(), region));
+                    continue;
                 }
                 ProcessedErrors(..) => {
                     bug!("should not encounter a `ProcessedErrors` yet: {:?}", error)
                 }
             }
+
+            // No changes to this error.
+            other_errors.push(error.clone());
         }
 
         // ok, let's pull together the errors, sorted in an order that
@@ -630,6 +651,19 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
                 format!("the associated type `{}`", p),
         };
 
+        if let SubregionOrigin::CompareImplMethodObligation {
+            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),
+                                              lint_id)
+                .emit();
+            return;
+        }
+
         let mut err = match *sub {
             ty::ReFree(ty::FreeRegion {bound_region: ty::BrNamed(..), ..}) => {
                 // Does the required lifetime have a nice name we can print?
@@ -947,6 +981,18 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
                     "");
                 err
             }
+            infer::CompareImplMethodObligation { span,
+                                                 item_name,
+                                                 impl_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),
+                                                  lint_id)
+            }
         }
     }
 
@@ -1792,6 +1838,11 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
                     "...so that references are valid when the destructor \
                      runs");
             }
+            infer::CompareImplMethodObligation { span, .. } => {
+                err.span_note(
+                    span,
+                    "...so that the definition in impl matches the definition from the trait");
+            }
         }
     }
 }

src/librustc/infer/mod.rs

@@ -360,6 +360,19 @@ pub enum SubregionOrigin<'tcx> {
 
     // Region constraint arriving from destructor safety
     SafeDestructor(Span),
+
+    // Comparing the signature and requirements of an impl method against
+    // the containing trait.
+    CompareImplMethodObligation {
+        span: Span,
+        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>,
+    },
 }
 
 /// Places that type/region parameters can appear.
@@ -1152,16 +1165,18 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
     }
 
     pub fn region_outlives_predicate(&self,
-                                     span: Span,
+                                     cause: &traits::ObligationCause<'tcx>,
                                      predicate: &ty::PolyRegionOutlivesPredicate<'tcx>)
         -> UnitResult<'tcx>
     {
         self.commit_if_ok(|snapshot| {
             let (ty::OutlivesPredicate(r_a, r_b), skol_map) =
                 self.skolemize_late_bound_regions(predicate, snapshot);
-            let origin = RelateRegionParamBound(span);
+            let origin =
+                SubregionOrigin::from_obligation_cause(cause,
+                                                       || RelateRegionParamBound(cause.span));
             self.sub_regions(origin, r_b, r_a); // `b : a` ==> `a <= b`
-            self.leak_check(false, span, &skol_map, snapshot)?;
+            self.leak_check(false, cause.span, &skol_map, snapshot)?;
             Ok(self.pop_skolemized(skol_map, snapshot))
         })
     }
@@ -1792,6 +1807,32 @@ impl<'tcx> SubregionOrigin<'tcx> {
             AddrOf(a) => a,
             AutoBorrow(a) => a,
             SafeDestructor(a) => a,
+            CompareImplMethodObligation { span, .. } => span,
+        }
+    }
+
+    pub fn from_obligation_cause<F>(cause: &traits::ObligationCause<'tcx>,
+                                    default: F)
+                                    -> Self
+        where F: FnOnce() -> Self
+    {
+        match cause.code {
+            traits::ObligationCauseCode::ReferenceOutlivesReferent(ref_type) =>
+                SubregionOrigin::ReferenceOutlivesReferent(ref_type, cause.span),
+
+            traits::ObligationCauseCode::CompareImplMethodObligation { item_name,
+                                                                       impl_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
         )
     }
 }