From 9f730e92f24ebd21b94112e46a0e44bdf7730e27 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Fri, 3 May 2024 21:42:34 +0000 Subject: [PATCH] Suggest setting lifetime in borrowck error involving types with elided lifetimes ``` error: lifetime may not live long enough --> $DIR/ex3-both-anon-regions-both-are-structs-2.rs:7:5 | LL | fn foo(mut x: Ref, y: Ref) { | ----- - has type `Ref<'_, '1>` | | | has type `Ref<'_, '2>` LL | x.b = y.b; | ^^^^^^^^^ assignment requires that `'1` must outlive `'2` | help: consider introducing a named lifetime parameter | LL | fn foo<'a>(mut x: Ref<'a, 'a>, y: Ref<'a, 'a>) { | ++++ ++++++++ ++++++++ ``` As can be seen above, it currently doesn't try to compare the `ty::Ty` lifetimes that diverged vs the `hir::Ty` to correctly suggest the following ``` help: consider introducing a named lifetime parameter | LL | fn foo<'a>(mut x: Ref<'_, 'a>, y: Ref<'_, 'a>) { | ++++ ++++++++ ++++++++ ``` but I believe this to still be an improvement over the status quo. CC #40990. --- compiler/rustc_infer/src/errors/mod.rs | 105 +++++++++++++----- ...ne-existing-name-if-else-using-impl.stderr | 5 + ...e-existing-name-return-type-is-anon.stderr | 5 + ...turn-one-existing-name-self-is-anon.stderr | 5 + .../ex2b-push-no-existing-names.stderr | 5 + ...oth-anon-regions-both-are-structs-2.stderr | 5 + ...oth-anon-regions-both-are-structs-3.stderr | 5 + ...-both-anon-regions-both-are-structs.stderr | 5 + ...3-both-anon-regions-one-is-struct-2.stderr | 5 + ...3-both-anon-regions-one-is-struct-3.stderr | 5 + ...3-both-anon-regions-one-is-struct-4.stderr | 5 + ...ex3-both-anon-regions-one-is-struct.stderr | 5 + ...f_types_pin_lifetime_mismatch-async.stderr | 5 + ...ry_self_types_pin_lifetime_mismatch.stderr | 5 + 14 files changed, 140 insertions(+), 30 deletions(-) diff --git a/compiler/rustc_infer/src/errors/mod.rs b/compiler/rustc_infer/src/errors/mod.rs index 2acaeac2439..03947239896 100644 --- a/compiler/rustc_infer/src/errors/mod.rs +++ b/compiler/rustc_infer/src/errors/mod.rs @@ -4,6 +4,7 @@ MultiSpan, SubdiagMessageOp, Subdiagnostic, }; use rustc_hir as hir; +use rustc_hir::intravisit::{walk_ty, Visitor}; use rustc_hir::FnRetTy; use rustc_macros::{Diagnostic, Subdiagnostic}; use rustc_middle::ty::print::TraitRefPrintOnlyTraitPath; @@ -355,18 +356,6 @@ fn add_to_diag_with>( _f: &F, ) { let mut mk_suggestion = || { - let ( - hir::Ty { kind: hir::TyKind::Ref(lifetime_sub, _), .. }, - hir::Ty { kind: hir::TyKind::Ref(lifetime_sup, _), .. }, - ) = (self.ty_sub, self.ty_sup) - else { - return false; - }; - - if !lifetime_sub.is_anonymous() || !lifetime_sup.is_anonymous() { - return false; - }; - let Some(anon_reg) = self.tcx.is_suitable_region(self.sub) else { return false; }; @@ -393,21 +382,77 @@ fn add_to_diag_with>( let suggestion_param_name = suggestion_param_name.map(|n| n.to_string()).unwrap_or_else(|| "'a".to_owned()); - debug!(?lifetime_sup.ident.span); - debug!(?lifetime_sub.ident.span); - let make_suggestion = |ident: Ident| { - let sugg = if ident.name == kw::Empty { - format!("{suggestion_param_name}, ") - } else if ident.name == kw::UnderscoreLifetime && ident.span.is_empty() { - format!("{suggestion_param_name} ") - } else { - suggestion_param_name.clone() - }; - (ident.span, sugg) - }; - let mut suggestions = - vec![make_suggestion(lifetime_sub.ident), make_suggestion(lifetime_sup.ident)]; + struct ImplicitLifetimeFinder { + suggestions: Vec<(Span, String)>, + suggestion_param_name: String, + } + impl<'v> Visitor<'v> for ImplicitLifetimeFinder { + fn visit_ty(&mut self, ty: &'v hir::Ty<'v>) { + let make_suggestion = |ident: Ident| { + if ident.name == kw::Empty && ident.span.is_empty() { + format!("{}, ", self.suggestion_param_name) + } else if ident.name == kw::UnderscoreLifetime && ident.span.is_empty() { + format!("{} ", self.suggestion_param_name) + } else { + self.suggestion_param_name.clone() + } + }; + match ty.kind { + hir::TyKind::Path(hir::QPath::Resolved(_, path)) => { + for segment in path.segments { + if let Some(args) = segment.args { + if args.args.iter().all(|arg| { + matches!( + arg, + hir::GenericArg::Lifetime(lifetime) + if lifetime.ident.name == kw::Empty + ) + }) { + self.suggestions.push(( + segment.ident.span.shrink_to_hi(), + format!( + "<{}>", + args.args + .iter() + .map(|_| self.suggestion_param_name.clone()) + .collect::>() + .join(", ") + ), + )); + } else { + for arg in args.args { + if let hir::GenericArg::Lifetime(lifetime) = arg + && lifetime.is_anonymous() + { + self.suggestions.push(( + lifetime.ident.span, + make_suggestion(lifetime.ident), + )); + } + } + } + } + } + } + hir::TyKind::Ref(lifetime, ..) if lifetime.is_anonymous() => { + self.suggestions + .push((lifetime.ident.span, make_suggestion(lifetime.ident))); + } + _ => {} + } + walk_ty(self, ty); + } + } + let mut visitor = ImplicitLifetimeFinder { + suggestions: vec![], + suggestion_param_name: suggestion_param_name.clone(), + }; + visitor.visit_ty(self.ty_sub); + visitor.visit_ty(self.ty_sup); + if visitor.suggestions.is_empty() { + return false; + } if introduce_new { let new_param_suggestion = if let Some(first) = generics.params.iter().find(|p| !p.name.ident().span.is_empty()) @@ -417,15 +462,15 @@ fn add_to_diag_with>( (generics.span, format!("<{suggestion_param_name}>")) }; - suggestions.push(new_param_suggestion); + visitor.suggestions.push(new_param_suggestion); } - - diag.multipart_suggestion( + diag.multipart_suggestion_verbose( fluent::infer_lifetime_param_suggestion, - suggestions, + visitor.suggestions, Applicability::MaybeIncorrect, ); diag.arg("is_impl", is_impl); + true }; if mk_suggestion() && self.add_note { diff --git a/tests/ui/lifetimes/lifetime-errors/ex1-return-one-existing-name-if-else-using-impl.stderr b/tests/ui/lifetimes/lifetime-errors/ex1-return-one-existing-name-if-else-using-impl.stderr index 5827b2fe695..6bc82910b06 100644 --- a/tests/ui/lifetimes/lifetime-errors/ex1-return-one-existing-name-if-else-using-impl.stderr +++ b/tests/ui/lifetimes/lifetime-errors/ex1-return-one-existing-name-if-else-using-impl.stderr @@ -8,6 +8,11 @@ LL | fn foo<'a>(x: &i32, y: &'a i32) -> &'a i32 { LL | LL | if x > y { x } else { y } | ^ associated function was supposed to return data with lifetime `'a` but it is returning data with lifetime `'1` + | +help: consider introducing a named lifetime parameter and update trait if needed + | +LL | fn foo<'a>(x: &'a i32, y: &'a i32) -> &'a i32 { + | ++ error: aborting due to 1 previous error diff --git a/tests/ui/lifetimes/lifetime-errors/ex1-return-one-existing-name-return-type-is-anon.stderr b/tests/ui/lifetimes/lifetime-errors/ex1-return-one-existing-name-return-type-is-anon.stderr index da454b8fd86..cdcdc0c07e3 100644 --- a/tests/ui/lifetimes/lifetime-errors/ex1-return-one-existing-name-return-type-is-anon.stderr +++ b/tests/ui/lifetimes/lifetime-errors/ex1-return-one-existing-name-return-type-is-anon.stderr @@ -8,6 +8,11 @@ LL | fn foo<'a>(&self, x: &'a i32) -> &i32 { LL | LL | x | ^ method was supposed to return data with lifetime `'1` but it is returning data with lifetime `'a` + | +help: consider introducing a named lifetime parameter and update trait if needed + | +LL | fn foo<'a>(&'a self, x: &'a i32) -> &i32 { + | ++ error: aborting due to 1 previous error diff --git a/tests/ui/lifetimes/lifetime-errors/ex1-return-one-existing-name-self-is-anon.stderr b/tests/ui/lifetimes/lifetime-errors/ex1-return-one-existing-name-self-is-anon.stderr index a1dfff883a0..f106d45b9ac 100644 --- a/tests/ui/lifetimes/lifetime-errors/ex1-return-one-existing-name-self-is-anon.stderr +++ b/tests/ui/lifetimes/lifetime-errors/ex1-return-one-existing-name-self-is-anon.stderr @@ -8,6 +8,11 @@ LL | fn foo<'a>(&self, x: &'a Foo) -> &'a Foo { LL | LL | if true { x } else { self } | ^^^^ method was supposed to return data with lifetime `'a` but it is returning data with lifetime `'1` + | +help: consider introducing a named lifetime parameter and update trait if needed + | +LL | fn foo<'a>(&'a self, x: &'a Foo) -> &'a Foo { + | ++ error: aborting due to 1 previous error diff --git a/tests/ui/lifetimes/lifetime-errors/ex2b-push-no-existing-names.stderr b/tests/ui/lifetimes/lifetime-errors/ex2b-push-no-existing-names.stderr index df1d03d5170..6f7127d4c4c 100644 --- a/tests/ui/lifetimes/lifetime-errors/ex2b-push-no-existing-names.stderr +++ b/tests/ui/lifetimes/lifetime-errors/ex2b-push-no-existing-names.stderr @@ -7,6 +7,11 @@ LL | fn foo(x: &mut Vec>, y: Ref) { | has type `&mut Vec>` LL | x.push(y); | ^^^^^^^^^ argument requires that `'1` must outlive `'2` + | +help: consider introducing a named lifetime parameter + | +LL | fn foo<'a>(x: &mut Vec>, y: Ref<'a, i32>) { + | ++++ +++ +++ error: aborting due to 1 previous error diff --git a/tests/ui/lifetimes/lifetime-errors/ex3-both-anon-regions-both-are-structs-2.stderr b/tests/ui/lifetimes/lifetime-errors/ex3-both-anon-regions-both-are-structs-2.stderr index 4f1cb1e7972..f8ad923cce8 100644 --- a/tests/ui/lifetimes/lifetime-errors/ex3-both-anon-regions-both-are-structs-2.stderr +++ b/tests/ui/lifetimes/lifetime-errors/ex3-both-anon-regions-both-are-structs-2.stderr @@ -7,6 +7,11 @@ LL | fn foo(mut x: Ref, y: Ref) { | has type `Ref<'_, '2>` LL | x.b = y.b; | ^^^^^^^^^ assignment requires that `'1` must outlive `'2` + | +help: consider introducing a named lifetime parameter + | +LL | fn foo<'a>(mut x: Ref<'a, 'a>, y: Ref<'a, 'a>) { + | ++++ ++++++++ ++++++++ error: aborting due to 1 previous error diff --git a/tests/ui/lifetimes/lifetime-errors/ex3-both-anon-regions-both-are-structs-3.stderr b/tests/ui/lifetimes/lifetime-errors/ex3-both-anon-regions-both-are-structs-3.stderr index 3a1947973b5..a1149779d2c 100644 --- a/tests/ui/lifetimes/lifetime-errors/ex3-both-anon-regions-both-are-structs-3.stderr +++ b/tests/ui/lifetimes/lifetime-errors/ex3-both-anon-regions-both-are-structs-3.stderr @@ -8,6 +8,11 @@ LL | fn foo(mut x: Ref) { | has type `Ref<'2, '_>` LL | x.a = x.b; | ^^^^^^^^^ assignment requires that `'1` must outlive `'2` + | +help: consider introducing a named lifetime parameter + | +LL | fn foo<'a>(mut x: Ref<'a, 'a>) { + | ++++ ++++++++ error: aborting due to 1 previous error diff --git a/tests/ui/lifetimes/lifetime-errors/ex3-both-anon-regions-both-are-structs.stderr b/tests/ui/lifetimes/lifetime-errors/ex3-both-anon-regions-both-are-structs.stderr index c778f77366a..017bfa71463 100644 --- a/tests/ui/lifetimes/lifetime-errors/ex3-both-anon-regions-both-are-structs.stderr +++ b/tests/ui/lifetimes/lifetime-errors/ex3-both-anon-regions-both-are-structs.stderr @@ -7,6 +7,11 @@ LL | fn foo(mut x: Vec, y: Ref) { | has type `Vec>` LL | x.push(y); | ^^^^^^^^^ argument requires that `'1` must outlive `'2` + | +help: consider introducing a named lifetime parameter + | +LL | fn foo<'a>(mut x: Vec>, y: Ref<'a>) { + | ++++ ++++ ++++ error: aborting due to 1 previous error diff --git a/tests/ui/lifetimes/lifetime-errors/ex3-both-anon-regions-one-is-struct-2.stderr b/tests/ui/lifetimes/lifetime-errors/ex3-both-anon-regions-one-is-struct-2.stderr index bbd62902d9f..0980de92d35 100644 --- a/tests/ui/lifetimes/lifetime-errors/ex3-both-anon-regions-one-is-struct-2.stderr +++ b/tests/ui/lifetimes/lifetime-errors/ex3-both-anon-regions-one-is-struct-2.stderr @@ -7,6 +7,11 @@ LL | fn foo(mut x: Ref, y: &u32) { | has type `Ref<'_, '1>` LL | y = x.b; | ^^^^^^^ assignment requires that `'1` must outlive `'2` + | +help: consider introducing a named lifetime parameter + | +LL | fn foo<'a>(mut x: Ref<'a, 'a>, y: &'a u32) { + | ++++ ++++++++ ++ error[E0384]: cannot assign to immutable argument `y` --> $DIR/ex3-both-anon-regions-one-is-struct-2.rs:4:5 diff --git a/tests/ui/lifetimes/lifetime-errors/ex3-both-anon-regions-one-is-struct-3.stderr b/tests/ui/lifetimes/lifetime-errors/ex3-both-anon-regions-one-is-struct-3.stderr index 60ca4168455..d9c7530b64c 100644 --- a/tests/ui/lifetimes/lifetime-errors/ex3-both-anon-regions-one-is-struct-3.stderr +++ b/tests/ui/lifetimes/lifetime-errors/ex3-both-anon-regions-one-is-struct-3.stderr @@ -7,6 +7,11 @@ LL | fn foo(mut y: Ref, x: &u32) { | has type `Ref<'_, '2>` LL | y.b = x; | ^^^^^^^ assignment requires that `'1` must outlive `'2` + | +help: consider introducing a named lifetime parameter + | +LL | fn foo<'a>(mut y: Ref<'a, 'a>, x: &'a u32) { + | ++++ ++++++++ ++ error: aborting due to 1 previous error diff --git a/tests/ui/lifetimes/lifetime-errors/ex3-both-anon-regions-one-is-struct-4.stderr b/tests/ui/lifetimes/lifetime-errors/ex3-both-anon-regions-one-is-struct-4.stderr index 98e490c3827..d07b821444c 100644 --- a/tests/ui/lifetimes/lifetime-errors/ex3-both-anon-regions-one-is-struct-4.stderr +++ b/tests/ui/lifetimes/lifetime-errors/ex3-both-anon-regions-one-is-struct-4.stderr @@ -7,6 +7,11 @@ LL | fn foo(mut y: Ref, x: &u32) { | has type `Ref<'_, '2>` LL | y.b = x; | ^^^^^^^ assignment requires that `'1` must outlive `'2` + | +help: consider introducing a named lifetime parameter + | +LL | fn foo<'a>(mut y: Ref<'a, 'a>, x: &'a u32) { + | ++++ ++++++++ ++ error: aborting due to 1 previous error diff --git a/tests/ui/lifetimes/lifetime-errors/ex3-both-anon-regions-one-is-struct.stderr b/tests/ui/lifetimes/lifetime-errors/ex3-both-anon-regions-one-is-struct.stderr index ea1cc1f18a4..ef28ddeacc5 100644 --- a/tests/ui/lifetimes/lifetime-errors/ex3-both-anon-regions-one-is-struct.stderr +++ b/tests/ui/lifetimes/lifetime-errors/ex3-both-anon-regions-one-is-struct.stderr @@ -7,6 +7,11 @@ LL | fn foo(mut x: Ref, y: &u32) { | has type `Ref<'_, '2>` LL | x.b = y; | ^^^^^^^ assignment requires that `'1` must outlive `'2` + | +help: consider introducing a named lifetime parameter + | +LL | fn foo<'a>(mut x: Ref<'a, 'a>, y: &'a u32) { + | ++++ ++++++++ ++ error: aborting due to 1 previous error diff --git a/tests/ui/self/arbitrary_self_types_pin_lifetime_mismatch-async.stderr b/tests/ui/self/arbitrary_self_types_pin_lifetime_mismatch-async.stderr index 0b4c0a7fece..bb643ab8272 100644 --- a/tests/ui/self/arbitrary_self_types_pin_lifetime_mismatch-async.stderr +++ b/tests/ui/self/arbitrary_self_types_pin_lifetime_mismatch-async.stderr @@ -34,6 +34,11 @@ LL | async fn bar<'a>(self: Alias<&Self>, arg: &'a ()) -> &() { arg } | | | | | let's call the lifetime of this reference `'1` | lifetime `'a` defined here + | +help: consider introducing a named lifetime parameter and update trait if needed + | +LL | async fn bar<'a>(self: Alias<&'a Self>, arg: &'a ()) -> &() { arg } + | ++ error: aborting due to 3 previous errors diff --git a/tests/ui/self/arbitrary_self_types_pin_lifetime_mismatch.stderr b/tests/ui/self/arbitrary_self_types_pin_lifetime_mismatch.stderr index 209dae9c1b3..bbb2ef4641e 100644 --- a/tests/ui/self/arbitrary_self_types_pin_lifetime_mismatch.stderr +++ b/tests/ui/self/arbitrary_self_types_pin_lifetime_mismatch.stderr @@ -33,6 +33,11 @@ LL | fn bar<'a>(self: Alias<&Self>, arg: &'a ()) -> &() { arg } | -- ---- has type `Pin<&'1 Foo>` ^^^ method was supposed to return data with lifetime `'1` but it is returning data with lifetime `'a` | | | lifetime `'a` defined here + | +help: consider introducing a named lifetime parameter and update trait if needed + | +LL | fn bar<'a>(self: Alias<&'a Self>, arg: &'a ()) -> &() { arg } + | ++ error: aborting due to 3 previous errors