Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Recover suggestions to introduce named lifetime under NLL #96409

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 34 additions & 1 deletion compiler/rustc_borrowck/src/diagnostics/region_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@

use rustc_errors::{Diagnostic, DiagnosticBuilder, ErrorGuaranteed};
use rustc_infer::infer::{
error_reporting::nice_region_error::{self, find_param_with_region, NiceRegionError},
error_reporting::nice_region_error::{
self, find_anon_type, find_param_with_region, suggest_adding_lifetime_params,
NiceRegionError,
},
error_reporting::unexpected_hidden_region_diagnostic,
NllRegionVariableOrigin, RelateParamBound,
};
Expand Down Expand Up @@ -630,6 +633,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
}

self.add_static_impl_trait_suggestion(&mut diag, *fr, fr_name, *outlived_fr);
self.suggest_adding_lifetime_params(&mut diag, *fr, *outlived_fr);

diag
}
Expand Down Expand Up @@ -694,4 +698,33 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
);
}
}

fn suggest_adding_lifetime_params(
&self,
diag: &mut Diagnostic,
sub: RegionVid,
sup: RegionVid,
) {
let (Some(sub), Some(sup)) = (self.to_error_region(sub), self.to_error_region(sup)) else {
return
};

let Some((ty_sub, _)) = self
.infcx
.tcx
.is_suitable_region(sub)
.and_then(|anon_reg| find_anon_type(self.infcx.tcx, sub, &anon_reg.boundregion)) else {
return
};

let Some((ty_sup, _)) = self
.infcx
.tcx
.is_suitable_region(sup)
.and_then(|anon_reg| find_anon_type(self.infcx.tcx, sup, &anon_reg.boundregion)) else {
return
};

suggest_adding_lifetime_params(self.infcx.tcx, sub, ty_sup, ty_sub, diag);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use crate::infer::error_reporting::nice_region_error::util::AnonymousParamInfo;
use crate::infer::error_reporting::nice_region_error::NiceRegionError;
use crate::infer::lexical_region_resolve::RegionResolutionError;
use crate::infer::SubregionOrigin;
use crate::infer::TyCtxt;

use rustc_errors::{struct_span_err, Applicability, Diagnostic, ErrorGuaranteed};
use rustc_hir as hir;
Expand Down Expand Up @@ -145,84 +146,83 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
}
}

self.suggest_adding_lifetime_params(sub, ty_sup, ty_sub, &mut err);
if suggest_adding_lifetime_params(self.tcx(), sub, ty_sup, ty_sub, &mut err) {
err.note("each elided lifetime in input position becomes a distinct lifetime");
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note was originally in suggest_adding_lifetime_params but by moving it here we can avoid having the note under NLL.
However if we do want the note, I can just move it back to where it comes from

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm. The note is kind of redundant with the NLL error output, I guess.


let reported = err.emit();
Some(reported)
}
}

