Skip to content

Commit

Permalink
Rollup merge of rust-lang#96409 - marmeladema:fix-nll-introduce-named…
Browse files Browse the repository at this point in the history
…-lifetime-suggestion, r=jackh726

Recover suggestions to introduce named lifetime under NLL

Fixes rust-lang#96157

r? ```@jackh726```

Built on top of rust-lang#96385 so only the second commit is relevant
  • Loading branch information
Dylan-DPC committed Apr 28, 2022
2 parents b3329f8 + 2c94218 commit cbfbc3b
Show file tree
Hide file tree
Showing 20 changed files with 331 additions and 76 deletions.
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");
}

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>(
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

0 comments on commit cbfbc3b

Please sign in to comment.