Skip to content

Commit

Permalink
Account for multiple impl/dyn Trait in return type when suggesting '_
Browse files Browse the repository at this point in the history
  • Loading branch information
@estebank
estebank committed Jun 19, 2020
1 parent a39c778 commit 562f496
Show file tree
Hide file tree
Showing 12 changed files with 363 additions and 77 deletions.
2 changes: 1 addition & 1 deletion src/librustc_infer/infer/error_reporting/nice_region_error/find_anon_type.rs
View file
Expand Up @@ -26,7 +26,7 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
&self,
region: Region<'tcx>,
br: &ty::BoundRegion,
) -> Option<(&hir::Ty<'_>, &hir::FnDecl<'_>)> {
) -> Option<(&hir::Ty<'tcx>, &hir::FnDecl<'tcx>)> {
if let Some(anon_reg) = self.tcx().is_suitable_region(region) {
let def_id = anon_reg.def_id;
if let Some(def_id) = def_id.as_local() {
Expand Down
13 changes: 9 additions & 4 deletions src/librustc_infer/infer/error_reporting/nice_region_error/named_anon_conflict.rs
View file
Expand Up @@ -2,7 +2,8 @@
//! where one region is named and the other is anonymous.
use crate::infer::error_reporting::nice_region_error::NiceRegionError;
use rustc_errors::{struct_span_err, Applicability, DiagnosticBuilder};
use rustc_hir::{FnRetTy, TyKind};
use rustc_hir::intravisit::Visitor;
use rustc_hir::FnRetTy;
use rustc_middle::ty;

impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
Expand Down Expand Up @@ -80,12 +81,16 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
}

if let FnRetTy::Return(ty) = &fndecl.output {
let mut v = ty::TraitObjectVisitor(vec![]);
rustc_hir::intravisit::walk_ty(&mut v, ty);
let mut v = ty::TraitObjectVisitor(vec![], self.tcx().hir());
v.visit_ty(ty);

debug!("try_report_named_anon_conflict: ret ty {:?}", ty);
if sub == &ty::ReStatic
&& (matches!(ty.kind, TyKind::OpaqueDef(_, _)) || v.0.len() == 1)
&& v.0
.into_iter()
.filter(|t| t.span.desugaring_kind().is_none())
.next()
.is_some()
{
debug!("try_report_named_anon_conflict: impl Trait + 'static");
// This is an `impl Trait` or `dyn Trait` return that evaluates de need of
Expand Down
76 changes: 49 additions & 27 deletions src/librustc_infer/infer/error_reporting/nice_region_error/static_impl_trait.rs
View file
Expand Up @@ -26,8 +26,11 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
);
let anon_reg_sup = self.tcx().is_suitable_region(sup_r)?;
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);
let fn_returns = self.tcx().return_type_impl_or_dyn_trait(anon_reg_sup.def_id);
if fn_returns.is_empty() {
return None;
}
debug!("try_report_static_impl_trait: fn_return={:?}", fn_returns);
if **sub_r == RegionKind::ReStatic {
let sp = var_origin.span();
let return_sp = sub_origin.span();
Expand Down Expand Up @@ -98,25 +101,26 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
);
}

// only apply this suggestion onto functions with
// explicit non-desugar'able return.
if fn_return.span.desugaring_kind().is_none() {
// 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_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);
// 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_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);
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 = self.tcx().hir().item(item_id.id);
Expand All @@ -143,7 +147,7 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
err.span_suggestion_verbose(
span,
&format!("{} `impl Trait`'s {}", consider, explicit_static),
lifetime_name,
lifetime_name.clone(),
Applicability::MaybeIncorrect,
);
err.span_suggestion_verbose(
Expand All @@ -152,6 +156,19 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
param_info.param_ty.to_string(),
Applicability::MaybeIncorrect,
);
} else if let Some(_) = opaque
.bounds
.iter()
.filter_map(|arg| match arg {
GenericBound::Outlives(Lifetime { name, span, .. })
if name.ident().to_string() == lifetime_name =>
{
Some(*span)
}
_ => None,
})
.next()
{
} else {
err.span_suggestion_verbose(
fn_return.span.shrink_to_hi(),
Expand All @@ -161,10 +178,10 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
captures = captures,
explicit = explicit,
),
plus_lt,
plus_lt.clone(),
Applicability::MaybeIncorrect,
);
};
}
}
TyKind::TraitObject(_, lt) => match lt.name {
LifetimeName::ImplicitObjectLifetimeDefault => {
Expand All @@ -176,15 +193,19 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
captures = captures,
explicit = explicit,
),
plus_lt,
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> + '_`.
err.span_suggestion_verbose(
lt.span,
&format!("{} trait object's {}", consider, explicit_static),
lifetime_name,
lifetime_name.clone(),
Applicability::MaybeIncorrect,
);
err.span_suggestion_verbose(
Expand All @@ -194,6 +215,7 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
Applicability::MaybeIncorrect,
);
}
_ => {}
},
_ => {}
}
Expand Down
32 changes: 6 additions & 26 deletions src/librustc_middle/ty/context.rs
View file
Expand Up @@ -37,6 +37,7 @@ use rustc_hir as hir;
use rustc_hir::def::{DefKind, Res};
use rustc_hir::def_id::{CrateNum, DefId, DefIdMap, DefIdSet, LocalDefId, LOCAL_CRATE};
use rustc_hir::definitions::{DefPathHash, Definitions};
use rustc_hir::intravisit::Visitor;
use rustc_hir::lang_items::{self, PanicLocationLangItem};
use rustc_hir::{HirId, ItemKind, ItemLocalId, ItemLocalMap, ItemLocalSet, Node, TraitCandidate};
use rustc_index::vec::{Idx, IndexVec};
Expand Down Expand Up @@ -1405,10 +1406,7 @@ impl<'tcx> TyCtxt<'tcx> {
})
}

