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.

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

Account for multiple impl/dyn Trait in return type when suggesting '_ #73496

Merged
merged 2 commits into from
Jun 23, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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()
estebank marked this conversation as resolved.
Show resolved Hide resolved
{
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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>> {
estebank marked this conversation as resolved.
Show resolved Hide resolved
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
Original file line number Diff line number Diff line change
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);
}
}
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Loading