Skip to content

Commit 29c603c

Browse files
matthiaskrgrmatthiaskrgr
authored
Rollup merge of rust-lang#124682 - estebank:issue-40990, r=pnkfelix
Suggest setting lifetime in borrowck error involving types with elided lifetimes ``` error: lifetime may not live long enough --> $DIR/ex3-both-anon-regions-both-are-structs-2.rs:7:5 | LL | fn foo(mut x: Ref, y: Ref) { | ----- - has type `Ref<'_, '1>` | | | has type `Ref<'_, '2>` LL | x.b = y.b; | ^^^^^^^^^ assignment requires that `'1` must outlive `'2` | help: consider introducing a named lifetime parameter | LL | fn foo<'a>(mut x: Ref<'a, 'a>, y: Ref<'a, 'a>) { | ++++ ++++++++ ++++++++ ``` As can be seen above, it currently doesn't try to compare the `ty::Ty` lifetimes that diverged vs the `hir::Ty` to correctly suggest the following ``` help: consider introducing a named lifetime parameter | LL | fn foo<'a>(mut x: Ref<'_, 'a>, y: Ref<'_, 'a>) { | ++++ ++++++++ ++++++++ ``` but I believe this to still be an improvement over the status quo. Fix rust-lang#40990.
2 parents 75cb5c5 + cf5702e commit 29c603c

File tree

42 files changed

+698
-216
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

42 files changed

+698
-216
lines changed

‎compiler/rustc_infer/messages.ftl‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,10 @@ infer_label_bad = {$bad_kind ->
164164
infer_lf_bound_not_satisfied = lifetime bound not satisfied
165165
infer_lifetime_mismatch = lifetime mismatch
166166
167-
infer_lifetime_param_suggestion = consider introducing a named lifetime parameter{$is_impl ->
167+
infer_lifetime_param_suggestion = consider {$is_reuse ->
168+
[true] reusing
169+
*[false] introducing
170+
} a named lifetime parameter{$is_impl ->
168171
[true] {" "}and update trait if needed
169172
*[false] {""}
170173
}

‎compiler/rustc_infer/src/errors/mod.rs‎

Lines changed: 127 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11
use hir::GenericParamKind;
2+
use rustc_data_structures::fx::FxHashSet;
23
use rustc_errors::{
34
codes::*, Applicability, Diag, DiagMessage, DiagStyledString, EmissionGuarantee, IntoDiagArg,
45
MultiSpan, SubdiagMessageOp, Subdiagnostic,
56
};
67
use rustc_hir as hir;
8+
use rustc_hir::intravisit::{walk_ty, Visitor};
79
use rustc_hir::FnRetTy;
810
use rustc_macros::{Diagnostic, Subdiagnostic};
911
use rustc_middle::ty::print::TraitRefPrintOnlyTraitPath;
@@ -355,31 +357,33 @@ impl Subdiagnostic for AddLifetimeParamsSuggestion<'_> {
355357
_f: &F,
356358
) {
357359
let mut mk_suggestion = || {
358-
let (
359-
hir::Ty { kind: hir::TyKind::Ref(lifetime_sub, _), .. },
360-
hir::Ty { kind: hir::TyKind::Ref(lifetime_sup, _), .. },
361-
) = (self.ty_sub, self.ty_sup)
362-
else {
363-
return false;
364-
};
365-
366-
if !lifetime_sub.is_anonymous() || !lifetime_sup.is_anonymous() {
367-
return false;
368-
};
369-
370360
let Some(anon_reg) = self.tcx.is_suitable_region(self.sub) else {
371361
return false;
372362
};
373363

374364
let node = self.tcx.hir_node_by_def_id(anon_reg.def_id);
375365
let is_impl = matches!(&node, hir::Node::ImplItem(_));
376-
let generics = match node {
366+
let (generics, parent_generics) = match node {
377367
hir::Node::Item(&hir::Item {
378368
kind: hir::ItemKind::Fn(_, ref generics, ..),
379369
..
380370
})
381371
| hir::Node::TraitItem(&hir::TraitItem { ref generics, .. })
382-
| hir::Node::ImplItem(&hir::ImplItem { ref generics, .. }) => generics,
372+
| hir::Node::ImplItem(&hir::ImplItem { ref generics, .. }) => (
373+
generics,
374+
match self.tcx.parent_hir_node(self.tcx.local_def_id_to_hir_id(anon_reg.def_id))
375+
{
376+
hir::Node::Item(hir::Item {
377+
kind: hir::ItemKind::Trait(_, _, ref generics, ..),
378+
..
379+
})
380+
| hir::Node::Item(hir::Item {
381+
kind: hir::ItemKind::Impl(hir::Impl { ref generics, .. }),
382+
..
383+
}) => Some(generics),
384+
_ => None,
385+
},
386+
),
383387
_ => return false,
384388
};
385389

@@ -390,24 +394,112 @@ impl Subdiagnostic for AddLifetimeParamsSuggestion<'_> {
390394
.map(|p| p.name.ident().name)
391395
.find(|i| *i != kw::UnderscoreLifetime);
392396
let introduce_new = suggestion_param_name.is_none();
397+
398+
let mut default = "'a".to_string();
399+
if let Some(parent_generics) = parent_generics {
400+
let used: FxHashSet<_> = parent_generics
401+
.params
402+
.iter()
403+
.filter(|p| matches!(p.kind, GenericParamKind::Lifetime { .. }))
404+
.map(|p| p.name.ident().name)
405+
.filter(|i| *i != kw::UnderscoreLifetime)
406+
.map(|l| l.to_string())
407+
.collect();
408+
if let Some(lt) =
409+
('a'..='z').map(|it| format!("'{it}")).find(|it| !used.contains(it))
410+
{
411+
// We want a lifetime that *isn't* present in the `trait` or `impl` that assoc
412+
// `fn` belongs to. We could suggest reusing one of their lifetimes, but it is
413+
// likely to be an over-constraining lifetime requirement, so we always add a
414+
// lifetime to the `fn`.
415+
default = lt;
416+
}
417+
}
393418
let suggestion_param_name =
394-
suggestion_param_name.map(|n| n.to_string()).unwrap_or_else(|| "'a".to_owned());
395-
396-
debug!(?lifetime_sup.ident.span);
397-
debug!(?lifetime_sub.ident.span);
398-
let make_suggestion = |ident: Ident| {
399-
let sugg = if ident.name == kw::Empty {
400-
format!("{suggestion_param_name}, ")
401-
} else if ident.name == kw::UnderscoreLifetime && ident.span.is_empty() {
402-
format!("{suggestion_param_name} ")
403-
} else {
404-
suggestion_param_name.clone()
405-
};
406-
(ident.span, sugg)
407-
};
408-
let mut suggestions =
409-
vec![make_suggestion(lifetime_sub.ident), make_suggestion(lifetime_sup.ident)];
419+
suggestion_param_name.map(|n| n.to_string()).unwrap_or_else(|| default);
420+
421+
struct ImplicitLifetimeFinder {
422+
suggestions: Vec<(Span, String)>,
423+
suggestion_param_name: String,
424+
}
410425

426+
impl<'v> Visitor<'v> for ImplicitLifetimeFinder {
427+
fn visit_ty(&mut self, ty: &'v hir::Ty<'v>) {
428+
let make_suggestion = |ident: Ident| {
429+
if ident.name == kw::Empty && ident.span.is_empty() {
430+
format!("{}, ", self.suggestion_param_name)
431+
} else if ident.name == kw::UnderscoreLifetime && ident.span.is_empty() {
432+
format!("{} ", self.suggestion_param_name)
433+
} else {
434+
self.suggestion_param_name.clone()
435+
}
436+
};
437+
match ty.kind {
438+
hir::TyKind::Path(hir::QPath::Resolved(_, path)) => {
439+
for segment in path.segments {
440+
if let Some(args) = segment.args {
441+
if args.args.iter().all(|arg| {
442+
matches!(
443+
arg,
444+
hir::GenericArg::Lifetime(lifetime)
445+
if lifetime.ident.name == kw::Empty
446+
)
447+
}) {
448+
self.suggestions.push((
449+
segment.ident.span.shrink_to_hi(),
450+
format!(
451+
"<{}>",
452+
args.args
453+
.iter()
454+
.map(|_| self.suggestion_param_name.clone())
455+
.collect::<Vec<_>>()
456+
.join(", ")
457+
),
458+
));
459+
} else {
460+
for arg in args.args {
461+
if let hir::GenericArg::Lifetime(lifetime) = arg
462+
&& lifetime.is_anonymous()
463+
{
464+
self.suggestions.push((
465+
lifetime.ident.span,
466+
make_suggestion(lifetime.ident),
467+
));
468+
}
469+
}
470+
}
471+
}
472+
}
473+
}
474+
hir::TyKind::Ref(lifetime, ..) if lifetime.is_anonymous() => {
475+
self.suggestions
476+
.push((lifetime.ident.span, make_suggestion(lifetime.ident)));
477+
}
478+
_ => {}
479+
}
480+
walk_ty(self, ty);
481+
}
482+
}
483+
let mut visitor = ImplicitLifetimeFinder {
484+
suggestions: vec![],
485+
suggestion_param_name: suggestion_param_name.clone(),
486+
};
487+
if let Some(fn_decl) = node.fn_decl()
488+
&& let hir::FnRetTy::Return(ty) = fn_decl.output
489+
{
490+
visitor.visit_ty(ty);
491+
}
492+
if visitor.suggestions.is_empty() {
493+
// Do not suggest constraining the `&self` param, but rather the return type.
494+
// If that is wrong (because it is not sufficient), a follow up error will tell the
495+
// user to fix it. This way we lower the chances of *over* constraining, but still
496+
// get the cake of "correctly" contrained in two steps.
497+
visitor.visit_ty(self.ty_sup);
498+
}
499+
visitor.visit_ty(self.ty_sub);
500+
if visitor.suggestions.is_empty() {
501+
return false;
502+
}
411503
if introduce_new {
412504
let new_param_suggestion = if let Some(first) =
413505
generics.params.iter().find(|p| !p.name.ident().span.is_empty())
@@ -417,15 +509,16 @@ impl Subdiagnostic for AddLifetimeParamsSuggestion<'_> {
417509
(generics.span, format!("<{suggestion_param_name}>"))
418510
};
419511

420-
suggestions.push(new_param_suggestion);
512+
visitor.suggestions.push(new_param_suggestion);
421513
}
422-
423-
diag.multipart_suggestion(
514+
diag.multipart_suggestion_verbose(
424515
fluent::infer_lifetime_param_suggestion,
425-
suggestions,
516+
visitor.suggestions,
426517
Applicability::MaybeIncorrect,
427518
);
428519
diag.arg("is_impl", is_impl);
520+
diag.arg("is_reuse", !introduce_new);
521+
429522
true
430523
};
431524
if mk_suggestion() && self.add_note {

‎tests/ui/lifetimes/issue-17728.stderr‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,8 @@ LL | Some(entry) => Ok(entry),
2929
|
3030
help: consider introducing a named lifetime parameter
3131
|
32-
LL | fn attemptTraverse<'a>(&'a self, room: &'a Room, directionStr: &str) -> Result<&Room, &str> {
33-
| ++++ ++ ++
32+
LL | fn attemptTraverse<'a>(&self, room: &'a Room, directionStr: &str) -> Result<&'a Room, &'a str> {
33+
| ++++ ++ ++ ++
3434

3535
error: aborting due to 2 previous errors
3636

‎tests/ui/lifetimes/issue-90170-elision-mismatch.stderr‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ LL | pub fn foo3<'a>(_other: &'a [u8], x: &mut Vec<&u8>, y: &u8) { x.push(y); }
3535
| | let's call the lifetime of this reference `'1`
3636
| let's call the lifetime of this reference `'2`
3737
|
38-
help: consider introducing a named lifetime parameter
38+
help: consider reusing a named lifetime parameter
3939
|
4040
LL | pub fn foo3<'a>(_other: &'a [u8], x: &mut Vec<&'a u8>, y: &'a u8) { x.push(y); }
4141
| ++ ++

‎tests/ui/lifetimes/lifetime-errors/ex1-return-one-existing-name-if-else-using-impl.stderr‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,11 @@ LL | fn foo<'a>(x: &i32, y: &'a i32) -> &'a i32 {
88
LL |
99
LL | if x > y { x } else { y }
1010
| ^ associated function was supposed to return data with lifetime `'a` but it is returning data with lifetime `'1`
11+
|
12+
help: consider reusing a named lifetime parameter and update trait if needed
13+
|
14+
LL | fn foo<'a>(x: &'a i32, y: &'a i32) -> &'a i32 {
15+
| ++
1116

1217
error: aborting due to 1 previous error
1318

‎tests/ui/lifetimes/lifetime-errors/ex1-return-one-existing-name-return-type-is-anon.fixed‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
//@ run-rustfix
2+
#![allow(dead_code)]
3+
struct Foo {
4+
field: i32,
5+
}
6+
7+
impl Foo {
8+
fn foo<'a>(&self, x: &'a i32) -> &'a i32 {
9+
x
10+
//~^ ERROR lifetime may not live long enough
11+
}
12+
}
13+
14+
fn main() {}

‎tests/ui/lifetimes/lifetime-errors/ex1-return-one-existing-name-return-type-is-anon.rs‎

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,14 @@
1+
//@ run-rustfix
2+
#![allow(dead_code)]
13
struct Foo {
2-
field: i32
4+
field: i32,
35
}
46

57
impl Foo {
6-
fn foo<'a>(&self, x: &'a i32) -> &i32 {
7-
8-
x
9-
//~^ ERROR lifetime may not live long enough
10-
11-
}
12-
8+
fn foo<'a>(&self, x: &'a i32) -> &i32 {
9+
x
10+
//~^ ERROR lifetime may not live long enough
11+
}
1312
}
1413

15-
fn main() { }
14+
fn main() {}

‎tests/ui/lifetimes/lifetime-errors/ex1-return-one-existing-name-return-type-is-anon.stderr‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,17 @@
11
error: lifetime may not live long enough
2-
--> $DIR/ex1-return-one-existing-name-return-type-is-anon.rs:8:5
2+
--> $DIR/ex1-return-one-existing-name-return-type-is-anon.rs:9:9
33
|
4-
LL | fn foo<'a>(&self, x: &'a i32) -> &i32 {
5-
| -- - let's call the lifetime of this reference `'1`
6-
| |
7-
| lifetime `'a` defined here
8-
LL |
9-
LL | x
10-
| ^ method was supposed to return data with lifetime `'1` but it is returning data with lifetime `'a`
4+
LL | fn foo<'a>(&self, x: &'a i32) -> &i32 {
5+
| -- - let's call the lifetime of this reference `'1`
6+
| |
7+
| lifetime `'a` defined here
8+
LL | x
9+
| ^ method was supposed to return data with lifetime `'1` but it is returning data with lifetime `'a`
10+
|
11+
help: consider reusing a named lifetime parameter and update trait if needed
12+
|
13+
LL | fn foo<'a>(&self, x: &'a i32) -> &'a i32 {
14+
| ++
1115

