When suggesting for lts, consider existing lifetime names

Fix #72404.

Author: estebank

src/librustc_resolve/late/diagnostics.rs

@@ -16,7 +16,7 @@ use rustc_hir::def_id::{DefId, CRATE_DEF_INDEX, LOCAL_CRATE};
 use rustc_hir::PrimTy;
 use rustc_session::config::nightly_options;
 use rustc_span::hygiene::MacroKind;
-use rustc_span::symbol::{kw, sym, Ident};
+use rustc_span::symbol::{kw, sym, Ident, Symbol};
 use rustc_span::{BytePos, Span, DUMMY_SP};
 
 use log::debug;
@@ -1244,7 +1244,8 @@ impl<'tcx> LifetimeContext<'_, 'tcx> {
         err: &mut DiagnosticBuilder<'_>,
         span: Span,
         count: usize,
-        lifetime_names: &FxHashSet<Ident>,
+        lifetime_names: &FxHashSet<Symbol>,
+        lifetime_spans: Vec<Span>,
         params: &[ElisionFailureInfo],
     ) {
         let snippet = self.tcx.sess.source_map().span_to_snippet(span).ok();
@@ -1258,11 +1259,60 @@ impl<'tcx> LifetimeContext<'_, 'tcx> {
             ),
         );
 
-        let suggest_existing = |err: &mut DiagnosticBuilder<'_>, sugg| {
+        let suggest_existing = |err: &mut DiagnosticBuilder<'_>,
+                                name: &str,
+                                formatter: &dyn Fn(&str) -> String| {
+            if let Some(MissingLifetimeSpot::HigherRanked { span: for_span, span_type }) =
+                self.missing_named_lifetime_spots.iter().rev().next()
+            {
+                // When we have `struct S<'a>(&'a dyn Fn(&X) -> &X);` we want to not only suggest
+                // using `'a`, but also introduce the concept of HRLTs by suggesting
+                // `struct S<'a>(&'a dyn for<'b> Fn(&X) -> &'b X);`. (#72404)
+                let mut introduce_suggestion = vec![];
+
+                let a_to_z_repeat_n = |n| {
+                    (b'a'..=b'z').map(move |c| {
+                        let mut s = '\''.to_string();
+                        s.extend(std::iter::repeat(char::from(c)).take(n));
+                        s
+                    })
+                };
+
+                // If all single char lifetime names are present, we wrap around and double the chars.
+                let lt_name = (1..)
+                    .flat_map(a_to_z_repeat_n)
+                    .find(|lt| !lifetime_names.contains(&Symbol::intern(&lt)))
+                    .unwrap();
+                let msg = format!(
+                    "consider making the {} lifetime-generic with a new `{}` lifetime",
+                    span_type.descr(),
+                    lt_name,
+                );
+                err.note(
+                    "for more information on higher-ranked polymorphism, visit \
+                    https://doc.rust-lang.org/nomicon/hrtb.html",
+                );
+                let for_sugg = span_type.suggestion(&lt_name);
+                for param in params {
+                    if let Ok(snippet) = self.tcx.sess.source_map().span_to_snippet(param.span) {
+                        if snippet.starts_with('&') && !snippet.starts_with("&'") {
+                            introduce_suggestion
+                                .push((param.span, format!("&{} {}", lt_name, &snippet[1..])));
+                        } else if snippet.starts_with("&'_ ") {
+                            introduce_suggestion
+                                .push((param.span, format!("&{} {}", lt_name, &snippet[4..])));
+                        }
+                    }
+                }
+                introduce_suggestion.push((*for_span, for_sugg.to_string()));
+                introduce_suggestion.push((span, formatter(&lt_name)));
+                err.multipart_suggestion(&msg, introduce_suggestion, Applicability::MaybeIncorrect);
+            }
+
             err.span_suggestion_verbose(
                 span,
                 &format!("consider using the `{}` lifetime", lifetime_names.iter().next().unwrap()),
-                sugg,
+                formatter(name),
                 Applicability::MaybeIncorrect,
             );
         };
