Skip to content

Commit e1b074a

Browse files
marmelademamarmeladema
committed
Recover most impl Trait and dyn Trait lifetime bound suggestions under NLL
1 parent fedbe5d commit e1b074a

File tree

14 files changed

+244
-148
lines changed

14 files changed

+244
-148
lines changed

‎compiler/rustc_borrowck/src/diagnostics/region_errors.rs‎

Lines changed: 42 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,17 @@
11
//! Error reporting machinery for lifetime errors.
22
3-
use rustc_errors::{Applicability, Diagnostic, DiagnosticBuilder, ErrorGuaranteed};
3+
use rustc_errors::{Diagnostic, DiagnosticBuilder, ErrorGuaranteed};
44
use rustc_infer::infer::{
5-
error_reporting::nice_region_error::NiceRegionError,
6-
error_reporting::unexpected_hidden_region_diagnostic, NllRegionVariableOrigin,
7-
RelateParamBound,
5+
error_reporting::nice_region_error::{self, find_param_with_region, NiceRegionError},
6+
error_reporting::unexpected_hidden_region_diagnostic,
7+
NllRegionVariableOrigin, RelateParamBound,
88
};
99
use rustc_middle::hir::place::PlaceBase;
1010
use rustc_middle::mir::{ConstraintCategory, ReturnConstraint};
11-
use rustc_middle::ty::subst::{InternalSubsts, Subst};
11+
use rustc_middle::ty::subst::InternalSubsts;
1212
use rustc_middle::ty::{self, RegionVid, Ty};
13-
use rustc_span::symbol::{kw, sym};
14-
use rustc_span::{BytePos, Span};
13+
use rustc_span::symbol::sym;
14+
use rustc_span::Span;
1515

1616
use crate::borrowck_errors;
1717

@@ -651,82 +651,47 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
651651
fr_name: RegionName,
652652
outlived_fr: RegionVid,
653653
) {
654-
if let (Some(f), Some(ty::ReStatic)) =
655-
(self.to_error_region(fr), self.to_error_region(outlived_fr).as_deref())
654+
if let (Some(f), Some(outlived_f)) =
655+
(self.to_error_region(fr), self.to_error_region(outlived_fr))
656656
{
657-
if let Some(&ty::Opaque(did, substs)) = self
657+
if *outlived_f != ty::ReStatic {
658+
return;
659+
}
660+
661+
let fn_returns = self
658662
.infcx
659663
.tcx
660664
.is_suitable_region(f)
661-
.map(|r| r.def_id)
662-
.and_then(|id| self.infcx.tcx.return_type_impl_trait(id))
663-
.map(|(ty, _)| ty.kind())
664-
{
665-
// Check whether or not the impl trait return type is intended to capture
666-
// data with the static lifetime.
667-
//
668-
// eg. check for `impl Trait + 'static` instead of `impl Trait`.
669-
let has_static_predicate = {
670-
let bounds = self.infcx.tcx.explicit_item_bounds(did);
671-
672-
let mut found = false;
673-
for (bound, _) in bounds {
674-
if let ty::PredicateKind::TypeOutlives(ty::OutlivesPredicate(_, r)) =
675-
bound.kind().skip_binder()
676-
{
677-
let r = r.subst(self.infcx.tcx, substs);
678-
if r.is_static() {
679-
found = true;
680-
break;
681-
} else {
682-
// If there's already a lifetime bound, don't
683-
// suggest anything.
684-
return;
685-
}
686-
}
687-
}
688-
689-
found
690-
};
665+
.map(|r| self.infcx.tcx.return_type_impl_or_dyn_traits(r.def_id))
666+
.unwrap_or_default();
691667

692-
debug!(
693-
"add_static_impl_trait_suggestion: has_static_predicate={:?}",
694-
has_static_predicate
695-
);
696-
let static_str = kw::StaticLifetime;
697-
// If there is a static predicate, then the only sensible suggestion is to replace
698-
// fr with `'static`.
699-
if has_static_predicate {
700-
diag.help(&format!("consider replacing `{fr_name}` with `{static_str}`"));
701-
} else {
702-
// Otherwise, we should suggest adding a constraint on the return type.
703-
let span = self.infcx.tcx.def_span(did);
704-
if let Ok(snippet) = self.infcx.tcx.sess.source_map().span_to_snippet(span) {
705-
let suggestable_fr_name = if fr_name.was_named() {
706-
fr_name.to_string()
707-
} else {
708-
"'_".to_string()
709-
};
710-
let span = if snippet.ends_with(';') {
711-
// `type X = impl Trait;`
712-
span.with_hi(span.hi() - BytePos(1))
713-
} else {
714-
span
715-
};
716-
let suggestion = format!(" + {suggestable_fr_name}");
717-
let span = span.shrink_to_hi();
718-
diag.span_suggestion(
719-
span,
720-
&format!(
721-
"to allow this `impl Trait` to capture borrowed data with lifetime \
722-
`{fr_name}`, add `{suggestable_fr_name}` as a bound",
723-
),
724-
suggestion,
725-
Applicability::MachineApplicable,
726-
);
727-
}
728-
}
668+
if fn_returns.is_empty() {
669+
return;
729670
}
671+
672+
let param = if let Some(param) = find_param_with_region(self.infcx.tcx, f, outlived_f) {
673+
param
674+
} else {
675+
return;
676+
};
677+
678+
let lifetime = if f.has_name() { fr_name.to_string() } else { "'_".to_string() };
679+
680+
let arg = match param.param.pat.simple_ident() {
681+
Some(simple_ident) => format!("argument `{}`", simple_ident),
682+
None => "the argument".to_string(),
683+
};
684+
let captures = format!("captures data from {}", arg);
685+
686+
return nice_region_error::suggest_new_region_bound(
687+
self.infcx.tcx,
688+
diag,
689+
fn_returns,
690+
lifetime,
691+
Some(arg),
692+
captures,
693+
Some((param.param_ty_span, param.param_ty.to_string())),
694+
);
730695
}
731696
}
732697
}

