review comments

Author: estebank

src/librustc_infer/infer/error_reporting/nice_region_error/trait_impl_difference.rs

@@ -63,78 +63,56 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
         err.span_label(trait_sp, &format!("expected {:?}", expected));
         let trait_fn_sig = tcx.fn_sig(trait_def_id);
 
+        // Check the `trait`'s method's output to look for type parameters that might have
+        // unconstrained lifetimes. If the method returns a type parameter and the `impl` has a
+        // borrow as the type parameter being implemented, the lifetimes will not match because
+        // a new lifetime is being introduced in the `impl` that is not present in the `trait`.
+        // Because this is confusing as hell the first time you see it, we give a short message
+        // explaining the situation and proposing constraining the type param with a named lifetime
+        // so that the `impl` will have one to tie them together.
         struct AssocTypeFinder(FxHashSet<ty::ParamTy>);
         impl<'tcx> ty::fold::TypeVisitor<'tcx> for AssocTypeFinder {
             fn visit_ty(&mut self, ty: Ty<'tcx>) -> bool {
                 debug!("assoc type finder ty {:?} {:?}", ty, ty.kind);
-                match ty.kind {
-                    ty::Param(param) => {
-                        self.0.insert(param);
-                    }
-                    _ => {}
+                if let ty::Param(param) = ty.kind {
+                    self.0.insert(param);
                 }
                 ty.super_visit_with(self)
             }
         }
         let mut visitor = AssocTypeFinder(FxHashSet::default());
         trait_fn_sig.output().visit_with(&mut visitor);
-
         if let Some(id) = tcx.hir().as_local_hir_id(trait_def_id) {
             let parent_id = tcx.hir().get_parent_item(id);
             let trait_item = tcx.hir().expect_item(parent_id);
             if let hir::ItemKind::Trait(_, _, generics, _, _) = &trait_item.kind {
                 for param_ty in visitor.0 {
                     if let Some(generic) = generics.get_named(param_ty.name) {
                         err.span_label(generic.span, &format!(
-                            "in order for `impl` items to be able to implement the method, this \
-                             type parameter might need a lifetime restriction like `{}: 'a`",
+                            "for `impl` items to implement the method, this type parameter might \
+                             need a lifetime restriction like `{}: 'a`",
                             param_ty.name,
                         ));
                     }
                 }
             }
         }
 
-        struct EarlyBoundRegionHighlighter(FxHashSet<DefId>);
-        impl<'tcx> ty::fold::TypeVisitor<'tcx> for EarlyBoundRegionHighlighter {
-            fn visit_region(&mut self, r: ty::Region<'tcx>) -> bool {
-                match *r {
-                    ty::ReFree(free) => {
-                        self.0.insert(free.scope);
-                    }
-                    ty::ReEarlyBound(bound) => {
-                        self.0.insert(bound.def_id);
-                    }
-                    _ => {}
-                }
-                r.super_visit_with(self)
-            }
-        }
-
-        let mut visitor = EarlyBoundRegionHighlighter(FxHashSet::default());
-        expected.visit_with(&mut visitor);
-
-        let note = !visitor.0.is_empty();
-
-        if let Some((expected, found)) = self
-            .tcx()
+        if let Some((expected, found)) = tcx
             .infer_ctxt()
             .enter(|infcx| infcx.expected_found_str_ty(&ExpectedFound { expected, found }))
         {
+            // Highlighted the differences when showing the "expected/found" note.
             err.note_expected_found(&"", expected, &"", found);
         } else {
             // This fallback shouldn't be necessary, but let's keep it in just in case.
             err.note(&format!("expected `{:?}`\n   found `{:?}`", expected, found));
         }
-        if note {
-            err.note(
-                "the lifetime requirements from the `trait` could not be fulfilled by the `impl`",
-            );
-            err.help(
-                "verify the lifetime relationships in the `trait` and `impl` between the \
-                 `self` argument, the other inputs and its output",
-            );
-        }
+        err.note("the lifetime requirements from the `trait` could not be satisfied by the `impl`");
+        err.help(
+            "verify the lifetime relationships in the `trait` and `impl` between the `self` \
+             argument, the other inputs and its output",
+        );
         err.emit();
     }
 }