@@ -1330,26 +1380,26 @@ impl<'tcx> LifetimeContext<'_, 'tcx> {
 
         match (lifetime_names.len(), lifetime_names.iter().next(), snippet.as_deref()) {
             (1, Some(name), Some("&")) => {
-                suggest_existing(err, format!("&{} ", name));
+                suggest_existing(err, &name.as_str()[..], &|name| format!("&{} ", name));
             }
             (1, Some(name), Some("'_")) => {
-                suggest_existing(err, name.to_string());
+                suggest_existing(err, &name.as_str()[..], &|n| n.to_string());
             }
             (1, Some(name), Some("")) => {
-                suggest_existing(err, format!("{}, ", name).repeat(count));
+                suggest_existing(err, &name.as_str()[..], &|n| format!("{}, ", n).repeat(count));
             }
             (1, Some(name), Some(snippet)) if !snippet.ends_with('>') => {
-                suggest_existing(
-                    err,
+                let f = |name: &str| {
                     format!(
                         "{}<{}>",
                         snippet,
                         std::iter::repeat(name.to_string())
                             .take(count)
                             .collect::<Vec<_>>()
                             .join(", ")
-                    ),
-                );
+                    )
+                };
+                suggest_existing(err, &name.as_str()[..], &f);
             }
             (0, _, Some("&")) if count == 1 => {
                 suggest_new(err, "&'a ");
@@ -1367,8 +1417,7 @@ impl<'tcx> LifetimeContext<'_, 'tcx> {
                 }
             }
             (n, ..) if n > 1 => {
-                let spans: Vec<Span> = lifetime_names.iter().map(|lt| lt.span).collect();
-                err.span_note(spans, "these named lifetimes are available to use");
+                err.span_note(lifetime_spans, "these named lifetimes are available to use");
                 if Some("") == snippet.as_deref() {
                     // This happens when we have `Foo<T>` where we point at the space before `T`,
                     // but this can be confusing so we give a suggestion with placeholders.

src/librustc_resolve/late/lifetimes.rs

@@ -2317,6 +2317,7 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> {
         let mut late_depth = 0;
         let mut scope = self.scope;
         let mut lifetime_names = FxHashSet::default();
+        let mut lifetime_spans = vec![];
         let error = loop {
             match *scope {
                 // Do not assign any resolution, it will be inferred.
@@ -2328,7 +2329,8 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> {
                     // collect named lifetimes for suggestions
                     for name in lifetimes.keys() {
                         if let hir::ParamName::Plain(name) = name {
-                            lifetime_names.insert(*name);
+                            lifetime_names.insert(name.name);
+                            lifetime_spans.push(name.span);
                         }
                     }
                     late_depth += 1;
@@ -2346,12 +2348,24 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> {
                         }
                         Elide::Exact(l) => l.shifted(late_depth),
                         Elide::Error(ref e) => {
-                            if let Scope::Binder { ref lifetimes, .. } = s {
-                                // collect named lifetimes for suggestions
-                                for name in lifetimes.keys() {
-                                    if let hir::ParamName::Plain(name) = name {
-                                        lifetime_names.insert(*name);
+                            let mut scope = s;
+                            loop {
+                                match scope {
+                                    Scope::Binder { ref lifetimes, s, .. } => {
+                                        // Collect named lifetimes for suggestions.
+                                        for name in lifetimes.keys() {
+                                            if let hir::ParamName::Plain(name) = name {
+                                                lifetime_names.insert(name.name);
+                                                lifetime_spans.push(name.span);
+                                            }
+                                        }
+                                        scope = s;
+                                    }
+                                    Scope::ObjectLifetimeDefault { ref s, .. }
+                                    | Scope::Elision { ref s, .. } => {
+                                        scope = s;
                                     }
+                                    _ => break,
                                 }
                             }
                             break Some(e);
@@ -2375,14 +2389,15 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> {
         if let Some(params) = error {
             // If there's no lifetime available, suggest `'static`.
             if self.report_elision_failure(&mut err, params) && lifetime_names.is_empty() {
-                lifetime_names.insert(Ident::with_dummy_span(kw::StaticLifetime));
+                lifetime_names.insert(kw::StaticLifetime);
             }
         }
         self.add_missing_lifetime_specifiers_label(
             &mut err,
             span,
             lifetime_refs.len(),
             &lifetime_names,
+            lifetime_spans,
             error.map(|p| &p[..]).unwrap_or(&[]),
         );
         err.emit();

src/test/ui/associated-types/bound-lifetime-in-binding-only.elision.stderr

@@ -5,6 +5,11 @@ LL | fn elision<T: 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
+   = note: for more information on higher-ranked polymorphism, visit https://doc.rust-lang.org/nomicon/hrtb.html
+help: consider making the bound lifetime-generic with a new `'a` lifetime
+   |
+LL | fn elision<T: for<'a> Fn() -> &'a i32>() {
+   |               ^^^^^^^         ^^^
 help: consider using the `'static` lifetime
    |
 LL | fn elision<T: Fn() -> &'static i32>() {

src/test/ui/associated-types/bound-lifetime-in-return-only.elision.stderr

@@ -5,6 +5,11 @@ 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
+   = note: for more information on higher-ranked polymorphism, visit https://doc.rust-lang.org/nomicon/hrtb.html
+help: consider making the type lifetime-generic with a new `'a` lifetime
+   |
+LL | fn elision(_: for<'a> fn() -> &'a i32) {
+   |               ^^^^^^^         ^^^
 help: consider using the `'static` lifetime
    |
 LL | fn elision(_: fn() -> &'static i32) {

src/test/ui/suggestions/missing-lt-for-hrtb.rs

@@ -0,0 +1,15 @@
+struct X<'a>(&'a ());
+struct S<'a>(&'a dyn Fn(&X) -> &X);
+//~^ ERROR missing lifetime specifier
+//~| ERROR missing lifetime specifier
+struct V<'a>(&'a dyn for<'b> Fn(&X) -> &X);
+//~^ ERROR missing lifetime specifier
+//~| ERROR missing lifetime specifier
+
+fn main() {
+    let x = S(&|x| {
+        println!("hi");
+        x
+    });
+    x.0(&X(&()));
+}

src/test/ui/suggestions/missing-lt-for-hrtb.stderr

@@ -0,0 +1,63 @@
+error[E0106]: missing lifetime specifier
+  --> $DIR/missing-lt-for-hrtb.rs:2:32
+   |
+LL | struct S<'a>(&'a dyn Fn(&X) -> &X);
+   |                         --     ^ expected named lifetime parameter
+   |
+   = help: this function's return type contains a borrowed value, but the signature does not say which one of argument 1's 2 lifetimes it is borrowed from
+   = note: for more information on higher-ranked polymorphism, visit https://doc.rust-lang.org/nomicon/hrtb.html
+help: consider making the bound lifetime-generic with a new `'b` lifetime
+   |
+LL | struct S<'a>(&'a dyn for<'b> Fn(&'b X) -> &'b X);
+   |                      ^^^^^^^    ^^^^^     ^^^
+help: consider using the `'a` lifetime
+   |
+LL | struct S<'a>(&'a dyn Fn(&X) -> &'a X);
+   |                                ^^^
+
+error[E0106]: missing lifetime specifier
+  --> $DIR/missing-lt-for-hrtb.rs:2:33
+   |
+LL | struct S<'a>(&'a dyn Fn(&X) -> &X);
+   |                         --      ^ expected named lifetime parameter
+   |
+   = help: this function's return type contains a borrowed value, but the signature does not say which one of argument 1's 2 lifetimes it is borrowed from
+   = note: for more information on higher-ranked polymorphism, visit https://doc.rust-lang.org/nomicon/hrtb.html
+help: consider making the bound lifetime-generic with a new `'b` lifetime
+   |
+LL | struct S<'a>(&'a dyn for<'b> Fn(&'b X) -> &X<'b>);
+   |                      ^^^^^^^    ^^^^^      ^^^^^
+help: consider using the `'a` lifetime
+   |
+LL | struct S<'a>(&'a dyn Fn(&X) -> &X<'a>);
+   |                                 ^^^^^
+
+error[E0106]: missing lifetime specifier
+  --> $DIR/missing-lt-for-hrtb.rs:5:40
+   |
+LL | struct V<'a>(&'a dyn for<'b> Fn(&X) -> &X);
+   |                                 --     ^ expected named lifetime parameter
+   |
+   = help: this function's return type contains a borrowed value, but the signature does not say which one of argument 1's 2 lifetimes it is borrowed from
+note: these named lifetimes are available to use
+  --> $DIR/missing-lt-for-hrtb.rs:5:10
+   |
+LL | struct V<'a>(&'a dyn for<'b> Fn(&X) -> &X);
+   |          ^^              ^^
+
+error[E0106]: missing lifetime specifier
+  --> $DIR/missing-lt-for-hrtb.rs:5:41
+   |
+LL | struct V<'a>(&'a dyn for<'b> Fn(&X) -> &X);
+   |                                 --      ^ expected named lifetime parameter
+   |
+   = help: this function's return type contains a borrowed value, but the signature does not say which one of argument 1's 2 lifetimes it is borrowed from
+note: these named lifetimes are available to use
+  --> $DIR/missing-lt-for-hrtb.rs:5:10
+   |
+LL | struct V<'a>(&'a dyn for<'b> Fn(&X) -> &X);
+   |          ^^              ^^
+
+error: aborting due to 4 previous errors
+
+For more information about this error, try `rustc --explain E0106`.