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

Author: bors

compiler/rustc_hir/src/hir.rs

@@ -3743,6 +3743,29 @@ impl<'hir> Node<'hir> {
         }
     }
 
+    /// Get the type for constants, assoc types, type aliases and statics.
+    pub fn ty(self) -> Option<&'hir Ty<'hir>> {
+        match self {
+            Node::Item(it) => match it.kind {
+                ItemKind::TyAlias(ty, _) | ItemKind::Static(ty, _, _) | ItemKind::Const(ty, _) => {
+                    Some(ty)
+                }
+                _ => None,
+            },
+            Node::TraitItem(it) => match it.kind {
+                TraitItemKind::Const(ty, _) => Some(ty),
+                TraitItemKind::Type(_, ty) => ty,
+                _ => None,
+            },
+            Node::ImplItem(it) => match it.kind {
+                ImplItemKind::Const(ty, _) => Some(ty),
+                ImplItemKind::Type(ty) => Some(ty),
+                _ => None,
+            },
+            _ => None,
+        }
+    }
+
     pub fn alias_ty(self) -> Option<&'hir Ty<'hir>> {
         match self {
             Node::Item(Item { kind: ItemKind::TyAlias(ty, ..), .. }) => Some(ty),

compiler/rustc_hir_analysis/src/check/check.rs

@@ -563,8 +563,8 @@ fn check_item_type(tcx: TyCtxt<'_>, id: hir::ItemId) {
             check_union(tcx, id.owner_id.def_id);
         }
         DefKind::OpaqueTy => {
-            let opaque = tcx.hir().expect_item(id.owner_id.def_id).expect_opaque_ty();
-            if let hir::OpaqueTyOrigin::FnReturn(fn_def_id) | hir::OpaqueTyOrigin::AsyncFn(fn_def_id) = opaque.origin
+            let origin = tcx.opaque_type_origin(id.owner_id.def_id);
+            if let hir::OpaqueTyOrigin::FnReturn(fn_def_id) | hir::OpaqueTyOrigin::AsyncFn(fn_def_id) = origin
                 && let hir::Node::TraitItem(trait_item) = tcx.hir().get_by_def_id(fn_def_id)
                 && let (_, hir::TraitFn::Required(..)) = trait_item.expect_fn()
             {

compiler/rustc_hir_analysis/src/check/wfcheck.rs

@@ -1543,8 +1543,8 @@ impl<'tcx> TypeVisitor<TyCtxt<'tcx>> for ImplTraitInTraitFinder<'_, 'tcx> {
         if let ty::Alias(ty::Opaque, unshifted_opaque_ty) = *ty.kind()
             && self.seen.insert(unshifted_opaque_ty.def_id)
             && let Some(opaque_def_id) = unshifted_opaque_ty.def_id.as_local()
-            && let opaque = tcx.hir().expect_item(opaque_def_id).expect_opaque_ty()
-            && let hir::OpaqueTyOrigin::FnReturn(source) | hir::OpaqueTyOrigin::AsyncFn(source) = opaque.origin
+            && let origin = tcx.opaque_type_origin(opaque_def_id)
+            && let hir::OpaqueTyOrigin::FnReturn(source) | hir::OpaqueTyOrigin::AsyncFn(source) = origin
             && source == self.fn_def_id
         {
             let opaque_ty = tcx.fold_regions(unshifted_opaque_ty, |re, _depth| {

compiler/rustc_infer/src/infer/error_reporting/note_and_explain.rs

@@ -260,7 +260,8 @@ impl<T> Trait<T> for X {
                     (ty::Alias(ty::Opaque, alias), _) | (_, ty::Alias(ty::Opaque, alias)) if alias.def_id.is_local() && matches!(tcx.def_kind(body_owner_def_id), DefKind::AssocFn | DefKind::AssocConst) => {
                         if tcx.is_type_alias_impl_trait(alias.def_id) {
                             if !tcx.opaque_types_defined_by(body_owner_def_id.expect_local()).contains(&alias.def_id.expect_local()) {
-                                diag.span_note(tcx.def_span(body_owner_def_id), "\
+                                let sp = tcx.def_ident_span(body_owner_def_id).unwrap_or_else(|| tcx.def_span(body_owner_def_id));
+                                diag.span_note(sp, "\
                                     this item must have the opaque type in its signature \
                                     in order to be able to register hidden types");
                             }

compiler/rustc_infer/src/infer/opaque_types.rs

@@ -378,7 +378,7 @@ impl<'tcx> InferCtxt<'tcx> {
             DefiningAnchor::Bind(bind) => bind,
         };
 
-        let origin = self.opaque_type_origin_unchecked(def_id);
+        let origin = self.tcx.opaque_type_origin(def_id);
         let in_definition_scope = match origin {
             // Async `impl Trait`
             hir::OpaqueTyOrigin::AsyncFn(parent) => parent == parent_def_id,
@@ -395,13 +395,6 @@ impl<'tcx> InferCtxt<'tcx> {
         };
         in_definition_scope.then_some(origin)
     }
-
-    /// Returns the origin of the opaque type `def_id` even if we are not in its
-    /// defining scope.
-    #[instrument(skip(self), level = "trace", ret)]
-    fn opaque_type_origin_unchecked(&self, def_id: LocalDefId) -> OpaqueTyOrigin {
-        self.tcx.hir().expect_item(def_id).expect_opaque_ty().origin
-    }
 }
 
 /// Visitor that requires that (almost) all regions in the type visited outlive

compiler/rustc_metadata/src/rmeta/encoder.rs

@@ -1138,8 +1138,8 @@ fn should_encode_type(tcx: TyCtxt<'_>, def_id: LocalDefId, def_kind: DefKind) ->
         | DefKind::InlineConst => true,
 
         DefKind::OpaqueTy => {
-            let opaque = tcx.hir().expect_item(def_id).expect_opaque_ty();
-            if let hir::OpaqueTyOrigin::FnReturn(fn_def_id) | hir::OpaqueTyOrigin::AsyncFn(fn_def_id) = opaque.origin
+            let origin = tcx.opaque_type_origin(def_id);
+            if let hir::OpaqueTyOrigin::FnReturn(fn_def_id) | hir::OpaqueTyOrigin::AsyncFn(fn_def_id) = origin
                 && let hir::Node::TraitItem(trait_item) = tcx.hir().get_by_def_id(fn_def_id)
                 && let (_, hir::TraitFn::Required(..)) = trait_item.expect_fn()
             {

compiler/rustc_middle/src/ty/context.rs

@@ -1192,6 +1192,12 @@ impl<'tcx> TyCtxt<'tcx> {
     pub fn local_visibility(self, def_id: LocalDefId) -> Visibility {
         self.visibility(def_id).expect_local()
     }
+
+    /// Returns the origin of the opaque type `def_id`.
+    #[instrument(skip(self), level = "trace", ret)]
+    pub fn opaque_type_origin(self, def_id: LocalDefId) -> hir::OpaqueTyOrigin {
+        self.hir().expect_item(def_id).expect_opaque_ty().origin
+    }
 }
 
 /// A trait implemented for all `X<'a>` types that can be safely and

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:?}");
     };

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]

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")
+        }
     }
 }