pub fn return_type_impl_or_dyn_trait(
&self,
scope_def_id: DefId,
) -> Option<&'tcx hir::Ty<'tcx>> {
pub fn return_type_impl_or_dyn_trait(&self, scope_def_id: DefId) -> Vec<&'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 @@ -1444,30 +1442,12 @@ impl<'tcx> TyCtxt<'tcx> {
),
..
}) => ty,
_ => return None,
_ => return vec![],
};

let ret_ty = self.type_of(scope_def_id);
match ret_ty.kind {
ty::FnDef(_, _) => {
let sig = ret_ty.fn_sig(*self);
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();
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]);
}
}
None
}
_ => None,
}
let mut v = TraitObjectVisitor(vec![], self.hir());
v.visit_ty(hir_output);
v.0
}

pub fn return_type_impl_trait(&self, scope_def_id: DefId) -> Option<(Ty<'tcx>, Span)> {
Expand Down
31 changes: 21 additions & 10 deletions src/librustc_middle/ty/diagnostics.rs
View file
Expand Up @@ -236,7 +236,9 @@ pub fn suggest_constraining_type_param(
}
}

pub struct TraitObjectVisitor<'tcx>(pub Vec<&'tcx hir::Ty<'tcx>>);
/// Collect al types that have an implicit `'static` obligation that we could suggest `'_` for.
pub struct TraitObjectVisitor<'tcx>(pub Vec<&'tcx hir::Ty<'tcx>>, pub crate::hir::map::Map<'tcx>);

impl<'v> hir::intravisit::Visitor<'v> for TraitObjectVisitor<'v> {
type Map = rustc_hir::intravisit::ErasedMap<'v>;

Expand All @@ -245,15 +247,24 @@ impl<'v> hir::intravisit::Visitor<'v> for TraitObjectVisitor<'v> {
}