‎compiler/rustc_infer/src/infer/error_reporting/nice_region_error/mod.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ mod trait_impl_difference;
1515
mod util;
1616

1717
pub use static_impl_trait::suggest_new_region_bound;
18+
pub use util::find_param_with_region;
1819

1920
impl<'cx, 'tcx> InferCtxt<'cx, 'tcx> {
2021
pub fn try_report_nice_region_error(&self, error: &RegionResolutionError<'tcx>) -> bool {

‎compiler/rustc_infer/src/infer/error_reporting/nice_region_error/util.rs‎

Lines changed: 73 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,15 @@
22
//! anonymous regions.
33
44
use crate::infer::error_reporting::nice_region_error::NiceRegionError;
5+
use crate::infer::TyCtxt;
56
use rustc_hir as hir;
67
use rustc_hir::def_id::LocalDefId;
78
use rustc_middle::ty::{self, Binder, DefIdTree, Region, Ty, TypeFoldable};
89
use rustc_span::Span;
910

1011
/// Information about the anonymous region we are searching for.
1112
#[derive(Debug)]
12-
pub(super) struct AnonymousParamInfo<'tcx> {
13+
pub struct AnonymousParamInfo<'tcx> {
1314
/// The parameter corresponding to the anonymous region.
1415
pub param: &'tcx hir::Param<'tcx>,
1516
/// The type corresponding to the anonymous region parameter.
@@ -22,76 +23,83 @@ pub(super) struct AnonymousParamInfo<'tcx> {
2223
pub is_first: bool,
2324
}
2425

26+
// This method walks the Type of the function body parameters using
27+
// `fold_regions()` function and returns the
28+
// &hir::Param of the function parameter corresponding to the anonymous
29+
// region and the Ty corresponding to the named region.
30+
// Currently only the case where the function declaration consists of
31+
// one named region and one anonymous region is handled.
32+
// Consider the example `fn foo<'a>(x: &'a i32, y: &i32) -> &'a i32`
33+
// Here, we would return the hir::Param for y, we return the type &'a
34+
// i32, which is the type of y but with the anonymous region replaced
35+
// with 'a, the corresponding bound region and is_first which is true if
36+
// the hir::Param is the first parameter in the function declaration.
37+
pub fn find_param_with_region<'tcx>(
38+
tcx: TyCtxt<'tcx>,
39+
anon_region: Region<'tcx>,
40+
replace_region: Region<'tcx>,
41+
) -> Option<AnonymousParamInfo<'tcx>> {
42+
let (id, bound_region) = match *anon_region {
43+
ty::ReFree(ref free_region) => (free_region.scope, free_region.bound_region),
44+
ty::ReEarlyBound(ebr) => {
45+
(tcx.parent(ebr.def_id).unwrap(), ty::BoundRegionKind::BrNamed(ebr.def_id, ebr.name))
46+
}
47+
_ => return None, // not a free region
48+
};
49+
50+
let hir = &tcx.hir();
51+
let hir_id = hir.local_def_id_to_hir_id(id.as_local()?);
52+
let body_id = hir.maybe_body_owned_by(hir_id)?;
53+
let body = hir.body(body_id);
54+
let owner_id = hir.body_owner(body_id);
55+
let fn_decl = hir.fn_decl_by_hir_id(owner_id).unwrap();
56+
let poly_fn_sig = tcx.fn_sig(id);
57+
let fn_sig = tcx.liberate_late_bound_regions(id, poly_fn_sig);
58+
body.params
59+
.iter()
60+
.take(if fn_sig.c_variadic {
61+
fn_sig.inputs().len()
62+
} else {
63+
assert_eq!(fn_sig.inputs().len(), body.params.len());
64+
body.params.len()
65+
})
66+
.enumerate()
67+
.find_map(|(index, param)| {
68+
// May return None; sometimes the tables are not yet populated.
69+
let ty = fn_sig.inputs()[index];
70+
let mut found_anon_region = false;
71+
let new_param_ty = tcx.fold_regions(ty, &mut false, |r, _| {
72+
if r == anon_region {
73+
found_anon_region = true;
74+
replace_region
75+
} else {
76+
r
77+
}
78+
});
79+
if found_anon_region {
80+
let ty_hir_id = fn_decl.inputs[index].hir_id;
81+
let param_ty_span = hir.span(ty_hir_id);
82+
let is_first = index == 0;
83+
Some(AnonymousParamInfo {
84+
param,
85+
param_ty: new_param_ty,
86+
param_ty_span,
87+
bound_region,
88+
is_first,
89+
})
90+
} else {
91+
None
92+
}
93+
})
94+
}
95+
2596
impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
26-
// This method walks the Type of the function body parameters using
27-
// `fold_regions()` function and returns the
28-
// &hir::Param of the function parameter corresponding to the anonymous
29-
// region and the Ty corresponding to the named region.
30-
// Currently only the case where the function declaration consists of
31-
// one named region and one anonymous region is handled.
32-
// Consider the example `fn foo<'a>(x: &'a i32, y: &i32) -> &'a i32`
33-
// Here, we would return the hir::Param for y, we return the type &'a
34-
// i32, which is the type of y but with the anonymous region replaced
35-
// with 'a, the corresponding bound region and is_first which is true if
36-
// the hir::Param is the first parameter in the function declaration.
3797
pub(super) fn find_param_with_region(
3898
&self,
3999
anon_region: Region<'tcx>,
40100
replace_region: Region<'tcx>,
41101
) -> Option<AnonymousParamInfo<'_>> {
42-
let (id, bound_region) = match *anon_region {
43-
ty::ReFree(ref free_region) => (free_region.scope, free_region.bound_region),
44-
ty::ReEarlyBound(ebr) => (
45-
self.tcx().parent(ebr.def_id).unwrap(),
46-
ty::BoundRegionKind::BrNamed(ebr.def_id, ebr.name),
47-
),
48-
_ => return None, // not a free region
49-
};
50-
51-
let hir = &self.tcx().hir();
52-
let hir_id = hir.local_def_id_to_hir_id(id.as_local()?);
53-
let body_id = hir.maybe_body_owned_by(hir_id)?;
54-
let body = hir.body(body_id);
55-
let owner_id = hir.body_owner(body_id);
56-
let fn_decl = hir.fn_decl_by_hir_id(owner_id).unwrap();
57-
let poly_fn_sig = self.tcx().fn_sig(id);
58-
let fn_sig = self.tcx().liberate_late_bound_regions(id, poly_fn_sig);
59-
body.params
60-
.iter()
61-
.take(if fn_sig.c_variadic {
62-
fn_sig.inputs().len()
63-
} else {
64-
assert_eq!(fn_sig.inputs().len(), body.params.len());
65-
body.params.len()
66-
})
67-
.enumerate()
68-
.find_map(|(index, param)| {
69-
// May return None; sometimes the tables are not yet populated.
70-
let ty = fn_sig.inputs()[index];
71-
let mut found_anon_region = false;
72-
let new_param_ty = self.tcx().fold_regions(ty, &mut false, |r, _| {
73-
if r == anon_region {
74-
found_anon_region = true;
75-
replace_region
76-
} else {
77-
r
78-
}
79-
});
80-
if found_anon_region {
81-
let ty_hir_id = fn_decl.inputs[index].hir_id;
82-
let param_ty_span = hir.span(ty_hir_id);
83-
let is_first = index == 0;
84-
Some(AnonymousParamInfo {
85-
param,
86-
param_ty: new_param_ty,
87-
param_ty_span,
88-
bound_region,
89-
is_first,
90-
})
91-
} else {
92-
None
93-
}
94-
})
102+
find_param_with_region(self.tcx(), anon_region, replace_region)
95103
}
96104

