From 439ef6d76279268eb80e33afffafa22597e22776 Mon Sep 17 00:00:00 2001 From: Fabian Wolff Date: Fri, 7 May 2021 19:44:32 +0200 Subject: [PATCH 1/3] Fix suggestions for missing return type lifetime parameters --- compiler/rustc_errors/src/diagnostic.rs | 24 ++ .../rustc_resolve/src/late/diagnostics.rs | 351 ++++++++++-------- compiler/rustc_resolve/src/late/lifetimes.rs | 15 +- src/test/ui/issues/issue-84592.rs | 17 + src/test/ui/issues/issue-84592.stderr | 17 + src/test/ui/return-elided-lifetime.rs | 37 ++ src/test/ui/return-elided-lifetime.stderr | 198 ++++++++++ .../ui/suggestions/missing-lt-for-hrtb.stderr | 4 + src/tools/tidy/src/ui_tests.rs | 2 +- 9 files changed, 514 insertions(+), 151 deletions(-) create mode 100644 src/test/ui/issues/issue-84592.rs create mode 100644 src/test/ui/issues/issue-84592.stderr create mode 100644 src/test/ui/return-elided-lifetime.rs create mode 100644 src/test/ui/return-elided-lifetime.stderr diff --git a/compiler/rustc_errors/src/diagnostic.rs b/compiler/rustc_errors/src/diagnostic.rs index b2f6a0c10142d..405dd2e68c6d0 100644 --- a/compiler/rustc_errors/src/diagnostic.rs +++ b/compiler/rustc_errors/src/diagnostic.rs @@ -299,6 +299,30 @@ impl Diagnostic { self } + /// [`Diagnostic::multipart_suggestion()`] but you can set the [`SuggestionStyle`]. + pub fn multipart_suggestion_with_style( + &mut self, + msg: &str, + suggestion: Vec<(Span, String)>, + applicability: Applicability, + style: SuggestionStyle, + ) -> &mut Self { + assert!(!suggestion.is_empty()); + self.suggestions.push(CodeSuggestion { + substitutions: vec![Substitution { + parts: suggestion + .into_iter() + .map(|(span, snippet)| SubstitutionPart { snippet, span }) + .collect(), + }], + msg: msg.to_owned(), + style, + applicability, + tool_metadata: Default::default(), + }); + self + } + /// Prints out a message with for a multipart suggestion without showing the suggested code. /// /// This is intended to be used for suggestions that are obvious in what the changes need to diff --git a/compiler/rustc_resolve/src/late/diagnostics.rs b/compiler/rustc_resolve/src/late/diagnostics.rs index 7561b3df3af70..ca4873bd51570 100644 --- a/compiler/rustc_resolve/src/late/diagnostics.rs +++ b/compiler/rustc_resolve/src/late/diagnostics.rs @@ -9,7 +9,7 @@ use rustc_ast::visit::FnKind; use rustc_ast::{self as ast, Expr, ExprKind, Item, ItemKind, NodeId, Path, Ty, TyKind}; use rustc_ast_pretty::pprust::path_segment_to_string; use rustc_data_structures::fx::FxHashSet; -use rustc_errors::{pluralize, struct_span_err, Applicability, DiagnosticBuilder}; +use rustc_errors::{pluralize, struct_span_err, Applicability, DiagnosticBuilder, SuggestionStyle}; use rustc_hir as hir; use rustc_hir::def::Namespace::{self, *}; use rustc_hir::def::{self, CtorKind, CtorOf, DefKind}; @@ -1687,12 +1687,12 @@ impl<'a: 'ast, 'ast> LateResolutionVisitor<'a, '_, 'ast> { impl<'tcx> LifetimeContext<'_, 'tcx> { crate fn report_missing_lifetime_specifiers( &self, - span: Span, + spans: Vec, count: usize, ) -> DiagnosticBuilder<'tcx> { struct_span_err!( self.tcx.sess, - span, + spans, E0106, "missing lifetime specifier{}", pluralize!(count) @@ -1821,81 +1821,107 @@ impl<'tcx> LifetimeContext<'_, 'tcx> { crate fn add_missing_lifetime_specifiers_label( &self, err: &mut DiagnosticBuilder<'_>, - span: Span, - count: usize, + spans: Vec, + counts: Vec, lifetime_names: &FxHashSet, lifetime_spans: Vec, params: &[ElisionFailureInfo], ) { - let snippet = self.tcx.sess.source_map().span_to_snippet(span).ok(); - - err.span_label( - span, - &format!( - "expected {} lifetime parameter{}", - if count == 1 { "named".to_string() } else { count.to_string() }, - pluralize!(count) - ), - ); + let snippets: Vec> = spans + .iter() + .copied() + .map(|span| self.tcx.sess.source_map().span_to_snippet(span).ok()) + .collect(); - 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![]; + for (span, count) in spans.iter().zip(counts.iter()) { + err.span_label( + span.clone(), + format!( + "expected {} lifetime parameter{}", + if *count == 1 { "named".to_string() } else { count.to_string() }, + pluralize!(*count), + ), + ); + } - 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 - }) - }; + let suggest_existing = + |err: &mut DiagnosticBuilder<'_>, + name: &str, + formatters: &Vec 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 \ + // 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 let Some(stripped) = snippet.strip_prefix("&'_ ") { - introduce_suggestion - .push((param.span, format!("&{} {}", lt_name, stripped))); + ); + 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 let Some(stripped) = snippet.strip_prefix("&'_ ") { + introduce_suggestion + .push((param.span, format!("&{} {}", lt_name, stripped))); + } } } + introduce_suggestion.push((*for_span, for_sugg)); + for (span, formatter) in spans.iter().copied().zip(formatters.iter()) { + if let Some(formatter) = formatter { + introduce_suggestion.push((span, formatter(<_name))); + } + } + err.multipart_suggestion_with_style( + &msg, + introduce_suggestion, + Applicability::MaybeIncorrect, + SuggestionStyle::ShowAlways, + ); } - introduce_suggestion.push((*for_span, for_sugg)); - 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()), - formatter(name), - Applicability::MaybeIncorrect, - ); - }; - let suggest_new = |err: &mut DiagnosticBuilder<'_>, sugg: &str| { + let mut spans_suggs: Vec<_> = Vec::new(); + for (span, fmt) in spans.iter().copied().zip(formatters.iter()) { + if let Some(formatter) = fmt { + spans_suggs.push((span, formatter(name))); + } + } + err.multipart_suggestion_with_style( + &format!( + "consider using the `{}` lifetime", + lifetime_names.iter().next().unwrap() + ), + spans_suggs, + Applicability::MaybeIncorrect, + SuggestionStyle::ShowAlways, + ); + }; + let suggest_new = |err: &mut DiagnosticBuilder<'_>, suggs: &Vec>| { for missing in self.missing_named_lifetime_spots.iter().rev() { let mut introduce_suggestion = vec![]; let msg; @@ -1940,38 +1966,44 @@ 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!( - "<{}>", + let mut spans_suggs = Vec::new(); + for ((span, snippet), count) in + spans.iter().copied().zip(snippets.iter()).zip(counts.iter().copied()) + { + 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(", ") - ), - ) + .join(", "), + ) + } else { + ( + span.shrink_to_hi(), + format!( + "<{}>", + std::iter::repeat("'static") + .take(count) + .collect::>() + .join(", ") + ), + ) + } } - } - _ => continue, - }; - err.span_suggestion_verbose( - span, + _ => continue, + }; + spans_suggs.push((span, sugg.to_string())); + } + err.multipart_suggestion_with_style( "consider using the `'static` lifetime", - sugg.to_string(), + spans_suggs, Applicability::MaybeIncorrect, + SuggestionStyle::ShowAlways, ); continue; } @@ -1986,8 +2018,17 @@ impl<'tcx> LifetimeContext<'_, 'tcx> { } } } - introduce_suggestion.push((span, sugg.to_string())); - err.multipart_suggestion(&msg, introduce_suggestion, Applicability::MaybeIncorrect); + for (span, sugg) in spans.iter().copied().zip(suggs.iter()) { + if let Some(sugg) = sugg { + introduce_suggestion.push((span, sugg.to_string())); + } + } + err.multipart_suggestion_with_style( + &msg, + introduce_suggestion, + Applicability::MaybeIncorrect, + SuggestionStyle::ShowAlways, + ); if should_break { break; } @@ -1995,68 +2036,86 @@ impl<'tcx> LifetimeContext<'_, 'tcx> { }; let lifetime_names: Vec<_> = lifetime_names.iter().collect(); - match (&lifetime_names[..], snippet.as_deref()) { - ([name], Some("&")) => { - suggest_existing(err, &name.as_str()[..], &|name| format!("&{} ", name)); - } - ([name], Some("'_")) => { - suggest_existing(err, &name.as_str()[..], &|n| n.to_string()); - } - ([name], Some("")) => { - suggest_existing(err, &name.as_str()[..], &|n| format!("{}, ", n).repeat(count)); - } - ([name], Some(snippet)) if !snippet.ends_with('>') => { - let f = |name: &str| { - format!( - "{}<{}>", - snippet, - std::iter::repeat(name.to_string()) - .take(count) - .collect::>() - .join(", ") - ) - }; - suggest_existing(err, &name.as_str()[..], &f); - } - ([], Some("&")) if count == 1 => { - suggest_new(err, "&'a "); - } - ([], Some("'_")) if count == 1 => { - suggest_new(err, "'a"); - } - ([], 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, - &std::iter::repeat("'a, ").take(count).collect::>().join(""), - ); - } else { - suggest_new( - err, - &format!( - "{}<{}>", - snippet, - std::iter::repeat("'a").take(count).collect::>().join(", ") - ), - ); + match &lifetime_names[..] { + [name] => { + let mut suggs: Vec String>>> = Vec::new(); + for (snippet, count) in snippets.iter().cloned().zip(counts.iter().copied()) { + if snippet == Some("&".to_string()) { + suggs.push(Some(Box::new(|name| format!("&{} ", name)))); + } else if snippet == Some("'_".to_string()) { + suggs.push(Some(Box::new(|n| n.to_string()))); + } else if snippet == Some("".to_string()) { + suggs.push(Some(Box::new(move |n| format!("{}, ", n).repeat(count)))); + } else if let Some(snippet) = snippet { + if !snippet.ends_with('>') { + suggs.push(Some(Box::new(move |name| { + format!( + "{}<{}>", + snippet, + std::iter::repeat(name.to_string()) + .take(count) + .collect::>() + .join(", ") + ) + }))); + } else { + suggs.push(None); + } + } else { + suggs.push(None); + } + } + suggest_existing(err, &name.as_str()[..], &suggs); + } + [] => { + let mut suggs: Vec> = Vec::new(); + for (snippet, count) in snippets.iter().cloned().zip(counts.iter().copied()) { + if snippet == Some("&".to_string()) { + suggs.push(Some("&'a ".to_string())); + } else if snippet == Some("'_".to_string()) { + suggs.push(Some("'a".to_string())); + } else if let Some(snippet) = snippet { + if snippet == "" { + suggs.push(Some( + std::iter::repeat("'a, ").take(count).collect::>().join(""), + )); + } else { + suggs.push(Some(format!( + "{}<{}>", + snippet, + std::iter::repeat("'a").take(count).collect::>().join(", ") + ))); + } + } else { + suggs.push(None); + } } + suggest_new(err, &suggs); } - (lts, ..) if lts.len() > 1 => { + lts if lts.len() > 1 => { err.span_note(lifetime_spans, "these named lifetimes are available to use"); - if Some("") == snippet.as_deref() { + + let mut spans_suggs: Vec<_> = Vec::new(); + for (span, snippet) in spans.iter().copied().zip(snippets.iter()) { + if Some("") == snippet.as_deref() { + spans_suggs.push((span, "'lifetime, ".to_string())); + } else if Some("&") == snippet.as_deref() { + spans_suggs.push((span, "&'lifetime ".to_string())); + } + } + + if spans_suggs.len() > 0 { // 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. - err.span_suggestion_verbose( - span, + err.multipart_suggestion_with_style( "consider using one of the available lifetimes here", - "'lifetime, ".repeat(count), + spans_suggs, Applicability::HasPlaceholders, + SuggestionStyle::ShowAlways, ); } } - _ => {} + _ => unreachable!(), } } diff --git a/compiler/rustc_resolve/src/late/lifetimes.rs b/compiler/rustc_resolve/src/late/lifetimes.rs index 174df09cbdbb2..c81269a46b21a 100644 --- a/compiler/rustc_resolve/src/late/lifetimes.rs +++ b/compiler/rustc_resolve/src/late/lifetimes.rs @@ -2956,7 +2956,6 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> { return; } - let span = lifetime_refs[0].span; let mut late_depth = 0; let mut scope = self.scope; let mut lifetime_names = FxHashSet::default(); @@ -3035,7 +3034,14 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> { } }; - let mut err = self.report_missing_lifetime_specifiers(span, lifetime_refs.len()); + let mut spans: Vec<_> = lifetime_refs.iter().map(|lt| lt.span).collect(); + spans.sort(); + let mut spans_dedup = spans.clone(); + spans_dedup.dedup(); + let counts: Vec<_> = + spans_dedup.iter().map(|sp| spans.iter().filter(|nsp| *nsp == sp).count()).collect(); + + let mut err = self.report_missing_lifetime_specifiers(spans.clone(), lifetime_refs.len()); if let Some(params) = error { // If there's no lifetime available, suggest `'static`. @@ -3043,10 +3049,11 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> { lifetime_names.insert(kw::StaticLifetime); } } + self.add_missing_lifetime_specifiers_label( &mut err, - span, - lifetime_refs.len(), + spans, + counts, &lifetime_names, lifetime_spans, error.unwrap_or(&[]), diff --git a/src/test/ui/issues/issue-84592.rs b/src/test/ui/issues/issue-84592.rs new file mode 100644 index 0000000000000..aa246aaa3d45e --- /dev/null +++ b/src/test/ui/issues/issue-84592.rs @@ -0,0 +1,17 @@ +/* Checks whether issue #84592 has been resolved. The issue was + * that in this example, there are two expected/missing lifetime + * parameters with *different spans*, leading to incorrect + * suggestions from rustc. + */ + +struct TwoLifetimes<'x, 'y> { + x: &'x (), + y: &'y (), +} + +fn two_lifetimes_needed(a: &(), b: &()) -> TwoLifetimes<'_, '_> { +//~^ ERROR missing lifetime specifiers [E0106] + TwoLifetimes { x: &(), y: &() } +} + +fn main() {} diff --git a/src/test/ui/issues/issue-84592.stderr b/src/test/ui/issues/issue-84592.stderr new file mode 100644 index 0000000000000..02f9241a6d2da --- /dev/null +++ b/src/test/ui/issues/issue-84592.stderr @@ -0,0 +1,17 @@ +error[E0106]: missing lifetime specifiers + --> $DIR/issue-84592.rs:12:57 + | +LL | fn two_lifetimes_needed(a: &(), b: &()) -> TwoLifetimes<'_, '_> { + | --- --- ^^ ^^ expected named lifetime parameter + | | + | expected named lifetime parameter + | + = help: this function's return type contains a borrowed value, but the signature does not say whether it is borrowed from `a` or `b` +help: consider introducing a named lifetime parameter + | +LL | fn two_lifetimes_needed<'a>(a: &'a (), b: &'a ()) -> TwoLifetimes<'a, 'a> { + | ^^^^ ^^^^^^ ^^^^^^ ^^ ^^ + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0106`. diff --git a/src/test/ui/return-elided-lifetime.rs b/src/test/ui/return-elided-lifetime.rs new file mode 100644 index 0000000000000..ca336bbb056d5 --- /dev/null +++ b/src/test/ui/return-elided-lifetime.rs @@ -0,0 +1,37 @@ +/* Checks all four scenarios possible in report_elision_failure() of + * rustc_resolve::late::lifetimes::LifetimeContext related to returning + * borrowed values, in various configurations. + */ + +fn f1() -> &i32 { loop {} } +//~^ ERROR missing lifetime specifier [E0106] +fn f1_() -> (&i32, &i32) { loop {} } +//~^ ERROR missing lifetime specifier [E0106] +//~^^ ERROR missing lifetime specifier [E0106] + +fn f2(a: i32, b: i32) -> &i32 { loop {} } +//~^ ERROR missing lifetime specifier [E0106] +fn f2_(a: i32, b: i32) -> (&i32, &i32) { loop {} } +//~^ ERROR missing lifetime specifier [E0106] +//~^^ ERROR missing lifetime specifier [E0106] + +struct S<'a, 'b> { a: &'a i32, b: &'b i32 } +fn f3(s: &S) -> &i32 { loop {} } +//~^ ERROR missing lifetime specifier [E0106] +fn f3_(s: &S, t: &S) -> (&i32, &i32) { loop {} } +//~^ ERROR missing lifetime specifier [E0106] +//~^^ ERROR missing lifetime specifier [E0106] + +fn f4<'a, 'b>(a: &'a i32, b: &'b i32) -> &i32 { loop {} } +//~^ ERROR missing lifetime specifier [E0106] +fn f4_<'a, 'b>(a: &'a i32, b: &'b i32) -> (&i32, &i32) { loop {} } +//~^ ERROR missing lifetime specifier [E0106] +//~^^ ERROR missing lifetime specifier [E0106] + +fn f5<'a>(a: &'a i32, b: &i32) -> &i32 { loop {} } +//~^ ERROR missing lifetime specifier [E0106] +fn f5_<'a>(a: &'a i32, b: &i32) -> (&i32, &i32) { loop {} } +//~^ ERROR missing lifetime specifier [E0106] +//~^^ ERROR missing lifetime specifier [E0106] + +fn main() {} diff --git a/src/test/ui/return-elided-lifetime.stderr b/src/test/ui/return-elided-lifetime.stderr new file mode 100644 index 0000000000000..888cd5e58abec --- /dev/null +++ b/src/test/ui/return-elided-lifetime.stderr @@ -0,0 +1,198 @@ +error[E0106]: missing lifetime specifier + --> $DIR/return-elided-lifetime.rs:6:12 + | +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 + | +LL | fn f1() -> &'static i32 { loop {} } + | ^^^^^^^^ + +error[E0106]: missing lifetime specifier + --> $DIR/return-elided-lifetime.rs:8:14 + | +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 + | +LL | fn f1_() -> (&'static i32, &i32) { loop {} } + | ^^^^^^^^ + +error[E0106]: missing lifetime specifier + --> $DIR/return-elided-lifetime.rs:8:20 + | +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 + | +LL | fn f1_() -> (&i32, &'static i32) { loop {} } + | ^^^^^^^^ + +error[E0106]: missing lifetime specifier + --> $DIR/return-elided-lifetime.rs:12:26 + | +LL | fn f2(a: i32, b: i32) -> &i32 { loop {} } + | ^ 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 `'static` lifetime + | +LL | fn f2(a: i32, b: i32) -> &'static i32 { loop {} } + | ^^^^^^^^ + +error[E0106]: missing lifetime specifier + --> $DIR/return-elided-lifetime.rs:14:28 + | +LL | fn f2_(a: i32, b: i32) -> (&i32, &i32) { loop {} } + | ^ 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 `'static` lifetime + | +LL | fn f2_(a: i32, b: i32) -> (&'static i32, &i32) { loop {} } + | ^^^^^^^^ + +error[E0106]: missing lifetime specifier + --> $DIR/return-elided-lifetime.rs:14:34 + | +LL | fn f2_(a: i32, b: i32) -> (&i32, &i32) { loop {} } + | ^ 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 `'static` lifetime + | +LL | fn f2_(a: i32, b: i32) -> (&i32, &'static i32) { loop {} } + | ^^^^^^^^ + +error[E0106]: missing lifetime specifier + --> $DIR/return-elided-lifetime.rs:19:17 + | +LL | fn f3(s: &S) -> &i32 { loop {} } + | -- ^ expected named lifetime parameter + | + = help: this function's return type contains a borrowed value, but the signature does not say which one of `s`'s 3 lifetimes it is borrowed from +help: consider introducing a named lifetime parameter + | +LL | fn f3<'a>(s: &'a S) -> &'a i32 { loop {} } + | ^^^^ ^^^^^ ^^^ + +error[E0106]: missing lifetime specifier + --> $DIR/return-elided-lifetime.rs:21:26 + | +LL | fn f3_(s: &S, t: &S) -> (&i32, &i32) { loop {} } + | -- -- ^ expected named lifetime parameter + | + = help: this function's return type contains a borrowed value, but the signature does not say whether it is borrowed from one of `s`'s 3 lifetimes or one of `t`'s 3 lifetimes +help: consider introducing a named lifetime parameter + | +LL | fn f3_<'a>(s: &'a S, t: &'a S) -> (&'a i32, &i32) { loop {} } + | ^^^^ ^^^^^ ^^^^^ ^^^ + +error[E0106]: missing lifetime specifier + --> $DIR/return-elided-lifetime.rs:21:32 + | +LL | fn f3_(s: &S, t: &S) -> (&i32, &i32) { loop {} } + | -- -- ^ expected named lifetime parameter + | + = help: this function's return type contains a borrowed value, but the signature does not say whether it is borrowed from one of `s`'s 3 lifetimes or one of `t`'s 3 lifetimes +help: consider introducing a named lifetime parameter + | +LL | fn f3_<'a>(s: &'a S, t: &'a S) -> (&i32, &'a i32) { loop {} } + | ^^^^ ^^^^^ ^^^^^ ^^^ + +error[E0106]: missing lifetime specifier + --> $DIR/return-elided-lifetime.rs:25:42 + | +LL | fn f4<'a, 'b>(a: &'a i32, b: &'b i32) -> &i32 { loop {} } + | ------- ------- ^ expected named lifetime parameter + | + = help: this function's return type contains a borrowed value, but the signature does not say whether it is borrowed from `a` or `b` +note: these named lifetimes are available to use + --> $DIR/return-elided-lifetime.rs:25:7 + | +LL | fn f4<'a, 'b>(a: &'a i32, b: &'b i32) -> &i32 { loop {} } + | ^^ ^^ +help: consider using one of the available lifetimes here + | +LL | fn f4<'a, 'b>(a: &'a i32, b: &'b i32) -> &'lifetime i32 { loop {} } + | ^^^^^^^^^^ + +error[E0106]: missing lifetime specifier + --> $DIR/return-elided-lifetime.rs:27:44 + | +LL | fn f4_<'a, 'b>(a: &'a i32, b: &'b i32) -> (&i32, &i32) { loop {} } + | ------- ------- ^ expected named lifetime parameter + | + = help: this function's return type contains a borrowed value, but the signature does not say whether it is borrowed from `a` or `b` +note: these named lifetimes are available to use + --> $DIR/return-elided-lifetime.rs:27:8 + | +LL | fn f4_<'a, 'b>(a: &'a i32, b: &'b i32) -> (&i32, &i32) { loop {} } + | ^^ ^^ +help: consider using one of the available lifetimes here + | +LL | fn f4_<'a, 'b>(a: &'a i32, b: &'b i32) -> (&'lifetime i32, &i32) { loop {} } + | ^^^^^^^^^^ + +error[E0106]: missing lifetime specifier + --> $DIR/return-elided-lifetime.rs:27:50 + | +LL | fn f4_<'a, 'b>(a: &'a i32, b: &'b i32) -> (&i32, &i32) { loop {} } + | ------- ------- ^ expected named lifetime parameter + | + = help: this function's return type contains a borrowed value, but the signature does not say whether it is borrowed from `a` or `b` +note: these named lifetimes are available to use + --> $DIR/return-elided-lifetime.rs:27:8 + | +LL | fn f4_<'a, 'b>(a: &'a i32, b: &'b i32) -> (&i32, &i32) { loop {} } + | ^^ ^^ +help: consider using one of the available lifetimes here + | +LL | fn f4_<'a, 'b>(a: &'a i32, b: &'b i32) -> (&i32, &'lifetime i32) { loop {} } + | ^^^^^^^^^^ + +error[E0106]: missing lifetime specifier + --> $DIR/return-elided-lifetime.rs:31:35 + | +LL | fn f5<'a>(a: &'a i32, b: &i32) -> &i32 { loop {} } + | ------- ---- ^ expected named lifetime parameter + | + = help: this function's return type contains a borrowed value, but the signature does not say whether it is borrowed from `a` or `b` +help: consider using the `'a` lifetime + | +LL | fn f5<'a>(a: &'a i32, b: &i32) -> &'a i32 { loop {} } + | ^^^ + +error[E0106]: missing lifetime specifier + --> $DIR/return-elided-lifetime.rs:33:37 + | +LL | fn f5_<'a>(a: &'a i32, b: &i32) -> (&i32, &i32) { loop {} } + | ------- ---- ^ expected named lifetime parameter + | + = help: this function's return type contains a borrowed value, but the signature does not say whether it is borrowed from `a` or `b` +help: consider using the `'a` lifetime + | +LL | fn f5_<'a>(a: &'a i32, b: &i32) -> (&'a i32, &i32) { loop {} } + | ^^^ + +error[E0106]: missing lifetime specifier + --> $DIR/return-elided-lifetime.rs:33:43 + | +LL | fn f5_<'a>(a: &'a i32, b: &i32) -> (&i32, &i32) { loop {} } + | ------- ---- ^ expected named lifetime parameter + | + = help: this function's return type contains a borrowed value, but the signature does not say whether it is borrowed from `a` or `b` +help: consider using the `'a` lifetime + | +LL | fn f5_<'a>(a: &'a i32, b: &i32) -> (&i32, &'a i32) { loop {} } + | ^^^ + +error: aborting due to 15 previous errors + +For more information about this error, try `rustc --explain E0106`. diff --git a/src/test/ui/suggestions/missing-lt-for-hrtb.stderr b/src/test/ui/suggestions/missing-lt-for-hrtb.stderr index 2cb63500e48b9..a7a44b511db8d 100644 --- a/src/test/ui/suggestions/missing-lt-for-hrtb.stderr +++ b/src/test/ui/suggestions/missing-lt-for-hrtb.stderr @@ -44,6 +44,10 @@ note: these named lifetimes are available to use | LL | struct V<'a>(&'a dyn for<'b> Fn(&X) -> &X); | ^^ ^^ +help: consider using one of the available lifetimes here + | +LL | struct V<'a>(&'a dyn for<'b> Fn(&X) -> &'lifetime X); + | ^^^^^^^^^^ error[E0106]: missing lifetime specifier --> $DIR/missing-lt-for-hrtb.rs:5:41 diff --git a/src/tools/tidy/src/ui_tests.rs b/src/tools/tidy/src/ui_tests.rs index 8334bc68ae729..791489b3960a5 100644 --- a/src/tools/tidy/src/ui_tests.rs +++ b/src/tools/tidy/src/ui_tests.rs @@ -8,7 +8,7 @@ use std::path::Path; const ENTRY_LIMIT: usize = 1000; // FIXME: The following limits should be reduced eventually. const ROOT_ENTRY_LIMIT: usize = 1388; -const ISSUES_ENTRY_LIMIT: usize = 2551; +const ISSUES_ENTRY_LIMIT: usize = 2557; fn check_entries(path: &Path, bad: &mut bool) { let dirs = walkdir::WalkDir::new(&path.join("test/ui")) From 3c0c3874fc75c730efba0702579bfeef34eecf31 Mon Sep 17 00:00:00 2001 From: Fabian Wolff Date: Sun, 9 May 2021 22:31:49 +0200 Subject: [PATCH 2/3] Implement @jackh726's suggestions --- .../rustc_resolve/src/late/diagnostics.rs | 131 ++++++++---------- compiler/rustc_resolve/src/late/lifetimes.rs | 9 +- .../ui/{issues => suggestions}/issue-84592.rs | 0 .../issue-84592.stderr | 0 src/tools/tidy/src/ui_tests.rs | 2 +- 5 files changed, 65 insertions(+), 77 deletions(-) rename src/test/ui/{issues => suggestions}/issue-84592.rs (100%) rename src/test/ui/{issues => suggestions}/issue-84592.stderr (100%) diff --git a/compiler/rustc_resolve/src/late/diagnostics.rs b/compiler/rustc_resolve/src/late/diagnostics.rs index ca4873bd51570..9b82d8afd2be1 100644 --- a/compiler/rustc_resolve/src/late/diagnostics.rs +++ b/compiler/rustc_resolve/src/late/diagnostics.rs @@ -1821,21 +1821,19 @@ impl<'tcx> LifetimeContext<'_, 'tcx> { crate fn add_missing_lifetime_specifiers_label( &self, err: &mut DiagnosticBuilder<'_>, - spans: Vec, - counts: Vec, + spans_with_counts: Vec<(Span, usize)>, lifetime_names: &FxHashSet, lifetime_spans: Vec, params: &[ElisionFailureInfo], ) { - let snippets: Vec> = spans + let snippets: Vec> = spans_with_counts .iter() - .copied() - .map(|span| self.tcx.sess.source_map().span_to_snippet(span).ok()) + .map(|(span, _)| self.tcx.sess.source_map().span_to_snippet(*span).ok()) .collect(); - for (span, count) in spans.iter().zip(counts.iter()) { + for (span, count) in &spans_with_counts { err.span_label( - span.clone(), + *span, format!( "expected {} lifetime parameter{}", if *count == 1 { "named".to_string() } else { count.to_string() }, @@ -1847,7 +1845,7 @@ impl<'tcx> LifetimeContext<'_, 'tcx> { let suggest_existing = |err: &mut DiagnosticBuilder<'_>, name: &str, - formatters: &Vec String>>>| { + formatters: Vec String>>>| { if let Some(MissingLifetimeSpot::HigherRanked { span: for_span, span_type }) = self.missing_named_lifetime_spots.iter().rev().next() { @@ -1892,9 +1890,9 @@ impl<'tcx> LifetimeContext<'_, 'tcx> { } } introduce_suggestion.push((*for_span, for_sugg)); - for (span, formatter) in spans.iter().copied().zip(formatters.iter()) { + for ((span, _), formatter) in spans_with_counts.iter().zip(formatters.iter()) { if let Some(formatter) = formatter { - introduce_suggestion.push((span, formatter(<_name))); + introduce_suggestion.push((*span, formatter(<_name))); } } err.multipart_suggestion_with_style( @@ -1905,12 +1903,12 @@ impl<'tcx> LifetimeContext<'_, 'tcx> { ); } - let mut spans_suggs: Vec<_> = Vec::new(); - for (span, fmt) in spans.iter().copied().zip(formatters.iter()) { - if let Some(formatter) = fmt { - spans_suggs.push((span, formatter(name))); - } - } + let spans_suggs: Vec<_> = formatters + .into_iter() + .filter_map(|fmt| fmt) + .zip(spans_with_counts.iter()) + .map(|(formatter, (span, _))| (*span, formatter(name))) + .collect(); err.multipart_suggestion_with_style( &format!( "consider using the `{}` lifetime", @@ -1921,7 +1919,7 @@ impl<'tcx> LifetimeContext<'_, 'tcx> { SuggestionStyle::ShowAlways, ); }; - let suggest_new = |err: &mut DiagnosticBuilder<'_>, suggs: &Vec>| { + let suggest_new = |err: &mut DiagnosticBuilder<'_>, suggs: Vec>| { for missing in self.missing_named_lifetime_spots.iter().rev() { let mut introduce_suggestion = vec![]; let msg; @@ -1967,8 +1965,8 @@ impl<'tcx> LifetimeContext<'_, 'tcx> { } MissingLifetimeSpot::Static => { let mut spans_suggs = Vec::new(); - for ((span, snippet), count) in - spans.iter().copied().zip(snippets.iter()).zip(counts.iter().copied()) + for ((span, count), snippet) in + spans_with_counts.iter().copied().zip(snippets.iter()) { let (span, sugg) = match snippet.as_deref() { Some("&") => (span.shrink_to_hi(), "'static ".to_owned()), @@ -2018,7 +2016,7 @@ impl<'tcx> LifetimeContext<'_, 'tcx> { } } } - for (span, sugg) in spans.iter().copied().zip(suggs.iter()) { + for ((span, _), sugg) in spans_with_counts.iter().copied().zip(suggs.iter()) { if let Some(sugg) = sugg { introduce_suggestion.push((span, sugg.to_string())); } @@ -2039,68 +2037,57 @@ impl<'tcx> LifetimeContext<'_, 'tcx> { match &lifetime_names[..] { [name] => { let mut suggs: Vec String>>> = Vec::new(); - for (snippet, count) in snippets.iter().cloned().zip(counts.iter().copied()) { - if snippet == Some("&".to_string()) { - suggs.push(Some(Box::new(|name| format!("&{} ", name)))); - } else if snippet == Some("'_".to_string()) { - suggs.push(Some(Box::new(|n| n.to_string()))); - } else if snippet == Some("".to_string()) { - suggs.push(Some(Box::new(move |n| format!("{}, ", n).repeat(count)))); - } else if let Some(snippet) = snippet { - if !snippet.ends_with('>') { - suggs.push(Some(Box::new(move |name| { - format!( - "{}<{}>", - snippet, - std::iter::repeat(name.to_string()) - .take(count) - .collect::>() - .join(", ") - ) - }))); - } else { - suggs.push(None); - } - } else { - suggs.push(None); - } + for (snippet, (_, count)) in snippets.iter().zip(spans_with_counts.iter().copied()) + { + suggs.push(match snippet.as_deref() { + Some("&") => Some(Box::new(|name| format!("&{} ", name))), + Some("'_") => Some(Box::new(|n| n.to_string())), + Some("") => Some(Box::new(move |n| format!("{}, ", n).repeat(count))), + Some(snippet) if !snippet.ends_with('>') => Some(Box::new(move |name| { + format!( + "{}<{}>", + snippet, + std::iter::repeat(name.to_string()) + .take(count) + .collect::>() + .join(", ") + ) + })), + _ => None, + }); } - suggest_existing(err, &name.as_str()[..], &suggs); + suggest_existing(err, &name.as_str()[..], suggs); } [] => { - let mut suggs: Vec> = Vec::new(); - for (snippet, count) in snippets.iter().cloned().zip(counts.iter().copied()) { - if snippet == Some("&".to_string()) { - suggs.push(Some("&'a ".to_string())); - } else if snippet == Some("'_".to_string()) { - suggs.push(Some("'a".to_string())); - } else if let Some(snippet) = snippet { - if snippet == "" { - suggs.push(Some( - std::iter::repeat("'a, ").take(count).collect::>().join(""), - )); - } else { - suggs.push(Some(format!( - "{}<{}>", - snippet, - std::iter::repeat("'a").take(count).collect::>().join(", ") - ))); + let mut suggs = Vec::new(); + for (snippet, (_, count)) in + snippets.iter().cloned().zip(spans_with_counts.iter().copied()) + { + suggs.push(match snippet.as_deref() { + Some("&") => Some("&'a ".to_string()), + Some("'_") => Some("'a".to_string()), + Some("") => { + Some(std::iter::repeat("'a, ").take(count).collect::>().join("")) } - } else { - suggs.push(None); - } + Some(snippet) => Some(format!( + "{}<{}>", + snippet, + std::iter::repeat("'a").take(count).collect::>().join(", "), + )), + None => None, + }); } - suggest_new(err, &suggs); + suggest_new(err, suggs); } lts if lts.len() > 1 => { err.span_note(lifetime_spans, "these named lifetimes are available to use"); let mut spans_suggs: Vec<_> = Vec::new(); - for (span, snippet) in spans.iter().copied().zip(snippets.iter()) { - if Some("") == snippet.as_deref() { - spans_suggs.push((span, "'lifetime, ".to_string())); - } else if Some("&") == snippet.as_deref() { - spans_suggs.push((span, "&'lifetime ".to_string())); + for ((span, _), snippet) in spans_with_counts.iter().copied().zip(snippets.iter()) { + match snippet.as_deref() { + Some("") => spans_suggs.push((span, "'lifetime, ".to_string())), + Some("&") => spans_suggs.push((span, "&'lifetime ".to_string())), + _ => {} } } diff --git a/compiler/rustc_resolve/src/late/lifetimes.rs b/compiler/rustc_resolve/src/late/lifetimes.rs index c81269a46b21a..e8d21af435887 100644 --- a/compiler/rustc_resolve/src/late/lifetimes.rs +++ b/compiler/rustc_resolve/src/late/lifetimes.rs @@ -3038,8 +3038,10 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> { spans.sort(); let mut spans_dedup = spans.clone(); spans_dedup.dedup(); - let counts: Vec<_> = - spans_dedup.iter().map(|sp| spans.iter().filter(|nsp| *nsp == sp).count()).collect(); + let spans_with_counts: Vec<_> = spans_dedup + .into_iter() + .map(|sp| (sp, spans.iter().filter(|nsp| *nsp == &sp).count())) + .collect(); let mut err = self.report_missing_lifetime_specifiers(spans.clone(), lifetime_refs.len()); @@ -3052,8 +3054,7 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> { self.add_missing_lifetime_specifiers_label( &mut err, - spans, - counts, + spans_with_counts, &lifetime_names, lifetime_spans, error.unwrap_or(&[]), diff --git a/src/test/ui/issues/issue-84592.rs b/src/test/ui/suggestions/issue-84592.rs similarity index 100% rename from src/test/ui/issues/issue-84592.rs rename to src/test/ui/suggestions/issue-84592.rs diff --git a/src/test/ui/issues/issue-84592.stderr b/src/test/ui/suggestions/issue-84592.stderr similarity index 100% rename from src/test/ui/issues/issue-84592.stderr rename to src/test/ui/suggestions/issue-84592.stderr diff --git a/src/tools/tidy/src/ui_tests.rs b/src/tools/tidy/src/ui_tests.rs index 791489b3960a5..8334bc68ae729 100644 --- a/src/tools/tidy/src/ui_tests.rs +++ b/src/tools/tidy/src/ui_tests.rs @@ -8,7 +8,7 @@ use std::path::Path; const ENTRY_LIMIT: usize = 1000; // FIXME: The following limits should be reduced eventually. const ROOT_ENTRY_LIMIT: usize = 1388; -const ISSUES_ENTRY_LIMIT: usize = 2557; +const ISSUES_ENTRY_LIMIT: usize = 2551; fn check_entries(path: &Path, bad: &mut bool) { let dirs = walkdir::WalkDir::new(&path.join("test/ui")) From 2448c7698ef186ead88bd7980f2cac28c55111a8 Mon Sep 17 00:00:00 2001 From: Fabian Wolff Date: Mon, 10 May 2021 14:59:54 +0200 Subject: [PATCH 3/3] More minor fixes suggested by @jackh726 --- compiler/rustc_errors/src/diagnostic.rs | 18 +++++------------- compiler/rustc_resolve/src/late/diagnostics.rs | 6 ++++-- .../return-elided-lifetime.rs | 0 .../return-elided-lifetime.stderr | 0 4 files changed, 9 insertions(+), 15 deletions(-) rename src/test/ui/{ => suggestions}/return-elided-lifetime.rs (100%) rename src/test/ui/{ => suggestions}/return-elided-lifetime.stderr (100%) diff --git a/compiler/rustc_errors/src/diagnostic.rs b/compiler/rustc_errors/src/diagnostic.rs index 405dd2e68c6d0..14ccced2c6a56 100644 --- a/compiler/rustc_errors/src/diagnostic.rs +++ b/compiler/rustc_errors/src/diagnostic.rs @@ -283,20 +283,12 @@ impl Diagnostic { suggestion: Vec<(Span, String)>, applicability: Applicability, ) -> &mut Self { - assert!(!suggestion.is_empty()); - self.suggestions.push(CodeSuggestion { - substitutions: vec![Substitution { - parts: suggestion - .into_iter() - .map(|(span, snippet)| SubstitutionPart { snippet, span }) - .collect(), - }], - msg: msg.to_owned(), - style: SuggestionStyle::ShowCode, + self.multipart_suggestion_with_style( + msg, + suggestion, applicability, - tool_metadata: Default::default(), - }); - self + SuggestionStyle::ShowCode, + ) } /// [`Diagnostic::multipart_suggestion()`] but you can set the [`SuggestionStyle`]. diff --git a/compiler/rustc_resolve/src/late/diagnostics.rs b/compiler/rustc_resolve/src/late/diagnostics.rs index 9b82d8afd2be1..fdde687d4866c 100644 --- a/compiler/rustc_resolve/src/late/diagnostics.rs +++ b/compiler/rustc_resolve/src/late/diagnostics.rs @@ -1905,9 +1905,11 @@ impl<'tcx> LifetimeContext<'_, 'tcx> { let spans_suggs: Vec<_> = formatters .into_iter() - .filter_map(|fmt| fmt) .zip(spans_with_counts.iter()) - .map(|(formatter, (span, _))| (*span, formatter(name))) + .filter_map(|(fmt, (span, _))| { + if let Some(formatter) = fmt { Some((formatter, span)) } else { None } + }) + .map(|(formatter, span)| (*span, formatter(name))) .collect(); err.multipart_suggestion_with_style( &format!( diff --git a/src/test/ui/return-elided-lifetime.rs b/src/test/ui/suggestions/return-elided-lifetime.rs similarity index 100% rename from src/test/ui/return-elided-lifetime.rs rename to src/test/ui/suggestions/return-elided-lifetime.rs diff --git a/src/test/ui/return-elided-lifetime.stderr b/src/test/ui/suggestions/return-elided-lifetime.stderr similarity index 100% rename from src/test/ui/return-elided-lifetime.stderr rename to src/test/ui/suggestions/return-elided-lifetime.stderr