src/test/ui/in-band-lifetimes/mismatched_trait_impl-2.stderr

@@ -11,7 +11,7 @@ LL |     fn deref(&self) -> &Self::Target;
    |
    = note: expected `fn(&Struct) -> &(dyn Trait + 'static)`
               found `fn(&Struct) -> &dyn Trait`
-   = note: the lifetime requirements from the `trait` could not be fulfilled by the `impl`
+   = note: the lifetime requirements from the `trait` could not be satisfied by the `impl`
    = help: verify the lifetime relationships in the `trait` and `impl` between the `self` argument, the other inputs and its output
 
 error: aborting due to previous error

src/test/ui/in-band-lifetimes/mismatched_trait_impl.nll.stderr

@@ -9,7 +9,7 @@ LL |     fn foo(&self, x: &u32, y: &'a u32) -> &'a u32 {
    |
    = note: expected `fn(&i32, &'a u32, &u32) -> &'a u32`
               found `fn(&i32, &u32, &u32) -> &u32`
-   = note: the lifetime requirements from the `trait` could not be fulfilled by the `impl`
+   = note: the lifetime requirements from the `trait` could not be satisfied by the `impl`
    = help: verify the lifetime relationships in the `trait` and `impl` between the `self` argument, the other inputs and its output
 
 error: aborting due to previous error

src/test/ui/in-band-lifetimes/mismatched_trait_impl.stderr

@@ -9,7 +9,7 @@ LL |     fn foo(&self, x: &u32, y: &'a u32) -> &'a u32 {
    |
    = note: expected `fn(&i32, &'a u32, &u32) -> &'a u32`
               found `fn(&i32, &u32, &u32) -> &u32`
-   = note: the lifetime requirements from the `trait` could not be fulfilled by the `impl`
+   = note: the lifetime requirements from the `trait` could not be satisfied by the `impl`
    = help: verify the lifetime relationships in the `trait` and `impl` between the `self` argument, the other inputs and its output
 
 error[E0623]: lifetime mismatch

src/test/ui/lifetimes/lifetime-mismatch-between-trait-and-impl.stderr

@@ -9,7 +9,7 @@ LL |     fn foo<'a>(x: &'a i32, y: &'a i32) -> &'a i32 {
    |
    = note: expected `fn(&i32, &'a i32) -> &'a i32`
               found `fn(&i32, &i32) -> &i32`
-   = note: the lifetime requirements from the `trait` could not be fulfilled by the `impl`
+   = note: the lifetime requirements from the `trait` could not be satisfied by the `impl`
    = help: verify the lifetime relationships in the `trait` and `impl` between the `self` argument, the other inputs and its output
 
 error: aborting due to previous error

src/test/ui/traits/trait-param-without-lifetime-constraint.stderr

@@ -2,7 +2,7 @@ error: `impl` item signature doesn't match `trait` item signature
   --> $DIR/trait-param-without-lifetime-constraint.rs:14:5
    |
 LL | pub trait HaveRelationship<To> {
-   |                            -- in order for `impl` items to be able to implement the method, this type parameter might need a lifetime restriction like `To: 'a`
+   |                            -- for `impl` items to implement the method, this type parameter might need a lifetime restriction like `To: 'a`
 LL |     fn get_relation(&self) -> To;
    |     ----------------------------- expected fn(&Article) -> &ProofReader
 ...
@@ -11,7 +11,7 @@ LL |     fn get_relation(&self) -> &ProofReader {
    |
    = note: expected `fn(&Article) -> &ProofReader`
               found `fn(&Article) -> &ProofReader`
-   = note: the lifetime requirements from the `trait` could not be fulfilled by the `impl`
+   = note: the lifetime requirements from the `trait` could not be satisfied by the `impl`
    = help: verify the lifetime relationships in the `trait` and `impl` between the `self` argument, the other inputs and its output
 
 error: aborting due to previous error