fn suggest_adding_lifetime_params(
&self,
sub: Region<'tcx>,
ty_sup: &Ty<'_>,
ty_sub: &Ty<'_>,
err: &mut Diagnostic,
) {
if let (
hir::Ty { kind: hir::TyKind::Rptr(lifetime_sub, _), .. },
hir::Ty { kind: hir::TyKind::Rptr(lifetime_sup, _), .. },
) = (ty_sub, ty_sup)
{
if lifetime_sub.name.is_elided() && lifetime_sup.name.is_elided() {
if let Some(anon_reg) = self.tcx().is_suitable_region(sub) {
let hir_id = self.tcx().hir().local_def_id_to_hir_id(anon_reg.def_id);

let node = self.tcx().hir().get(hir_id);
let is_impl = matches!(&node, hir::Node::ImplItem(_));
let generics = match node {
hir::Node::Item(&hir::Item {
kind: hir::ItemKind::Fn(_, ref generics, ..),
..
})
| hir::Node::TraitItem(&hir::TraitItem { ref generics, .. })
| hir::Node::ImplItem(&hir::ImplItem { ref generics, .. }) => generics,
_ => return,
};

let (suggestion_param_name, introduce_new) = generics
.params
.iter()
.find(|p| matches!(p.kind, GenericParamKind::Lifetime { .. }))
.and_then(|p| self.tcx().sess.source_map().span_to_snippet(p.span).ok())
.map(|name| (name, false))
.unwrap_or_else(|| ("'a".to_string(), true));

let mut suggestions = vec![
if let hir::LifetimeName::Underscore = lifetime_sub.name {
(lifetime_sub.span, suggestion_param_name.clone())
} else {
(lifetime_sub.span.shrink_to_hi(), suggestion_param_name.clone() + " ")
},
if let hir::LifetimeName::Underscore = lifetime_sup.name {
(lifetime_sup.span, suggestion_param_name.clone())
} else {
(lifetime_sup.span.shrink_to_hi(), suggestion_param_name.clone() + " ")
},
];

if introduce_new {
let new_param_suggestion = match &generics.params {
[] => (generics.span, format!("<{}>", suggestion_param_name)),
[first, ..] => {
(first.span.shrink_to_lo(), format!("{}, ", suggestion_param_name))
}
};

suggestions.push(new_param_suggestion);
}

let mut sugg = String::from("consider introducing a named lifetime parameter");
if is_impl {
sugg.push_str(" and update trait if needed");
}
err.multipart_suggestion(
sugg.as_str(),
suggestions,
Applicability::MaybeIncorrect,
);
err.note("each elided lifetime in input position becomes a distinct lifetime");
}
}
}
pub fn suggest_adding_lifetime_params<'tcx>(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Method moved as a free standing function

tcx: TyCtxt<'tcx>,
sub: Region<'tcx>,
ty_sup: &Ty<'_>,
ty_sub: &Ty<'_>,
err: &mut Diagnostic,
) -> bool {
let (
hir::Ty { kind: hir::TyKind::Rptr(lifetime_sub, _), .. },
hir::Ty { kind: hir::TyKind::Rptr(lifetime_sup, _), .. },
) = (ty_sub, ty_sup) else {
return false;
};

if !lifetime_sub.name.is_elided() || !lifetime_sup.name.is_elided() {
return false;
};

let Some(anon_reg) = tcx.is_suitable_region(sub) else {
return false;
};

let hir_id = tcx.hir().local_def_id_to_hir_id(anon_reg.def_id);

let node = tcx.hir().get(hir_id);
let is_impl = matches!(&node, hir::Node::ImplItem(_));
let generics = match node {
hir::Node::Item(&hir::Item { kind: hir::ItemKind::Fn(_, ref generics, ..), .. })
| hir::Node::TraitItem(&hir::TraitItem { ref generics, .. })
| hir::Node::ImplItem(&hir::ImplItem { ref generics, .. }) => generics,
_ => return false,
};

let (suggestion_param_name, introduce_new) = generics
.params
.iter()
.find(|p| matches!(p.kind, GenericParamKind::Lifetime { .. }))
.and_then(|p| tcx.sess.source_map().span_to_snippet(p.span).ok())
.map(|name| (name, false))
.unwrap_or_else(|| ("'a".to_string(), true));

let mut suggestions = vec![
if let hir::LifetimeName::Underscore = lifetime_sub.name {
(lifetime_sub.span, suggestion_param_name.clone())
} else {
(lifetime_sub.span.shrink_to_hi(), suggestion_param_name.clone() + " ")
},
if let hir::LifetimeName::Underscore = lifetime_sup.name {
(lifetime_sup.span, suggestion_param_name.clone())
} else {
(lifetime_sup.span.shrink_to_hi(), suggestion_param_name.clone() + " ")
},
];

if introduce_new {
let new_param_suggestion = match &generics.params {
[] => (generics.span, format!("<{}>", suggestion_param_name)),
[first, ..] => (first.span.shrink_to_lo(), format!("{}, ", suggestion_param_name)),
};

suggestions.push(new_param_suggestion);
}

let mut sugg = String::from("consider introducing a named lifetime parameter");
if is_impl {
sugg.push_str(" and update trait if needed");
}
err.multipart_suggestion(sugg.as_str(), suggestions, Applicability::MaybeIncorrect);

true
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use rustc_middle::ty::{self, Region, TyCtxt};
/// ```
/// The function returns the nested type corresponding to the anonymous region
/// for e.g., `&u8` and `Vec<&u8>`.
pub(crate) fn find_anon_type<'tcx>(
pub fn find_anon_type<'tcx>(
tcx: TyCtxt<'tcx>,
region: Region<'tcx>,
br: &ty::BoundRegionKind,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ mod static_impl_trait;
mod trait_impl_difference;
mod util;

pub use different_lifetimes::suggest_adding_lifetime_params;
pub use find_anon_type::find_anon_type;
pub use static_impl_trait::suggest_new_region_bound;
pub use util::find_param_with_region;

Expand Down
15 changes: 15 additions & 0 deletions src/test/ui/lifetimes/issue-90170-elision-mismatch.nll.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@ LL | pub fn foo(x: &mut Vec<&u8>, y: &u8) { x.push(y); }
| | |
| | let's call the lifetime of this reference `'1`
| let's call the lifetime of this reference `'2`
|
help: consider introducing a named lifetime parameter
|
LL | pub fn foo<'a>(x: &mut Vec<&'a u8>, y: &'a u8) { x.push(y); }
| ++++ ++ ++

error: lifetime may not live long enough
--> $DIR/issue-90170-elision-mismatch.rs:5:44
Expand All @@ -15,6 +20,11 @@ LL | pub fn foo2(x: &mut Vec<&'_ u8>, y: &u8) { x.push(y); }
| | |
| | let's call the lifetime of this reference `'1`
| let's call the lifetime of this reference `'2`
|
help: consider introducing a named lifetime parameter
|
LL | pub fn foo2<'a>(x: &mut Vec<&'a u8>, y: &'a u8) { x.push(y); }
| ++++ ~~ ++

error: lifetime may not live long enough
--> $DIR/issue-90170-elision-mismatch.rs:7:63
Expand All @@ -24,6 +34,11 @@ LL | pub fn foo3<'a>(_other: &'a [u8], x: &mut Vec<&u8>, y: &u8) { x.push(y); }
| | |
| | let's call the lifetime of this reference `'1`
| let's call the lifetime of this reference `'2`
|
help: consider introducing a named lifetime parameter
|
LL | pub fn foo3<'a>(_other: &'a [u8], x: &mut Vec<&'a u8>, y: &'a u8) { x.push(y); }
| ++ ++

