Skip to content

Commit

Permalink
Re-use logic for adding a suggestion when a lifetime bound is missing…
Browse files Browse the repository at this point in the history
… on an impl trait
  • Loading branch information
oli-obk committed Oct 13, 2021
1 parent be39963 commit 888ba50
Show file tree
Hide file tree
Showing 6 changed files with 150 additions and 91 deletions.
3 changes: 1 addition & 2 deletions compiler/rustc_hir/src/hir.rs
Expand Up @@ -2235,8 +2235,7 @@ pub enum TyKind<'hir> {
///
/// Type parameters may be stored in each `PathSegment`.
Path(QPath<'hir>),
/// An opaque type definition itself. This is currently only used for the
/// `opaque type Foo: Trait` item that `impl Trait` in desugars to.
/// An opaque type definition itself. This is only used for `impl Trait`.
///
/// The generic argument list contains the lifetimes (and in the future
/// possibly parameters) that are actually bound on the `impl Trait`.
Expand Down
12 changes: 12 additions & 0 deletions compiler/rustc_infer/src/infer/error_reporting/mod.rs
Expand Up @@ -267,6 +267,18 @@ pub fn unexpected_hidden_region_diagnostic(
hidden_region,
"",
);
if let Some(reg_info) = tcx.is_suitable_region(hidden_region) {
let fn_returns = tcx.return_type_impl_or_dyn_traits(reg_info.def_id);
nice_region_error::suggest_new_region_bound(
tcx,
&mut err,
fn_returns,
hidden_region.to_string(),
None,
format!("captures {}", hidden_region),
None,
)
}
}
_ => {
// Ugh. This is a painful case: the hidden region is not one
Expand Down
Expand Up @@ -14,6 +14,8 @@ mod static_impl_trait;
mod trait_impl_difference;
mod util;

pub use static_impl_trait::suggest_new_region_bound;

impl<'cx, 'tcx> InferCtxt<'cx, 'tcx> {
pub fn try_report_nice_region_error(&self, error: &RegionResolutionError<'tcx>) -> bool {
NiceRegionError::new(self, error.clone()).try_report().is_some()
Expand Down
Expand Up @@ -217,128 +217,159 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
));
}

