Auto merge of #112891 - oli-obk:impl_trait_in_assoc_tys_cleanup, r=compiler-errors

Various impl trait in assoc tys cleanups

r? `@compiler-errors`

All commits except for the last are pure refactorings. 274dab5bd658c97886a8987340bf50ae57900c39 allows struct fields to participate in deciding whether a function has an opaque in its signature.

best reviewed commit by commit

3 files changed, 85 insertions, 49 deletions

diff --git a/compiler/rustc_ty_utils/src/assoc.rs b/compiler/rustc_ty_utils/src/assoc.rs
index 658ab03c0f4..5b731641e9d 100644
--- a/compiler/rustc_ty_utils/src/assoc.rs
+++ b/compiler/rustc_ty_utils/src/assoc.rs
@@ -259,7 +259,7 @@ fn associated_type_for_impl_trait_in_trait(
     opaque_ty_def_id: LocalDefId,
 ) -> LocalDefId {
     let (hir::OpaqueTyOrigin::FnReturn(fn_def_id) | hir::OpaqueTyOrigin::AsyncFn(fn_def_id)) =
-        tcx.hir().expect_item(opaque_ty_def_id).expect_opaque_ty().origin
+        tcx.opaque_type_origin(opaque_ty_def_id)
     else {
         bug!("expected opaque for {opaque_ty_def_id:?}");
     };
diff --git a/compiler/rustc_ty_utils/src/errors.rs b/compiler/rustc_ty_utils/src/errors.rs
index 553bf40ef3a..947d4bbe86e 100644
--- a/compiler/rustc_ty_utils/src/errors.rs
+++ b/compiler/rustc_ty_utils/src/errors.rs
@@ -113,7 +113,7 @@ pub struct DuplicateArg<'tcx> {
 }
 
 #[derive(Diagnostic)]
-#[diag(ty_utils_impl_trait_not_param)]
+#[diag(ty_utils_impl_trait_not_param, code = "E0792")]
 pub struct NotParam<'tcx> {
     pub arg: GenericArg<'tcx>,
     #[primary_span]
diff --git a/compiler/rustc_ty_utils/src/opaque_types.rs b/compiler/rustc_ty_utils/src/opaque_types.rs
index 34c7b9f4451..29de8bf0e53 100644
--- a/compiler/rustc_ty_utils/src/opaque_types.rs
+++ b/compiler/rustc_ty_utils/src/opaque_types.rs
@@ -1,5 +1,4 @@
 use rustc_data_structures::fx::FxHashSet;
-use rustc_errors::ErrorGuaranteed;
 use rustc_hir::{def::DefKind, def_id::LocalDefId};
 use rustc_middle::query::Providers;
 use rustc_middle::ty::util::{CheckRegions, NotUniqueParam};
@@ -19,21 +18,26 @@ struct OpaqueTypeCollector<'tcx> {
 
     /// Avoid infinite recursion due to recursive declarations.
     seen: FxHashSet<LocalDefId>,
