Skip to content

Commit

Permalink
Increase accuracy of lifetime bound on trait object impl suggestion
Browse files Browse the repository at this point in the history
  • Loading branch information
estebank committed Jun 28, 2020
1 parent e6599ad commit bdb39ab
Show file tree
Hide file tree
Showing 18 changed files with 321 additions and 114 deletions.
12 changes: 11 additions & 1 deletion src/librustc_hir/hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2197,7 +2197,17 @@ pub enum IsAsync {
NotAsync,
}

#[derive(Copy, Clone, PartialEq, RustcEncodable, RustcDecodable, Debug, HashStable_Generic)]
#[derive(
Copy,
Clone,
PartialEq,
RustcEncodable,
RustcDecodable,
Debug,
HashStable_Generic,
Eq,
Hash
)]
pub enum Defaultness {
Default { has_value: bool },
Final,
Expand Down
10 changes: 6 additions & 4 deletions src/librustc_infer/infer/combine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,13 @@ use crate::traits::{Obligation, PredicateObligations};

use rustc_ast::ast;
use rustc_hir::def_id::DefId;
use rustc_middle::traits::ObligationCause;
use rustc_middle::ty::error::TypeError;
use rustc_middle::ty::relate::{self, Relate, RelateResult, TypeRelation};
use rustc_middle::ty::subst::SubstsRef;
use rustc_middle::ty::{self, InferConst, ToPredicate, Ty, TyCtxt, TypeFoldable};
use rustc_middle::ty::{IntType, UintType};
use rustc_span::{Span, DUMMY_SP};
use rustc_span::DUMMY_SP;

#[derive(Clone)]
pub struct CombineFields<'infcx, 'tcx> {
Expand Down Expand Up @@ -367,10 +368,11 @@ impl<'infcx, 'tcx> CombineFields<'infcx, 'tcx> {
};

debug!("generalize: for_universe = {:?}", for_universe);
debug!("generalize: trace = {:?}", self.trace);

let mut generalize = Generalizer {
infcx: self.infcx,
span: self.trace.cause.span,
cause: &self.trace.cause,
for_vid_sub_root: self.infcx.inner.borrow_mut().type_variables().sub_root_var(for_vid),
for_universe,
ambient_variance,
Expand Down Expand Up @@ -414,7 +416,7 @@ struct Generalizer<'cx, 'tcx> {
infcx: &'cx InferCtxt<'cx, 'tcx>,

/// The span, used when creating new type variables and things.
span: Span,
cause: &'cx ObligationCause<'tcx>,

/// The vid of the type variable that is in the process of being
/// instantiated; if we find this within the type we are folding,
Expand Down Expand Up @@ -639,7 +641,7 @@ impl TypeRelation<'tcx> for Generalizer<'_, 'tcx> {

// FIXME: This is non-ideal because we don't give a
// very descriptive origin for this region variable.
Ok(self.infcx.next_region_var_in_universe(MiscVariable(self.span), self.for_universe))
Ok(self.infcx.next_region_var_in_universe(MiscVariable(self.cause.span), self.for_universe))
}

fn consts(
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_infer/infer/error_reporting/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2007,7 +2007,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
infer::MiscVariable(_) => String::new(),
infer::PatternRegion(_) => " for pattern".to_string(),
infer::AddrOfRegion(_) => " for borrow expression".to_string(),
infer::Autoref(_) => " for autoref".to_string(),
infer::Autoref(_, _) => " for autoref".to_string(),
infer::Coercion(_) => " for automatic coercion".to_string(),
infer::LateBoundRegion(_, br, infer::FnCall) => {
format!(" for lifetime parameter {}in function call", br_string(br))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,14 @@

use crate::infer::error_reporting::nice_region_error::NiceRegionError;
use crate::infer::lexical_region_resolve::RegionResolutionError;
use crate::infer::{SubregionOrigin, TypeTrace};
use crate::traits::ObligationCauseCode;
use rustc_errors::{struct_span_err, Applicability, DiagnosticBuilder, ErrorReported};
use rustc_hir::def::{DefKind, Res};
use rustc_hir::def_id::{DefId, LOCAL_CRATE};
use rustc_hir::{
GenericBound, Item, ItemKind, Lifetime, LifetimeName, Node, Path, PolyTraitRef, TraitRef,
TyKind,
};
use rustc_middle::ty::{self, RegionKind, Ty, TypeFoldable, TypeVisitor};
use rustc_hir::intravisit::{walk_ty, ErasedMap, NestedVisitorMap, Visitor};
use rustc_hir::{self as hir, GenericBound, Item, ItemKind, Lifetime, LifetimeName, Node, TyKind};
use rustc_middle::ty::{self, AssocItemContainer, RegionKind, Ty, TypeFoldable, TypeVisitor};
use rustc_span::Span;

impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
/// Print the error message for lifetime errors when the return type is a static impl Trait.
Expand All @@ -27,6 +27,39 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
) if **sub_r == RegionKind::ReStatic => {
(var_origin, sub_origin, sub_r, sup_origin, sup_r)
}
RegionResolutionError::ConcreteFailure(
SubregionOrigin::Subtype(box TypeTrace { cause, .. }),
sub_r,
sup_r,
) if **sub_r == RegionKind::ReStatic => {
// This is for the implicit `'static` requirement coming from `impl dyn Trait {}`.
if let ObligationCauseCode::UnifyReceiver(assoc) = &cause.code {
let param = self.find_param_with_region(sup_r, sub_r)?;
let lifetime = if sup_r.has_name() {
format!("lifetime `{}`", sup_r)
} else {
"an anonymous lifetime `'_`".to_string()
};
let mut err = struct_span_err!(
tcx.sess,
cause.span,
E0759,
"cannot infer an appropriate lifetime"
);
err.span_label(param.param_ty_span, &format!("this data with {}...", lifetime));
err.span_label(
cause.span,
"...is captured and required to live as long as `'static` here",
);
if self.find_impl_on_dyn_trait(&mut err, param.param_ty, &assoc.container) {
err.emit();
return Some(ErrorReported);
} else {
err.cancel();
}
}
return None;
}
_ => return None,
};
debug!(
Expand Down Expand Up @@ -96,7 +129,11 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
);
}

self.find_impl_on_dyn_trait(&mut err, param.param_ty);
if let SubregionOrigin::Subtype(box TypeTrace { cause, .. }) = &sup_origin {
if let ObligationCauseCode::UnifyReceiver(assoc) = &cause.code {
self.find_impl_on_dyn_trait(&mut err, param.param_ty, &assoc.container);
}
}

let fn_returns = tcx.return_type_impl_or_dyn_traits(anon_reg_sup.def_id);
debug!("try_report_static_impl_trait: fn_return={:?}", fn_returns);
Expand Down Expand Up @@ -222,63 +259,86 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {

/// When we call a method coming from an `impl Foo for dyn Bar`, `dyn Bar` introduces a default
/// `'static` obligation. Find `impl` blocks that are implemented
fn find_impl_on_dyn_trait(&self, err: &mut DiagnosticBuilder<'_>, ty: Ty<'_>) -> bool {
fn find_impl_on_dyn_trait(
&self,
err: &mut DiagnosticBuilder<'_>,
ty: Ty<'_>,
container: &AssocItemContainer,
) -> bool {
let tcx = self.tcx();
let mut suggested = false;

// Find the trait object types in the argument.
let mut v = TraitObjectVisitor(vec![]);
v.visit_ty(ty);
debug!("TraitObjectVisitor {:?}", v.0);

// Find all the `impl`s in the local scope that can be called on the type parameter.
// FIXME: this doesn't find `impl dyn Trait { /**/ }`.
let container_id = match container {
// When the obligation comes from an `impl Foo for dyn Bar {}`, we
// have the `DefId` of the `trait` itself, not the relevant `impl`
// block. Because of this, we have to look at all the `trait`s
// available, and filter out all that are not of `Foo` (this `def_id`)
// and not of `Bar` (the `filter_map` later in this method).
AssocItemContainer::TraitContainer(def_id) => def_id,

// When the obligation comes from an `impl dyn Trait {}`, we already
// have the `DefId` of the relevant `Item`, so we use it directly.
AssocItemContainer::ImplContainer(def_id) => {
if let Some(Node::Item(Item { kind: ItemKind::Impl { self_ty, .. }, .. })) =
tcx.hir().get_if_local(*def_id)
{
for found_did in &v.0 {
let mut hir_v = HirTraitObjectVisitor(vec![], *found_did);
hir_v.visit_ty(self_ty);
if let [span] = &hir_v.0[..] {
err.span_suggestion_verbose(
span.shrink_to_hi(),
"this `impl` introduces an implicit `'static` requirement, \
consider changing it",
" + '_".to_string(),
Applicability::MaybeIncorrect,
);
suggested = true;
}
}
}
return suggested;
}
};

// Find all the `impl`s in the local scope that can be called on the type parameter. And
// retain all that are `impl`s of the trait that originated the `'static` obligation.
// This doesn't find `impl dyn Trait { /**/ }`, but that case is handled above.
let impl_self_tys = tcx
.all_traits(LOCAL_CRATE)
.iter()
.flat_map(|trait_did| tcx.hir().trait_impls(*trait_did))
.filter_map(|impl_node| {
let impl_did = tcx.hir().local_def_id(*impl_node);
if let Some(Node::Item(Item { kind: ItemKind::Impl { self_ty, .. }, .. })) =
tcx.hir().get_if_local(impl_did.to_def_id())
{
Some(self_ty)
} else {
None
match tcx.hir().get_if_local(impl_did.to_def_id()) {
Some(Node::Item(Item {
kind: ItemKind::Impl { self_ty, of_trait: Some(of_trait), .. },
..
})) if of_trait.trait_def_id() == Some(*container_id) => Some(self_ty),
_ => None,
}
});
let mut suggested = false;

// Given all the `impl`s of the relevant `trait`, look for those that are implemented for
// the trait object in the `fn` parameter type.
for self_ty in impl_self_tys {
if let TyKind::TraitObject(
poly_trait_refs,
Lifetime { name: LifetimeName::ImplicitObjectLifetimeDefault, .. },
) = self_ty.kind
{
for p in poly_trait_refs {
if let PolyTraitRef {
trait_ref:
TraitRef { path: Path { res: Res::Def(DefKind::Trait, did), .. }, .. },
..
} = p
{
for found_did in &v.0 {
if did == found_did {
// We've found an `impl Foo for dyn Bar {}`.
// FIXME: we should change this so it also works for
// `impl Foo for Box<dyn Bar> {}`.
err.span_suggestion_verbose(
self_ty.span.shrink_to_hi(),
"this `impl` introduces an implicit `'static` requirement, \
consider changing it",
" + '_".to_string(),
Applicability::MaybeIncorrect,
);
suggested = true;
}
}
}
for found_did in &v.0 {
let mut hir_v = HirTraitObjectVisitor(vec![], *found_did);
hir_v.visit_ty(self_ty);
if let [span] = &hir_v.0[..] {
err.span_suggestion_verbose(
span.shrink_to_hi(),
"this `impl` introduces an implicit `'static` requirement, \
consider changing it",
" + '_".to_string(),
Applicability::MaybeIncorrect,
);
suggested = true;
}
err.emit();
return Some(ErrorReported);
}
}
suggested
Expand All @@ -301,3 +361,31 @@ impl TypeVisitor<'_> for TraitObjectVisitor {
}
}
}

/// Collect all `hir::Ty<'_>` `Span`s for trait objects with an implicit lifetime.
struct HirTraitObjectVisitor(Vec<Span>, DefId);

impl<'tcx> Visitor<'tcx> for HirTraitObjectVisitor {
type Map = ErasedMap<'tcx>;

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

fn visit_ty(&mut self, t: &'tcx hir::Ty<'tcx>) {
match t.kind {
TyKind::TraitObject(
poly_trait_refs,
Lifetime { name: LifetimeName::ImplicitObjectLifetimeDefault, .. },
) => {
for ptr in poly_trait_refs {
if Some(self.1) == ptr.trait_ref.trait_def_id() {
self.0.push(ptr.span);
}
}
}
_ => {}
}
walk_ty(self, t);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,20 +7,18 @@ use rustc_hir::def_id::LocalDefId;
use rustc_middle::ty::{self, DefIdTree, Region, Ty};
use rustc_span::Span;

// The struct contains the information about the anonymous region
// we are searching for.
/// Information about the anonymous region we are searching for.
#[derive(Debug)]
pub(super) struct AnonymousParamInfo<'tcx> {
// the parameter corresponding to the anonymous region
/// The parameter corresponding to the anonymous region.
pub param: &'tcx hir::Param<'tcx>,
// the type corresponding to the anonymopus region parameter
/// The type corresponding to the anonymous region parameter.
pub param_ty: Ty<'tcx>,
// the ty::BoundRegion corresponding to the anonymous region
/// The ty::BoundRegion corresponding to the anonymous region.
pub bound_region: ty::BoundRegion,
// param_ty_span contains span of parameter type
/// The `Span` of the parameter type.
pub param_ty_span: Span,
// corresponds to id the argument is the first parameter
// in the declaration
/// Signals that the argument is the first parameter in the declaration.
pub is_first: bool,
}

Expand Down
18 changes: 9 additions & 9 deletions src/librustc_infer/infer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -463,7 +463,7 @@ pub enum RegionVariableOrigin {
AddrOfRegion(Span),

/// Regions created as part of an autoref of a method receiver
Autoref(Span),
Autoref(Span, ty::AssocItem),

/// Regions created as part of an automatic coercion
Coercion(Span),
Expand Down Expand Up @@ -1798,15 +1798,15 @@ impl<'tcx> SubregionOrigin<'tcx> {
impl RegionVariableOrigin {
pub fn span(&self) -> Span {
match *self {
MiscVariable(a) => a,
PatternRegion(a) => a,
AddrOfRegion(a) => a,
Autoref(a) => a,
Coercion(a) => a,
EarlyBoundRegion(a, ..) => a,
LateBoundRegion(a, ..) => a,
MiscVariable(a)
| PatternRegion(a)
| AddrOfRegion(a)
| Autoref(a, _)
| Coercion(a)
| EarlyBoundRegion(a, ..)
| LateBoundRegion(a, ..)
| UpvarRegion(_, a) => a,
BoundRegionInCoherence(_) => rustc_span::DUMMY_SP,
UpvarRegion(_, a) => a,
NLL(..) => bug!("NLL variable used with `span`"),
}
}
Expand Down
1 change: 1 addition & 0 deletions src/librustc_infer/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
//! This API is completely unstable and subject to change.

#![doc(html_root_url = "https://doc.rust-lang.org/nightly/")]
#![feature(bindings_after_at)]
#![feature(bool_to_option)]
#![feature(box_patterns)]
#![feature(box_syntax)]
Expand Down
2 changes: 2 additions & 0 deletions src/librustc_middle/traits/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,8 @@ pub enum ObligationCauseCode<'tcx> {
/// Method receiver
MethodReceiver,

UnifyReceiver(Rc<ty::AssocItem>),

/// `return` with no expression
ReturnNoExpression,

Expand Down
1 change: 1 addition & 0 deletions src/librustc_middle/traits/structural_impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,7 @@ impl<'a, 'tcx> Lift<'tcx> for traits::ObligationCauseCode<'a> {
super::StartFunctionType => Some(super::StartFunctionType),
super::IntrinsicType => Some(super::IntrinsicType),
super::MethodReceiver => Some(super::MethodReceiver),
super::UnifyReceiver(ref assoc) => Some(super::UnifyReceiver(assoc.clone())),
super::BlockTailExpression(id) => Some(super::BlockTailExpression(id)),
super::TrivialBound => Some(super::TrivialBound),
}
Expand Down
Loading

0 comments on commit bdb39ab

Please sign in to comment.