Skip to content

Commit d3871f5

Browse files
oli-obkoli-obk
committed
Re-use logic for adding a suggestion when a lifetime bound is missing on an impl trait
1 parent b7191b3 commit d3871f5

File tree

6 files changed

+150
-91
lines changed

6 files changed

+150
-91
lines changed

‎compiler/rustc_hir/src/hir.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2235,8 +2235,7 @@ pub enum TyKind<'hir> {
22352235
///
22362236
/// Type parameters may be stored in each `PathSegment`.
22372237
Path(QPath<'hir>),
2238-
/// An opaque type definition itself. This is currently only used for the
2239-
/// `opaque type Foo: Trait` item that `impl Trait` in desugars to.
2238+
/// An opaque type definition itself. This is only used for `impl Trait`.
22402239
///
22412240
/// The generic argument list contains the lifetimes (and in the future
22422241
/// possibly parameters) that are actually bound on the `impl Trait`.

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -267,6 +267,18 @@ pub fn unexpected_hidden_region_diagnostic(
267267
hidden_region,
268268
"",
269269
);
270+
if let Some(reg_info) = tcx.is_suitable_region(hidden_region) {
271+
let fn_returns = tcx.return_type_impl_or_dyn_traits(reg_info.def_id);
272+
nice_region_error::suggest_new_region_bound(
273+
tcx,
274+
&mut err,
275+
fn_returns,
276+
hidden_region.to_string(),
277+
None,
278+
format!("captures {}", hidden_region),
279+
None,
280+
)
281+
}
270282
}
271283
_ => {
272284
// Ugh. This is a painful case: the hidden region is not one

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ mod static_impl_trait;
1414
mod trait_impl_difference;
1515
mod util;
1616

17+
pub use static_impl_trait::suggest_new_region_bound;
18+
1719
impl<'cx, 'tcx> InferCtxt<'cx, 'tcx> {
1820
pub fn try_report_nice_region_error(&self, error: &RegionResolutionError<'tcx>) -> bool {
1921
NiceRegionError::new(self, error.clone()).try_report().is_some()

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

Lines changed: 120 additions & 89 deletions
Original file line numberDiff line numberDiff line change
@@ -217,128 +217,159 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
217217
));
218218
}
219219

