Increase accuracy of lifetime bound on trait object impl suggestion

Author: estebank

src/librustc_hir/hir.rs

@@ -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,

src/librustc_infer/infer/combine.rs

@@ -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> {
@@ -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,
@@ -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,
@@ -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(

src/librustc_infer/infer/error_reporting/mod.rs

@@ -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))

src/librustc_infer/infer/error_reporting/nice_region_error/static_impl_trait.rs

@@ -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.
@@ -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!(
@@ -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);
@@ -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
@@ -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);
+    }
+}

src/librustc_infer/infer/error_reporting/nice_region_error/util.rs

@@ -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,
 }
 

src/librustc_infer/infer/mod.rs

@@ -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),
@@ -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`"),
         }
     }

src/librustc_infer/lib.rs

@@ -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)]

src/librustc_middle/traits/mod.rs

@@ -299,6 +299,8 @@ pub enum ObligationCauseCode<'tcx> {
     /// Method receiver
     MethodReceiver,
 
+    UnifyReceiver(Rc<ty::AssocItem>),
+
     /// `return` with no expression
     ReturnNoExpression,
 

src/librustc_middle/traits/structural_impls.rs

@@ -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),
         }