97105
// Here, we check for the case where the anonymous region

‎src/test/ui/impl-trait/issues/issue-88236-2.nll.stderr‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,14 @@ LL | fn make_bad_impl<'b>(x: &'b ()) -> impl for<'a> Hrtb<'a, Assoc = impl Send
2424
LL | x
2525
| ^ returning this value requires that `'b` must outlive `'static`
2626
|
27-
help: to allow this `impl Trait` to capture borrowed data with lifetime `'b`, add `'b` as a bound
27+
help: to declare that the `impl Trait` captures data from argument `x`, you can add an explicit `'b` lifetime bound
2828
|
2929
LL | fn make_bad_impl<'b>(x: &'b ()) -> impl for<'a> Hrtb<'a, Assoc = impl Send + 'a> + 'b {
3030
| ++++
31+
help: to declare that the `impl Trait` captures data from argument `x`, you can add an explicit `'b` lifetime bound
32+
|
33+
LL | fn make_bad_impl<'b>(x: &'b ()) -> impl for<'a> Hrtb<'a, Assoc = impl Send + 'a + 'b> {
34+
| ++++
3135

3236
error: implementation of `Hrtb` is not general enough
3337
--> $DIR/issue-88236-2.rs:20:5

‎src/test/ui/impl-trait/must_outlive_least_region_or_bound.nll.stderr‎

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,15 +32,29 @@ LL | fn elided2(x: &i32) -> impl Copy + 'static { x }
3232
| |
3333
| let's call the lifetime of this reference `'1`
3434
|
35-
= help: consider replacing `'1` with `'static`
35+
help: consider changing the `impl Trait`'s explicit `'static` bound to the lifetime of argument `x`
36+
|
37+
LL | fn elided2(x: &i32) -> impl Copy + '_ { x }
38+
| ~~
39+
help: alternatively, add an explicit `'static` bound to this reference
40+
|
41+
LL | fn elided2(x: &'static i32) -> impl Copy + 'static { x }
42+
| ~~~~~~~~~~~~
3643

3744
error: lifetime may not live long enough
3845
--> $DIR/must_outlive_least_region_or_bound.rs:11:55
3946
|
4047
LL | fn explicit2<'a>(x: &'a i32) -> impl Copy + 'static { x }
4148
| -- lifetime `'a` defined here ^ returning this value requires that `'a` must outlive `'static`
4249
|
43-
= help: consider replacing `'a` with `'static`
50+
help: consider changing the `impl Trait`'s explicit `'static` bound to the lifetime of argument `x`
51+
|
52+
LL | fn explicit2<'a>(x: &'a i32) -> impl Copy + 'a { x }
53+
| ~~
54+
help: alternatively, add an explicit `'static` bound to this reference
55+
|
56+
LL | fn explicit2<'a>(x: &'static i32) -> impl Copy + 'static { x }
57+
| ~~~~~~~~~~~~
4458