+
+    span: Option<Span>,
 }
 
 impl<'tcx> OpaqueTypeCollector<'tcx> {
-    fn collect(
-        tcx: TyCtxt<'tcx>,
-        item: LocalDefId,
-        val: ty::Binder<'tcx, impl TypeVisitable<TyCtxt<'tcx>>>,
-    ) -> Vec<LocalDefId> {
-        let mut collector = Self { tcx, opaques: Vec::new(), item, seen: Default::default() };
-        val.skip_binder().visit_with(&mut collector);
-        collector.opaques
+    fn new(tcx: TyCtxt<'tcx>, item: LocalDefId) -> Self {
+        Self { tcx, opaques: Vec::new(), item, seen: Default::default(), span: None }
     }
 
     fn span(&self) -> Span {
-        self.tcx.def_span(self.item)
+        self.span.unwrap_or_else(|| {
+            self.tcx.def_ident_span(self.item).unwrap_or_else(|| self.tcx.def_span(self.item))
+        })
+    }
+
+    fn visit_spanned(&mut self, span: Span, value: impl TypeVisitable<TyCtxt<'tcx>>) {
+        let old = self.span;
+        self.span = Some(span);
+        value.visit_with(self);
+        self.span = old;
     }
 
     fn parent_trait_ref(&self) -> Option<ty::TraitRef<'tcx>> {
@@ -60,53 +64,57 @@ impl<'tcx> OpaqueTypeCollector<'tcx> {
 }
 
 impl<'tcx> TypeVisitor<TyCtxt<'tcx>> for OpaqueTypeCollector<'tcx> {
-    type BreakTy = ErrorGuaranteed;
-
     #[instrument(skip(self), ret, level = "trace")]
-    fn visit_ty(&mut self, t: Ty<'tcx>) -> ControlFlow<ErrorGuaranteed> {
+    fn visit_ty(&mut self, t: Ty<'tcx>) -> ControlFlow<!> {
+        t.super_visit_with(self)?;
         match t.kind() {
             ty::Alias(ty::Opaque, alias_ty) if alias_ty.def_id.is_local() => {
                 if !self.seen.insert(alias_ty.def_id.expect_local()) {
                     return ControlFlow::Continue(());
                 }
+
+                self.opaques.push(alias_ty.def_id.expect_local());
+
                 match self.tcx.uses_unique_generic_params(alias_ty.substs, CheckRegions::Bound) {
                     Ok(()) => {
                         // FIXME: implement higher kinded lifetime bounds on nested opaque types. They are not
                         // supported at all, so this is sound to do, but once we want to support them, you'll
                         // start seeing the error below.
 
-                        self.opaques.push(alias_ty.def_id.expect_local());
-
                         // Collect opaque types nested within the associated type bounds of this opaque type.
-                        for (pred, _span) in self
+                        // We use identity substs here, because we already know that the opaque type uses
+                        // only generic parameters, and thus substituting would not give us more information.
+                        for (pred, span) in self
                             .tcx
                             .explicit_item_bounds(alias_ty.def_id)
-                            .subst_iter_copied(self.tcx, alias_ty.substs)
+                            .subst_identity_iter_copied()
                         {
                             trace!(?pred);
-                            pred.visit_with(self)?;
+                            self.visit_spanned(span, pred);
                         }
-
-                        ControlFlow::Continue(())
                     }
                     Err(NotUniqueParam::NotParam(arg)) => {
-                        let err = self.tcx.sess.emit_err(NotParam {
+                        self.tcx.sess.emit_err(NotParam {
                             arg,
                             span: self.span(),
                             opaque_span: self.tcx.def_span(alias_ty.def_id),
                         });
-                        ControlFlow::Break(err)
                     }
                     Err(NotUniqueParam::DuplicateParam(arg)) => {
-                        let err = self.tcx.sess.emit_err(DuplicateArg {
+                        self.tcx.sess.emit_err(DuplicateArg {
                             arg,
                             span: self.span(),
                             opaque_span: self.tcx.def_span(alias_ty.def_id),
                         });
-                        ControlFlow::Break(err)
                     }
                 }
             }
+            ty::Alias(ty::Weak, alias_ty) if alias_ty.def_id.is_local() => {
+                self.tcx
+                    .type_of(alias_ty.def_id)
+                    .subst(self.tcx, alias_ty.substs)
+                    .visit_with(self)?;
+            }
             ty::Alias(ty::Projection, alias_ty) => {
                 // This avoids having to do normalization of `Self::AssocTy` by only
                 // supporting the case of a method defining opaque types from assoc types
@@ -136,26 +144,44 @@ impl<'tcx> TypeVisitor<TyCtxt<'tcx>> for OpaqueTypeCollector<'tcx> {
                                 ty::InternalSubsts::identity_for_item(self.tcx, parent),
                             );
 
-                            if !check_substs_compatible(self.tcx, assoc, impl_substs) {
+                            if check_substs_compatible(self.tcx, assoc, impl_substs) {
+                                return self
+                                    .tcx
+                                    .type_of(assoc.def_id)
+                                    .subst(self.tcx, impl_substs)
+                                    .visit_with(self);
+                            } else {
                                 self.tcx.sess.delay_span_bug(
                                     self.tcx.def_span(assoc.def_id),
                                     "item had incorrect substs",
                                 );
-                                return ControlFlow::Continue(());
                             }
-
-                            return self
-                                .tcx
-                                .type_of(assoc.def_id)
-                                .subst(self.tcx, impl_substs)
-                                .visit_with(self);
                         }
                     }
                 }
-                t.super_visit_with(self)
             }
-            _ => t.super_visit_with(self),
+            ty::Adt(def, _) if def.did().is_local() => {
+                if !self.seen.insert(def.did().expect_local()) {
+                    return ControlFlow::Continue(());
+                }
+                for variant in def.variants().iter() {
+                    for field in variant.fields.iter() {
+                        // Don't use the `ty::Adt` substs, we either
+                        // * found the opaque in the substs
+                        // * will find the opaque in the unsubstituted fields
+                        // The only other situation that can occur is that after substituting,
+                        // some projection resolves to an opaque that we would have otherwise
+                        // not found. While we could substitute and walk those, that would mean we
+                        // would have to walk all substitutions of an Adt, which can quickly
+                        // degenerate into looking at an exponential number of types.
+                        let ty = self.tcx.type_of(field.did).subst_identity();
+                        self.visit_spanned(self.tcx.def_span(field.did), ty);
+                    }
+                }
+            }
+            _ => trace!(kind=?t.kind()),
         }
+        ControlFlow::Continue(())
     }
 }
 
@@ -166,21 +192,29 @@ fn opaque_types_defined_by<'tcx>(tcx: TyCtxt<'tcx>, item: LocalDefId) -> &'tcx [
     match kind {
         // We're also doing this for `AssocTy` for the wf checks in `check_opaque_meets_bounds`
         DefKind::Fn | DefKind::AssocFn | DefKind::AssocTy | DefKind::AssocConst => {
-            let defined_opaques = match kind {
-                DefKind::Fn => {
-                    OpaqueTypeCollector::collect(tcx, item, tcx.fn_sig(item).subst_identity())
+            let mut collector = OpaqueTypeCollector::new(tcx, item);
+            match kind {
+                // Walk over the signature of the function-like to find the opaques.
+                DefKind::AssocFn | DefKind::Fn => {
+                    let ty_sig = tcx.fn_sig(item).subst_identity();
+                    let hir_sig = tcx.hir().get_by_def_id(item).fn_sig().unwrap();
+                    // Walk over the inputs and outputs manually in order to get good spans for them.
+                    collector.visit_spanned(hir_sig.decl.output.span(), ty_sig.output());
+                    for (hir, ty) in hir_sig.decl.inputs.iter().zip(ty_sig.inputs().iter()) {
+                        collector.visit_spanned(hir.span, ty.map_bound(|x| *x));
+                    }
                 }
-                DefKind::AssocFn => {
-                    OpaqueTypeCollector::collect(tcx, item, tcx.fn_sig(item).subst_identity())
+                // Walk over the type of the item to find opaques.
+                DefKind::AssocTy | DefKind::AssocConst => {
+                    let span = match tcx.hir().get_by_def_id(item).ty() {
+                        Some(ty) => ty.span,
+                        _ => tcx.def_span(item),
+                    };
+                    collector.visit_spanned(span, tcx.type_of(item).subst_identity());
                 }
-                DefKind::AssocTy | DefKind::AssocConst => OpaqueTypeCollector::collect(
-                    tcx,
-                    item,
-                    ty::Binder::dummy(tcx.type_of(item).subst_identity()),
-                ),
                 _ => unreachable!(),
-            };
-            tcx.arena.alloc_from_iter(defined_opaques)
+            }
+            tcx.arena.alloc_from_iter(collector.opaques)
         }
         DefKind::Mod
         | DefKind::Struct
@@ -209,7 +243,9 @@ fn opaque_types_defined_by<'tcx>(tcx: TyCtxt<'tcx>, item: LocalDefId) -> &'tcx [
         | DefKind::GlobalAsm
         | DefKind::Impl { .. }
         | DefKind::Closure
-        | DefKind::Generator => &[],
+        | DefKind::Generator => {
+            span_bug!(tcx.def_span(item), "{kind:?} is type checked as part of its parent")
+        }
     }
 }