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.

Sign up for GitHub

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 most impl Trait and dyn Trait lifetime bound suggestions under NLL #96385

Merged
merged 1 commit into from
Apr 27, 2022
Merged

Recover most impl Trait and dyn Trait lifetime bound suggestions under NLL #96385

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
119 changes: 42 additions & 77 deletions compiler/rustc_borrowck/src/diagnostics/region_errors.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
//! Error reporting machinery for lifetime errors.

use rustc_errors::{Applicability, Diagnostic, DiagnosticBuilder, ErrorGuaranteed};
use rustc_errors::{Diagnostic, DiagnosticBuilder, ErrorGuaranteed};
use rustc_infer::infer::{
error_reporting::nice_region_error::NiceRegionError,
error_reporting::unexpected_hidden_region_diagnostic, NllRegionVariableOrigin,
RelateParamBound,
error_reporting::nice_region_error::{self, find_param_with_region, NiceRegionError},
error_reporting::unexpected_hidden_region_diagnostic,
NllRegionVariableOrigin, RelateParamBound,
};
use rustc_middle::hir::place::PlaceBase;
use rustc_middle::mir::{ConstraintCategory, ReturnConstraint};
use rustc_middle::ty::subst::{InternalSubsts, Subst};
use rustc_middle::ty::subst::InternalSubsts;
use rustc_middle::ty::{self, RegionVid, Ty};
use rustc_span::symbol::{kw, sym};
use rustc_span::{BytePos, Span};
use rustc_span::symbol::sym;
use rustc_span::Span;

use crate::borrowck_errors;

Expand Down Expand Up @@ -651,82 +651,47 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
fr_name: RegionName,
outlived_fr: RegionVid,
) {
if let (Some(f), Some(ty::ReStatic)) =
(self.to_error_region(fr), self.to_error_region(outlived_fr).as_deref())
if let (Some(f), Some(outlived_f)) =
(self.to_error_region(fr), self.to_error_region(outlived_fr))
{
if let Some(&ty::Opaque(did, substs)) = self
if *outlived_f != ty::ReStatic {
return;
}

let fn_returns = self
.infcx
.tcx
.is_suitable_region(f)
.map(|r| r.def_id)
.and_then(|id| self.infcx.tcx.return_type_impl_trait(id))
.map(|(ty, _)| ty.kind())
{
// Check whether or not the impl trait return type is intended to capture
// data with the static lifetime.
//
// eg. check for `impl Trait + 'static` instead of `impl Trait`.
let has_static_predicate = {
let bounds = self.infcx.tcx.explicit_item_bounds(did);

let mut found = false;
for (bound, _) in bounds {
if let ty::PredicateKind::TypeOutlives(ty::OutlivesPredicate(_, r)) =
bound.kind().skip_binder()
{
let r = r.subst(self.infcx.tcx, substs);
if r.is_static() {
found = true;
break;
} else {
// If there's already a lifetime bound, don't
// suggest anything.
return;
}
}
}

found
};
.map(|r| self.infcx.tcx.return_type_impl_or_dyn_traits(r.def_id))
.unwrap_or_default();

debug!(
"add_static_impl_trait_suggestion: has_static_predicate={:?}",
has_static_predicate
);
let static_str = kw::StaticLifetime;
// If there is a static predicate, then the only sensible suggestion is to replace
// fr with `'static`.
if has_static_predicate {
diag.help(&format!("consider replacing `{fr_name}` with `{static_str}`"));
} else {
// Otherwise, we should suggest adding a constraint on the return type.
let span = self.infcx.tcx.def_span(did);
if let Ok(snippet) = self.infcx.tcx.sess.source_map().span_to_snippet(span) {
let suggestable_fr_name = if fr_name.was_named() {
fr_name.to_string()
} else {
"'_".to_string()
};
let span = if snippet.ends_with(';') {
// `type X = impl Trait;`
span.with_hi(span.hi() - BytePos(1))
} else {
span
};
let suggestion = format!(" + {suggestable_fr_name}");
let span = span.shrink_to_hi();
diag.span_suggestion(
span,
&format!(
"to allow this `impl Trait` to capture borrowed data with lifetime \
`{fr_name}`, add `{suggestable_fr_name}` as a bound",
),
suggestion,
Applicability::MachineApplicable,
);
}
}
if fn_returns.is_empty() {
return;
}

let param = if let Some(param) = find_param_with_region(self.infcx.tcx, f, outlived_f) {
param
} else {
return;
};

let lifetime = if f.has_name() { fr_name.to_string() } else { "'_".to_string() };

let arg = match param.param.pat.simple_ident() {
Some(simple_ident) => format!("argument `{}`", simple_ident),
None => "the argument".to_string(),
};
let captures = format!("captures data from {}", arg);

return nice_region_error::suggest_new_region_bound(
self.infcx.tcx,
diag,
fn_returns,
lifetime,
Some(arg),
captures,
Some((param.param_ty_span, param.param_ty.to_string())),
);
}
}
}
1 change: 1 addition & 0 deletions compiler/rustc_infer/src/infer/error_reporting/nice_region_error/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ mod trait_impl_difference;
mod util;

