Skip to content

Commit

Permalink
Rollup merge of #85050 - FabianWolff:issue-84592, r=jackh726
Browse files Browse the repository at this point in the history
Fix suggestions for missing return type lifetime specifiers

This pull request aims to fix #84592. The issue is that the current code seems to assume that there is only a single relevant span pointing to the missing lifetime, and only looks at the first one:
https://github.com/rust-lang/rust/blob/e5f83d24aee866a14753a7cedbb4e301dfe5bef5/compiler/rustc_resolve/src/late/lifetimes.rs#L2959

This is incorrect, though, and leads to incorrect error messages and invalid suggestions. For instance, the example from #84592:
```rust
struct TwoLifetimes<'x, 'y> {
    x: &'x (),
    y: &'y (),
}

fn two_lifetimes_needed(a: &(), b: &()) -> TwoLifetimes<'_, '_> {
    TwoLifetimes { x: &(), y: &() }
}
```
currently leads to:
```
error[E0106]: missing lifetime specifiers
 --> src/main.rs:6:57
  |
6 | fn two_lifetimes_needed(a: &(), b: &()) -> TwoLifetimes<'_, '_> {
  |                            ---     ---                  ^^ expected 2 lifetime parameters
  |
  = 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
  |
6 | fn two_lifetimes_needed<'a>(a: &'a (), b: &'a ()) -> TwoLifetimes<'_<'a, 'a>, '_> {
  |                        ^^^^    ^^^^^^     ^^^^^^                  ^^^^^^^^^^
```
There are two problems:
- The error message is wrong. There is only _one_ lifetime parameter expected at the location pointed to by the error message (and another one at a separate location).
- The suggestion is incorrect and will not lead to correct code.

With the changes in this PR, I get the following output:
```
error[E0106]: missing lifetime specifiers
 --> p.rs:6:57
  |
6 | 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
  |
6 | 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`.
```
Mainly, I changed `add_missing_lifetime_specifiers_label()` to receive a _vector_ of spans (and counts) instead of just one, and adjusted its body accordingly.
  • Loading branch information
Dylan-DPC authored May 10, 2021
2 parents 1b30245 + 2448c76 commit 0740015
Show file tree
Hide file tree
Showing 8 changed files with 493 additions and 148 deletions.
18 changes: 17 additions & 1 deletion compiler/rustc_errors/src/diagnostic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,22 @@ impl Diagnostic {
msg: &str,
suggestion: Vec<(Span, String)>,
applicability: Applicability,
) -> &mut Self {
self.multipart_suggestion_with_style(
msg,
suggestion,
applicability,
SuggestionStyle::ShowCode,
)
}

/// [`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 {
Expand All @@ -292,7 +308,7 @@ impl Diagnostic {
.collect(),
}],
msg: msg.to_owned(),
style: SuggestionStyle::ShowCode,
style,
applicability,
tool_metadata: Default::default(),
});
Expand Down
334 changes: 191 additions & 143 deletions compiler/rustc_resolve/src/late/diagnostics.rs

Large diffs are not rendered by default.

16 changes: 12 additions & 4 deletions compiler/rustc_resolve/src/late/lifetimes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -3035,18 +3034,27 @@ 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 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());

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(kw::StaticLifetime);
}
}

self.add_missing_lifetime_specifiers_label(
&mut err,
span,
lifetime_refs.len(),
spans_with_counts,
&lifetime_names,
lifetime_spans,
error.unwrap_or(&[]),
Expand Down
17 changes: 17 additions & 0 deletions src/test/ui/suggestions/issue-84592.rs
Original file line number Diff line number Diff line change
@@ -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() {}
17 changes: 17 additions & 0 deletions src/test/ui/suggestions/issue-84592.stderr
Original file line number Diff line number Diff line change
@@ -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`.
4 changes: 4 additions & 0 deletions src/test/ui/suggestions/missing-lt-for-hrtb.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
37 changes: 37 additions & 0 deletions src/test/ui/suggestions/return-elided-lifetime.rs
Original file line number Diff line number Diff line change
@@ -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() {}
198 changes: 198 additions & 0 deletions src/test/ui/suggestions/return-elided-lifetime.stderr
Original file line number Diff line number Diff line change
@@ -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`.

0 comments on commit 0740015

Please sign in to comment.