4559
error[E0621]: explicit lifetime required in the type of `x`
4660
--> $DIR/must_outlive_least_region_or_bound.rs:13:41
@@ -57,14 +71,30 @@ LL | fn elided5(x: &i32) -> (Box<dyn Debug>, impl Debug) { (Box::new(x), x) }
5771
| - ^^^^^^^^^^^^^^^^ returning this value requires that `'1` must outlive `'static`
5872
| |
5973
| let's call the lifetime of this reference `'1`
74+
|
75+
help: to declare that the trait object captures data from argument `x`, you can add an explicit `'_` lifetime bound
76+
|
77+
LL | fn elided5(x: &i32) -> (Box<dyn Debug + '_>, impl Debug) { (Box::new(x), x) }
78+
| ++++
79+
help: to declare that the `impl Trait` captures data from argument `x`, you can add an explicit `'_` lifetime bound
80+
|
81+
LL | fn elided5(x: &i32) -> (Box<dyn Debug>, impl Debug + '_) { (Box::new(x), x) }
82+
| ++++
6083

6184
error: lifetime may not live long enough
6285
--> $DIR/must_outlive_least_region_or_bound.rs:29:69
6386
|
6487
LL | fn with_bound<'a>(x: &'a i32) -> impl LifetimeTrait<'a> + 'static { x }
6588
| -- lifetime `'a` defined here ^ returning this value requires that `'a` must outlive `'static`
6689
|
67-
= help: consider replacing `'a` with `'static`
90+
help: consider changing the `impl Trait`'s explicit `'static` bound to the lifetime of argument `x`
91+
|
92+
LL | fn with_bound<'a>(x: &'a i32) -> impl LifetimeTrait<'a> + 'a { x }
93+
| ~~
94+
help: alternatively, add an explicit `'static` bound to this reference
95+
|
96+
LL | fn with_bound<'a>(x: &'static i32) -> impl LifetimeTrait<'a> + 'static { x }
97+
| ~~~~~~~~~~~~
6898

6999
error[E0700]: hidden type for `impl Trait` captures lifetime that does not appear in bounds
70100
--> $DIR/must_outlive_least_region_or_bound.rs:34:5

0 commit comments

Comments
 (0)