Skip to content

Commit

Permalink
Rollup merge of rust-lang#72804 - estebank:opaque-missing-lts-in-fn-2…
Browse files Browse the repository at this point in the history
…, r=nikomatsakis

Further tweak lifetime errors involving `dyn Trait` and `impl Trait` in return position

* Suggest substituting `'static` lifetime in impl/dyn `Trait + 'static` instead of `Trait + 'static + '_`
* When `'static` is explicit, also suggest constraining argument with it
* Reduce verbosity of suggestion message and mention lifetime in label
* Tweak output for overlapping required/captured spans
* Give these errors an error code

Follow up to rust-lang#72543.

r? @nikomatsakis
  • Loading branch information
RalfJung authored Jun 13, 2020
2 parents 2b763fb + 6145918 commit fb9a119
Show file tree
Hide file tree
Showing 25 changed files with 611 additions and 205 deletions.
1 change: 1 addition & 0 deletions src/librustc_error_codes/error_codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -439,6 +439,7 @@ E0752: include_str!("./error_codes/E0752.md"),
E0753: include_str!("./error_codes/E0753.md"),
E0754: include_str!("./error_codes/E0754.md"),
E0758: include_str!("./error_codes/E0758.md"),
E0759: include_str!("./error_codes/E0759.md"),
E0760: include_str!("./error_codes/E0760.md"),
E0761: include_str!("./error_codes/E0761.md"),
E0762: include_str!("./error_codes/E0762.md"),
Expand Down
67 changes: 67 additions & 0 deletions src/librustc_error_codes/error_codes/E0759.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
A `'static` requirement in a return type involving a trait is not fulfilled.

Erroneous code examples:

```compile_fail,E0759
use std::fmt::Debug;
fn foo(x: &i32) -> impl Debug {
x
}
```

```compile_fail,E0759
# use std::fmt::Debug;
fn bar(x: &i32) -> Box<dyn Debug> {
Box::new(x)
}
```

These examples have the same semantics as the following:

```compile_fail,E0759
# use std::fmt::Debug;
fn foo(x: &i32) -> impl Debug + 'static {
x
}
```

```compile_fail,E0759
# use std::fmt::Debug;
fn bar(x: &i32) -> Box<dyn Debug + 'static> {
Box::new(x)
}
```

Both [`dyn Trait`] and [`impl Trait`] in return types have a an implicit
`'static` requirement, meaning that the value implementing them that is being
returned has to be either a `'static` borrow or an owned value.

In order to change the requirement from `'static` to be a lifetime derived from
its arguments, you can add an explicit bound, either to an anonymous lifetime
`'_` or some appropriate named lifetime.

```
# use std::fmt::Debug;
fn foo(x: &i32) -> impl Debug + '_ {
x
}
fn bar(x: &i32) -> Box<dyn Debug + '_> {
Box::new(x)
}
```

These are equivalent to the following explicit lifetime annotations:

```
# use std::fmt::Debug;
fn foo<'a>(x: &'a i32) -> impl Debug + 'a {
x
}
fn bar<'a>(x: &'a i32) -> Box<dyn Debug + 'a> {
Box::new(x)
}
```

[`dyn Trait`]: https://doc.rust-lang.org/book/ch17-02-trait-objects.html#using-trait-objects-that-allow-for-values-of-different-types
[`impl Trait`]: https://doc.rust-lang.org/book/ch10-02-traits.html#returning-types-that-implement-traits
3 changes: 1 addition & 2 deletions src/librustc_infer/infer/error_reporting/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2037,8 +2037,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
self.tcx.sess,
var_origin.span(),
E0495,
"cannot infer an appropriate lifetime{} \
due to conflicting requirements",
"cannot infer an appropriate lifetime{} due to conflicting requirements",
var_description
)
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
//! Error Reporting for static impl Traits.
use crate::infer::error_reporting::msg_span_from_free_region;
use crate::infer::error_reporting::nice_region_error::NiceRegionError;
use crate::infer::lexical_region_resolve::RegionResolutionError;
use rustc_errors::{Applicability, ErrorReported};
use rustc_errors::{struct_span_err, Applicability, ErrorReported};
use rustc_hir::{GenericBound, ItemKind, Lifetime, LifetimeName, TyKind};
use rustc_middle::ty::RegionKind;

impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
/// Print the error message for lifetime errors when the return type is a static impl Trait.
pub(super) fn try_report_static_impl_trait(&self) -> Option<ErrorReported> {
debug!("try_report_static_impl_trait(error={:?})", self.error);
if let Some(ref error) = self.error {
if let RegionResolutionError::SubSupConflict(
_,
Expand All @@ -17,18 +18,36 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
sub_r,
sup_origin,
sup_r,
) = error.clone()
) = error
{
debug!(
"try_report_static_impl_trait(var={:?}, sub={:?} {:?} sup={:?} {:?})",
var_origin, sub_origin, sub_r, sup_origin, sup_r
);
let anon_reg_sup = self.tcx().is_suitable_region(sup_r)?;
let (fn_return_span, is_dyn) =
self.tcx().return_type_impl_or_dyn_trait(anon_reg_sup.def_id)?;
if sub_r == &RegionKind::ReStatic {
debug!("try_report_static_impl_trait: anon_reg_sup={:?}", anon_reg_sup);
let fn_return = self.tcx().return_type_impl_or_dyn_trait(anon_reg_sup.def_id)?;
debug!("try_report_static_impl_trait: fn_return={:?}", fn_return);
if **sub_r == RegionKind::ReStatic {
let sp = var_origin.span();
let return_sp = sub_origin.span();
let mut err =
self.tcx().sess.struct_span_err(sp, "cannot infer an appropriate lifetime");
let param_info = self.find_param_with_region(sup_r, sub_r)?;
err.span_label(param_info.param_ty_span, "data with this lifetime...");
let (lifetime_name, lifetime) = if sup_r.has_name() {
(sup_r.to_string(), format!("lifetime `{}`", sup_r))
} else {
("'_".to_owned(), "an anonymous lifetime `'_`".to_string())
};
let mut err = struct_span_err!(
self.tcx().sess,
sp,
E0759,
"cannot infer an appropriate lifetime"
);
err.span_label(
param_info.param_ty_span,
&format!("this data with {}...", lifetime),
);
debug!("try_report_static_impl_trait: param_info={:?}", param_info);

// We try to make the output have fewer overlapping spans if possible.
if (sp == sup_origin.span() || !return_sp.overlaps(sup_origin.span()))
Expand All @@ -38,41 +57,146 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {

// Customize the spans and labels depending on their relative order so
// that split sentences flow correctly.
if sup_origin.span().shrink_to_hi() <= return_sp.shrink_to_lo() {
err.span_label(sup_origin.span(), "...is captured here...");
err.span_label(return_sp, "...and required to be `'static` by this");
if sup_origin.span().overlaps(return_sp) && sp == sup_origin.span() {
// Avoid the following:
//
// error: cannot infer an appropriate lifetime
// --> $DIR/must_outlive_least_region_or_bound.rs:18:50
// |
// LL | fn foo(x: &i32) -> Box<dyn Debug> { Box::new(x) }
// | ---- ---------^-
//
// and instead show:
//
// error: cannot infer an appropriate lifetime
// --> $DIR/must_outlive_least_region_or_bound.rs:18:50
// |
// LL | fn foo(x: &i32) -> Box<dyn Debug> { Box::new(x) }
// | ---- ^
err.span_label(
sup_origin.span(),
"...is captured here, requiring it to live as long as `'static`",
);
} else {
err.span_label(return_sp, "...is required to be `'static` by this...");
err.span_label(sup_origin.span(), "...and is captured here");
err.span_label(sup_origin.span(), "...is captured here...");
if return_sp < sup_origin.span() {
err.span_note(
return_sp,
"...and is required to live as long as `'static` here",
);
} else {
err.span_label(
return_sp,
"...and is required to live as long as `'static` here",
);
}
}
} else {
err.span_label(
return_sp,
"...is captured and required to be `'static` here",
"...is captured and required to live as long as `'static` here",
);
}

let (lifetime, _) = msg_span_from_free_region(self.tcx(), sup_r);

let lifetime_name =
if sup_r.has_name() { sup_r.to_string() } else { "'_".to_owned() };
// only apply this suggestion onto functions with
// explicit non-desugar'able return.
if fn_return_span.desugaring_kind().is_none() {
let msg = format!(
"to permit non-static references in {} `{} Trait` value, you can add \
an explicit bound for {}",
if is_dyn { "a" } else { "an" },
if is_dyn { "dyn" } else { "impl" },
lifetime,
);
if fn_return.span.desugaring_kind().is_none() {
// FIXME: account for the need of parens in `&(dyn Trait + '_)`
err.span_suggestion_verbose(
fn_return_span.shrink_to_hi(),
&msg,
format!(" + {}", lifetime_name),
Applicability::MaybeIncorrect,
);

let consider = "consider changing the";
let declare = "to declare that the";
let arg = match param_info.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);
match fn_return.kind {
TyKind::Def(item_id, _) => {
let item = self.tcx().hir().item(item_id.id);
let opaque = if let ItemKind::OpaqueTy(opaque) = &item.kind {
opaque
} else {
err.emit();
return Some(ErrorReported);
};

if let Some(span) = opaque
.bounds
.iter()
.filter_map(|arg| match arg {
GenericBound::Outlives(Lifetime {
name: LifetimeName::Static,
span,
..
}) => Some(*span),
_ => None,
})
.next()
{
err.span_suggestion_verbose(
span,
&format!("{} `impl Trait`'s {}", consider, explicit_static),
lifetime_name,
Applicability::MaybeIncorrect,
);
err.span_suggestion_verbose(
param_info.param_ty_span,
add_static_bound,
param_info.param_ty.to_string(),
Applicability::MaybeIncorrect,
);
} 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,
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,
Applicability::MaybeIncorrect,
);
}
_ => {
err.span_suggestion_verbose(
lt.span,
&format!("{} trait object's {}", consider, explicit_static),
lifetime_name,
Applicability::MaybeIncorrect,
);
err.span_suggestion_verbose(
param_info.param_ty_span,
add_static_bound,
param_info.param_ty.to_string(),
Applicability::MaybeIncorrect,
);
}
},
_ => {}
}
}
err.emit();
return Some(ErrorReported);
Expand Down
13 changes: 9 additions & 4 deletions src/librustc_middle/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1383,7 +1383,10 @@ impl<'tcx> TyCtxt<'tcx> {
})
}

pub fn return_type_impl_or_dyn_trait(&self, scope_def_id: DefId) -> Option<(Span, bool)> {
pub fn return_type_impl_or_dyn_trait(
&self,
scope_def_id: DefId,
) -> Option<&'tcx hir::Ty<'tcx>> {
let hir_id = self.hir().as_local_hir_id(scope_def_id.expect_local());
let hir_output = match self.hir().get(hir_id) {
Node::Item(hir::Item {
Expand Down Expand Up @@ -1429,15 +1432,17 @@ impl<'tcx> TyCtxt<'tcx> {
let output = self.erase_late_bound_regions(&sig.output());
if output.is_impl_trait() {
let fn_decl = self.hir().fn_decl_by_hir_id(hir_id).unwrap();
Some((fn_decl.output.span(), false))
if let hir::FnRetTy::Return(ty) = fn_decl.output {
return Some(ty);
}
} else {
let mut v = TraitObjectVisitor(vec![]);
rustc_hir::intravisit::walk_ty(&mut v, hir_output);
if v.0.len() == 1 {
return Some((v.0[0], true));
return Some(v.0[0]);
}
None
}
None
}
_ => None,
}
Expand Down
13 changes: 8 additions & 5 deletions src/librustc_middle/ty/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -236,21 +236,24 @@ pub fn suggest_constraining_type_param(
}
}

pub struct TraitObjectVisitor(pub Vec<rustc_span::Span>);
impl<'v> hir::intravisit::Visitor<'v> for TraitObjectVisitor {
pub struct TraitObjectVisitor<'tcx>(pub Vec<&'tcx hir::Ty<'tcx>>);
impl<'v> hir::intravisit::Visitor<'v> for TraitObjectVisitor<'v> {
type Map = rustc_hir::intravisit::ErasedMap<'v>;

fn nested_visit_map(&mut self) -> hir::intravisit::NestedVisitorMap<Self::Map> {
hir::intravisit::NestedVisitorMap::None
}

fn visit_ty(&mut self, ty: &hir::Ty<'_>) {
fn visit_ty(&mut self, ty: &'v hir::Ty<'v>) {
if let hir::TyKind::TraitObject(
_,
hir::Lifetime { name: hir::LifetimeName::ImplicitObjectLifetimeDefault, .. },
hir::Lifetime {
name: hir::LifetimeName::ImplicitObjectLifetimeDefault | hir::LifetimeName::Static,
..
},
) = ty.kind
{
self.0.push(ty.span);
self.0.push(ty);
}
}
}
7 changes: 4 additions & 3 deletions src/test/ui/async-await/issues/issue-62097.stderr
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
error: cannot infer an appropriate lifetime
error[E0759]: cannot infer an appropriate lifetime
--> $DIR/issue-62097.rs:12:31
|
LL | pub async fn run_dummy_fn(&self) {
| ^^^^^
| |
| data with this lifetime...
| this data with an anonymous lifetime `'_`...
| ...is captured here...
LL | foo(|| self.bar()).await;
| --- ...and required to be `'static` by this
| --- ...and is required to live as long as `'static` here

error: aborting due to previous error

For more information about this error, try `rustc --explain E0759`.
Loading

0 comments on commit fb9a119

Please sign in to comment.