From 0f7205f2020ff5acefac83e354acea4c69f2ea0e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Mon, 10 Aug 2020 12:30:31 -0700 Subject: [PATCH 1/5] Fix suggestion to use lifetime in type --- src/librustc_resolve/late/diagnostics.rs | 8 +++++++- .../mismatched_types/issue-74918-missing-lifetime.stderr | 4 ++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/librustc_resolve/late/diagnostics.rs b/src/librustc_resolve/late/diagnostics.rs index c57c0e51941..c9e83864a93 100644 --- a/src/librustc_resolve/late/diagnostics.rs +++ b/src/librustc_resolve/late/diagnostics.rs @@ -1349,7 +1349,13 @@ impl<'tcx> LifetimeContext<'_, 'tcx> { suggest_new(err, "'a"); } (0, _, Some(snippet)) if !snippet.ends_with('>') && count == 1 => { - suggest_new(err, &format!("{}<'a>", snippet)); + if snippet == "" { + // This happens when we have `type Bar<'a> = Foo` where we point at the space + // before `T`. We will suggest `type Bar<'a> = Foo<'a, T>`. + suggest_new(err, "'a, "); + } else { + suggest_new(err, &format!("{}<'a>", snippet)); + } } (n, ..) if n > 1 => { let spans: Vec = lifetime_names.iter().map(|lt| lt.span).collect(); diff --git a/src/test/ui/mismatched_types/issue-74918-missing-lifetime.stderr b/src/test/ui/mismatched_types/issue-74918-missing-lifetime.stderr index da3056eac90..d260addef48 100644 --- a/src/test/ui/mismatched_types/issue-74918-missing-lifetime.stderr +++ b/src/test/ui/mismatched_types/issue-74918-missing-lifetime.stderr @@ -6,8 +6,8 @@ LL | type Item = IteratorChunk; | help: consider introducing a named lifetime parameter | -LL | type Item<'a> = IteratorChunk<<'a>T, S>; - | ^^^^ ^^^^ +LL | type Item<'a> = IteratorChunk<'a, T, S>; + | ^^^^ ^^^ error: `impl` item signature doesn't match `trait` item signature --> $DIR/issue-74918-missing-lifetime.rs:11:5 From 7956b1cef71ef633fb0e5e55451c5bb9ee9c59e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Mon, 10 Aug 2020 13:26:16 -0700 Subject: [PATCH 2/5] Assoc `const`s don't have generics Fix #74264. --- src/librustc_resolve/late/diagnostics.rs | 11 ++++++++++- src/librustc_resolve/late/lifetimes.rs | 6 ++++-- .../missing-lifetime-in-assoc-const-type.rs | 5 +++++ .../missing-lifetime-in-assoc-const-type.stderr | 15 +++++++++++++++ 4 files changed, 34 insertions(+), 3 deletions(-) create mode 100644 src/test/ui/suggestions/missing-lifetime-in-assoc-const-type.rs create mode 100644 src/test/ui/suggestions/missing-lifetime-in-assoc-const-type.stderr diff --git a/src/librustc_resolve/late/diagnostics.rs b/src/librustc_resolve/late/diagnostics.rs index c9e83864a93..e9c463c7a3e 100644 --- a/src/librustc_resolve/late/diagnostics.rs +++ b/src/librustc_resolve/late/diagnostics.rs @@ -17,7 +17,7 @@ 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::{BytePos, Span}; +use rustc_span::{BytePos, Span, DUMMY_SP}; use log::debug; @@ -1273,6 +1273,15 @@ impl<'tcx> LifetimeContext<'_, 'tcx> { let should_break; introduce_suggestion.push(match missing { MissingLifetimeSpot::Generics(generics) => { + if generics.span == DUMMY_SP { + // Account for malformed generics in the HIR. This shouldn't happen, + // but if we make a mistake elsewhere, mainly by keeping something in + // `missing_named_lifetime_spots` that we shouldn't, like associated + // `const`s or making a mistake in the AST lowering we would provide + // non-sensical suggestions. Guard against that by skipping these. + // (#74264) + continue; + } msg = "consider introducing a named lifetime parameter".to_string(); should_break = true; if let Some(param) = generics.params.iter().find(|p| match p.kind { diff --git a/src/librustc_resolve/late/lifetimes.rs b/src/librustc_resolve/late/lifetimes.rs index 0b881b089de..6cb92843766 100644 --- a/src/librustc_resolve/late/lifetimes.rs +++ b/src/librustc_resolve/late/lifetimes.rs @@ -711,9 +711,9 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> { fn visit_trait_item(&mut self, trait_item: &'tcx hir::TraitItem<'tcx>) { use self::hir::TraitItemKind::*; - self.missing_named_lifetime_spots.push((&trait_item.generics).into()); match trait_item.kind { Fn(ref sig, _) => { + self.missing_named_lifetime_spots.push((&trait_item.generics).into()); let tcx = self.tcx; self.visit_early_late( Some(tcx.hir().get_parent_item(trait_item.hir_id)), @@ -721,8 +721,10 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> { &trait_item.generics, |this| intravisit::walk_trait_item(this, trait_item), ); + self.missing_named_lifetime_spots.pop(); } Type(bounds, ref ty) => { + self.missing_named_lifetime_spots.push((&trait_item.generics).into()); let generics = &trait_item.generics; let mut index = self.next_early_index(); debug!("visit_ty: index = {}", index); @@ -757,6 +759,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> { this.visit_ty(ty); } }); + self.missing_named_lifetime_spots.pop(); } Const(_, _) => { // Only methods and types support generics. @@ -764,7 +767,6 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> { intravisit::walk_trait_item(self, trait_item); } } - self.missing_named_lifetime_spots.pop(); } fn visit_impl_item(&mut self, impl_item: &'tcx hir::ImplItem<'tcx>) { diff --git a/src/test/ui/suggestions/missing-lifetime-in-assoc-const-type.rs b/src/test/ui/suggestions/missing-lifetime-in-assoc-const-type.rs new file mode 100644 index 00000000000..b9e0108fe71 --- /dev/null +++ b/src/test/ui/suggestions/missing-lifetime-in-assoc-const-type.rs @@ -0,0 +1,5 @@ +trait ZstAssert: Sized { + const TYPE_NAME: &str = ""; //~ ERROR missing lifetime specifier +} + +fn main() {} diff --git a/src/test/ui/suggestions/missing-lifetime-in-assoc-const-type.stderr b/src/test/ui/suggestions/missing-lifetime-in-assoc-const-type.stderr new file mode 100644 index 00000000000..ee9d1a15d2a --- /dev/null +++ b/src/test/ui/suggestions/missing-lifetime-in-assoc-const-type.stderr @@ -0,0 +1,15 @@ +error[E0106]: missing lifetime specifier + --> $DIR/missing-lifetime-in-assoc-const-type.rs:2:22 + | +LL | const TYPE_NAME: &str = ""; + | ^ expected named lifetime parameter + | +help: consider introducing a named lifetime parameter + | +LL | trait ZstAssert<'a>: Sized { +LL | const TYPE_NAME: &'a str = ""; + | + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0106`. From b9585fda7bc32db3b129aa7ec43322ae0f089635 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Mon, 10 Aug 2020 16:42:57 -0700 Subject: [PATCH 3/5] When suggesting `for` lts, consider existing lifetime names Fix #72404. --- src/librustc_resolve/late/diagnostics.rs | 75 +++++++++++++++---- src/librustc_resolve/late/lifetimes.rs | 29 +++++-- ...nd-lifetime-in-binding-only.elision.stderr | 5 ++ ...und-lifetime-in-return-only.elision.stderr | 5 ++ .../ui/suggestions/missing-lt-for-hrtb.rs | 15 ++++ .../ui/suggestions/missing-lt-for-hrtb.stderr | 63 ++++++++++++++++ 6 files changed, 172 insertions(+), 20 deletions(-) create mode 100644 src/test/ui/suggestions/missing-lt-for-hrtb.rs create mode 100644 src/test/ui/suggestions/missing-lt-for-hrtb.stderr diff --git a/src/librustc_resolve/late/diagnostics.rs b/src/librustc_resolve/late/diagnostics.rs index e9c463c7a3e..5b8d8dd0635 100644 --- a/src/librustc_resolve/late/diagnostics.rs +++ b/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, + lifetime_names: &FxHashSet, + lifetime_spans: Vec, 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(<))) + .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(<_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(<_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,17 +1380,16 @@ 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, @@ -1348,8 +1397,9 @@ impl<'tcx> LifetimeContext<'_, 'tcx> { .take(count) .collect::>() .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 = 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` where we point at the space before `T`, // but this can be confusing so we give a suggestion with placeholders. diff --git a/src/librustc_resolve/late/lifetimes.rs b/src/librustc_resolve/late/lifetimes.rs index 6cb92843766..e5accc7fde9 100644 --- a/src/librustc_resolve/late/lifetimes.rs +++ b/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,7 +2389,7 @@ 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( @@ -2383,6 +2397,7 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> { span, lifetime_refs.len(), &lifetime_names, + lifetime_spans, error.map(|p| &p[..]).unwrap_or(&[]), ); err.emit(); diff --git a/src/test/ui/associated-types/bound-lifetime-in-binding-only.elision.stderr b/src/test/ui/associated-types/bound-lifetime-in-binding-only.elision.stderr index 00f44129cc8..6c68cc7bc61 100644 --- a/src/test/ui/associated-types/bound-lifetime-in-binding-only.elision.stderr +++ b/src/test/ui/associated-types/bound-lifetime-in-binding-only.elision.stderr @@ -5,6 +5,11 @@ 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 + = 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 Fn() -> &'a i32>() { + | ^^^^^^^ ^^^ help: consider using the `'static` lifetime | LL | fn elision &'static i32>() { diff --git a/src/test/ui/associated-types/bound-lifetime-in-return-only.elision.stderr b/src/test/ui/associated-types/bound-lifetime-in-return-only.elision.stderr index a5242707c71..93d2f8e7911 100644 --- a/src/test/ui/associated-types/bound-lifetime-in-return-only.elision.stderr +++ b/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) { diff --git a/src/test/ui/suggestions/missing-lt-for-hrtb.rs b/src/test/ui/suggestions/missing-lt-for-hrtb.rs new file mode 100644 index 00000000000..a90a90122ad --- /dev/null +++ b/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(&())); +} diff --git a/src/test/ui/suggestions/missing-lt-for-hrtb.stderr b/src/test/ui/suggestions/missing-lt-for-hrtb.stderr new file mode 100644 index 00000000000..2cb63500e48 --- /dev/null +++ b/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`. From becd479482935a63633861595e276bae6533b25b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Tue, 11 Aug 2020 11:05:59 -0700 Subject: [PATCH 4/5] review comment: simplify code by using slice pat --- src/librustc_resolve/late/diagnostics.rs | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/src/librustc_resolve/late/diagnostics.rs b/src/librustc_resolve/late/diagnostics.rs index 5b8d8dd0635..9dbde5e7852 100644 --- a/src/librustc_resolve/late/diagnostics.rs +++ b/src/librustc_resolve/late/diagnostics.rs @@ -1378,17 +1378,18 @@ impl<'tcx> LifetimeContext<'_, 'tcx> { } }; - match (lifetime_names.len(), lifetime_names.iter().next(), snippet.as_deref()) { - (1, Some(name), Some("&")) => { + let lifetime_names: Vec<_> = lifetime_names.into_iter().collect(); + match (&lifetime_names[..], snippet.as_deref()) { + ([name], Some("&")) => { suggest_existing(err, &name.as_str()[..], &|name| format!("&{} ", name)); } - (1, Some(name), Some("'_")) => { + ([name], Some("'_")) => { suggest_existing(err, &name.as_str()[..], &|n| n.to_string()); } - (1, Some(name), Some("")) => { + ([name], Some("")) => { suggest_existing(err, &name.as_str()[..], &|n| format!("{}, ", n).repeat(count)); } - (1, Some(name), Some(snippet)) if !snippet.ends_with('>') => { + ([name], Some(snippet)) if !snippet.ends_with('>') => { let f = |name: &str| { format!( "{}<{}>", @@ -1401,13 +1402,13 @@ impl<'tcx> LifetimeContext<'_, 'tcx> { }; suggest_existing(err, &name.as_str()[..], &f); } - (0, _, Some("&")) if count == 1 => { + ([], Some("&")) if count == 1 => { suggest_new(err, "&'a "); } - (0, _, Some("'_")) if count == 1 => { + ([], Some("'_")) if count == 1 => { suggest_new(err, "'a"); } - (0, _, Some(snippet)) if !snippet.ends_with('>') && count == 1 => { + ([], Some(snippet)) if !snippet.ends_with('>') && count == 1 => { if snippet == "" { // This happens when we have `type Bar<'a> = Foo` where we point at the space // before `T`. We will suggest `type Bar<'a> = Foo<'a, T>`. @@ -1416,7 +1417,7 @@ impl<'tcx> LifetimeContext<'_, 'tcx> { suggest_new(err, &format!("{}<'a>", snippet)); } } - (n, ..) if n > 1 => { + (lts, ..) if lts.len() > 1 => { err.span_note(lifetime_spans, "these named lifetimes are available to use"); if Some("") == snippet.as_deref() { // This happens when we have `Foo` where we point at the space before `T`, From 6a3deb0ae04c4cb6400b30fecd1cbbee0506348b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Tue, 11 Aug 2020 13:02:14 -0700 Subject: [PATCH 5/5] Suggest using `'static` in assoc consts and suggest when multiple lts are needed --- src/librustc_resolve/late/diagnostics.rs | 54 ++++++++++++++- src/librustc_resolve/late/lifetimes.rs | 12 +++- src/test/ui/error-codes/E0106.stderr | 9 +++ .../missing-lifetime-in-assoc-const-type.rs | 13 +++- ...issing-lifetime-in-assoc-const-type.stderr | 68 +++++++++++++++++-- 5 files changed, 144 insertions(+), 12 deletions(-) diff --git a/src/librustc_resolve/late/diagnostics.rs b/src/librustc_resolve/late/diagnostics.rs index 9dbde5e7852..6e7a004f853 100644 --- a/src/librustc_resolve/late/diagnostics.rs +++ b/src/librustc_resolve/late/diagnostics.rs @@ -33,6 +33,7 @@ enum AssocSuggestion { crate enum MissingLifetimeSpot<'tcx> { Generics(&'tcx hir::Generics<'tcx>), HigherRanked { span: Span, span_type: ForLifetimeSpanType }, + Static, } crate enum ForLifetimeSpanType { @@ -1186,6 +1187,7 @@ impl<'tcx> LifetimeContext<'_, 'tcx> { https://doc.rust-lang.org/nomicon/hrtb.html", ); } + _ => {} } } if nightly_options::is_nightly_build() @@ -1358,6 +1360,42 @@ impl<'tcx> LifetimeContext<'_, 'tcx> { ); (*span, span_type.suggestion("'a")) } + MissingLifetimeSpot::Static => { + let (span, sugg) = match snippet.as_deref() { + Some("&") => (span.shrink_to_hi(), "'static ".to_owned()), + Some("'_") => (span, "'static".to_owned()), + Some(snippet) if !snippet.ends_with('>') => { + if snippet == "" { + ( + span, + std::iter::repeat("'static") + .take(count) + .collect::>() + .join(", "), + ) + } else { + ( + span.shrink_to_hi(), + format!( + "<{}>", + std::iter::repeat("'static") + .take(count) + .collect::>() + .join(", ") + ), + ) + } + } + _ => continue, + }; + err.span_suggestion_verbose( + span, + "consider using the `'static` lifetime", + sugg.to_string(), + Applicability::MaybeIncorrect, + ); + continue; + } }); for param in params { if let Ok(snippet) = self.tcx.sess.source_map().span_to_snippet(param.span) { @@ -1408,13 +1446,23 @@ impl<'tcx> LifetimeContext<'_, 'tcx> { ([], Some("'_")) if count == 1 => { suggest_new(err, "'a"); } - ([], Some(snippet)) if !snippet.ends_with('>') && count == 1 => { + ([], Some(snippet)) if !snippet.ends_with('>') => { if snippet == "" { // This happens when we have `type Bar<'a> = Foo` where we point at the space // before `T`. We will suggest `type Bar<'a> = Foo<'a, T>`. - suggest_new(err, "'a, "); + suggest_new( + err, + &std::iter::repeat("'a, ").take(count).collect::>().join(""), + ); } else { - suggest_new(err, &format!("{}<'a>", snippet)); + suggest_new( + err, + &format!( + "{}<{}>", + snippet, + std::iter::repeat("'a").take(count).collect::>().join(", ") + ), + ); } } (lts, ..) if lts.len() > 1 => { diff --git a/src/librustc_resolve/late/lifetimes.rs b/src/librustc_resolve/late/lifetimes.rs index e5accc7fde9..2046419d984 100644 --- a/src/librustc_resolve/late/lifetimes.rs +++ b/src/librustc_resolve/late/lifetimes.rs @@ -764,26 +764,30 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> { Const(_, _) => { // Only methods and types support generics. assert!(trait_item.generics.params.is_empty()); + self.missing_named_lifetime_spots.push(MissingLifetimeSpot::Static); intravisit::walk_trait_item(self, trait_item); + self.missing_named_lifetime_spots.pop(); } } } fn visit_impl_item(&mut self, impl_item: &'tcx hir::ImplItem<'tcx>) { use self::hir::ImplItemKind::*; - self.missing_named_lifetime_spots.push((&impl_item.generics).into()); match impl_item.kind { Fn(ref sig, _) => { + self.missing_named_lifetime_spots.push((&impl_item.generics).into()); let tcx = self.tcx; self.visit_early_late( Some(tcx.hir().get_parent_item(impl_item.hir_id)), &sig.decl, &impl_item.generics, |this| intravisit::walk_impl_item(this, impl_item), - ) + ); + self.missing_named_lifetime_spots.pop(); } TyAlias(ref ty) => { let generics = &impl_item.generics; + self.missing_named_lifetime_spots.push(generics.into()); let mut index = self.next_early_index(); let mut non_lifetime_count = 0; debug!("visit_ty: index = {}", index); @@ -812,14 +816,16 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> { this.visit_generics(generics); this.visit_ty(ty); }); + self.missing_named_lifetime_spots.pop(); } Const(_, _) => { // Only methods and types support generics. assert!(impl_item.generics.params.is_empty()); + self.missing_named_lifetime_spots.push(MissingLifetimeSpot::Static); intravisit::walk_impl_item(self, impl_item); + self.missing_named_lifetime_spots.pop(); } } - self.missing_named_lifetime_spots.pop(); } fn visit_lifetime(&mut self, lifetime_ref: &'tcx hir::Lifetime) { diff --git a/src/test/ui/error-codes/E0106.stderr b/src/test/ui/error-codes/E0106.stderr index a23bcbfd71a..ac70e887626 100644 --- a/src/test/ui/error-codes/E0106.stderr +++ b/src/test/ui/error-codes/E0106.stderr @@ -51,6 +51,15 @@ error[E0106]: missing lifetime specifiers | LL | buzz: Buzz, | ^^^^ expected 2 lifetime parameters + | +help: consider introducing a named lifetime parameter + | +LL | struct Quux<'a> { +LL | baz: Baz, +LL | +LL | +LL | buzz: Buzz<'a, 'a>, + | error: aborting due to 5 previous errors diff --git a/src/test/ui/suggestions/missing-lifetime-in-assoc-const-type.rs b/src/test/ui/suggestions/missing-lifetime-in-assoc-const-type.rs index b9e0108fe71..38332627f4c 100644 --- a/src/test/ui/suggestions/missing-lifetime-in-assoc-const-type.rs +++ b/src/test/ui/suggestions/missing-lifetime-in-assoc-const-type.rs @@ -1,5 +1,16 @@ trait ZstAssert: Sized { - const TYPE_NAME: &str = ""; //~ ERROR missing lifetime specifier + const A: &str = ""; //~ ERROR missing lifetime specifier + const B: S = S { s: &() }; //~ ERROR missing lifetime specifier + const C: &'_ str = ""; //~ ERROR missing lifetime specifier + const D: T = T { a: &(), b: &() }; //~ ERROR missing lifetime specifier +} + +struct S<'a> { + s: &'a (), +} +struct T<'a, 'b> { + a: &'a (), + b: &'b (), } fn main() {} diff --git a/src/test/ui/suggestions/missing-lifetime-in-assoc-const-type.stderr b/src/test/ui/suggestions/missing-lifetime-in-assoc-const-type.stderr index ee9d1a15d2a..b20778ce208 100644 --- a/src/test/ui/suggestions/missing-lifetime-in-assoc-const-type.stderr +++ b/src/test/ui/suggestions/missing-lifetime-in-assoc-const-type.stderr @@ -1,15 +1,73 @@ error[E0106]: missing lifetime specifier - --> $DIR/missing-lifetime-in-assoc-const-type.rs:2:22 + --> $DIR/missing-lifetime-in-assoc-const-type.rs:2:14 | -LL | const TYPE_NAME: &str = ""; - | ^ expected named lifetime parameter +LL | const A: &str = ""; + | ^ expected named lifetime parameter | +help: consider using the `'static` lifetime + | +LL | const A: &'static str = ""; + | ^^^^^^^ help: consider introducing a named lifetime parameter | LL | trait ZstAssert<'a>: Sized { -LL | const TYPE_NAME: &'a str = ""; +LL | const A: &'a str = ""; | -error: aborting due to previous error +error[E0106]: missing lifetime specifier + --> $DIR/missing-lifetime-in-assoc-const-type.rs:3:14 + | +LL | const B: S = S { s: &() }; + | ^ expected named lifetime parameter + | +help: consider using the `'static` lifetime + | +LL | const B: S<'static> = S { s: &() }; + | ^^^^^^^^^ +help: consider introducing a named lifetime parameter + | +LL | trait ZstAssert<'a>: Sized { +LL | const A: &str = ""; +LL | const B: S<'a> = S { s: &() }; + | + +error[E0106]: missing lifetime specifier + --> $DIR/missing-lifetime-in-assoc-const-type.rs:4:15 + | +LL | const C: &'_ str = ""; + | ^^ expected named lifetime parameter + | +help: consider using the `'static` lifetime + | +LL | const C: &'static str = ""; + | ^^^^^^^ +help: consider introducing a named lifetime parameter + | +LL | trait ZstAssert<'a>: Sized { +LL | const A: &str = ""; +LL | const B: S = S { s: &() }; +LL | const C: &'a str = ""; + | + +error[E0106]: missing lifetime specifiers + --> $DIR/missing-lifetime-in-assoc-const-type.rs:5:14 + | +LL | const D: T = T { a: &(), b: &() }; + | ^ expected 2 lifetime parameters + | +help: consider using the `'static` lifetime + | +LL | const D: T<'static, 'static> = T { a: &(), b: &() }; + | ^^^^^^^^^^^^^^^^^^ +help: consider introducing a named lifetime parameter + | +LL | trait ZstAssert<'a>: Sized { +LL | const A: &str = ""; +LL | const B: S = S { s: &() }; +LL | const C: &'_ str = ""; +LL | const D: T<'a, 'a> = T { a: &(), b: &() }; + | + +error: aborting due to 4 previous errors For more information about this error, try `rustc --explain E0106`.