1216
error: aborting due to 1 previous error
1317

‎tests/ui/lifetimes/lifetime-errors/ex1-return-one-existing-name-self-is-anon.stderr‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,11 @@ LL | fn foo<'a>(&self, x: &'a Foo) -> &'a Foo {
88
LL |
99
LL | if true { x } else { self }
1010
| ^^^^ method was supposed to return data with lifetime `'a` but it is returning data with lifetime `'1`
11+
|
12+
help: consider reusing a named lifetime parameter and update trait if needed
13+
|
14+
LL | fn foo<'a>(&'a self, x: &'a Foo) -> &'a Foo {
15+
| ++
1116

1217
error: aborting due to 1 previous error
1318

‎tests/ui/lifetimes/lifetime-errors/ex2b-push-no-existing-names.stderr‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,11 @@ LL | fn foo(x: &mut Vec<Ref<i32>>, y: Ref<i32>) {
77
| has type `&mut Vec<Ref<'2, i32>>`
88
LL | x.push(y);
99
| ^^^^^^^^^ argument requires that `'1` must outlive `'2`
10+
|
11+
help: consider introducing a named lifetime parameter
12+
|
13+
LL | fn foo<'a>(x: &mut Vec<Ref<'a, i32>>, y: Ref<'a, i32>) {
14+
| ++++ +++ +++
1015

1116
error: aborting due to 1 previous error
1217

0 commit comments

Comments
 (0)