Improve error message for opaque captures

Author: compiler-errors

compiler/rustc_error_codes/src/error_codes/E0657.md

@@ -1,57 +1,26 @@
-A lifetime bound on a trait implementation was captured at an incorrect place.
+An `impl Trait` captured a higher-ranked lifetime, which is not supported.
+
+Currently, `impl Trait` typess are only allowed to capture lifetimes from
+their parent items, and not from any `for<'a>` binders in scope.
 
 Erroneous code example:
 
 ```compile_fail,E0657
-trait Id<T> {}
-trait Lt<'a> {}
-
-impl<'a> Lt<'a> for () {}
-impl<T> Id<T> for T {}
-
-fn free_fn_capture_hrtb_in_impl_trait()
-    -> Box<for<'a> Id<impl Lt<'a>>> // error!
-{
-    Box::new(())
-}
+trait BorrowInto<'a> {
+    type Target;
 
-struct Foo;
-impl Foo {
-    fn impl_fn_capture_hrtb_in_impl_trait()
-        -> Box<for<'a> Id<impl Lt<'a>>> // error!
-    {
-        Box::new(())
-    }
+    fn borrow_into(&'a self) -> Self::Target;
 }
-```
 
-Here, you have used the inappropriate lifetime in the `impl Trait`,
-The `impl Trait` can only capture lifetimes bound at the fn or impl
-level.
+impl<'a> BorrowInto<'a> for () {
+    type Target = &'a ();
 
-To fix this we have to define the lifetime at the function or impl
-level and use that lifetime in the `impl Trait`. For example you can
-define the lifetime at the function:
-
-```
-trait Id<T> {}
-trait Lt<'a> {}
-
-impl<'a> Lt<'a> for () {}
-impl<T> Id<T> for T {}
-
-fn free_fn_capture_hrtb_in_impl_trait<'b>()
-    -> Box<for<'a> Id<impl Lt<'b>>> // ok!
-{
-    Box::new(())
+    fn borrow_into(&'a self) -> Self::Target {
+        self
+    }
 }
 
-struct Foo;
-impl Foo {
-    fn impl_fn_capture_hrtb_in_impl_trait<'b>()
-        -> Box<for<'a> Id<impl Lt<'b>>> // ok!
-    {
-        Box::new(())
-    }
+fn opaque() -> impl for<'a> BorrowInto<'a, Target = impl Sized + 'a> {
+    ()
 }
 ```

compiler/rustc_hir_analysis/messages.ftl

@@ -312,6 +312,10 @@ hir_analysis_only_current_traits_primitive = only traits defined in the current
 
 hir_analysis_only_current_traits_ty = `{$ty}` is not defined in the current crate
 
+hir_analysis_opaque_captures_higher_ranked_lifetime = `impl Trait` cannot capture {$bad_place}
+    .label = `impl Trait` implicitly captures all lifetimes in scope
+    .note = lifetime declared here
+
 hir_analysis_paren_sugar_attribute = the `#[rustc_paren_sugar]` attribute is a temporary means of controlling which traits can use parenthetical notation
     .help = add `#![feature(unboxed_closures)]` to the crate attributes to use it
 

compiler/rustc_hir_analysis/src/collect/resolve_bound_vars.rs

@@ -8,7 +8,6 @@
 
 use rustc_ast::visit::walk_list;
 use rustc_data_structures::fx::{FxHashSet, FxIndexMap, FxIndexSet};
-use rustc_errors::{codes::*, struct_span_code_err};
 use rustc_hir as hir;
 use rustc_hir::def::{DefKind, Res};
 use rustc_hir::def_id::LocalDefId;
@@ -719,34 +718,42 @@ impl<'a, 'tcx> Visitor<'tcx> for BoundVarContext<'a, 'tcx> {
                     // and ban them. Type variables instantiated inside binders aren't
                     // well-supported at the moment, so this doesn't work.
                     // In the future, this should be fixed and this error should be removed.
-                    let def = self.map.defs.get(&lifetime.hir_id).cloned();
-                    let Some(ResolvedArg::LateBound(_, _, def_id)) = def else { continue };
-                    let Some(def_id) = def_id.as_local() else { continue };
-                    let hir_id = self.tcx.local_def_id_to_hir_id(def_id);
-                    // Ensure that the parent of the def is an item, not HRTB
-                    let parent_id = self.tcx.parent_hir_id(hir_id);
-                    if !parent_id.is_owner() {
-                        struct_span_code_err!(
-                            self.tcx.dcx(),
-                            lifetime.ident.span,
-                            E0657,
-                            "`impl Trait` can only capture lifetimes bound at the fn or impl level"
-                        )
-                        .emit();
-                        self.uninsert_lifetime_on_error(lifetime, def.unwrap());
-                    }
-                    if let hir::Node::Item(hir::Item {
-                        kind: hir::ItemKind::OpaqueTy { .. }, ..
-                    }) = self.tcx.hir_node(parent_id)
+                    let def = self.map.defs.get(&lifetime.hir_id).copied();
+                    let Some(ResolvedArg::LateBound(_, _, lifetime_def_id)) = def else { continue };
+                    let Some(lifetime_def_id) = lifetime_def_id.as_local() else { continue };
+                    let lifetime_hir_id = self.tcx.local_def_id_to_hir_id(lifetime_def_id);
+
+                    let bad_place = match self.tcx.hir_node(self.tcx.parent_hir_id(lifetime_hir_id))
                     {
-                        self.tcx.dcx().struct_span_err(
-                            lifetime.ident.span,
-                            "higher kinded lifetime bounds on nested opaque types are not supported yet",
-                        )
-                        .with_span_note(self.tcx.def_span(def_id), "lifetime declared here")
-                        .emit();
-                        self.uninsert_lifetime_on_error(lifetime, def.unwrap());
-                    }
+                        // Opaques do not declare their own lifetimes, so if a lifetime comes from an opaque
+                        // it must be a reified late-bound lifetime from a trait goal.
+                        hir::Node::Item(hir::Item {
+                            kind: hir::ItemKind::OpaqueTy { .. }, ..
+                        }) => "higher-ranked lifetime from outer `impl Trait`",
+                        // Other items are fine.
+                        hir::Node::Item(_) | hir::Node::TraitItem(_) | hir::Node::ImplItem(_) => {
+                            continue;
+                        }
+                        hir::Node::Ty(_) => "higher-ranked lifetime from function pointer",
+                        _ => "higher-ranked lifetime",
+                    };
+
+                    let (span, label) = if lifetime.ident.span == self.tcx.def_span(lifetime_def_id)
+                    {
+                        let opaque_span = self.tcx.def_span(item_id.owner_id);
+                        (opaque_span, Some(opaque_span))
+                    } else {
+                        (lifetime.ident.span, None)
+                    };
+
+                    // Ensure that the parent of the def is an item, not HRTB
+                    self.tcx.dcx().emit_err(errors::OpaqueCapturesHigherRankedLifetime {
+                        span,
+                        label,
+                        decl_span: self.tcx.def_span(lifetime_def_id),
+                        bad_place,
+                    });
+                    self.uninsert_lifetime_on_error(lifetime, def.unwrap());
                 }
             }
             _ => intravisit::walk_ty(self, ty),

compiler/rustc_hir_analysis/src/errors.rs

@@ -1607,3 +1607,15 @@ pub struct UnnamedFieldsReprFieldDefined {
     #[primary_span]
     pub span: Span,
 }
+
+#[derive(Diagnostic)]
+#[diag(hir_analysis_opaque_captures_higher_ranked_lifetime, code = E0657)]
+pub struct OpaqueCapturesHigherRankedLifetime {
+    #[primary_span]
+    pub span: Span,
+    #[label]
+    pub label: Option<Span>,
+    #[note]
+    pub decl_span: Span,
+    pub bad_place: &'static str,
+}