debug!("try_report_static_impl_trait: fn_return={:?}", fn_returns);
// FIXME: account for the need of parens in `&(dyn Trait + '_)`
let consider = "consider changing the";
let declare = "to declare that the";
let arg = match param.param.pat.simple_ident() {
Some(simple_ident) => format!("argument `{}`", simple_ident),
None => "the argument".to_string(),
};
let explicit = format!("you can add an explicit `{}` lifetime bound", lifetime_name);
let explicit_static = format!("explicit `'static` bound to the lifetime of {}", arg);
let captures = format!("captures data from {}", arg);
let add_static_bound = "alternatively, add an explicit `'static` bound to this reference";
let plus_lt = format!(" + {}", lifetime_name);
for fn_return in fn_returns {
if fn_return.span.desugaring_kind().is_some() {
// Skip `async` desugaring `impl Future`.
continue;
}
match fn_return.kind {
TyKind::OpaqueDef(item_id, _) => {
let item = tcx.hir().item(item_id);
let opaque = if let ItemKind::OpaqueTy(opaque) = &item.kind {
opaque
} else {
err.emit();
return Some(ErrorReported);
};
suggest_new_region_bound(
tcx,
&mut err,
fn_returns,
lifetime_name,
Some(arg),
captures,
Some((param.param_ty_span, param.param_ty.to_string())),
);

if let Some(span) = opaque
.bounds
.iter()
.filter_map(|arg| match arg {
GenericBound::Outlives(Lifetime {
name: LifetimeName::Static,
span,
..
}) => Some(*span),
_ => None,
})
.next()
{
err.emit();
Some(ErrorReported)
}
}

pub fn suggest_new_region_bound(
tcx: TyCtxt<'tcx>,
err: &mut DiagnosticBuilder<'_>,
fn_returns: Vec<&rustc_hir::Ty<'_>>,
lifetime_name: String,
arg: Option<String>,
captures: String,
param: Option<(Span, String)>,
) {
debug!("try_report_static_impl_trait: fn_return={:?}", fn_returns);
// FIXME: account for the need of parens in `&(dyn Trait + '_)`
let consider = "consider changing the";
let declare = "to declare that the";
let explicit = format!("you can add an explicit `{}` lifetime bound", lifetime_name);
let explicit_static =
arg.map(|arg| format!("explicit `'static` bound to the lifetime of {}", arg));
let add_static_bound = "alternatively, add an explicit `'static` bound to this reference";
let plus_lt = format!(" + {}", lifetime_name);
for fn_return in fn_returns {
if fn_return.span.desugaring_kind().is_some() {
// Skip `async` desugaring `impl Future`.
continue;
}
match fn_return.kind {
TyKind::OpaqueDef(item_id, _) => {
let item = tcx.hir().item(item_id);
let opaque = if let ItemKind::OpaqueTy(opaque) = &item.kind {
opaque
} else {
return;
};

if let Some(span) = opaque
.bounds
.iter()
.filter_map(|arg| match arg {
GenericBound::Outlives(Lifetime {
name: LifetimeName::Static,
span,
..
}) => Some(*span),
_ => None,
})
.next()
{
if let Some(explicit_static) = &explicit_static {
err.span_suggestion_verbose(
span,
&format!("{} `impl Trait`'s {}", consider, explicit_static),
lifetime_name.clone(),
Applicability::MaybeIncorrect,
);
}
if let Some((param_span, param_ty)) = param.clone() {
err.span_suggestion_verbose(
param.param_ty_span,
param_span,
add_static_bound,
param.param_ty.to_string(),
Applicability::MaybeIncorrect,
);
} else if opaque
.bounds
.iter()
.filter_map(|arg| match arg {
GenericBound::Outlives(Lifetime { name, span, .. })
if name.ident().to_string() == lifetime_name =>
{
Some(*span)
}
_ => None,
})
.next()
.is_some()
{
} else {
err.span_suggestion_verbose(
fn_return.span.shrink_to_hi(),
&format!(
"{declare} `impl Trait` {captures}, {explicit}",
declare = declare,
captures = captures,
explicit = explicit,
),
plus_lt.clone(),
param_ty,
Applicability::MaybeIncorrect,
);
}
} else if opaque
.bounds
.iter()
.filter_map(|arg| match arg {
GenericBound::Outlives(Lifetime { name, span, .. })
if name.ident().to_string() == lifetime_name =>
{
Some(*span)
}
_ => None,
})
.next()
.is_some()
{
} else {
err.span_suggestion_verbose(
fn_return.span.shrink_to_hi(),
&format!(
"{declare} `impl Trait` {captures}, {explicit}",
declare = declare,
captures = captures,
explicit = explicit,
),
plus_lt.clone(),
Applicability::MaybeIncorrect,
);
}
TyKind::TraitObject(_, lt, _) => match lt.name {
LifetimeName::ImplicitObjectLifetimeDefault => {
err.span_suggestion_verbose(
fn_return.span.shrink_to_hi(),
&format!(
"{declare} trait object {captures}, {explicit}",
declare = declare,
captures = captures,
explicit = explicit,
),
plus_lt.clone(),
Applicability::MaybeIncorrect,
);
}
name if name.ident().to_string() != lifetime_name => {
// With this check we avoid suggesting redundant bounds. This
// would happen if there are nested impl/dyn traits and only
// one of them has the bound we'd suggest already there, like
// in `impl Foo<X = dyn Bar> + '_`.
}
TyKind::TraitObject(_, lt, _) => match lt.name {
LifetimeName::ImplicitObjectLifetimeDefault => {
err.span_suggestion_verbose(
fn_return.span.shrink_to_hi(),
&format!(
"{declare} trait object {captures}, {explicit}",
declare = declare,
captures = captures,
explicit = explicit,
),
plus_lt.clone(),
Applicability::MaybeIncorrect,
);
}
name if name.ident().to_string() != lifetime_name => {
// With this check we avoid suggesting redundant bounds. This
// would happen if there are nested impl/dyn traits and only
// one of them has the bound we'd suggest already there, like
// in `impl Foo<X = dyn Bar> + '_`.
if let Some(explicit_static) = &explicit_static {
err.span_suggestion_verbose(
lt.span,
&format!("{} trait object's {}", consider, explicit_static),
lifetime_name.clone(),
Applicability::MaybeIncorrect,
);
}
if let Some((param_span, param_ty)) = param.clone() {
err.span_suggestion_verbose(
param.param_ty_span,
param_span,
add_static_bound,
param.param_ty.to_string(),
param_ty,
Applicability::MaybeIncorrect,
);
}
_ => {}
},
}
_ => {}
}
},
_ => {}
}
err.emit();
Some(ErrorReported)
}
}

impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
fn get_impl_ident_and_self_ty_from_trait(
&self,
def_id: DefId,
Expand Down
10 changes: 10 additions & 0 deletions src/test/ui/impl-trait/hidden-lifetimes.stderr
Expand Up @@ -5,6 +5,11 @@ LL | fn hide_ref<'a, 'b, T: 'static>(x: &'a mut &'b T) -> impl Swap + 'a {
| -- ^^^^^^^^^^^^^^
| |
| hidden type `&'a mut &'b T` captures the lifetime `'b` as defined here
|
help: to declare that the `impl Trait` captures 'b, you can add an explicit `'b` lifetime bound
|
LL | fn hide_ref<'a, 'b, T: 'static>(x: &'a mut &'b T) -> impl Swap + 'a + 'b {
| ++++

error[E0700]: hidden type for `impl Trait` captures lifetime that does not appear in bounds
--> $DIR/hidden-lifetimes.rs:45:70
Expand All @@ -13,6 +18,11 @@ LL | fn hide_rc_refcell<'a, 'b: 'a, T: 'static>(x: Rc<RefCell<&'b T>>) -> impl S
| -- ^^^^^^^^^^^^^^
| |
| hidden type `Rc<RefCell<&'b T>>` captures the lifetime `'b` as defined here
|
help: to declare that the `impl Trait` captures 'b, you can add an explicit `'b` lifetime bound
|
LL | fn hide_rc_refcell<'a, 'b: 'a, T: 'static>(x: Rc<RefCell<&'b T>>) -> impl Swap + 'a + 'b {
| ++++

error: aborting due to 2 previous errors

Expand Down
5 changes: 5 additions & 0 deletions src/test/ui/impl-trait/region-escape-via-bound.stderr
Expand Up @@ -6,6 +6,11 @@ LL | fn foo(x: Cell<&'x u32>) -> impl Trait<'y>
LL |
LL | where 'x: 'y
| -- hidden type `Cell<&'x u32>` captures the lifetime `'x` as defined here
|
help: to declare that the `impl Trait` captures 'x, you can add an explicit `'x` lifetime bound
|
LL | fn foo(x: Cell<&'x u32>) -> impl Trait<'y> + 'x
| ++++

error: aborting due to previous error

Expand Down

0 comments on commit 888ba50

Please sign in to comment.