From 38112321908e824a56943f8e2a461510206e2409 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sun, 22 Dec 2019 17:53:50 -0800 Subject: [PATCH] review comments --- .../trait_impl_difference.rs | 58 ++++++------------- .../mismatched_trait_impl-2.stderr | 2 +- .../mismatched_trait_impl.nll.stderr | 2 +- .../mismatched_trait_impl.stderr | 2 +- ...ime-mismatch-between-trait-and-impl.stderr | 2 +- ...t-param-without-lifetime-constraint.stderr | 4 +- 6 files changed, 24 insertions(+), 46 deletions(-) diff --git a/src/librustc_infer/infer/error_reporting/nice_region_error/trait_impl_difference.rs b/src/librustc_infer/infer/error_reporting/nice_region_error/trait_impl_difference.rs index 5aa55d253aa1..7ad7e8897df3 100644 --- a/src/librustc_infer/infer/error_reporting/nice_region_error/trait_impl_difference.rs +++ b/src/librustc_infer/infer/error_reporting/nice_region_error/trait_impl_difference.rs @@ -63,22 +63,25 @@ 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); 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); @@ -86,8 +89,8 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> { 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, )); } @@ -95,46 +98,21 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> { } } - struct EarlyBoundRegionHighlighter(FxHashSet); - 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(); } } diff --git a/src/test/ui/in-band-lifetimes/mismatched_trait_impl-2.stderr b/src/test/ui/in-band-lifetimes/mismatched_trait_impl-2.stderr index d8a2fa14333e..05413c3a2923 100644 --- a/src/test/ui/in-band-lifetimes/mismatched_trait_impl-2.stderr +++ b/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 diff --git a/src/test/ui/in-band-lifetimes/mismatched_trait_impl.nll.stderr b/src/test/ui/in-band-lifetimes/mismatched_trait_impl.nll.stderr index 88edf10a66f0..c58f78b6719a 100644 --- a/src/test/ui/in-band-lifetimes/mismatched_trait_impl.nll.stderr +++ b/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 diff --git a/src/test/ui/in-band-lifetimes/mismatched_trait_impl.stderr b/src/test/ui/in-band-lifetimes/mismatched_trait_impl.stderr index dcdc6d330c0e..4dbd4ac6a852 100644 --- a/src/test/ui/in-band-lifetimes/mismatched_trait_impl.stderr +++ b/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 diff --git a/src/test/ui/lifetimes/lifetime-mismatch-between-trait-and-impl.stderr b/src/test/ui/lifetimes/lifetime-mismatch-between-trait-and-impl.stderr index fa2ea83deefb..0e56473c5b6a 100644 --- a/src/test/ui/lifetimes/lifetime-mismatch-between-trait-and-impl.stderr +++ b/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 diff --git a/src/test/ui/traits/trait-param-without-lifetime-constraint.stderr b/src/test/ui/traits/trait-param-without-lifetime-constraint.stderr index 912d3a8d28de..724062b1a589 100644 --- a/src/test/ui/traits/trait-param-without-lifetime-constraint.stderr +++ b/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 { - | -- 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