From b7a23bc08b2cc9b1ddfac18ea2019d5150d93e0e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Tue, 14 Nov 2023 20:36:49 +0000 Subject: [PATCH 1/9] On borrow return type, suggest borrowing from arg or owned return type When we encounter a function with a return type that has an anonymous lifetime with no argument to borrow from, besides suggesting the `'static` lifetime we now also suggest changing the arguments to be borrows or changing the return type to be an owned type. ``` error[E0106]: missing lifetime specifier --> $DIR/variadic-ffi-6.rs:7:6 | LL | ) -> &usize { | ^ expected named lifetime parameter | = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static` | LL | ) -> &'static usize { | +++++++ help: instead, you are more likely to want to change one of the arguments to be borrowed... | LL | x: &usize, | + help: ...or alternatively, to want to return an owned value | LL - ) -> &usize { LL + ) -> usize { | ``` Fix #85843. --- .../rustc_resolve/src/late/diagnostics.rs | 138 ++++++++++++++++-- src/tools/tidy/src/ui_tests.rs | 2 +- ...nd-lifetime-in-binding-only.elision.stderr | 7 +- ...und-lifetime-in-return-only.elision.stderr | 7 +- tests/ui/c-variadic/variadic-ffi-6.stderr | 11 +- tests/ui/foreign-fn-return-lifetime.fixed | 8 - tests/ui/foreign-fn-return-lifetime.rs | 2 - tests/ui/foreign-fn-return-lifetime.stderr | 9 +- .../issue-70304.stderr | 2 +- .../ui/impl-trait/impl-fn-hrtb-bounds.stderr | 2 +- tests/ui/issues/issue-13497.stderr | 6 +- tests/ui/lifetimes/issue-26638.stderr | 16 +- ...urn-type-requires-explicit-lifetime.stderr | 50 ++++++- tests/ui/self/elision/nested-item.stderr | 11 +- .../impl-trait-missing-lifetime-gated.stderr | 48 +++++- .../impl-trait-missing-lifetime.stderr | 4 +- .../missing-lifetime-specifier.stderr | 15 +- .../suggestions/return-elided-lifetime.stderr | 22 ++- .../underscore-lifetime-binders.stderr | 2 +- 19 files changed, 305 insertions(+), 57 deletions(-) delete mode 100644 tests/ui/foreign-fn-return-lifetime.fixed diff --git a/compiler/rustc_resolve/src/late/diagnostics.rs b/compiler/rustc_resolve/src/late/diagnostics.rs index d62d7fdcae0c2..a32897270a8ad 100644 --- a/compiler/rustc_resolve/src/late/diagnostics.rs +++ b/compiler/rustc_resolve/src/late/diagnostics.rs @@ -8,7 +8,7 @@ use crate::{PathResult, PathSource, Segment}; use rustc_hir::def::Namespace::{self, *}; use rustc_ast::ptr::P; -use rustc_ast::visit::{FnCtxt, FnKind, LifetimeCtxt}; +use rustc_ast::visit::{walk_ty, FnCtxt, FnKind, LifetimeCtxt, Visitor}; use rustc_ast::{ self as ast, AssocItemKind, Expr, ExprKind, GenericParam, GenericParamKind, Item, ItemKind, MethodCall, NodeId, Path, Ty, TyKind, DUMMY_NODE_ID, @@ -2811,6 +2811,7 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { .collect(); debug!(?in_scope_lifetimes); + let mut maybe_static = false; debug!(?function_param_lifetimes); if let Some((param_lifetimes, params)) = &function_param_lifetimes { let elided_len = param_lifetimes.len(); @@ -2849,9 +2850,10 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { if num_params == 0 { err.help( - "this function's return type contains a borrowed value, \ - but there is no value for it to be borrowed from", + "this function's return type contains a borrowed value, but there is no value \ + for it to be borrowed from", ); + maybe_static = true; if in_scope_lifetimes.is_empty() { in_scope_lifetimes = vec![( Ident::with_dummy_span(kw::StaticLifetime), @@ -2860,10 +2862,10 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { } } else if elided_len == 0 { err.help( - "this function's return type contains a borrowed value with \ - an elided lifetime, but the lifetime cannot be derived from \ - the arguments", + "this function's return type contains a borrowed value with an elided \ + lifetime, but the lifetime cannot be derived from the arguments", ); + maybe_static = true; if in_scope_lifetimes.is_empty() { in_scope_lifetimes = vec![( Ident::with_dummy_span(kw::StaticLifetime), @@ -2872,13 +2874,13 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { } } else if num_params == 1 { err.help(format!( - "this function's return type contains a borrowed value, \ - but the signature does not say which {m} it is borrowed from" + "this function's return type contains a borrowed value, but the signature does \ + not say which {m} it is borrowed from", )); } else { err.help(format!( - "this function's return type contains a borrowed value, \ - but the signature does not say whether it is borrowed from {m}" + "this function's return type contains a borrowed value, but the signature does \ + not say whether it is borrowed from {m}", )); } } @@ -2943,11 +2945,107 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { ); } 1 => { + let post = if maybe_static { + let owned = if let [lt] = &lifetime_refs[..] + && lt.kind != MissingLifetimeKind::Ampersand + { + ", or if you will only have owned values" + } else { + "" + }; + format!( + ", but this is uncommon unless you're returning a borrowed value from a \ + `const` or a `static`{owned}", + ) + } else { + String::new() + }; err.multipart_suggestion_verbose( - format!("consider using the `{existing_name}` lifetime"), + format!("consider using the `{existing_name}` lifetime{post}"), spans_suggs, Applicability::MaybeIncorrect, ); + if maybe_static { + // FIXME: what follows are general suggestions, but we'd want to perform some + // minimal flow analysis to provide more accurate suggestions. For example, if + // we identified that the return expression references only one argument, we + // would suggest borrowing only that argument, and we'd skip the prior + // "use `'static`" suggestion entirely. + if let [lt] = &lifetime_refs[..] && lt.kind == MissingLifetimeKind::Ampersand { + let pre = if let Some((kind, _span)) = + self.diagnostic_metadata.current_function + && let FnKind::Fn(_, _, sig, _, _, _) = kind + && !sig.decl.inputs.is_empty() + && let sugg = sig + .decl + .inputs + .iter() + .filter_map(|param| { + if param.ty.span.contains(lt.span) { + // We don't want to suggest `fn elision(_: &fn() -> &i32)` + // when we have `fn elision(_: fn() -> &i32)` + None + } else if let TyKind::CVarArgs = param.ty.kind { + // Don't suggest `&...` for ffi fn with varargs + None + } else { + Some((param.ty.span.shrink_to_lo(), "&".to_string())) + } + }) + .collect::>() + && !sugg.is_empty() + { + + let (the, s) = if sig.decl.inputs.len() == 1 { + ("the", "") + } else { + ("one of the", "s") + }; + err.multipart_suggestion_verbose( + format!( + "instead, you are more likely to want to change {the} \ + argument{s} to be borrowed...", + ), + sugg, + Applicability::MaybeIncorrect, + ); + "...or alternatively," + } else { + "instead, you are more likely" + }; + let mut sugg = vec![(lt.span, String::new())]; + if let Some((kind, _span)) = + self.diagnostic_metadata.current_function + && let FnKind::Fn(_, _, sig, _, _, _) = kind + && let ast::FnRetTy::Ty(ty) = &sig.decl.output + { + let mut lt_finder = LifetimeFinder { lifetime: lt.span, found: None }; + lt_finder.visit_ty(&ty); + + if let Some(ty) = lt_finder.found { + if let TyKind::Path(None, Path { segments, .. }) = &ty.kind + && segments.len() == 1 + && segments[0].ident.name == sym::str + { + // Don't suggest `-> str`, suggest `-> String`. + sugg = vec![(lt.span.with_hi(ty.span.hi()), "String".to_string())]; + } + if let TyKind::Slice(inner_ty) = &ty.kind { + // Don't suggest `-> [T]`, suggest `-> Vec`. + sugg = vec![ + (lt.span.with_hi(inner_ty.span.lo()), "Vec<".to_string()), + (ty.span.with_lo(inner_ty.span.hi()), ">".to_string()), + ]; + } + } + }; + err.multipart_suggestion_verbose( + format!("{pre} to want to return an owned value"), + sugg, + Applicability::MaybeIncorrect, + ); + } + } // Record as using the suggested resolution. let (_, (_, res)) = in_scope_lifetimes[0]; @@ -2977,7 +3075,7 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { fn mk_where_bound_predicate( path: &Path, poly_trait_ref: &ast::PolyTraitRef, - ty: &ast::Ty, + ty: &Ty, ) -> Option { use rustc_span::DUMMY_SP; let modified_segments = { @@ -3054,6 +3152,22 @@ pub(super) fn signal_lifetime_shadowing(sess: &Session, orig: Ident, shadower: I err.emit(); } +struct LifetimeFinder<'ast> { + lifetime: Span, + found: Option<&'ast Ty>, +} + +impl<'ast> Visitor<'ast> for LifetimeFinder<'ast> { + fn visit_ty(&mut self, t: &'ast Ty) { + if t.span.lo() == self.lifetime.lo() + && let TyKind::Ref(_, mut_ty) = &t.kind + { + self.found = Some(&mut_ty.ty); + } + walk_ty(self, t) + } +} + /// Shadowing involving a label is only a warning for historical reasons. //FIXME: make this a proper lint. pub(super) fn signal_label_shadowing(sess: &Session, orig: Span, shadower: Ident) { diff --git a/src/tools/tidy/src/ui_tests.rs b/src/tools/tidy/src/ui_tests.rs index dfa386b49de7c..40149f8f1c3b6 100644 --- a/src/tools/tidy/src/ui_tests.rs +++ b/src/tools/tidy/src/ui_tests.rs @@ -11,7 +11,7 @@ use std::path::{Path, PathBuf}; const ENTRY_LIMIT: usize = 900; // FIXME: The following limits should be reduced eventually. const ISSUES_ENTRY_LIMIT: usize = 1852; -const ROOT_ENTRY_LIMIT: usize = 867; +const ROOT_ENTRY_LIMIT: usize = 866; const EXPECTED_TEST_FILE_EXTENSIONS: &[&str] = &[ "rs", // test source files diff --git a/tests/ui/associated-types/bound-lifetime-in-binding-only.elision.stderr b/tests/ui/associated-types/bound-lifetime-in-binding-only.elision.stderr index 4de4afb6e9246..bf22cb3b1aa82 100644 --- a/tests/ui/associated-types/bound-lifetime-in-binding-only.elision.stderr +++ b/tests/ui/associated-types/bound-lifetime-in-binding-only.elision.stderr @@ -5,10 +5,15 @@ LL | fn elision &i32>() { | ^ expected named lifetime parameter | = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from -help: consider using the `'static` lifetime +help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static` | LL | fn elision &'static i32>() { | +++++++ +help: instead, you are more likely to want to return an owned value + | +LL - fn elision &i32>() { +LL + fn elision i32>() { + | error: aborting due to previous error diff --git a/tests/ui/associated-types/bound-lifetime-in-return-only.elision.stderr b/tests/ui/associated-types/bound-lifetime-in-return-only.elision.stderr index 7753d186504f8..405d5d9615ba2 100644 --- a/tests/ui/associated-types/bound-lifetime-in-return-only.elision.stderr +++ b/tests/ui/associated-types/bound-lifetime-in-return-only.elision.stderr @@ -5,10 +5,15 @@ LL | fn elision(_: fn() -> &i32) { | ^ expected named lifetime parameter | = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from -help: consider using the `'static` lifetime +help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static` | LL | fn elision(_: fn() -> &'static i32) { | +++++++ +help: instead, you are more likely to want to return an owned value + | +LL - fn elision(_: fn() -> &i32) { +LL + fn elision(_: fn() -> i32) { + | error: aborting due to previous error diff --git a/tests/ui/c-variadic/variadic-ffi-6.stderr b/tests/ui/c-variadic/variadic-ffi-6.stderr index 4c7792d965019..b3497d31a0cbf 100644 --- a/tests/ui/c-variadic/variadic-ffi-6.stderr +++ b/tests/ui/c-variadic/variadic-ffi-6.stderr @@ -5,10 +5,19 @@ LL | ) -> &usize { | ^ expected named lifetime parameter | = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from -help: consider using the `'static` lifetime +help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static` | LL | ) -> &'static usize { | +++++++ +help: instead, you are more likely to want to change one of the arguments to be borrowed... + | +LL | x: &usize, + | + +help: ...or alternatively, to want to return an owned value + | +LL - ) -> &usize { +LL + ) -> usize { + | error: aborting due to previous error diff --git a/tests/ui/foreign-fn-return-lifetime.fixed b/tests/ui/foreign-fn-return-lifetime.fixed deleted file mode 100644 index 143d6343d26cd..0000000000000 --- a/tests/ui/foreign-fn-return-lifetime.fixed +++ /dev/null @@ -1,8 +0,0 @@ -// run-rustfix - -extern "C" { - pub fn g(_: &u8) -> &u8; // OK - pub fn f() -> &'static u8; //~ ERROR missing lifetime specifier -} - -fn main() {} diff --git a/tests/ui/foreign-fn-return-lifetime.rs b/tests/ui/foreign-fn-return-lifetime.rs index 76fe50a340ae4..35595bab36d42 100644 --- a/tests/ui/foreign-fn-return-lifetime.rs +++ b/tests/ui/foreign-fn-return-lifetime.rs @@ -1,5 +1,3 @@ -// run-rustfix - extern "C" { pub fn g(_: &u8) -> &u8; // OK pub fn f() -> &u8; //~ ERROR missing lifetime specifier diff --git a/tests/ui/foreign-fn-return-lifetime.stderr b/tests/ui/foreign-fn-return-lifetime.stderr index df1a23a19edd5..d63d13ec8017a 100644 --- a/tests/ui/foreign-fn-return-lifetime.stderr +++ b/tests/ui/foreign-fn-return-lifetime.stderr @@ -1,14 +1,19 @@ error[E0106]: missing lifetime specifier - --> $DIR/foreign-fn-return-lifetime.rs:5:19 + --> $DIR/foreign-fn-return-lifetime.rs:3:19 | LL | pub fn f() -> &u8; | ^ expected named lifetime parameter | = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from -help: consider using the `'static` lifetime +help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static` | LL | pub fn f() -> &'static u8; | +++++++ +help: instead, you are more likely to want to return an owned value + | +LL - pub fn f() -> &u8; +LL + pub fn f() -> u8; + | error: aborting due to previous error diff --git a/tests/ui/generic-associated-types/issue-70304.stderr b/tests/ui/generic-associated-types/issue-70304.stderr index 99339e9685959..9b02c1b076837 100644 --- a/tests/ui/generic-associated-types/issue-70304.stderr +++ b/tests/ui/generic-associated-types/issue-70304.stderr @@ -11,7 +11,7 @@ LL | fn create_doc() -> impl Document = DocCursorImpl<'_>> { | ^^ expected named lifetime parameter | = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from -help: consider using the `'static` lifetime +help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static`, or if you will only have owned values | LL | fn create_doc() -> impl Document = DocCursorImpl<'static>> { | ~~~~~~~ diff --git a/tests/ui/impl-trait/impl-fn-hrtb-bounds.stderr b/tests/ui/impl-trait/impl-fn-hrtb-bounds.stderr index 443ffeb55cdee..a5982a5542a54 100644 --- a/tests/ui/impl-trait/impl-fn-hrtb-bounds.stderr +++ b/tests/ui/impl-trait/impl-fn-hrtb-bounds.stderr @@ -5,7 +5,7 @@ LL | fn d() -> impl Fn() -> (impl Debug + '_) { | ^^ expected named lifetime parameter | = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from -help: consider using the `'static` lifetime +help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static`, or if you will only have owned values | LL | fn d() -> impl Fn() -> (impl Debug + 'static) { | ~~~~~~~ diff --git a/tests/ui/issues/issue-13497.stderr b/tests/ui/issues/issue-13497.stderr index 4b1d979da36e3..5f9aa7162fa64 100644 --- a/tests/ui/issues/issue-13497.stderr +++ b/tests/ui/issues/issue-13497.stderr @@ -5,10 +5,14 @@ LL | &str | ^ expected named lifetime parameter | = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from -help: consider using the `'static` lifetime +help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static` | LL | &'static str | +++++++ +help: instead, you are more likely to want to return an owned value + | +LL | String + | ~~~~~~ error: aborting due to previous error diff --git a/tests/ui/lifetimes/issue-26638.stderr b/tests/ui/lifetimes/issue-26638.stderr index e61158a5d4d02..bea590237bc64 100644 --- a/tests/ui/lifetimes/issue-26638.stderr +++ b/tests/ui/lifetimes/issue-26638.stderr @@ -17,10 +17,18 @@ LL | fn parse_type_2(iter: fn(&u8)->&u8) -> &str { iter() } | ^ expected named lifetime parameter | = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from -help: consider using the `'static` lifetime +help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static` | LL | fn parse_type_2(iter: fn(&u8)->&u8) -> &'static str { iter() } | +++++++ +help: instead, you are more likely to want to change the argument to be borrowed... + | +LL | fn parse_type_2(iter: &fn(&u8)->&u8) -> &str { iter() } + | + +help: ...or alternatively, to want to return an owned value + | +LL | fn parse_type_2(iter: fn(&u8)->&u8) -> String { iter() } + | ~~~~~~ error[E0106]: missing lifetime specifier --> $DIR/issue-26638.rs:10:22 @@ -29,10 +37,14 @@ LL | fn parse_type_3() -> &str { unimplemented!() } | ^ expected named lifetime parameter | = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from -help: consider using the `'static` lifetime +help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static` | LL | fn parse_type_3() -> &'static str { unimplemented!() } | +++++++ +help: instead, you are more likely to want to return an owned value + | +LL | fn parse_type_3() -> String { unimplemented!() } + | ~~~~~~ error[E0308]: mismatched types --> $DIR/issue-26638.rs:1:69 diff --git a/tests/ui/lifetimes/lifetime-elision-return-type-requires-explicit-lifetime.stderr b/tests/ui/lifetimes/lifetime-elision-return-type-requires-explicit-lifetime.stderr index 5eee953ef189f..caba886a3f7c7 100644 --- a/tests/ui/lifetimes/lifetime-elision-return-type-requires-explicit-lifetime.stderr +++ b/tests/ui/lifetimes/lifetime-elision-return-type-requires-explicit-lifetime.stderr @@ -5,10 +5,15 @@ LL | fn f() -> &isize { | ^ expected named lifetime parameter | = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from -help: consider using the `'static` lifetime +help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static` | LL | fn f() -> &'static isize { | +++++++ +help: instead, you are more likely to want to return an owned value + | +LL - fn f() -> &isize { +LL + fn f() -> isize { + | error[E0106]: missing lifetime specifier --> $DIR/lifetime-elision-return-type-requires-explicit-lifetime.rs:7:33 @@ -41,10 +46,19 @@ LL | fn i(_x: isize) -> &isize { | ^ expected named lifetime parameter | = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from -help: consider using the `'static` lifetime +help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static` | LL | fn i(_x: isize) -> &'static isize { | +++++++ +help: instead, you are more likely to want to change the argument to be borrowed... + | +LL | fn i(_x: &isize) -> &isize { + | + +help: ...or alternatively, to want to return an owned value + | +LL - fn i(_x: isize) -> &isize { +LL + fn i(_x: isize) -> isize { + | error[E0106]: missing lifetime specifier --> $DIR/lifetime-elision-return-type-requires-explicit-lifetime.rs:34:24 @@ -53,10 +67,19 @@ LL | fn j(_x: StaticStr) -> &isize { | ^ expected named lifetime parameter | = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from -help: consider using the `'static` lifetime +help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static` | LL | fn j(_x: StaticStr) -> &'static isize { | +++++++ +help: instead, you are more likely to want to change the argument to be borrowed... + | +LL | fn j(_x: &StaticStr) -> &isize { + | + +help: ...or alternatively, to want to return an owned value + | +LL - fn j(_x: StaticStr) -> &isize { +LL + fn j(_x: StaticStr) -> isize { + | error[E0106]: missing lifetime specifier --> $DIR/lifetime-elision-return-type-requires-explicit-lifetime.rs:40:49 @@ -65,10 +88,19 @@ LL | fn k<'a, T: WithLifetime<'a>>(_x: T::Output) -> &isize { | ^ expected named lifetime parameter | = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from -help: consider using the `'a` lifetime +help: consider using the `'a` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static` | LL | fn k<'a, T: WithLifetime<'a>>(_x: T::Output) -> &'a isize { | ++ +help: instead, you are more likely to want to change the argument to be borrowed... + | +LL | fn k<'a, T: WithLifetime<'a>>(_x: &T::Output) -> &isize { + | + +help: ...or alternatively, to want to return an owned value + | +LL - fn k<'a, T: WithLifetime<'a>>(_x: T::Output) -> &isize { +LL + fn k<'a, T: WithLifetime<'a>>(_x: T::Output) -> isize { + | error[E0106]: missing lifetime specifier --> $DIR/lifetime-elision-return-type-requires-explicit-lifetime.rs:45:37 @@ -77,10 +109,18 @@ LL | fn l<'a>(_: &'a str, _: &'a str) -> &str { "" } | ------- ------- ^ expected named lifetime parameter | = help: this function's return type contains a borrowed value with an elided lifetime, but the lifetime cannot be derived from the arguments -help: consider using the `'a` lifetime +help: consider using the `'a` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static` | LL | fn l<'a>(_: &'a str, _: &'a str) -> &'a str { "" } | ++ +help: instead, you are more likely to want to change one of the arguments to be borrowed... + | +LL | fn l<'a>(_: &&'a str, _: &&'a str) -> &str { "" } + | + + +help: ...or alternatively, to want to return an owned value + | +LL | fn l<'a>(_: &'a str, _: &'a str) -> String { "" } + | ~~~~~~ error: aborting due to 7 previous errors diff --git a/tests/ui/self/elision/nested-item.stderr b/tests/ui/self/elision/nested-item.stderr index 752fd82332c38..5b7845559105b 100644 --- a/tests/ui/self/elision/nested-item.stderr +++ b/tests/ui/self/elision/nested-item.stderr @@ -21,10 +21,19 @@ LL | fn wrap(self: Wrap<{ fn bar(&self) {} }>) -> &() { | ^ expected named lifetime parameter | = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from -help: consider using the `'static` lifetime +help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static` | LL | fn wrap(self: Wrap<{ fn bar(&self) {} }>) -> &'static () { | +++++++ +help: instead, you are more likely to want to change the argument to be borrowed... + | +LL | fn wrap(self: &Wrap<{ fn bar(&self) {} }>) -> &() { + | + +help: ...or alternatively, to want to return an owned value + | +LL - fn wrap(self: Wrap<{ fn bar(&self) {} }>) -> &() { +LL + fn wrap(self: Wrap<{ fn bar(&self) {} }>) -> () { + | error[E0412]: cannot find type `Wrap` in this scope --> $DIR/nested-item.rs:5:15 diff --git a/tests/ui/suggestions/impl-trait-missing-lifetime-gated.stderr b/tests/ui/suggestions/impl-trait-missing-lifetime-gated.stderr index 50806a6725500..c4d156825a339 100644 --- a/tests/ui/suggestions/impl-trait-missing-lifetime-gated.stderr +++ b/tests/ui/suggestions/impl-trait-missing-lifetime-gated.stderr @@ -5,10 +5,19 @@ LL | fn g(mut x: impl Iterator) -> Option<&()> { x.next() } | ^ expected named lifetime parameter | = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from -help: consider using the `'static` lifetime +help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static` | LL | fn g(mut x: impl Iterator) -> Option<&'static ()> { x.next() } | +++++++ +help: instead, you are more likely to want to change the argument to be borrowed... + | +LL | fn g(mut x: &impl Iterator) -> Option<&()> { x.next() } + | + +help: ...or alternatively, to want to return an owned value + | +LL - fn g(mut x: impl Iterator) -> Option<&()> { x.next() } +LL + fn g(mut x: impl Iterator) -> Option<()> { x.next() } + | error[E0106]: missing lifetime specifier --> $DIR/impl-trait-missing-lifetime-gated.rs:19:60 @@ -17,10 +26,19 @@ LL | async fn i(mut x: impl Iterator) -> Option<&()> { x.next() | ^ expected named lifetime parameter | = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from -help: consider using the `'static` lifetime +help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static` | LL | async fn i(mut x: impl Iterator) -> Option<&'static ()> { x.next() } | +++++++ +help: instead, you are more likely to want to change the argument to be borrowed... + | +LL | async fn i(mut x: &impl Iterator) -> Option<&()> { x.next() } + | + +help: ...or alternatively, to want to return an owned value + | +LL - async fn i(mut x: impl Iterator) -> Option<&()> { x.next() } +LL + async fn i(mut x: impl Iterator) -> Option<()> { x.next() } + | error[E0106]: missing lifetime specifier --> $DIR/impl-trait-missing-lifetime-gated.rs:27:58 @@ -29,7 +47,7 @@ LL | fn g(mut x: impl Iterator) -> Option<&'_ ()> { x.next() | ^^ expected named lifetime parameter | = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from -help: consider using the `'static` lifetime +help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static`, or if you will only have owned values | LL | fn g(mut x: impl Iterator) -> Option<&'static ()> { x.next() } | ~~~~~~~ @@ -41,7 +59,7 @@ LL | async fn i(mut x: impl Iterator) -> Option<&'_ ()> { x.n | ^^ expected named lifetime parameter | = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from -help: consider using the `'static` lifetime +help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static`, or if you will only have owned values | LL | async fn i(mut x: impl Iterator) -> Option<&'static ()> { x.next() } | ~~~~~~~ @@ -53,10 +71,19 @@ LL | fn g(mut x: impl Foo) -> Option<&()> { x.next() } | ^ expected named lifetime parameter | = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from -help: consider using the `'static` lifetime +help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static` | LL | fn g(mut x: impl Foo) -> Option<&'static ()> { x.next() } | +++++++ +help: instead, you are more likely to want to change the argument to be borrowed... + | +LL | fn g(mut x: &impl Foo) -> Option<&()> { x.next() } + | + +help: ...or alternatively, to want to return an owned value + | +LL - fn g(mut x: impl Foo) -> Option<&()> { x.next() } +LL + fn g(mut x: impl Foo) -> Option<()> { x.next() } + | error[E0106]: missing lifetime specifier --> $DIR/impl-trait-missing-lifetime-gated.rs:58:41 @@ -65,10 +92,19 @@ LL | fn g(mut x: impl Foo<()>) -> Option<&()> { x.next() } | ^ expected named lifetime parameter | = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from -help: consider using the `'static` lifetime +help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static` | LL | fn g(mut x: impl Foo<()>) -> Option<&'static ()> { x.next() } | +++++++ +help: instead, you are more likely to want to change the argument to be borrowed... + | +LL | fn g(mut x: &impl Foo<()>) -> Option<&()> { x.next() } + | + +help: ...or alternatively, to want to return an owned value + | +LL - fn g(mut x: impl Foo<()>) -> Option<&()> { x.next() } +LL + fn g(mut x: impl Foo<()>) -> Option<()> { x.next() } + | error[E0658]: anonymous lifetimes in `impl Trait` are unstable --> $DIR/impl-trait-missing-lifetime-gated.rs:6:35 diff --git a/tests/ui/suggestions/impl-trait-missing-lifetime.stderr b/tests/ui/suggestions/impl-trait-missing-lifetime.stderr index 2c29cfa0b910f..37c2ad2aaeaaf 100644 --- a/tests/ui/suggestions/impl-trait-missing-lifetime.stderr +++ b/tests/ui/suggestions/impl-trait-missing-lifetime.stderr @@ -5,7 +5,7 @@ LL | fn g(mut x: impl Iterator) -> Option<&'_ ()> { x.next() } | ^^ expected named lifetime parameter | = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from -help: consider using the `'static` lifetime +help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static`, or if you will only have owned values | LL | fn g(mut x: impl Iterator) -> Option<&'static ()> { x.next() } | ~~~~~~~ @@ -17,7 +17,7 @@ LL | async fn i(mut x: impl Iterator) -> Option<&'_ ()> { x.next( | ^^ expected named lifetime parameter | = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from -help: consider using the `'static` lifetime +help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static`, or if you will only have owned values | LL | async fn i(mut x: impl Iterator) -> Option<&'static ()> { x.next() } | ~~~~~~~ diff --git a/tests/ui/suggestions/missing-lifetime-specifier.stderr b/tests/ui/suggestions/missing-lifetime-specifier.stderr index fa4bc2fa79d7c..484a9b3ad8ddb 100644 --- a/tests/ui/suggestions/missing-lifetime-specifier.stderr +++ b/tests/ui/suggestions/missing-lifetime-specifier.stderr @@ -5,7 +5,7 @@ LL | static a: RefCell>>> = RefCell::new(HashMap:: | ^^^ expected 2 lifetime parameters | = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from -help: consider using the `'static` lifetime +help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static`, or if you will only have owned values | LL | static a: RefCell>>>> = RefCell::new(HashMap::new()); | ++++++++++++++++++ @@ -32,7 +32,7 @@ LL | static b: RefCell>>> = RefCell::new(HashMap: | expected named lifetime parameter | = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from -help: consider using the `'static` lifetime +help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static` | LL | static b: RefCell>>>> = RefCell::new(HashMap::new()); | +++++++ ++++++++++++++++++ @@ -59,7 +59,7 @@ LL | static c: RefCell>>>> = RefCell::new(Hash | ^ expected 2 lifetime parameters | = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from -help: consider using the `'static` lifetime +help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static`, or if you will only have owned values | LL | static c: RefCell>>>> = RefCell::new(HashMap::new()); | +++++++++++++++++ @@ -86,7 +86,7 @@ LL | static d: RefCell>>>> = RefCell::new(Has | expected named lifetime parameter | = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from -help: consider using the `'static` lifetime +help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static` | LL | static d: RefCell>>>> = RefCell::new(HashMap::new()); | +++++++ +++++++++++++++++ @@ -113,10 +113,15 @@ LL | static f: RefCell>>>> = RefCell | ^ expected named lifetime parameter | = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from -help: consider using the `'static` lifetime +help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static` | LL | static f: RefCell>>>> = RefCell::new(HashMap::new()); | +++++++ +help: instead, you are more likely to want to return an owned value + | +LL - static f: RefCell>>>> = RefCell::new(HashMap::new()); +LL + static f: RefCell>>>> = RefCell::new(HashMap::new()); + | error[E0106]: missing lifetime specifier --> $DIR/missing-lifetime-specifier.rs:47:44 diff --git a/tests/ui/suggestions/return-elided-lifetime.stderr b/tests/ui/suggestions/return-elided-lifetime.stderr index 273d95bc747d3..54e0cfdf9a326 100644 --- a/tests/ui/suggestions/return-elided-lifetime.stderr +++ b/tests/ui/suggestions/return-elided-lifetime.stderr @@ -5,10 +5,15 @@ LL | fn f1() -> &i32 { loop {} } | ^ expected named lifetime parameter | = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from -help: consider using the `'static` lifetime +help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static` | LL | fn f1() -> &'static i32 { loop {} } | +++++++ +help: instead, you are more likely to want to return an owned value + | +LL - fn f1() -> &i32 { loop {} } +LL + fn f1() -> i32 { loop {} } + | error[E0106]: missing lifetime specifiers --> $DIR/return-elided-lifetime.rs:8:14 @@ -19,7 +24,7 @@ LL | fn f1_() -> (&i32, &i32) { loop {} } | expected named lifetime parameter | = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from -help: consider using the `'static` lifetime +help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static` | LL | fn f1_() -> (&'static i32, &'static i32) { loop {} } | +++++++ +++++++ @@ -31,10 +36,19 @@ LL | fn f2(a: i32, b: i32) -> &i32 { loop {} } | ^ expected named lifetime parameter | = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from -help: consider using the `'static` lifetime +help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static` | LL | fn f2(a: i32, b: i32) -> &'static i32 { loop {} } | +++++++ +help: instead, you are more likely to want to change one of the arguments to be borrowed... + | +LL | fn f2(a: &i32, b: &i32) -> &i32 { loop {} } + | + + +help: ...or alternatively, to want to return an owned value + | +LL - fn f2(a: i32, b: i32) -> &i32 { loop {} } +LL + fn f2(a: i32, b: i32) -> i32 { loop {} } + | error[E0106]: missing lifetime specifiers --> $DIR/return-elided-lifetime.rs:13:28 @@ -45,7 +59,7 @@ LL | fn f2_(a: i32, b: i32) -> (&i32, &i32) { loop {} } | expected named lifetime parameter | = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from -help: consider using the `'static` lifetime +help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static` | LL | fn f2_(a: i32, b: i32) -> (&'static i32, &'static i32) { loop {} } | +++++++ +++++++ diff --git a/tests/ui/underscore-lifetime/underscore-lifetime-binders.stderr b/tests/ui/underscore-lifetime/underscore-lifetime-binders.stderr index 50401791effb8..cd74d27dcb55f 100644 --- a/tests/ui/underscore-lifetime/underscore-lifetime-binders.stderr +++ b/tests/ui/underscore-lifetime/underscore-lifetime-binders.stderr @@ -28,7 +28,7 @@ LL | fn meh() -> Box Meh<'_>> | ^^ expected named lifetime parameter | = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from -help: consider using the `'static` lifetime +help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static`, or if you will only have owned values | LL | fn meh() -> Box Meh<'static>> | ~~~~~~~ From 5fce361d589c870d5fd2f29ff9f0cd3b6485b5be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Tue, 14 Nov 2023 22:36:05 +0000 Subject: [PATCH 2/9] Account for impl Trait in lifetime suggestion When encountering ```rust fn g(mut x: impl Iterator) -> Option<&()> { /* */ } ``` Suggest ```rust fn g<'a>(mut x: impl Iterator) -> Option<&'a ()> { /* */ } ``` --- .../rustc_resolve/src/late/diagnostics.rs | 77 +++++++++++++++++-- .../impl-trait-missing-lifetime-gated.stderr | 24 +++--- 2 files changed, 84 insertions(+), 17 deletions(-) diff --git a/compiler/rustc_resolve/src/late/diagnostics.rs b/compiler/rustc_resolve/src/late/diagnostics.rs index a32897270a8ad..2998609c7c81f 100644 --- a/compiler/rustc_resolve/src/late/diagnostics.rs +++ b/compiler/rustc_resolve/src/late/diagnostics.rs @@ -2988,6 +2988,9 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { } else if let TyKind::CVarArgs = param.ty.kind { // Don't suggest `&...` for ffi fn with varargs None + } else if let TyKind::ImplTrait(..) = ¶m.ty.kind { + // We handle these in the next `else if` branch. + None } else { Some((param.ty.span.shrink_to_lo(), "&".to_string())) } @@ -3010,6 +3013,64 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { Applicability::MaybeIncorrect, ); "...or alternatively," + } else if let Some((kind, _span)) = + self.diagnostic_metadata.current_function + && let FnKind::Fn(_, _, sig, _, _, _) = kind + && let ast::FnRetTy::Ty(ret_ty) = &sig.decl.output + && !sig.decl.inputs.is_empty() + && let arg_refs = sig + .decl + .inputs + .iter() + .filter_map(|param| match ¶m.ty.kind { + TyKind::ImplTrait(_, bounds) => Some(bounds), + _ => None, + }) + .flat_map(|bounds| bounds.into_iter()) + .collect::>() + && !arg_refs.is_empty() + { + // We have a situation like + // fn g(mut x: impl Iterator) -> Option<&()> + // So we look at every ref in the trait bound. If there's any, we + // suggest + // fn g<'a>(mut x: impl Iterator) -> Option<&'a ()> + let mut lt_finder = LifetimeFinder { + lifetime: lt.span, + found: None, + seen: vec![], + }; + for bound in arg_refs { + if let ast::GenericBound::Trait(trait_ref, _) = bound { + lt_finder.visit_trait_ref(&trait_ref.trait_ref); + } + } + lt_finder.visit_ty(ret_ty); + let spans_suggs: Vec<_> = lt_finder.seen.iter().filter_map(|ty| { + match &ty.kind { + TyKind::Ref(_, mut_ty) => { + let span = ty.span.with_hi(mut_ty.ty.span.lo()); + Some((span, "&'a ".to_string())) + } + _ => None + } + }).collect(); + self.suggest_introducing_lifetime( + err, + None, + |err, higher_ranked, span, message, intro_sugg| { + info!(?span, ?message, ?intro_sugg); + err.multipart_suggestion_verbose( + message, + std::iter::once((span, intro_sugg)) + .chain(spans_suggs.iter().cloned()) + .collect(), + Applicability::MaybeIncorrect, + ); + higher_ranked + }, + ); + "...or alternatively," } else { "instead, you are more likely" }; @@ -3019,7 +3080,11 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { && let FnKind::Fn(_, _, sig, _, _, _) = kind && let ast::FnRetTy::Ty(ty) = &sig.decl.output { - let mut lt_finder = LifetimeFinder { lifetime: lt.span, found: None }; + let mut lt_finder = LifetimeFinder { + lifetime: lt.span, + found: None, + seen: vec![], + }; lt_finder.visit_ty(&ty); if let Some(ty) = lt_finder.found { @@ -3155,14 +3220,16 @@ pub(super) fn signal_lifetime_shadowing(sess: &Session, orig: Ident, shadower: I struct LifetimeFinder<'ast> { lifetime: Span, found: Option<&'ast Ty>, + seen: Vec<&'ast Ty>, } impl<'ast> Visitor<'ast> for LifetimeFinder<'ast> { fn visit_ty(&mut self, t: &'ast Ty) { - if t.span.lo() == self.lifetime.lo() - && let TyKind::Ref(_, mut_ty) = &t.kind - { - self.found = Some(&mut_ty.ty); + if let TyKind::Ref(_, mut_ty) = &t.kind { + self.seen.push(t); + if t.span.lo() == self.lifetime.lo() { + self.found = Some(&mut_ty.ty); + } } walk_ty(self, t) } diff --git a/tests/ui/suggestions/impl-trait-missing-lifetime-gated.stderr b/tests/ui/suggestions/impl-trait-missing-lifetime-gated.stderr index c4d156825a339..3425092366f6a 100644 --- a/tests/ui/suggestions/impl-trait-missing-lifetime-gated.stderr +++ b/tests/ui/suggestions/impl-trait-missing-lifetime-gated.stderr @@ -9,10 +9,10 @@ help: consider using the `'static` lifetime, but this is uncommon unless you're | LL | fn g(mut x: impl Iterator) -> Option<&'static ()> { x.next() } | +++++++ -help: instead, you are more likely to want to change the argument to be borrowed... +help: consider introducing a named lifetime parameter | -LL | fn g(mut x: &impl Iterator) -> Option<&()> { x.next() } - | + +LL | fn g<'a>(mut x: impl Iterator) -> Option<&'a ()> { x.next() } + | ++++ ~~~ ~~~ help: ...or alternatively, to want to return an owned value | LL - fn g(mut x: impl Iterator) -> Option<&()> { x.next() } @@ -30,10 +30,10 @@ help: consider using the `'static` lifetime, but this is uncommon unless you're | LL | async fn i(mut x: impl Iterator) -> Option<&'static ()> { x.next() } | +++++++ -help: instead, you are more likely to want to change the argument to be borrowed... +help: consider introducing a named lifetime parameter | -LL | async fn i(mut x: &impl Iterator) -> Option<&()> { x.next() } - | + +LL | async fn i<'a>(mut x: impl Iterator) -> Option<&'a ()> { x.next() } + | ++++ ~~~ ~~~ help: ...or alternatively, to want to return an owned value | LL - async fn i(mut x: impl Iterator) -> Option<&()> { x.next() } @@ -75,10 +75,10 @@ help: consider using the `'static` lifetime, but this is uncommon unless you're | LL | fn g(mut x: impl Foo) -> Option<&'static ()> { x.next() } | +++++++ -help: instead, you are more likely to want to change the argument to be borrowed... +help: consider introducing a named lifetime parameter | -LL | fn g(mut x: &impl Foo) -> Option<&()> { x.next() } - | + +LL | fn g<'a>(mut x: impl Foo) -> Option<&'a ()> { x.next() } + | ++++ ~~~ help: ...or alternatively, to want to return an owned value | LL - fn g(mut x: impl Foo) -> Option<&()> { x.next() } @@ -96,10 +96,10 @@ help: consider using the `'static` lifetime, but this is uncommon unless you're | LL | fn g(mut x: impl Foo<()>) -> Option<&'static ()> { x.next() } | +++++++ -help: instead, you are more likely to want to change the argument to be borrowed... +help: consider introducing a named lifetime parameter | -LL | fn g(mut x: &impl Foo<()>) -> Option<&()> { x.next() } - | + +LL | fn g<'a>(mut x: impl Foo<()>) -> Option<&'a ()> { x.next() } + | ++++ ~~~ help: ...or alternatively, to want to return an owned value | LL - fn g(mut x: impl Foo<()>) -> Option<&()> { x.next() } From d30252e3593af0c335f86de4242d581ffa11e270 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Tue, 14 Nov 2023 22:41:45 +0000 Subject: [PATCH 3/9] Tweak wording --- compiler/rustc_resolve/src/late/diagnostics.rs | 8 ++++---- tests/ui/c-variadic/variadic-ffi-6.stderr | 2 +- tests/ui/lifetimes/issue-26638.stderr | 2 +- ...-elision-return-type-requires-explicit-lifetime.stderr | 8 ++++---- tests/ui/self/elision/nested-item.stderr | 2 +- .../suggestions/impl-trait-missing-lifetime-gated.stderr | 8 ++++---- tests/ui/suggestions/return-elided-lifetime.stderr | 2 +- 7 files changed, 16 insertions(+), 16 deletions(-) diff --git a/compiler/rustc_resolve/src/late/diagnostics.rs b/compiler/rustc_resolve/src/late/diagnostics.rs index 2998609c7c81f..2cd3e67338252 100644 --- a/compiler/rustc_resolve/src/late/diagnostics.rs +++ b/compiler/rustc_resolve/src/late/diagnostics.rs @@ -3012,7 +3012,7 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { sugg, Applicability::MaybeIncorrect, ); - "...or alternatively," + "...or alternatively, you might want" } else if let Some((kind, _span)) = self.diagnostic_metadata.current_function && let FnKind::Fn(_, _, sig, _, _, _) = kind @@ -3070,9 +3070,9 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { higher_ranked }, ); - "...or alternatively," + "alternatively, you might want" } else { - "instead, you are more likely" + "instead, you are more likely to want" }; let mut sugg = vec![(lt.span, String::new())]; if let Some((kind, _span)) = @@ -3105,7 +3105,7 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { } }; err.multipart_suggestion_verbose( - format!("{pre} to want to return an owned value"), + format!("{pre} to return an owned value"), sugg, Applicability::MaybeIncorrect, ); diff --git a/tests/ui/c-variadic/variadic-ffi-6.stderr b/tests/ui/c-variadic/variadic-ffi-6.stderr index b3497d31a0cbf..3511127a9fa32 100644 --- a/tests/ui/c-variadic/variadic-ffi-6.stderr +++ b/tests/ui/c-variadic/variadic-ffi-6.stderr @@ -13,7 +13,7 @@ help: instead, you are more likely to want to change one of the arguments to be | LL | x: &usize, | + -help: ...or alternatively, to want to return an owned value +help: ...or alternatively, you might want to return an owned value | LL - ) -> &usize { LL + ) -> usize { diff --git a/tests/ui/lifetimes/issue-26638.stderr b/tests/ui/lifetimes/issue-26638.stderr index bea590237bc64..ee958686259aa 100644 --- a/tests/ui/lifetimes/issue-26638.stderr +++ b/tests/ui/lifetimes/issue-26638.stderr @@ -25,7 +25,7 @@ help: instead, you are more likely to want to change the argument to be borrowed | LL | fn parse_type_2(iter: &fn(&u8)->&u8) -> &str { iter() } | + -help: ...or alternatively, to want to return an owned value +help: ...or alternatively, you might want to return an owned value | LL | fn parse_type_2(iter: fn(&u8)->&u8) -> String { iter() } | ~~~~~~ diff --git a/tests/ui/lifetimes/lifetime-elision-return-type-requires-explicit-lifetime.stderr b/tests/ui/lifetimes/lifetime-elision-return-type-requires-explicit-lifetime.stderr index caba886a3f7c7..37ebb178ec621 100644 --- a/tests/ui/lifetimes/lifetime-elision-return-type-requires-explicit-lifetime.stderr +++ b/tests/ui/lifetimes/lifetime-elision-return-type-requires-explicit-lifetime.stderr @@ -54,7 +54,7 @@ help: instead, you are more likely to want to change the argument to be borrowed | LL | fn i(_x: &isize) -> &isize { | + -help: ...or alternatively, to want to return an owned value +help: ...or alternatively, you might want to return an owned value | LL - fn i(_x: isize) -> &isize { LL + fn i(_x: isize) -> isize { @@ -75,7 +75,7 @@ help: instead, you are more likely to want to change the argument to be borrowed | LL | fn j(_x: &StaticStr) -> &isize { | + -help: ...or alternatively, to want to return an owned value +help: ...or alternatively, you might want to return an owned value | LL - fn j(_x: StaticStr) -> &isize { LL + fn j(_x: StaticStr) -> isize { @@ -96,7 +96,7 @@ help: instead, you are more likely to want to change the argument to be borrowed | LL | fn k<'a, T: WithLifetime<'a>>(_x: &T::Output) -> &isize { | + -help: ...or alternatively, to want to return an owned value +help: ...or alternatively, you might want to return an owned value | LL - fn k<'a, T: WithLifetime<'a>>(_x: T::Output) -> &isize { LL + fn k<'a, T: WithLifetime<'a>>(_x: T::Output) -> isize { @@ -117,7 +117,7 @@ help: instead, you are more likely to want to change one of the arguments to be | LL | fn l<'a>(_: &&'a str, _: &&'a str) -> &str { "" } | + + -help: ...or alternatively, to want to return an owned value +help: ...or alternatively, you might want to return an owned value | LL | fn l<'a>(_: &'a str, _: &'a str) -> String { "" } | ~~~~~~ diff --git a/tests/ui/self/elision/nested-item.stderr b/tests/ui/self/elision/nested-item.stderr index 5b7845559105b..7bad26fa13303 100644 --- a/tests/ui/self/elision/nested-item.stderr +++ b/tests/ui/self/elision/nested-item.stderr @@ -29,7 +29,7 @@ help: instead, you are more likely to want to change the argument to be borrowed | LL | fn wrap(self: &Wrap<{ fn bar(&self) {} }>) -> &() { | + -help: ...or alternatively, to want to return an owned value +help: ...or alternatively, you might want to return an owned value | LL - fn wrap(self: Wrap<{ fn bar(&self) {} }>) -> &() { LL + fn wrap(self: Wrap<{ fn bar(&self) {} }>) -> () { diff --git a/tests/ui/suggestions/impl-trait-missing-lifetime-gated.stderr b/tests/ui/suggestions/impl-trait-missing-lifetime-gated.stderr index 3425092366f6a..a1ab43922147e 100644 --- a/tests/ui/suggestions/impl-trait-missing-lifetime-gated.stderr +++ b/tests/ui/suggestions/impl-trait-missing-lifetime-gated.stderr @@ -13,7 +13,7 @@ help: consider introducing a named lifetime parameter | LL | fn g<'a>(mut x: impl Iterator) -> Option<&'a ()> { x.next() } | ++++ ~~~ ~~~ -help: ...or alternatively, to want to return an owned value +help: alternatively, you might want to return an owned value | LL - fn g(mut x: impl Iterator) -> Option<&()> { x.next() } LL + fn g(mut x: impl Iterator) -> Option<()> { x.next() } @@ -34,7 +34,7 @@ help: consider introducing a named lifetime parameter | LL | async fn i<'a>(mut x: impl Iterator) -> Option<&'a ()> { x.next() } | ++++ ~~~ ~~~ -help: ...or alternatively, to want to return an owned value +help: alternatively, you might want to return an owned value | LL - async fn i(mut x: impl Iterator) -> Option<&()> { x.next() } LL + async fn i(mut x: impl Iterator) -> Option<()> { x.next() } @@ -79,7 +79,7 @@ help: consider introducing a named lifetime parameter | LL | fn g<'a>(mut x: impl Foo) -> Option<&'a ()> { x.next() } | ++++ ~~~ -help: ...or alternatively, to want to return an owned value +help: alternatively, you might want to return an owned value | LL - fn g(mut x: impl Foo) -> Option<&()> { x.next() } LL + fn g(mut x: impl Foo) -> Option<()> { x.next() } @@ -100,7 +100,7 @@ help: consider introducing a named lifetime parameter | LL | fn g<'a>(mut x: impl Foo<()>) -> Option<&'a ()> { x.next() } | ++++ ~~~ -help: ...or alternatively, to want to return an owned value +help: alternatively, you might want to return an owned value | LL - fn g(mut x: impl Foo<()>) -> Option<&()> { x.next() } LL + fn g(mut x: impl Foo<()>) -> Option<()> { x.next() } diff --git a/tests/ui/suggestions/return-elided-lifetime.stderr b/tests/ui/suggestions/return-elided-lifetime.stderr index 54e0cfdf9a326..7bfffd30184f8 100644 --- a/tests/ui/suggestions/return-elided-lifetime.stderr +++ b/tests/ui/suggestions/return-elided-lifetime.stderr @@ -44,7 +44,7 @@ help: instead, you are more likely to want to change one of the arguments to be | LL | fn f2(a: &i32, b: &i32) -> &i32 { loop {} } | + + -help: ...or alternatively, to want to return an owned value +help: ...or alternatively, you might want to return an owned value | LL - fn f2(a: i32, b: i32) -> &i32 { loop {} } LL + fn f2(a: i32, b: i32) -> i32 { loop {} } From dec7f00e158f04054320f63796fea1445ac18917 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Wed, 15 Nov 2023 04:07:19 +0000 Subject: [PATCH 4/9] Fix incorrect lifetime suggestion --- .../rustc_resolve/src/late/diagnostics.rs | 4 ++-- ...urn-type-requires-explicit-lifetime.stderr | 21 ++----------------- 2 files changed, 4 insertions(+), 21 deletions(-) diff --git a/compiler/rustc_resolve/src/late/diagnostics.rs b/compiler/rustc_resolve/src/late/diagnostics.rs index 2cd3e67338252..ce981c333d88a 100644 --- a/compiler/rustc_resolve/src/late/diagnostics.rs +++ b/compiler/rustc_resolve/src/late/diagnostics.rs @@ -2853,8 +2853,8 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { "this function's return type contains a borrowed value, but there is no value \ for it to be borrowed from", ); - maybe_static = true; if in_scope_lifetimes.is_empty() { + maybe_static = true; in_scope_lifetimes = vec![( Ident::with_dummy_span(kw::StaticLifetime), (DUMMY_NODE_ID, LifetimeRes::Static), @@ -2865,8 +2865,8 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { "this function's return type contains a borrowed value with an elided \ lifetime, but the lifetime cannot be derived from the arguments", ); - maybe_static = true; if in_scope_lifetimes.is_empty() { + maybe_static = true; in_scope_lifetimes = vec![( Ident::with_dummy_span(kw::StaticLifetime), (DUMMY_NODE_ID, LifetimeRes::Static), diff --git a/tests/ui/lifetimes/lifetime-elision-return-type-requires-explicit-lifetime.stderr b/tests/ui/lifetimes/lifetime-elision-return-type-requires-explicit-lifetime.stderr index 37ebb178ec621..23ef36888f079 100644 --- a/tests/ui/lifetimes/lifetime-elision-return-type-requires-explicit-lifetime.stderr +++ b/tests/ui/lifetimes/lifetime-elision-return-type-requires-explicit-lifetime.stderr @@ -88,19 +88,10 @@ LL | fn k<'a, T: WithLifetime<'a>>(_x: T::Output) -> &isize { | ^ expected named lifetime parameter | = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from -help: consider using the `'a` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static` +help: consider using the `'a` lifetime | LL | fn k<'a, T: WithLifetime<'a>>(_x: T::Output) -> &'a isize { | ++ -help: instead, you are more likely to want to change the argument to be borrowed... - | -LL | fn k<'a, T: WithLifetime<'a>>(_x: &T::Output) -> &isize { - | + -help: ...or alternatively, you might want to return an owned value - | -LL - fn k<'a, T: WithLifetime<'a>>(_x: T::Output) -> &isize { -LL + fn k<'a, T: WithLifetime<'a>>(_x: T::Output) -> isize { - | error[E0106]: missing lifetime specifier --> $DIR/lifetime-elision-return-type-requires-explicit-lifetime.rs:45:37 @@ -109,18 +100,10 @@ LL | fn l<'a>(_: &'a str, _: &'a str) -> &str { "" } | ------- ------- ^ expected named lifetime parameter | = help: this function's return type contains a borrowed value with an elided lifetime, but the lifetime cannot be derived from the arguments -help: consider using the `'a` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static` +help: consider using the `'a` lifetime | LL | fn l<'a>(_: &'a str, _: &'a str) -> &'a str { "" } | ++ -help: instead, you are more likely to want to change one of the arguments to be borrowed... - | -LL | fn l<'a>(_: &&'a str, _: &&'a str) -> &str { "" } - | + + -help: ...or alternatively, you might want to return an owned value - | -LL | fn l<'a>(_: &'a str, _: &'a str) -> String { "" } - | ~~~~~~ error: aborting due to 7 previous errors From 2a92d820c79e56be8191bdf64e8b2dc7c96210df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Wed, 15 Nov 2023 16:56:41 +0000 Subject: [PATCH 5/9] Suggest 'a when trait object assoc type has '_ --- .../rustc_resolve/src/late/diagnostics.rs | 33 ++++++++++++------- .../impl-trait-missing-lifetime-gated.stderr | 8 +++++ .../impl-trait-missing-lifetime.stderr | 8 +++++ 3 files changed, 37 insertions(+), 12 deletions(-) diff --git a/compiler/rustc_resolve/src/late/diagnostics.rs b/compiler/rustc_resolve/src/late/diagnostics.rs index ce981c333d88a..984516baa18f4 100644 --- a/compiler/rustc_resolve/src/late/diagnostics.rs +++ b/compiler/rustc_resolve/src/late/diagnostics.rs @@ -2971,9 +2971,13 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { // we identified that the return expression references only one argument, we // would suggest borrowing only that argument, and we'd skip the prior // "use `'static`" suggestion entirely. - if let [lt] = &lifetime_refs[..] && lt.kind == MissingLifetimeKind::Ampersand { - let pre = if let Some((kind, _span)) = - self.diagnostic_metadata.current_function + if let [lt] = &lifetime_refs[..] + && (lt.kind == MissingLifetimeKind::Ampersand + || lt.kind == MissingLifetimeKind::Underscore) + { + let pre = if lt.kind == MissingLifetimeKind::Ampersand + && let Some((kind, _span)) = + self.diagnostic_metadata.current_function && let FnKind::Fn(_, _, sig, _, _, _) = kind && !sig.decl.inputs.is_empty() && let sugg = sig @@ -3013,8 +3017,10 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { Applicability::MaybeIncorrect, ); "...or alternatively, you might want" - } else if let Some((kind, _span)) = - self.diagnostic_metadata.current_function + } else if (lt.kind == MissingLifetimeKind::Ampersand + || lt.kind == MissingLifetimeKind::Underscore) + && let Some((kind, _span)) = + self.diagnostic_metadata.current_function && let FnKind::Fn(_, _, sig, _, _, _) = kind && let ast::FnRetTy::Ty(ret_ty) = &sig.decl.output && !sig.decl.inputs.is_empty() @@ -3059,7 +3065,6 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { err, None, |err, higher_ranked, span, message, intro_sugg| { - info!(?span, ?message, ?intro_sugg); err.multipart_suggestion_verbose( message, std::iter::once((span, intro_sugg)) @@ -3093,7 +3098,9 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { && segments[0].ident.name == sym::str { // Don't suggest `-> str`, suggest `-> String`. - sugg = vec![(lt.span.with_hi(ty.span.hi()), "String".to_string())]; + sugg = vec![ + (lt.span.with_hi(ty.span.hi()), "String".to_string()), + ]; } if let TyKind::Slice(inner_ty) = &ty.kind { // Don't suggest `-> [T]`, suggest `-> Vec`. @@ -3104,11 +3111,13 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { } } }; - err.multipart_suggestion_verbose( - format!("{pre} to return an owned value"), - sugg, - Applicability::MaybeIncorrect, - ); + if lt.kind == MissingLifetimeKind::Ampersand { + err.multipart_suggestion_verbose( + format!("{pre} to return an owned value"), + sugg, + Applicability::MaybeIncorrect, + ); + } } } diff --git a/tests/ui/suggestions/impl-trait-missing-lifetime-gated.stderr b/tests/ui/suggestions/impl-trait-missing-lifetime-gated.stderr index a1ab43922147e..383cba75e285b 100644 --- a/tests/ui/suggestions/impl-trait-missing-lifetime-gated.stderr +++ b/tests/ui/suggestions/impl-trait-missing-lifetime-gated.stderr @@ -51,6 +51,10 @@ help: consider using the `'static` lifetime, but this is uncommon unless you're | LL | fn g(mut x: impl Iterator) -> Option<&'static ()> { x.next() } | ~~~~~~~ +help: consider introducing a named lifetime parameter + | +LL | fn g<'a>(mut x: impl Iterator) -> Option<&'a ()> { x.next() } + | ++++ ~~~ ~~~ error[E0106]: missing lifetime specifier --> $DIR/impl-trait-missing-lifetime-gated.rs:37:64 @@ -63,6 +67,10 @@ help: consider using the `'static` lifetime, but this is uncommon unless you're | LL | async fn i(mut x: impl Iterator) -> Option<&'static ()> { x.next() } | ~~~~~~~ +help: consider introducing a named lifetime parameter + | +LL | async fn i<'a>(mut x: impl Iterator) -> Option<&'a ()> { x.next() } + | ++++ ~~~ ~~~ error[E0106]: missing lifetime specifier --> $DIR/impl-trait-missing-lifetime-gated.rs:47:37 diff --git a/tests/ui/suggestions/impl-trait-missing-lifetime.stderr b/tests/ui/suggestions/impl-trait-missing-lifetime.stderr index 37c2ad2aaeaaf..98b0ab82b3ce9 100644 --- a/tests/ui/suggestions/impl-trait-missing-lifetime.stderr +++ b/tests/ui/suggestions/impl-trait-missing-lifetime.stderr @@ -9,6 +9,10 @@ help: consider using the `'static` lifetime, but this is uncommon unless you're | LL | fn g(mut x: impl Iterator) -> Option<&'static ()> { x.next() } | ~~~~~~~ +help: consider introducing a named lifetime parameter + | +LL | fn g<'a>(mut x: impl Iterator) -> Option<&'a ()> { x.next() } + | ++++ ~~~ ~~~ error[E0106]: missing lifetime specifier --> $DIR/impl-trait-missing-lifetime.rs:16:60 @@ -21,6 +25,10 @@ help: consider using the `'static` lifetime, but this is uncommon unless you're | LL | async fn i(mut x: impl Iterator) -> Option<&'static ()> { x.next() } | ~~~~~~~ +help: consider introducing a named lifetime parameter + | +LL | async fn i<'a>(mut x: impl Iterator) -> Option<&'a ()> { x.next() } + | ++++ ~~~ ~~~ error: lifetime may not live long enough --> $DIR/impl-trait-missing-lifetime.rs:16:69 From 85f26ade8dc3364f29a2c0b0d8f5fe2068df8969 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Wed, 15 Nov 2023 18:00:43 +0000 Subject: [PATCH 6/9] Account for '_ in lifetime suggestion --- .../rustc_resolve/src/late/diagnostics.rs | 19 +++++++++++++++---- .../impl-trait-missing-lifetime-gated.stderr | 10 ++++++++++ .../impl-trait-missing-lifetime.stderr | 10 ++++++++++ 3 files changed, 35 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_resolve/src/late/diagnostics.rs b/compiler/rustc_resolve/src/late/diagnostics.rs index 984516baa18f4..f7560d9949b0b 100644 --- a/compiler/rustc_resolve/src/late/diagnostics.rs +++ b/compiler/rustc_resolve/src/late/diagnostics.rs @@ -3079,6 +3079,7 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { } else { "instead, you are more likely to want" }; + let mut owned_sugg = lt.kind == MissingLifetimeKind::Ampersand; let mut sugg = vec![(lt.span, String::new())]; if let Some((kind, _span)) = self.diagnostic_metadata.current_function @@ -3092,6 +3093,17 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { }; lt_finder.visit_ty(&ty); + if let [Ty { span, kind: TyKind::Ref(_, mut_ty), ..}] + = <_finder.seen[..] + { + // We might have a situation like + // fn g(mut x: impl Iterator) -> Option<&'_ ()> + // but `lt.span` only points at `'_`, so to suggest `-> Option<()>` + // we need to find a more accurate span to end up with + // fn g<'a>(mut x: impl Iterator) -> Option<()> + sugg = vec![(span.with_hi(mut_ty.ty.span.lo()), String::new())]; + owned_sugg = true; + } if let Some(ty) = lt_finder.found { if let TyKind::Path(None, Path { segments, .. }) = &ty.kind && segments.len() == 1 @@ -3101,8 +3113,7 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { sugg = vec![ (lt.span.with_hi(ty.span.hi()), "String".to_string()), ]; - } - if let TyKind::Slice(inner_ty) = &ty.kind { + } else if let TyKind::Slice(inner_ty) = &ty.kind { // Don't suggest `-> [T]`, suggest `-> Vec`. sugg = vec![ (lt.span.with_hi(inner_ty.span.lo()), "Vec<".to_string()), @@ -3110,8 +3121,8 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { ]; } } - }; - if lt.kind == MissingLifetimeKind::Ampersand { + } + if owned_sugg { err.multipart_suggestion_verbose( format!("{pre} to return an owned value"), sugg, diff --git a/tests/ui/suggestions/impl-trait-missing-lifetime-gated.stderr b/tests/ui/suggestions/impl-trait-missing-lifetime-gated.stderr index 383cba75e285b..2dfaa7311948f 100644 --- a/tests/ui/suggestions/impl-trait-missing-lifetime-gated.stderr +++ b/tests/ui/suggestions/impl-trait-missing-lifetime-gated.stderr @@ -55,6 +55,11 @@ help: consider introducing a named lifetime parameter | LL | fn g<'a>(mut x: impl Iterator) -> Option<&'a ()> { x.next() } | ++++ ~~~ ~~~ +help: alternatively, you might want to return an owned value + | +LL - fn g(mut x: impl Iterator) -> Option<&'_ ()> { x.next() } +LL + fn g(mut x: impl Iterator) -> Option<()> { x.next() } + | error[E0106]: missing lifetime specifier --> $DIR/impl-trait-missing-lifetime-gated.rs:37:64 @@ -71,6 +76,11 @@ help: consider introducing a named lifetime parameter | LL | async fn i<'a>(mut x: impl Iterator) -> Option<&'a ()> { x.next() } | ++++ ~~~ ~~~ +help: alternatively, you might want to return an owned value + | +LL - async fn i(mut x: impl Iterator) -> Option<&'_ ()> { x.next() } +LL + async fn i(mut x: impl Iterator) -> Option<()> { x.next() } + | error[E0106]: missing lifetime specifier --> $DIR/impl-trait-missing-lifetime-gated.rs:47:37 diff --git a/tests/ui/suggestions/impl-trait-missing-lifetime.stderr b/tests/ui/suggestions/impl-trait-missing-lifetime.stderr index 98b0ab82b3ce9..c1dbaae0649a0 100644 --- a/tests/ui/suggestions/impl-trait-missing-lifetime.stderr +++ b/tests/ui/suggestions/impl-trait-missing-lifetime.stderr @@ -13,6 +13,11 @@ help: consider introducing a named lifetime parameter | LL | fn g<'a>(mut x: impl Iterator) -> Option<&'a ()> { x.next() } | ++++ ~~~ ~~~ +help: alternatively, you might want to return an owned value + | +LL - fn g(mut x: impl Iterator) -> Option<&'_ ()> { x.next() } +LL + fn g(mut x: impl Iterator) -> Option<()> { x.next() } + | error[E0106]: missing lifetime specifier --> $DIR/impl-trait-missing-lifetime.rs:16:60 @@ -29,6 +34,11 @@ help: consider introducing a named lifetime parameter | LL | async fn i<'a>(mut x: impl Iterator) -> Option<&'a ()> { x.next() } | ++++ ~~~ ~~~ +help: alternatively, you might want to return an owned value + | +LL - async fn i(mut x: impl Iterator) -> Option<&'_ ()> { x.next() } +LL + async fn i(mut x: impl Iterator) -> Option<()> { x.next() } + | error: lifetime may not live long enough --> $DIR/impl-trait-missing-lifetime.rs:16:69 From bb9d720a161221dac6ad7f0fb1cb5e94ab157ab2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Wed, 15 Nov 2023 20:09:19 +0000 Subject: [PATCH 7/9] Do not consider traits as ownable in suggestion --- .../rustc_resolve/src/late/diagnostics.rs | 62 ++++++++++++++++--- .../missing-lifetime-specifier.stderr | 5 -- 2 files changed, 55 insertions(+), 12 deletions(-) diff --git a/compiler/rustc_resolve/src/late/diagnostics.rs b/compiler/rustc_resolve/src/late/diagnostics.rs index f7560d9949b0b..86846aa214aed 100644 --- a/compiler/rustc_resolve/src/late/diagnostics.rs +++ b/compiler/rustc_resolve/src/late/diagnostics.rs @@ -3105,15 +3105,63 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { owned_sugg = true; } if let Some(ty) = lt_finder.found { - if let TyKind::Path(None, Path { segments, .. }) = &ty.kind + if let TyKind::Path(None, path @ Path { segments, .. }) = &ty.kind && segments.len() == 1 - && segments[0].ident.name == sym::str { - // Don't suggest `-> str`, suggest `-> String`. - sugg = vec![ - (lt.span.with_hi(ty.span.hi()), "String".to_string()), - ]; - } else if let TyKind::Slice(inner_ty) = &ty.kind { + if segments[0].ident.name == sym::str { + // Don't suggest `-> str`, suggest `-> String`. + sugg = vec![ + (lt.span.with_hi(ty.span.hi()), "String".to_string()), + ]; + } else { + // Check if the path being borrowed is likely to be owned. + let path: Vec<_> = Segment::from_path(path); + match self.resolve_path(&path, Some(TypeNS), None) { + PathResult::Module( + ModuleOrUniformRoot::Module(module), + ) => { + match module.res() { + Some(Res::PrimTy(..)) => {} + Some(Res::Def( + DefKind::Struct + | DefKind::Union + | DefKind::Enum + | DefKind::ForeignTy + | DefKind::AssocTy + | DefKind::OpaqueTy + | DefKind::TyParam, + _, + )) => {} + _ => { // Do not suggest in all other cases. + owned_sugg = false; + } + } + } + PathResult::NonModule(res) => { + match res.base_res() { + Res::PrimTy(..) => {} + Res::Def( + DefKind::Struct + | DefKind::Union + | DefKind::Enum + | DefKind::ForeignTy + | DefKind::AssocTy + | DefKind::OpaqueTy + | DefKind::TyParam, + _, + ) => {} + _ => { // Do not suggest in all other cases. + owned_sugg = false; + } + } + } + _ => { // Do not suggest in all other cases. + owned_sugg = false; + } + } + } + } + if let TyKind::Slice(inner_ty) = &ty.kind { // Don't suggest `-> [T]`, suggest `-> Vec`. sugg = vec![ (lt.span.with_hi(inner_ty.span.lo()), "Vec<".to_string()), diff --git a/tests/ui/suggestions/missing-lifetime-specifier.stderr b/tests/ui/suggestions/missing-lifetime-specifier.stderr index 484a9b3ad8ddb..e41f547ce9b23 100644 --- a/tests/ui/suggestions/missing-lifetime-specifier.stderr +++ b/tests/ui/suggestions/missing-lifetime-specifier.stderr @@ -117,11 +117,6 @@ help: consider using the `'static` lifetime, but this is uncommon unless you're | LL | static f: RefCell>>>> = RefCell::new(HashMap::new()); | +++++++ -help: instead, you are more likely to want to return an owned value - | -LL - static f: RefCell>>>> = RefCell::new(HashMap::new()); -LL + static f: RefCell>>>> = RefCell::new(HashMap::new()); - | error[E0106]: missing lifetime specifier --> $DIR/missing-lifetime-specifier.rs:47:44 From 02bea16c08d6c0202f7cb1658280bd7cf97560d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Wed, 15 Nov 2023 20:22:54 +0000 Subject: [PATCH 8/9] Rely in resolve and not on path name for `&str` -> `String` suggestion --- .../rustc_resolve/src/late/diagnostics.rs | 103 +++++++++--------- 1 file changed, 54 insertions(+), 49 deletions(-) diff --git a/compiler/rustc_resolve/src/late/diagnostics.rs b/compiler/rustc_resolve/src/late/diagnostics.rs index 86846aa214aed..1ffe8a9a26ef4 100644 --- a/compiler/rustc_resolve/src/late/diagnostics.rs +++ b/compiler/rustc_resolve/src/late/diagnostics.rs @@ -3105,60 +3105,65 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { owned_sugg = true; } if let Some(ty) = lt_finder.found { - if let TyKind::Path(None, path @ Path { segments, .. }) = &ty.kind - && segments.len() == 1 - { - if segments[0].ident.name == sym::str { - // Don't suggest `-> str`, suggest `-> String`. - sugg = vec![ - (lt.span.with_hi(ty.span.hi()), "String".to_string()), - ]; - } else { - // Check if the path being borrowed is likely to be owned. - let path: Vec<_> = Segment::from_path(path); - match self.resolve_path(&path, Some(TypeNS), None) { - PathResult::Module( - ModuleOrUniformRoot::Module(module), - ) => { - match module.res() { - Some(Res::PrimTy(..)) => {} - Some(Res::Def( - DefKind::Struct - | DefKind::Union - | DefKind::Enum - | DefKind::ForeignTy - | DefKind::AssocTy - | DefKind::OpaqueTy - | DefKind::TyParam, - _, - )) => {} - _ => { // Do not suggest in all other cases. - owned_sugg = false; - } + if let TyKind::Path(None, path) = &ty.kind { + // Check if the path being borrowed is likely to be owned. + let path: Vec<_> = Segment::from_path(path); + match self.resolve_path(&path, Some(TypeNS), None) { + PathResult::Module( + ModuleOrUniformRoot::Module(module), + ) => { + match module.res() { + Some(Res::PrimTy(PrimTy::Str)) => { + // Don't suggest `-> str`, suggest `-> String`. + sugg = vec![( + lt.span.with_hi(ty.span.hi()), + "String".to_string(), + )]; } - } - PathResult::NonModule(res) => { - match res.base_res() { - Res::PrimTy(..) => {} - Res::Def( - DefKind::Struct - | DefKind::Union - | DefKind::Enum - | DefKind::ForeignTy - | DefKind::AssocTy - | DefKind::OpaqueTy - | DefKind::TyParam, - _, - ) => {} - _ => { // Do not suggest in all other cases. - owned_sugg = false; - } + Some(Res::PrimTy(..)) => {} + Some(Res::Def( + DefKind::Struct + | DefKind::Union + | DefKind::Enum + | DefKind::ForeignTy + | DefKind::AssocTy + | DefKind::OpaqueTy + | DefKind::TyParam, + _, + )) => {} + _ => { // Do not suggest in all other cases. + owned_sugg = false; } } - _ => { // Do not suggest in all other cases. - owned_sugg = false; + } + PathResult::NonModule(res) => { + match res.base_res() { + Res::PrimTy(PrimTy::Str) => { + // Don't suggest `-> str`, suggest `-> String`. + sugg = vec![( + lt.span.with_hi(ty.span.hi()), + "String".to_string(), + )]; + } + Res::PrimTy(..) => {} + Res::Def( + DefKind::Struct + | DefKind::Union + | DefKind::Enum + | DefKind::ForeignTy + | DefKind::AssocTy + | DefKind::OpaqueTy + | DefKind::TyParam, + _, + ) => {} + _ => { // Do not suggest in all other cases. + owned_sugg = false; + } } } + _ => { // Do not suggest in all other cases. + owned_sugg = false; + } } } if let TyKind::Slice(inner_ty) = &ty.kind { From eee4cc661677be3bf58db12ea1ee598e822762ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 16 Nov 2023 16:55:25 +0000 Subject: [PATCH 9/9] let-chain fmt --- .../rustc_resolve/src/late/diagnostics.rs | 55 ++++++++----------- 1 file changed, 24 insertions(+), 31 deletions(-) diff --git a/compiler/rustc_resolve/src/late/diagnostics.rs b/compiler/rustc_resolve/src/late/diagnostics.rs index 1ffe8a9a26ef4..bccce8699c52f 100644 --- a/compiler/rustc_resolve/src/late/diagnostics.rs +++ b/compiler/rustc_resolve/src/late/diagnostics.rs @@ -2976,8 +2976,7 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { || lt.kind == MissingLifetimeKind::Underscore) { let pre = if lt.kind == MissingLifetimeKind::Ampersand - && let Some((kind, _span)) = - self.diagnostic_metadata.current_function + && let Some((kind, _span)) = self.diagnostic_metadata.current_function && let FnKind::Fn(_, _, sig, _, _, _) = kind && !sig.decl.inputs.is_empty() && let sugg = sig @@ -3002,7 +3001,6 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { .collect::>() && !sugg.is_empty() { - let (the, s) = if sig.decl.inputs.len() == 1 { ("the", "") } else { @@ -3018,9 +3016,8 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { ); "...or alternatively, you might want" } else if (lt.kind == MissingLifetimeKind::Ampersand - || lt.kind == MissingLifetimeKind::Underscore) - && let Some((kind, _span)) = - self.diagnostic_metadata.current_function + || lt.kind == MissingLifetimeKind::Underscore) + && let Some((kind, _span)) = self.diagnostic_metadata.current_function && let FnKind::Fn(_, _, sig, _, _, _) = kind && let ast::FnRetTy::Ty(ret_ty) = &sig.decl.output && !sig.decl.inputs.is_empty() @@ -3041,26 +3038,25 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { // So we look at every ref in the trait bound. If there's any, we // suggest // fn g<'a>(mut x: impl Iterator) -> Option<&'a ()> - let mut lt_finder = LifetimeFinder { - lifetime: lt.span, - found: None, - seen: vec![], - }; + let mut lt_finder = + LifetimeFinder { lifetime: lt.span, found: None, seen: vec![] }; for bound in arg_refs { if let ast::GenericBound::Trait(trait_ref, _) = bound { lt_finder.visit_trait_ref(&trait_ref.trait_ref); } } lt_finder.visit_ty(ret_ty); - let spans_suggs: Vec<_> = lt_finder.seen.iter().filter_map(|ty| { - match &ty.kind { + let spans_suggs: Vec<_> = lt_finder + .seen + .iter() + .filter_map(|ty| match &ty.kind { TyKind::Ref(_, mut_ty) => { let span = ty.span.with_hi(mut_ty.ty.span.lo()); Some((span, "&'a ".to_string())) } - _ => None - } - }).collect(); + _ => None, + }) + .collect(); self.suggest_introducing_lifetime( err, None, @@ -3081,20 +3077,16 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { }; let mut owned_sugg = lt.kind == MissingLifetimeKind::Ampersand; let mut sugg = vec![(lt.span, String::new())]; - if let Some((kind, _span)) = - self.diagnostic_metadata.current_function + if let Some((kind, _span)) = self.diagnostic_metadata.current_function && let FnKind::Fn(_, _, sig, _, _, _) = kind && let ast::FnRetTy::Ty(ty) = &sig.decl.output { - let mut lt_finder = LifetimeFinder { - lifetime: lt.span, - found: None, - seen: vec![], - }; + let mut lt_finder = + LifetimeFinder { lifetime: lt.span, found: None, seen: vec![] }; lt_finder.visit_ty(&ty); - if let [Ty { span, kind: TyKind::Ref(_, mut_ty), ..}] - = <_finder.seen[..] + if let [Ty { span, kind: TyKind::Ref(_, mut_ty), .. }] = + <_finder.seen[..] { // We might have a situation like // fn g(mut x: impl Iterator) -> Option<&'_ ()> @@ -3109,9 +3101,7 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { // Check if the path being borrowed is likely to be owned. let path: Vec<_> = Segment::from_path(path); match self.resolve_path(&path, Some(TypeNS), None) { - PathResult::Module( - ModuleOrUniformRoot::Module(module), - ) => { + PathResult::Module(ModuleOrUniformRoot::Module(module)) => { match module.res() { Some(Res::PrimTy(PrimTy::Str)) => { // Don't suggest `-> str`, suggest `-> String`. @@ -3131,7 +3121,8 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { | DefKind::TyParam, _, )) => {} - _ => { // Do not suggest in all other cases. + _ => { + // Do not suggest in all other cases. owned_sugg = false; } } @@ -3156,12 +3147,14 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { | DefKind::TyParam, _, ) => {} - _ => { // Do not suggest in all other cases. + _ => { + // Do not suggest in all other cases. owned_sugg = false; } } } - _ => { // Do not suggest in all other cases. + _ => { + // Do not suggest in all other cases. owned_sugg = false; } }