fn visit_ty(&mut self, ty: &'v hir::Ty<'v>) {
if let hir::TyKind::TraitObject(
_,
hir::Lifetime {
name: hir::LifetimeName::ImplicitObjectLifetimeDefault | hir::LifetimeName::Static,
..
},
) = ty.kind
{
self.0.push(ty);
match ty.kind {
hir::TyKind::TraitObject(
_,
hir::Lifetime {
name:
hir::LifetimeName::ImplicitObjectLifetimeDefault | hir::LifetimeName::Static,
..
},
) => {
self.0.push(ty);
}
hir::TyKind::OpaqueDef(item_id, _) => {
self.0.push(ty);
let item = self.1.expect_item(item_id.id);
hir::intravisit::walk_item(self, item);
}
_ => {}
}
hir::intravisit::walk_ty(self, ty);
}
}
16 changes: 12 additions & 4 deletions src/test/ui/impl-trait/must_outlive_least_region_or_bound.nll.stderr
View file
Expand Up @@ -53,7 +53,15 @@ LL | fn foo<'a>(x: &i32) -> impl Copy + 'a { x }
| help: add explicit lifetime `'a` to the type of `x`: `&'a i32`

error: lifetime may not live long enough
--> $DIR/must_outlive_least_region_or_bound.rs:33:69
--> $DIR/must_outlive_least_region_or_bound.rs:30:24
|
LL | fn elided5(x: &i32) -> (Box<dyn Debug>, impl Debug) { (Box::new(x), x) }
| - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ opaque type requires that `'1` must outlive `'static`
| |
| let's call the lifetime of this reference `'1`

error: lifetime may not live long enough
--> $DIR/must_outlive_least_region_or_bound.rs:37: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`
Expand All @@ -62,7 +70,7 @@ LL | fn with_bound<'a>(x: &'a i32) -> impl LifetimeTrait<'a> + 'static { x }
= help: consider replacing `'a` with `'static`

error: lifetime may not live long enough
--> $DIR/must_outlive_least_region_or_bound.rs:38:61
--> $DIR/must_outlive_least_region_or_bound.rs:42:61
|
LL | fn move_lifetime_into_fn<'a, 'b>(x: &'a u32, y: &'b u32) -> impl Fn(&'a u32) {
| -- -- lifetime `'b` defined here ^^^^^^^^^^^^^^^^ opaque type requires that `'b` must outlive `'a`
Expand All @@ -72,14 +80,14 @@ LL | fn move_lifetime_into_fn<'a, 'b>(x: &'a u32, y: &'b u32) -> impl Fn(&'a u32
= help: consider adding the following bound: `'b: 'a`

error[E0310]: the parameter type `T` may not live long enough
--> $DIR/must_outlive_least_region_or_bound.rs:43:51
--> $DIR/must_outlive_least_region_or_bound.rs:47:51
|
LL | fn ty_param_wont_outlive_static<T:Debug>(x: T) -> impl Debug + 'static {
| ^^^^^^^^^^^^^^^^^^^^
|
= help: consider adding an explicit lifetime bound `T: 'static`...

error: aborting due to 8 previous errors
error: aborting due to 9 previous errors

Some errors have detailed explanations: E0310, E0621.
For more information about an error, try `rustc --explain E0310`.
4 changes: 4 additions & 0 deletions src/test/ui/impl-trait/must_outlive_least_region_or_bound.rs
View file
Expand Up @@ -27,6 +27,10 @@ fn elided4(x: &i32) -> Box<dyn Debug + 'static> { Box::new(x) }
fn explicit4<'a>(x: &'a i32) -> Box<dyn Debug + 'static> { Box::new(x) }
//~^ ERROR cannot infer an appropriate lifetime

fn elided5(x: &i32) -> (Box<dyn Debug>, impl Debug) { (Box::new(x), x) }
//~^ ERROR cannot infer an appropriate lifetime
//~| ERROR cannot infer an appropriate lifetime

trait LifetimeTrait<'a> {}
impl<'a> LifetimeTrait<'a> for &'a i32 {}

Expand Down

0 comments on commit 562f496

Please sign in to comment.