Better errors when we don't have implicit statics in trait objects

Author: jackh726

compiler/rustc_ast_lowering/src/path.rs

@@ -336,6 +336,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
                             insertion_sp,
                             suggestion,
                         );
+                        err.note("assuming a `'static` lifetime...");
                         err.emit();
                     }
                     AnonymousLifetimeMode::PassThrough | AnonymousLifetimeMode::ReportError => {

compiler/rustc_infer/src/infer/error_reporting/nice_region_error/mismatched_static_lifetime.rs

@@ -6,6 +6,7 @@ use crate::infer::error_reporting::note_and_explain_region;
 use crate::infer::lexical_region_resolve::RegionResolutionError;
 use crate::infer::{SubregionOrigin, TypeTrace};
 use crate::traits::ObligationCauseCode;
+use rustc_data_structures::stable_set::FxHashSet;
 use rustc_errors::{Applicability, ErrorReported};
 use rustc_hir as hir;
 use rustc_hir::intravisit::Visitor;
@@ -42,13 +43,12 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
         multi_span
             .push_span_label(binding_span, "introduces a `'static` lifetime requirement".into());
         err.span_note(multi_span, "because this has an unmet lifetime requirement");
-        note_and_explain_region(self.tcx(), &mut err, "...", sup, "...");
+        note_and_explain_region(self.tcx(), &mut err, "", sup, "...");
         if let Some(impl_node) = self.tcx().hir().get_if_local(*impl_def_id) {
-            let ty = self.tcx().type_of(*impl_def_id);
-            let mut v = super::static_impl_trait::TraitObjectVisitor(vec![]);
-            v.visit_ty(ty);
-            let matching_def_ids = v.0;
+            // If an impl is local, then maybe this isn't what they want. Try to
+            // be as helpful as possible with implicit lifetimes.
 
+            // First, let's get the hir self type of the impl
             let impl_self_ty = match impl_node {
                 hir::Node::Item(hir::Item {
                     kind: hir::ItemKind::Impl(hir::Impl { self_ty, .. }),
@@ -57,26 +57,45 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
                 _ => bug!("Node not an impl."),
             };
 
-            for matching_def_id in matching_def_ids {
+            // Next, let's figure out the set of trait objects with implict static bounds
+            let ty = self.tcx().type_of(*impl_def_id);
+            let mut v = super::static_impl_trait::TraitObjectVisitor(FxHashSet::default());
+            v.visit_ty(ty);
+            let mut traits = vec![];
+            for matching_def_id in v.0 {
                 let mut hir_v =
-                    super::static_impl_trait::HirTraitObjectVisitor(vec![], matching_def_id);
+                    super::static_impl_trait::HirTraitObjectVisitor(&mut traits, matching_def_id);
                 hir_v.visit_ty(&impl_self_ty);
+            }
 
-                let mut multi_span: MultiSpan = hir_v.0.clone().into();
-                for span in &hir_v.0 {
-                    multi_span.push_span_label(
-                        *span,
-                        "this has an implicit `'static` lifetime requirement".to_string(),
-                    );
+            if traits.is_empty() {
+                // If there are no trait object traits to point at, either because
+                // there aren't trait objects or because none are implicit, then just
+                // write a single note on the impl itself.
+
+                let impl_span = self.tcx().def_span(*impl_def_id);
+                err.span_note(impl_span, "...does not necessarily outlive the static lifetime introduced by the compatible `impl`");
+            } else {
+                // Otherwise, point at all implicit static lifetimes
+
+                err.note("...does not necessarily outlive the static lifetime introduced by the compatible `impl`");
+                for span in &traits {
+                    err.span_note(*span, "this has an implicit `'static` lifetime requirement");
+                    // It would be nice to put this immediately under the above note, but they get
+                    // pushed to the end.
                     err.span_suggestion_verbose(
                         span.shrink_to_hi(),
                         "consider relaxing the implicit `'static` requirement",
                         " + '_".to_string(),
                         Applicability::MaybeIncorrect,
                     );
                 }
-                err.span_note(multi_span, "...does not necessarily outlive the static lifetime introduced by the compatible `impl`");
             }
+        } else {
+            // Otherwise just point out the impl.
+
+            let impl_span = self.tcx().def_span(*impl_def_id);
+            err.span_note(impl_span, "...does not necessarily outlive the static lifetime introduced by the compatible `impl`");
         }
         err.emit();
         Some(ErrorReported)

compiler/rustc_infer/src/infer/error_reporting/nice_region_error/static_impl_trait.rs

@@ -4,6 +4,7 @@ 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, UnifyReceiverContext};
+use rustc_data_structures::stable_set::FxHashSet;
 use rustc_errors::{struct_span_err, Applicability, DiagnosticBuilder, ErrorReported};
 use rustc_hir::def_id::DefId;
 use rustc_hir::intravisit::{walk_ty, ErasedMap, NestedVisitorMap, Visitor};
@@ -193,13 +194,12 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
                 // Same case of `impl Foo for dyn Bar { fn qux(&self) {} }` introducing a `'static`
                 // lifetime as above, but called using a fully-qualified path to the method:
                 // `Foo::qux(bar)`.
-                let mut v = TraitObjectVisitor(vec![]);
+                let mut v = TraitObjectVisitor(FxHashSet::default());
                 v.visit_ty(param.param_ty);
                 if let Some((ident, self_ty)) =
-                    self.get_impl_ident_and_self_ty_from_trait(item_def_id, &v.0[..])
+                    self.get_impl_ident_and_self_ty_from_trait(item_def_id, &v.0)
                 {
-                    if self.suggest_constrain_dyn_trait_in_impl(&mut err, &v.0[..], ident, self_ty)
-                    {
+                    if self.suggest_constrain_dyn_trait_in_impl(&mut err, &v.0, ident, self_ty) {
                         override_error_code = Some(ident);
                     }
                 }
@@ -340,7 +340,7 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
     fn get_impl_ident_and_self_ty_from_trait(
         &self,
         def_id: DefId,
-        trait_objects: &[DefId],
+        trait_objects: &FxHashSet<DefId>,
     ) -> Option<(Ident, &'tcx hir::Ty<'tcx>)> {
         let tcx = self.tcx();
         match tcx.hir().get_if_local(def_id) {
@@ -377,9 +377,10 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
                                         // multiple `impl`s for the same trait like
                                         // `impl Foo for Box<dyn Bar>` and `impl Foo for dyn Bar`.
                                         // In that case, only the first one will get suggestions.
-                                        let mut hir_v = HirTraitObjectVisitor(vec![], *did);
+                                        let mut traits = vec![];
+                                        let mut hir_v = HirTraitObjectVisitor(&mut traits, *did);
                                         hir_v.visit_ty(self_ty);
-                                        !hir_v.0.is_empty()
+                                        !traits.is_empty()
                                     }) =>
                                     {
                                         Some(self_ty)
@@ -421,33 +422,34 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
             _ => return false,
         };
 
-        let mut v = TraitObjectVisitor(vec![]);
+        let mut v = TraitObjectVisitor(FxHashSet::default());
         v.visit_ty(ty);
 
         // Get the `Ident` of the method being called and the corresponding `impl` (to point at
         // `Bar` in `impl Foo for dyn Bar {}` and the definition of the method being called).
         let (ident, self_ty) =
-            match self.get_impl_ident_and_self_ty_from_trait(instance.def_id(), &v.0[..]) {
+            match self.get_impl_ident_and_self_ty_from_trait(instance.def_id(), &v.0) {
                 Some((ident, self_ty)) => (ident, self_ty),
                 None => return false,
             };
 
         // Find the trait object types in the argument, so we point at *only* the trait object.
-        self.suggest_constrain_dyn_trait_in_impl(err, &v.0[..], ident, self_ty)
+        self.suggest_constrain_dyn_trait_in_impl(err, &v.0, ident, self_ty)
     }
 
     fn suggest_constrain_dyn_trait_in_impl(
         &self,
         err: &mut DiagnosticBuilder<'_>,
-        found_dids: &[DefId],
+        found_dids: &FxHashSet<DefId>,
         ident: Ident,
         self_ty: &hir::Ty<'_>,
     ) -> bool {
         let mut suggested = false;
         for found_did in found_dids {
-            let mut hir_v = HirTraitObjectVisitor(vec![], *found_did);
+            let mut traits = vec![];
+            let mut hir_v = HirTraitObjectVisitor(&mut traits, *found_did);
             hir_v.visit_ty(&self_ty);
-            for span in &hir_v.0 {
+            for span in &traits {
                 let mut multi_span: MultiSpan = vec![*span].into();
                 multi_span.push_span_label(
                     *span,
@@ -472,14 +474,14 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
 }
 
 /// Collect all the trait objects in a type that could have received an implicit `'static` lifetime.
-pub(super) struct TraitObjectVisitor(pub(super) Vec<DefId>);
+pub(super) struct TraitObjectVisitor(pub(super) FxHashSet<DefId>);
 
 impl TypeVisitor<'_> for TraitObjectVisitor {
     fn visit_ty(&mut self, t: Ty<'_>) -> ControlFlow<Self::BreakTy> {
         match t.kind() {
             ty::Dynamic(preds, RegionKind::ReStatic) => {
                 if let Some(def_id) = preds.principal_def_id() {
-                    self.0.push(def_id);
+                    self.0.insert(def_id);
                 }
                 ControlFlow::CONTINUE
             }
@@ -489,9 +491,9 @@ impl TypeVisitor<'_> for TraitObjectVisitor {
 }
 
 /// Collect all `hir::Ty<'_>` `Span`s for trait objects with an implicit lifetime.
-pub(super) struct HirTraitObjectVisitor(pub(super) Vec<Span>, pub(super) DefId);
+pub(super) struct HirTraitObjectVisitor<'a>(pub(super) &'a mut Vec<Span>, pub(super) DefId);
 
-impl<'tcx> Visitor<'tcx> for HirTraitObjectVisitor {
+impl<'a, 'tcx> Visitor<'tcx> for HirTraitObjectVisitor<'a> {
     type Map = ErasedMap<'tcx>;
 
     fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {

src/test/ui/async-await/async-fn-path-elision.stderr

@@ -3,6 +3,8 @@ error[E0726]: implicit elided lifetime not allowed here
    |
 LL | async fn error(lt: HasLifetime) {
    |                    ^^^^^^^^^^^- help: indicate the anonymous lifetime: `<'_>`
+   |
+   = note: assuming a `'static` lifetime...
 
 error: aborting due to previous error
 

src/test/ui/generic-associated-types/issue-78113-lifetime-mismatch-dyn-trait-box.rs

@@ -13,7 +13,28 @@ pub trait B {
 }
 
 impl B for () {
+    // `'a` doesn't match implicit `'static`: suggest `'_`
     type T<'a> = Box<dyn A + 'a>; //~ incompatible lifetime on type
 }
 
+trait C {}
+impl C for Box<dyn A + 'static> {}
+pub trait D {
+    type T<'a>: C;
+}
+impl D for () {
+    // `'a` doesn't match explicit `'static`: we *should* suggest removing `'static`
+    type T<'a> = Box<dyn A + 'a>; //~ incompatible lifetime on type
+}
+
+trait E {}
+impl E for (Box<dyn A>, Box<dyn A>) {}
+pub trait F {
+    type T<'a>: E;
+}
+impl F for () {
+    // `'a` doesn't match explicit `'static`: suggest `'_`
+    type T<'a> = (Box<dyn A + 'a>, Box<dyn A + 'a>); //~ incompatible lifetime on type
+}
+
 fn main() {}

src/test/ui/generic-associated-types/issue-78113-lifetime-mismatch-dyn-trait-box.stderr

@@ -1,5 +1,5 @@
 error: incompatible lifetime on type
-  --> $DIR/issue-78113-lifetime-mismatch-dyn-trait-box.rs:16:5
+  --> $DIR/issue-78113-lifetime-mismatch-dyn-trait-box.rs:17:5
    |
 LL |     type T<'a> = Box<dyn A + 'a>;
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -9,20 +9,79 @@ note: because this has an unmet lifetime requirement
    |
 LL |     type T<'a>: A;
    |                 ^ introduces a `'static` lifetime requirement
-note: ...the lifetime `'a` as defined on the associated item at 16:12...
-  --> $DIR/issue-78113-lifetime-mismatch-dyn-trait-box.rs:16:12
+note: the lifetime `'a` as defined on the associated item at 17:12...
+  --> $DIR/issue-78113-lifetime-mismatch-dyn-trait-box.rs:17:12
    |
 LL |     type T<'a> = Box<dyn A + 'a>;
    |            ^^
-note: ...does not necessarily outlive the static lifetime introduced by the compatible `impl`
+   = note: ...does not necessarily outlive the static lifetime introduced by the compatible `impl`
+note: this has an implicit `'static` lifetime requirement
   --> $DIR/issue-78113-lifetime-mismatch-dyn-trait-box.rs:9:20
    |
 LL | impl A for Box<dyn A> {}
-   |                    ^ this has an implicit `'static` lifetime requirement
+   |                    ^
 help: consider relaxing the implicit `'static` requirement
    |
 LL | impl A for Box<dyn A + '_> {}
    |                      ^^^^
 
-error: aborting due to previous error
+error: incompatible lifetime on type
+  --> $DIR/issue-78113-lifetime-mismatch-dyn-trait-box.rs:27:5
+   |
+LL |     type T<'a> = Box<dyn A + 'a>;
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+note: because this has an unmet lifetime requirement
+  --> $DIR/issue-78113-lifetime-mismatch-dyn-trait-box.rs:23:17
+   |
+LL |     type T<'a>: C;
+   |                 ^ introduces a `'static` lifetime requirement
+note: the lifetime `'a` as defined on the associated item at 27:12...
+  --> $DIR/issue-78113-lifetime-mismatch-dyn-trait-box.rs:27:12
+   |
+LL |     type T<'a> = Box<dyn A + 'a>;
+   |            ^^
+note: ...does not necessarily outlive the static lifetime introduced by the compatible `impl`
+  --> $DIR/issue-78113-lifetime-mismatch-dyn-trait-box.rs:21:1
+   |
+LL | impl C for Box<dyn A + 'static> {}
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: incompatible lifetime on type
+  --> $DIR/issue-78113-lifetime-mismatch-dyn-trait-box.rs:37:5
+   |
+LL |     type T<'a> = (Box<dyn A + 'a>, Box<dyn A + 'a>);
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+note: because this has an unmet lifetime requirement
+  --> $DIR/issue-78113-lifetime-mismatch-dyn-trait-box.rs:33:17
+   |
+LL |     type T<'a>: E;
+   |                 ^ introduces a `'static` lifetime requirement
+note: the lifetime `'a` as defined on the associated item at 37:12...
+  --> $DIR/issue-78113-lifetime-mismatch-dyn-trait-box.rs:37:12
+   |
+LL |     type T<'a> = (Box<dyn A + 'a>, Box<dyn A + 'a>);
+   |            ^^
+   = note: ...does not necessarily outlive the static lifetime introduced by the compatible `impl`
+note: this has an implicit `'static` lifetime requirement
+  --> $DIR/issue-78113-lifetime-mismatch-dyn-trait-box.rs:31:21
+   |
+LL | impl E for (Box<dyn A>, Box<dyn A>) {}
+   |                     ^
+note: this has an implicit `'static` lifetime requirement
+  --> $DIR/issue-78113-lifetime-mismatch-dyn-trait-box.rs:31:33
+   |
+LL | impl E for (Box<dyn A>, Box<dyn A>) {}
+   |                                 ^
+help: consider relaxing the implicit `'static` requirement
+   |
+LL | impl E for (Box<dyn A + '_>, Box<dyn A>) {}
+   |                       ^^^^
+help: consider relaxing the implicit `'static` requirement
+   |
+LL | impl E for (Box<dyn A>, Box<dyn A + '_>) {}
+   |                                   ^^^^
+
+error: aborting due to 3 previous errors
 

src/test/ui/impl-header-lifetime-elision/path-elided.stderr

@@ -3,6 +3,8 @@ error[E0726]: implicit elided lifetime not allowed here
    |
 LL | impl MyTrait for Foo {
    |                  ^^^- help: indicate the anonymous lifetime: `<'_>`
+   |
+   = note: assuming a `'static` lifetime...
 
 error: aborting due to previous error
 

src/test/ui/impl-header-lifetime-elision/trait-elided.stderr

@@ -3,6 +3,8 @@ error[E0726]: implicit elided lifetime not allowed here
    |
 LL | impl MyTrait for u32 {
    |      ^^^^^^^- help: indicate the anonymous lifetime: `<'_>`
+   |
+   = note: assuming a `'static` lifetime...
 
 error: aborting due to previous error
 

src/test/ui/issues/issue-10412.stderr

@@ -45,6 +45,8 @@ error[E0726]: implicit elided lifetime not allowed here
    |
 LL | impl<'self> Serializable<str> for &'self str {
    |             ^^^^^^^^^^^^^^^^^ help: indicate the anonymous lifetime: `Serializable<'_, str>`
+   |
+   = note: assuming a `'static` lifetime...
 
 error[E0277]: the size for values of type `str` cannot be known at compilation time
   --> $DIR/issue-10412.rs:6:13

src/test/ui/wf/wf-in-foreign-fn-decls-issue-80468.stderr

@@ -3,6 +3,8 @@ error[E0726]: implicit elided lifetime not allowed here
    |
 LL | impl Trait for Ref {}
    |                ^^^- help: indicate the anonymous lifetime: `<'_>`
+   |
+   = note: assuming a `'static` lifetime...
 
 error: incompatible lifetime on type
   --> $DIR/wf-in-foreign-fn-decls-issue-80468.rs:16:21
@@ -15,11 +17,16 @@ note: because this has an unmet lifetime requirement
    |
 LL | pub struct Wrapper<T: Trait>(T);
    |                       ^^^^^ introduces a `'static` lifetime requirement
-note: ...the anonymous lifetime #1 defined on the method body at 16:5...
+note: the anonymous lifetime #1 defined on the method body at 16:5...
   --> $DIR/wf-in-foreign-fn-decls-issue-80468.rs:16:5
    |
 LL |     pub fn repro(_: Wrapper<Ref>);
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+note: ...does not necessarily outlive the static lifetime introduced by the compatible `impl`
+  --> $DIR/wf-in-foreign-fn-decls-issue-80468.rs:13:1
+   |
+LL | impl Trait for Ref {}
+   | ^^^^^^^^^^^^^^^^^^^^^
 
 error: aborting due to 2 previous errors