From f213acf4db81a33308ab2d53b5927108d63a2d7f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Wed, 27 May 2020 20:58:05 -0700 Subject: [PATCH] review comments: change wording and visual output --- .../trait_impl_difference.rs | 63 +++++-------------- .../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 | 14 ++--- 6 files changed, 26 insertions(+), 59 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 8180dd5ef34..1b6e5c2116b 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 @@ -4,16 +4,13 @@ use crate::infer::error_reporting::nice_region_error::NiceRegionError; use crate::infer::lexical_region_resolve::RegionResolutionError; use crate::infer::{Subtype, TyCtxtInferExt, ValuePairs}; use crate::traits::ObligationCauseCode::CompareImplMethodObligation; -use rustc_data_structures::fx::FxIndexSet; use rustc_errors::ErrorReported; use rustc_hir as hir; use rustc_hir::def_id::DefId; use rustc_hir::intravisit::Visitor; -use rustc_hir::ItemKind; use rustc_middle::ty::error::ExpectedFound; -use rustc_middle::ty::fold::TypeFoldable; use rustc_middle::ty::{self, Ty, TyCtxt}; -use rustc_span::Span; +use rustc_span::{MultiSpan, Span}; impl<'a, 'tcx> NiceRegionError<'a, 'tcx> { /// Print the error message for lifetime errors when the `impl` doesn't conform to the `trait`. @@ -63,41 +60,6 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> { .struct_span_err(sp, "`impl` item signature doesn't match `trait` item signature"); err.span_label(sp, &format!("found `{:?}`", found)); 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(FxIndexSet); - impl<'tcx> ty::fold::TypeVisitor<'tcx> for AssocTypeFinder { - fn visit_ty(&mut self, ty: Ty<'tcx>) -> bool { - if let ty::Param(param) = ty.kind { - self.0.insert(param); - } - ty.super_visit_with(self) - } - } - let mut visitor = AssocTypeFinder(FxIndexSet::default()); - trait_fn_sig.output().visit_with(&mut visitor); - if let Some(id) = trait_def_id.as_local().map(|id| tcx.hir().as_local_hir_id(id)) { - let parent_id = tcx.hir().get_parent_item(id); - let trait_item = tcx.hir().expect_item(parent_id); - if let 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, - "this type parameter might not have a lifetime compatible with the \ - `impl`", - ); - } - } - } - } // Get the span of all the used type parameters in the method. let assoc_item = self.tcx().associated_item(trait_def_id); @@ -114,11 +76,12 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> { } _ => {} } - for span in visitor.types { - err.span_label( + let mut type_param_span: MultiSpan = + visitor.types.iter().cloned().collect::>().into(); + for &span in &visitor.types { + type_param_span.push_span_label( span, - "you might want to borrow this type parameter in the trait to make it match the \ - `impl`", + "consider borrowing this type parameter in the trait".to_string(), ); } @@ -132,11 +95,17 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> { // This fallback shouldn't be necessary, but let's keep it in just in case. err.note(&format!("expected `{:?}`\n found `{:?}`", expected, found)); } - 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.span_help( + type_param_span, + "the lifetime requirements from the `impl` do not correspond to the requirements in \ + the `trait`", ); + if visitor.types.is_empty() { + 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 61a8674a231..b93d98ca39f 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 satisfied by the `impl` + = help: the lifetime requirements from the `impl` do not correspond to the requirements in the `trait` = 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 908a81bedb0..149c2aeb958 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 satisfied by the `impl` + = help: the lifetime requirements from the `impl` do not correspond to the requirements in the `trait` = 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 ca610291455..9a0bd827850 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 satisfied by the `impl` + = help: the lifetime requirements from the `impl` do not correspond to the requirements in the `trait` = 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 53f6aae5ff6..060e6954403 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 satisfied by the `impl` + = help: the lifetime requirements from the `impl` do not correspond to the requirements in the `trait` = 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 a7be2d3365c..4942dbe480b 100644 --- a/src/test/ui/traits/trait-param-without-lifetime-constraint.stderr +++ b/src/test/ui/traits/trait-param-without-lifetime-constraint.stderr @@ -1,21 +1,19 @@ error: `impl` item signature doesn't match `trait` item signature --> $DIR/trait-param-without-lifetime-constraint.rs:14:5 | -LL | pub trait HaveRelationship { - | -- this type parameter might not have a lifetime compatible with the `impl` LL | fn get_relation(&self) -> To; - | ----------------------------- - | | | - | | you might want to borrow this type parameter in the trait to make it match the `impl` - | expected `fn(&Article) -> &ProofReader` + | ----------------------------- expected `fn(&Article) -> &ProofReader` ... LL | fn get_relation(&self) -> &ProofReader { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ found `fn(&Article) -> &ProofReader` | = note: expected `fn(&Article) -> &ProofReader` found `fn(&Article) -> &ProofReader` - = 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 +help: the lifetime requirements from the `impl` do not correspond to the requirements in the `trait` + --> $DIR/trait-param-without-lifetime-constraint.rs:10:31 + | +LL | fn get_relation(&self) -> To; + | ^^ consider borrowing this type parameter in the trait error: aborting due to previous error