error: aborting due to 3 previous errors

Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,11 @@ LL | fn foo(&mut (ref mut v, w): &mut (&u8, &u8), x: &u8) {
| let's call the lifetime of this reference `'2`
LL | *v = x;
| ^^^^^^ assignment requires that `'1` must outlive `'2`
|
help: consider introducing a named lifetime parameter
|
LL | fn foo<'a>(&mut (ref mut v, w): &mut (&'a u8, &u8), x: &'a u8) {
| ++++ ++ ++

error: aborting due to previous error

Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,11 @@ LL | fn foo(z: &mut Vec<(&u8,&u8)>, (x, y): (&u8, &u8)) {
| let's call the lifetime of this reference `'2`
LL | z.push((x,y));
| ^^^^^^^^^^^^^ argument requires that `'1` must outlive `'2`
|
help: consider introducing a named lifetime parameter
|
LL | fn foo<'a>(z: &mut Vec<(&'a u8,&u8)>, (x, y): (&'a u8, &u8)) {
| ++++ ++ ++

error: lifetime may not live long enough
--> $DIR/ex3-both-anon-regions-3.rs:2:5
Expand All @@ -17,6 +22,11 @@ LL | fn foo(z: &mut Vec<(&u8,&u8)>, (x, y): (&u8, &u8)) {
| let's call the lifetime of this reference `'4`
LL | z.push((x,y));
| ^^^^^^^^^^^^^ argument requires that `'3` must outlive `'4`
|
help: consider introducing a named lifetime parameter
|
LL | fn foo<'a>(z: &mut Vec<(&u8,&'a u8)>, (x, y): (&u8, &'a u8)) {
| ++++ ++ ++

error: aborting due to 2 previous errors

Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,11 @@ LL | fn foo<'a>(&self, x: &i32) -> &i32 {
| let's call the lifetime of this reference `'2`
LL | x
| ^ associated function was supposed to return data with lifetime `'2` but it is returning data with lifetime `'1`
|
help: consider introducing a named lifetime parameter and update trait if needed
|
LL | fn foo<'a>(&'a self, x: &'a i32) -> &i32 {
| ++ ++

error: aborting due to previous error

Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,11 @@ LL | fn foo<'a>(&self, x: &Foo) -> &Foo {
| let's call the lifetime of this reference `'2`
LL | if true { x } else { self }
| ^ associated function was supposed to return data with lifetime `'2` but it is returning data with lifetime `'1`
|
help: consider introducing a named lifetime parameter and update trait if needed
|
LL | fn foo<'a>(&'a self, x: &'a Foo) -> &Foo {
| ++ ++

error: aborting due to previous error

Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,11 @@ LL | fn foo(x:fn(&u8, &u8), y: Vec<&u8>, z: &u8) {
| let's call the lifetime of this reference `'2`
LL | y.push(z);
| ^^^^^^^^^ argument requires that `'1` must outlive `'2`
|
help: consider introducing a named lifetime parameter
|
LL | fn foo<'a>(x:fn(&u8, &u8), y: Vec<&'a u8>, z: &'a u8) {
| ++++ ++ ++

error[E0596]: cannot borrow `y` as mutable, as it is not declared as mutable
--> $DIR/ex3-both-anon-regions-using-fn-items.rs:2:3
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,11 @@ LL | fn foo(x: &mut Vec<&u8>, y: &u8) {
| let's call the lifetime of this reference `'2`
LL | x.push(y);
| ^^^^^^^^^ argument requires that `'1` must outlive `'2`
|
help: consider introducing a named lifetime parameter and update trait if needed
|
LL | fn foo<'a>(x: &mut Vec<&'a u8>, y: &'a u8) {
| ++++ ++ ++

error: aborting due to previous error

Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,11 @@ LL | fn foo(x:Box<dyn Fn(&u8, &u8)> , y: Vec<&u8>, z: &u8) {
| let's call the lifetime of this reference `'2`
LL | y.push(z);
| ^^^^^^^^^ argument requires that `'1` must outlive `'2`
|
help: consider introducing a named lifetime parameter
|
LL | fn foo<'a>(x:Box<dyn Fn(&'a u8, &'a u8)> , y: Vec<&u8>, z: &u8) {
| ++++ ++ ++

error[E0596]: cannot borrow `y` as mutable, as it is not declared as mutable
--> $DIR/ex3-both-anon-regions-using-trait-objects.rs:2:3
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,11 @@ LL | fn foo(x: &mut Vec<&u8>, y: &u8) {
| let's call the lifetime of this reference `'2`
LL | x.push(y);
| ^^^^^^^^^ argument requires that `'1` must outlive `'2`
|
help: consider introducing a named lifetime parameter
|
LL | fn foo<'a>(x: &mut Vec<&'a u8>, y: &'a u8) {
| ++++ ++ ++

error: aborting due to previous error

Loading