pub use static_impl_trait::suggest_new_region_bound;
pub use util::find_param_with_region;

impl<'cx, 'tcx> InferCtxt<'cx, 'tcx> {
pub fn try_report_nice_region_error(&self, error: &RegionResolutionError<'tcx>) -> bool {
Expand Down
138 changes: 73 additions & 65 deletions compiler/rustc_infer/src/infer/error_reporting/nice_region_error/util.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,15 @@
//! anonymous regions.

use crate::infer::error_reporting::nice_region_error::NiceRegionError;
use crate::infer::TyCtxt;
use rustc_hir as hir;
use rustc_hir::def_id::LocalDefId;
use rustc_middle::ty::{self, Binder, DefIdTree, Region, Ty, TypeFoldable};
use rustc_span::Span;

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

// This method walks the Type of the function body parameters using
// `fold_regions()` function and returns the
// &hir::Param of the function parameter corresponding to the anonymous
// region and the Ty corresponding to the named region.
// Currently only the case where the function declaration consists of
// one named region and one anonymous region is handled.
// Consider the example `fn foo<'a>(x: &'a i32, y: &i32) -> &'a i32`
// Here, we would return the hir::Param for y, we return the type &'a
// i32, which is the type of y but with the anonymous region replaced
// with 'a, the corresponding bound region and is_first which is true if
// the hir::Param is the first parameter in the function declaration.
pub fn find_param_with_region<'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.

This has just been split out from NiceRegionError::find_param_with_region as a free standing function.

tcx: TyCtxt<'tcx>,
anon_region: Region<'tcx>,
replace_region: Region<'tcx>,
) -> Option<AnonymousParamInfo<'tcx>> {
let (id, bound_region) = match *anon_region {
ty::ReFree(ref free_region) => (free_region.scope, free_region.bound_region),
ty::ReEarlyBound(ebr) => {
(tcx.parent(ebr.def_id).unwrap(), ty::BoundRegionKind::BrNamed(ebr.def_id, ebr.name))
}
_ => return None, // not a free region
};

let hir = &tcx.hir();
let hir_id = hir.local_def_id_to_hir_id(id.as_local()?);
let body_id = hir.maybe_body_owned_by(hir_id)?;
let body = hir.body(body_id);
let owner_id = hir.body_owner(body_id);
let fn_decl = hir.fn_decl_by_hir_id(owner_id).unwrap();
let poly_fn_sig = tcx.fn_sig(id);
let fn_sig = tcx.liberate_late_bound_regions(id, poly_fn_sig);
body.params
.iter()
.take(if fn_sig.c_variadic {
fn_sig.inputs().len()
} else {
assert_eq!(fn_sig.inputs().len(), body.params.len());
body.params.len()
})
.enumerate()
.find_map(|(index, param)| {
// May return None; sometimes the tables are not yet populated.
let ty = fn_sig.inputs()[index];
let mut found_anon_region = false;
let new_param_ty = tcx.fold_regions(ty, &mut false, |r, _| {
if r == anon_region {
found_anon_region = true;
replace_region
} else {
r
}
});
if found_anon_region {
let ty_hir_id = fn_decl.inputs[index].hir_id;
let param_ty_span = hir.span(ty_hir_id);
let is_first = index == 0;
Some(AnonymousParamInfo {
param,
param_ty: new_param_ty,
param_ty_span,
bound_region,
is_first,
})
} else {
None
}
})
}

impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
// This method walks the Type of the function body parameters using
// `fold_regions()` function and returns the
// &hir::Param of the function parameter corresponding to the anonymous
// region and the Ty corresponding to the named region.
// Currently only the case where the function declaration consists of
// one named region and one anonymous region is handled.
// Consider the example `fn foo<'a>(x: &'a i32, y: &i32) -> &'a i32`
// Here, we would return the hir::Param for y, we return the type &'a
// i32, which is the type of y but with the anonymous region replaced
// with 'a, the corresponding bound region and is_first which is true if
// the hir::Param is the first parameter in the function declaration.
pub(super) fn find_param_with_region(
&self,
anon_region: Region<'tcx>,
replace_region: Region<'tcx>,
) -> Option<AnonymousParamInfo<'_>> {
let (id, bound_region) = match *anon_region {
ty::ReFree(ref free_region) => (free_region.scope, free_region.bound_region),
ty::ReEarlyBound(ebr) => (
self.tcx().parent(ebr.def_id).unwrap(),
ty::BoundRegionKind::BrNamed(ebr.def_id, ebr.name),
),
_ => return None, // not a free region
};

let hir = &self.tcx().hir();
let hir_id = hir.local_def_id_to_hir_id(id.as_local()?);
let body_id = hir.maybe_body_owned_by(hir_id)?;
let body = hir.body(body_id);
let owner_id = hir.body_owner(body_id);
let fn_decl = hir.fn_decl_by_hir_id(owner_id).unwrap();
let poly_fn_sig = self.tcx().fn_sig(id);
let fn_sig = self.tcx().liberate_late_bound_regions(id, poly_fn_sig);
body.params
.iter()
.take(if fn_sig.c_variadic {
fn_sig.inputs().len()
} else {
assert_eq!(fn_sig.inputs().len(), body.params.len());
body.params.len()
})
.enumerate()
.find_map(|(index, param)| {
// May return None; sometimes the tables are not yet populated.
let ty = fn_sig.inputs()[index];
let mut found_anon_region = false;
let new_param_ty = self.tcx().fold_regions(ty, &mut false, |r, _| {
if r == anon_region {
found_anon_region = true;
replace_region
} else {
r
}
});
if found_anon_region {
let ty_hir_id = fn_decl.inputs[index].hir_id;
let param_ty_span = hir.span(ty_hir_id);
let is_first = index == 0;
Some(AnonymousParamInfo {
param,
param_ty: new_param_ty,
param_ty_span,
bound_region,
is_first,
})
} else {
None
}
})
find_param_with_region(self.tcx(), anon_region, replace_region)
}

// Here, we check for the case where the anonymous region
Expand Down
6 changes: 5 additions & 1 deletion src/test/ui/impl-trait/issues/issue-88236-2.nll.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,14 @@ LL | fn make_bad_impl<'b>(x: &'b ()) -> impl for<'a> Hrtb<'a, Assoc = impl Send
LL | x
| ^ returning this value requires that `'b` must outlive `'static`
|
help: to allow this `impl Trait` to capture borrowed data with lifetime `'b`, add `'b` as a bound
help: to declare that the `impl Trait` captures data from argument `x`, you can add an explicit `'b` lifetime bound
|
LL | fn make_bad_impl<'b>(x: &'b ()) -> impl for<'a> Hrtb<'a, Assoc = impl Send + 'a> + 'b {
| ++++
help: to declare that the `impl Trait` captures data from argument `x`, you can add an explicit `'b` lifetime bound
|
LL | fn make_bad_impl<'b>(x: &'b ()) -> impl for<'a> Hrtb<'a, Assoc = impl Send + 'a + 'b> {
| ++++

error: implementation of `Hrtb` is not general enough
--> $DIR/issue-88236-2.rs:20:5
Expand Down
36 changes: 33 additions & 3 deletions src/test/ui/impl-trait/must_outlive_least_region_or_bound.nll.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,15 +32,29 @@ LL | fn elided2(x: &i32) -> impl Copy + 'static { x }
| |
| let's call the lifetime of this reference `'1`
|
= help: consider replacing `'1` with `'static`
help: consider changing the `impl Trait`'s explicit `'static` bound to the lifetime of argument `x`
|
LL | fn elided2(x: &i32) -> impl Copy + '_ { x }
| ~~
help: alternatively, add an explicit `'static` bound to this reference
|
LL | fn elided2(x: &'static i32) -> impl Copy + 'static { x }
| ~~~~~~~~~~~~

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

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

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

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