220-
debug!("try_report_static_impl_trait: fn_return={:?}", fn_returns);
221-
// FIXME: account for the need of parens in `&(dyn Trait + '_)`
222-
let consider = "consider changing the";
223-
let declare = "to declare that the";
224220
let arg = match param.param.pat.simple_ident() {
225221
Some(simple_ident) => format!("argument `{}`", simple_ident),
226222
None => "the argument".to_string(),
227223
};
228-
let explicit = format!("you can add an explicit `{}` lifetime bound", lifetime_name);
229-
let explicit_static = format!("explicit `'static` bound to the lifetime of {}", arg);
230224
let captures = format!("captures data from {}", arg);
231-
let add_static_bound = "alternatively, add an explicit `'static` bound to this reference";
232-
let plus_lt = format!(" + {}", lifetime_name);
233-
for fn_return in fn_returns {
234-
if fn_return.span.desugaring_kind().is_some() {
235-
// Skip `async` desugaring `impl Future`.
236-
continue;
237-
}
238-
match fn_return.kind {
239-
TyKind::OpaqueDef(item_id, _) => {
240-
let item = tcx.hir().item(item_id);
241-
let opaque = if let ItemKind::OpaqueTy(opaque) = &item.kind {
242-
opaque
243-
} else {
244-
err.emit();
245-
return Some(ErrorReported);
246-
};
225+
suggest_new_region_bound(
226+
tcx,
227+
&mut err,
228+
fn_returns,
229+
lifetime_name,
230+
Some(arg),
231+
captures,
232+
Some((param.param_ty_span, param.param_ty.to_string())),
233+
);
247234

248-
if let Some(span) = opaque
249-
.bounds
250-
.iter()
251-
.filter_map(|arg| match arg {
252-
GenericBound::Outlives(Lifetime {
253-
name: LifetimeName::Static,
254-
span,
255-
..
256-
}) => Some(*span),
257-
_ => None,
258-
})
259-
.next()
260-
{
235+
err.emit();
236+
Some(ErrorReported)
237+
}
238+
}
239+
240+
pub fn suggest_new_region_bound(
241+
tcx: TyCtxt<'tcx>,
242+
err: &mut DiagnosticBuilder<'_>,
243+
fn_returns: Vec<&rustc_hir::Ty<'_>>,
244+
lifetime_name: String,
245+
arg: Option<String>,
246+
captures: String,
247+
param: Option<(Span, String)>,
248+
) {
249+
debug!("try_report_static_impl_trait: fn_return={:?}", fn_returns);
250+
// FIXME: account for the need of parens in `&(dyn Trait + '_)`
251+
let consider = "consider changing the";
252+
let declare = "to declare that the";
253+
let explicit = format!("you can add an explicit `{}` lifetime bound", lifetime_name);
254+
let explicit_static =
255+
arg.map(|arg| format!("explicit `'static` bound to the lifetime of {}", arg));
256+
let add_static_bound = "alternatively, add an explicit `'static` bound to this reference";
257+
let plus_lt = format!(" + {}", lifetime_name);
258+
for fn_return in fn_returns {
259+
if fn_return.span.desugaring_kind().is_some() {
260+
// Skip `async` desugaring `impl Future`.
261+
continue;
262+
}
263+
match fn_return.kind {
264+
TyKind::OpaqueDef(item_id, _) => {
265+
let item = tcx.hir().item(item_id);
266+
let opaque = if let ItemKind::OpaqueTy(opaque) = &item.kind {
267+
opaque
268+
} else {
269+
return;
270+
};
271+
272+
if let Some(span) = opaque
273+
.bounds
274+
.iter()
275+
.filter_map(|arg| match arg {
276+
GenericBound::Outlives(Lifetime {
277+
name: LifetimeName::Static,
278+
span,
279+
..
280+
}) => Some(*span),
281+
_ => None,
282+
})
283+
.next()
284+
{
285+
if let Some(explicit_static) = &explicit_static {
261286
err.span_suggestion_verbose(
262287
span,
263288
&format!("{} `impl Trait`'s {}", consider, explicit_static),
264289
lifetime_name.clone(),
265290
Applicability::MaybeIncorrect,
266291
);
292+
}
293+
if let Some((param_span, param_ty)) = param.clone() {
267294
err.span_suggestion_verbose(
268-
param.param_ty_span,
295+
param_span,
269296
add_static_bound,
270-
param.param_ty.to_string(),
271-
Applicability::MaybeIncorrect,
272-
);
273-
} else if opaque
274-
.bounds
275-
.iter()
276-
.filter_map(|arg| match arg {
277-
GenericBound::Outlives(Lifetime { name, span, .. })
278-
if name.ident().to_string() == lifetime_name =>
279-
{
280-
Some(*span)
281-
}
282-
_ => None,
283-
})
284-
.next()
285-
.is_some()
286-
{
287-
} else {
288-
err.span_suggestion_verbose(
289-
fn_return.span.shrink_to_hi(),
290-
&format!(
291-
"{declare} `impl Trait` {captures}, {explicit}",
292-
declare = declare,
293-
captures = captures,
294-
explicit = explicit,
295-
),
296-
plus_lt.clone(),
297+
param_ty,
297298
Applicability::MaybeIncorrect,
298299
);
299300
}
301+
} else if opaque
302+
.bounds
303+
.iter()
304+
.filter_map(|arg| match arg {
305+
GenericBound::Outlives(Lifetime { name, span, .. })
306+
if name.ident().to_string() == lifetime_name =>
307+
{
308+
Some(*span)
309+
}
310+
_ => None,
311+
})
312+
.next()
313+
.is_some()
314+
{
315+
} else {
316+
err.span_suggestion_verbose(
317+
fn_return.span.shrink_to_hi(),
318+
&format!(
319+
"{declare} `impl Trait` {captures}, {explicit}",
320+
declare = declare,
321+
captures = captures,
322+
explicit = explicit,
323+
),
324+
plus_lt.clone(),
325+
Applicability::MaybeIncorrect,
326+
);
300327
}
301-
TyKind::TraitObject(_, lt, _) => match lt.name {
302-
LifetimeName::ImplicitObjectLifetimeDefault => {
303-
err.span_suggestion_verbose(
304-
fn_return.span.shrink_to_hi(),
305-
&format!(
306-
"{declare} trait object {captures}, {explicit}",
307-
declare = declare,
308-
captures = captures,
309-
explicit = explicit,
310-
),
311-
plus_lt.clone(),
312-
Applicability::MaybeIncorrect,
313-
);
314-
}
315-
name if name.ident().to_string() != lifetime_name => {
316-
// With this check we avoid suggesting redundant bounds. This
317-
// would happen if there are nested impl/dyn traits and only
318-
// one of them has the bound we'd suggest already there, like
319-
// in `impl Foo<X = dyn Bar> + '_`.
328+
}
329+
TyKind::TraitObject(_, lt, _) => match lt.name {
330+
LifetimeName::ImplicitObjectLifetimeDefault => {
331+
err.span_suggestion_verbose(
332+
fn_return.span.shrink_to_hi(),
333+
&format!(
334+
"{declare} trait object {captures}, {explicit}",
335+
declare = declare,
336+
captures = captures,
337+
explicit = explicit,
338+
),
339+
plus_lt.clone(),
340+
Applicability::MaybeIncorrect,
341+
);
342+
}
343+
name if name.ident().to_string() != lifetime_name => {
344+
// With this check we avoid suggesting redundant bounds. This
345+
// would happen if there are nested impl/dyn traits and only
346+
// one of them has the bound we'd suggest already there, like
347+
// in `impl Foo<X = dyn Bar> + '_`.
348+
if let Some(explicit_static) = &explicit_static {
320349
err.span_suggestion_verbose(
321350
lt.span,
322351
&format!("{} trait object's {}", consider, explicit_static),
323352
lifetime_name.clone(),
324353
Applicability::MaybeIncorrect,
325354
);
355+
}
356+
if let Some((param_span, param_ty)) = param.clone() {
326357
err.span_suggestion_verbose(
327-
param.param_ty_span,
358+
param_span,
328359
add_static_bound,
329-
param.param_ty.to_string(),
360+
param_ty,
330361
Applicability::MaybeIncorrect,
331362
);
332363
}
333-
_ => {}
334-
},
364+
}
335365
_ => {}
336-
}
366+
},
367+
_ => {}
337368
}
338-
err.emit();
339-
Some(ErrorReported)
340369
}
370+
}
341371

372+
impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
342373
fn get_impl_ident_and_self_ty_from_trait(
343374
&self,
344375
def_id: DefId,

‎src/test/ui/impl-trait/hidden-lifetimes.stderr

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,11 @@ LL | fn hide_ref<'a, 'b, T: 'static>(x: &'a mut &'b T) -> impl Swap + 'a {
55
| -- ^^^^^^^^^^^^^^
66
| |
77
| hidden type `&'a mut &'b T` captures the lifetime `'b` as defined here
8+
|
9+
help: to declare that the `impl Trait` captures 'b, you can add an explicit `'b` lifetime bound
10+
|
11+
LL | fn hide_ref<'a, 'b, T: 'static>(x: &'a mut &'b T) -> impl Swap + 'a + 'b {
12+
| ++++
813

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

1727
error: aborting due to 2 previous errors
1828

‎src/test/ui/impl-trait/region-escape-via-bound.stderr

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,11 @@ LL | fn foo(x: Cell<&'x u32>) -> impl Trait<'y>
66
LL |
77
LL | where 'x: 'y
88
| -- hidden type `Cell<&'x u32>` captures the lifetime `'x` as defined here
9+
|
10+
help: to declare that the `impl Trait` captures 'x, you can add an explicit `'x` lifetime bound
11+
|
12+
LL | fn foo(x: Cell<&'x u32>) -> impl Trait<'y> + 'x
13+
| ++++
914

1015
error: aborting due to previous error
1116

0 commit comments

Comments
 (0)