Auto merge of #117076 - oli-obk:privacy_visitor_types, r=petrochenkov

Refactor type visitor walking

r? `@petrochenkov`

pulling out the uncontroversial parts of https://github.com/rust-lang/rust/pull/113671

compiler/rustc_hir/src/hir.rs

@@ -3781,6 +3781,7 @@ impl<'hir> Node<'hir> {
                 ItemKind::TyAlias(ty, _)
                 | ItemKind::Static(ty, _, _)
                 | ItemKind::Const(ty, _, _) => Some(ty),
+                ItemKind::Impl(impl_item) => Some(&impl_item.self_ty),
                 _ => None,
             },
             Node::TraitItem(it) => match it.kind {

compiler/rustc_privacy/src/lib.rs

@@ -210,22 +210,7 @@ where
                     }
                 }
             }
-            ty::Alias(ty::Weak, alias) => {
+            ty::Alias(kind @ (ty::Inherent | ty::Weak | ty::Projection), data) => {
-                self.def_id_visitor.visit_def_id(alias.def_id, "type alias", &ty);
-            }
-            ty::Alias(ty::Projection, proj) => {
-                if V::SKIP_ASSOC_TYS {
-                    // Visitors searching for minimal visibility/reachability want to
-                    // conservatively approximate associated types like `<Type as Trait>::Alias`
-                    // as visible/reachable even if both `Type` and `Trait` are private.
-                    // Ideally, associated types should be substituted in the same way as
-                    // free type aliases, but this isn't done yet.
-                    return ControlFlow::Continue(());
-                }
-                // This will also visit args if necessary, so we don't need to recurse.
-                return self.visit_projection_ty(proj);
-            }
-            ty::Alias(ty::Inherent, data) => {
                 if V::SKIP_ASSOC_TYS {
                     // Visitors searching for minimal visibility/reachability want to
                     // conservatively approximate associated types like `Type::Alias`
@@ -235,9 +220,14 @@ where
                     return ControlFlow::Continue(());
                 }
 
+                let kind = match kind {
+                    ty::Inherent | ty::Projection => "associated type",
+                    ty::Weak => "type alias",
+                    ty::Opaque => unreachable!(),
+                };
                 self.def_id_visitor.visit_def_id(
                     data.def_id,
-                    "associated type",
+                    kind,
                     &LazyDefPathStr { def_id: data.def_id, tcx },
                 )?;
 

compiler/rustc_trait_selection/src/traits/project.rs

@@ -898,7 +898,10 @@ impl<'tcx> TypeFolder<TyCtxt<'tcx>> for BoundVarReplacer<'_, 'tcx> {
                 if debruijn.as_usize() + 1
                     > self.current_index.as_usize() + self.universe_indices.len() =>
             {
-                bug!("Bound vars outside of `self.universe_indices`");
+                bug!(
+                    "Bound vars {r:#?} outside of `self.universe_indices`: {:#?}",
+                    self.universe_indices
+                );
             }
             ty::ReLateBound(debruijn, br) if debruijn >= self.current_index => {
                 let universe = self.universe_for(debruijn);
@@ -916,7 +919,10 @@ impl<'tcx> TypeFolder<TyCtxt<'tcx>> for BoundVarReplacer<'_, 'tcx> {
                 if debruijn.as_usize() + 1
                     > self.current_index.as_usize() + self.universe_indices.len() =>
             {
-                bug!("Bound vars outside of `self.universe_indices`");
+                bug!(
+                    "Bound vars {t:#?} outside of `self.universe_indices`: {:#?}",
+                    self.universe_indices
+                );
             }
             ty::Bound(debruijn, bound_ty) if debruijn >= self.current_index => {
                 let universe = self.universe_for(debruijn);
@@ -935,7 +941,10 @@ impl<'tcx> TypeFolder<TyCtxt<'tcx>> for BoundVarReplacer<'_, 'tcx> {
                 if debruijn.as_usize() + 1
                     > self.current_index.as_usize() + self.universe_indices.len() =>
             {
-                bug!("Bound vars outside of `self.universe_indices`");
+                bug!(
+                    "Bound vars {ct:#?} outside of `self.universe_indices`: {:#?}",
+                    self.universe_indices
+                );
             }
             ty::ConstKind::Bound(debruijn, bound_const) if debruijn >= self.current_index => {
                 let universe = self.universe_for(debruijn);

compiler/rustc_ty_utils/src/lib.rs

@@ -9,6 +9,7 @@
 #![cfg_attr(not(bootstrap), feature(rustdoc_internals))]
 #![cfg_attr(not(bootstrap), allow(internal_features))]
 #![feature(assert_matches)]
+#![feature(associated_type_defaults)]
 #![feature(iterator_try_collect)]
 #![feature(let_chains)]
 #![feature(if_let_guard)]
@@ -39,6 +40,7 @@ mod layout_sanity_check;
 mod needs_drop;
 mod opaque_types;
 pub mod representability;
+pub mod sig_types;
 mod structural_match;
 mod ty;
 

compiler/rustc_ty_utils/src/opaque_types.rs

@@ -53,14 +53,10 @@ impl<'tcx> OpaqueTypeCollector<'tcx> {
 
     fn parent(&self) -> Option<LocalDefId> {
         match self.tcx.def_kind(self.item) {
-            DefKind::AnonConst | DefKind::InlineConst | DefKind::Fn | DefKind::TyAlias => None,
             DefKind::AssocFn | DefKind::AssocTy | DefKind::AssocConst => {
                 Some(self.tcx.local_parent(self.item))
             }
-            other => span_bug!(
+            _ => None,
-                self.tcx.def_span(self.item),
-                "unhandled item with opaque types: {other:?}"
-            ),
         }
     }
 
@@ -98,14 +94,6 @@ impl<'tcx> OpaqueTypeCollector<'tcx> {
         hir_id == scope
     }
 
-    fn collect_body_and_predicate_taits(&mut self) {
-        // Look at all where bounds.
-        self.tcx.predicates_of(self.item).instantiate_identity(self.tcx).visit_with(self);
-        // An item is allowed to constrain opaques declared within its own body (but not nested within
-        // nested functions).
-        self.collect_taits_declared_in_body();
-    }
-
     #[instrument(level = "trace", skip(self))]
     fn collect_taits_declared_in_body(&mut self) {
         let body = self.tcx.hir().body(self.tcx.hir().body_owned_by(self.item)).value;
@@ -132,6 +120,13 @@ impl<'tcx> OpaqueTypeCollector<'tcx> {
     }
 }
 
+impl<'tcx> super::sig_types::SpannedTypeVisitor<'tcx> for OpaqueTypeCollector<'tcx> {
+    fn visit(&mut self, span: Span, value: impl TypeVisitable<TyCtxt<'tcx>>) -> ControlFlow<!> {
+        self.visit_spanned(span, value);
+        ControlFlow::Continue(())
+    }
+}
+
 impl<'tcx> TypeVisitor<TyCtxt<'tcx>> for OpaqueTypeCollector<'tcx> {
     #[instrument(skip(self), ret, level = "trace")]
     fn visit_ty(&mut self, t: Ty<'tcx>) -> ControlFlow<!> {
@@ -273,37 +268,20 @@ fn opaque_types_defined_by<'tcx>(tcx: TyCtxt<'tcx>, item: LocalDefId) -> &'tcx [
     let kind = tcx.def_kind(item);
     trace!(?kind);
     let mut collector = OpaqueTypeCollector::new(tcx, item);
+    super::sig_types::walk_types(tcx, item, &mut collector);
     match kind {
-        // Walk over the signature of the function-like to find the opaques.
+        DefKind::AssocFn
-        DefKind::AssocFn | DefKind::Fn => {
+        | DefKind::Fn
-            let ty_sig = tcx.fn_sig(item).instantiate_identity();
+        | DefKind::Static(_)
-            let hir_sig = tcx.hir().get_by_def_id(item).fn_sig().unwrap();
+        | DefKind::Const
-            // Walk over the inputs and outputs manually in order to get good spans for them.
+        | DefKind::AssocConst
-            collector.visit_spanned(hir_sig.decl.output.span(), ty_sig.output());
+        | DefKind::AnonConst => {
-            for (hir, ty) in hir_sig.decl.inputs.iter().zip(ty_sig.inputs().iter()) {
+            collector.collect_taits_declared_in_body();
-                collector.visit_spanned(hir.span, ty.map_bound(|x| *x));
         }
-            collector.collect_body_and_predicate_taits();
+        DefKind::OpaqueTy
-        }
+        | DefKind::TyAlias
-        // Walk over the type of the item to find opaques.
+        | DefKind::AssocTy
-        DefKind::Static(_) | DefKind::Const | DefKind::AssocConst | DefKind::AnonConst => {
+        | DefKind::Mod
-            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).instantiate_identity());
-            collector.collect_body_and_predicate_taits();
-        }
-        // We're also doing this for `AssocTy` for the wf checks in `check_opaque_meets_bounds`
-        DefKind::TyAlias | DefKind::AssocTy => {
-            tcx.type_of(item).instantiate_identity().visit_with(&mut collector);
-        }
-        DefKind::OpaqueTy => {
-            for (pred, span) in tcx.explicit_item_bounds(item).instantiate_identity_iter_copied() {
-                collector.visit_spanned(span, pred);
-            }
-        }
-        DefKind::Mod
         | DefKind::Struct
         | DefKind::Union
         | DefKind::Enum
@@ -322,9 +300,10 @@ fn opaque_types_defined_by<'tcx>(tcx: TyCtxt<'tcx>, item: LocalDefId) -> &'tcx [
         | DefKind::LifetimeParam
         | DefKind::GlobalAsm
         | DefKind::Impl { .. } => {}
-        // Closures and coroutines are type checked with their parent, so there is no difference here.
+        // Closures and coroutines are type checked with their parent, so we need to allow all
+        // opaques from the closure signature *and* from the parent body.
         DefKind::Closure | DefKind::Coroutine | DefKind::InlineConst => {
-            return tcx.opaque_types_defined_by(tcx.local_parent(item));
+            collector.opaques.extend(tcx.opaque_types_defined_by(tcx.local_parent(item)));
         }
     }
     tcx.arena.alloc_from_iter(collector.opaques)

compiler/rustc_ty_utils/src/sig_types.rs

@@ -0,0 +1,129 @@
+//! This module contains helpers for walking all types of
+//! a signature, while preserving spans as much as possible
+
+use std::ops::ControlFlow;
+
+use rustc_hir::{def::DefKind, def_id::LocalDefId};
+use rustc_middle::ty::{self, TyCtxt};
+use rustc_span::Span;
+use rustc_type_ir::visit::TypeVisitable;
+
+pub trait SpannedTypeVisitor<'tcx> {
+    type BreakTy = !;
+    fn visit(
+        &mut self,
+        span: Span,
+        value: impl TypeVisitable<TyCtxt<'tcx>>,
+    ) -> ControlFlow<Self::BreakTy>;
+}
+
+pub fn walk_types<'tcx, V: SpannedTypeVisitor<'tcx>>(
+    tcx: TyCtxt<'tcx>,
+    item: LocalDefId,
+    visitor: &mut V,
+) -> ControlFlow<V::BreakTy> {
+    let kind = tcx.def_kind(item);
+    trace!(?kind);
+    match kind {
+        DefKind::Coroutine => {
+            match tcx.type_of(item).instantiate_identity().kind() {
+                ty::Coroutine(_, args, _) => visitor.visit(tcx.def_span(item), args.as_coroutine().sig())?,
+                _ => bug!(),
+            }
+            for (pred, span) in tcx.predicates_of(item).instantiate_identity(tcx) {
+                visitor.visit(span, pred)?;
+            }
+        }
+        // Walk over the signature of the function-like
+        DefKind::Closure | DefKind::AssocFn | DefKind::Fn => {
+            let ty_sig = match kind {
+                DefKind::Closure => match tcx.type_of(item).instantiate_identity().kind() {
+                    ty::Closure(_, args) => args.as_closure().sig(),
+                    _ => bug!(),
+                },
+                _ => tcx.fn_sig(item).instantiate_identity(),
+            };
+            let hir_sig = tcx.hir().get_by_def_id(item).fn_decl().unwrap();
+            // Walk over the inputs and outputs manually in order to get good spans for them.
+            visitor.visit(hir_sig.output.span(), ty_sig.output());
+            for (hir, ty) in hir_sig.inputs.iter().zip(ty_sig.inputs().iter()) {
+                visitor.visit(hir.span, ty.map_bound(|x| *x))?;
+            }
+            for (pred, span) in tcx.predicates_of(item).instantiate_identity(tcx) {
+                visitor.visit(span, pred)?;
+            }
+        }
+        // Walk over the type behind the alias
+        DefKind::TyAlias {..} | DefKind::AssocTy |
+        // Walk over the type of the item
+        DefKind::Static(_) | DefKind::Const | DefKind::AssocConst | DefKind::AnonConst => {
+            let span = match tcx.hir().get_by_def_id(item).ty() {
+                Some(ty) => ty.span,
+                _ => tcx.def_span(item),
+            };
+            visitor.visit(span,  tcx.type_of(item).instantiate_identity());
+            for (pred, span) in tcx.predicates_of(item).instantiate_identity(tcx) {
+                visitor.visit(span, pred)?;
+            }
+        }
+        DefKind::OpaqueTy => {
+            for (pred, span) in tcx.explicit_item_bounds(item).instantiate_identity_iter_copied() {
+                visitor.visit(span, pred)?;
+            }
+        }
+        // Look at field types
+        DefKind::Struct | DefKind::Union | DefKind::Enum => {
+            let span = tcx.def_ident_span(item).unwrap();
+            visitor.visit(span,  tcx.type_of(item).instantiate_identity());
+            for (pred, span) in tcx.predicates_of(item).instantiate_identity(tcx) {
+                visitor.visit(span, pred)?;
+            }
+        }
+        // Does not have a syntactical signature
+        DefKind::InlineConst => {}
+        DefKind::Impl { of_trait } => {
+            if of_trait {
+                let span = tcx.hir().get_by_def_id(item).expect_item().expect_impl().of_trait.unwrap().path.span;
+                let args = &tcx.impl_trait_ref(item).unwrap().instantiate_identity().args[1..];
+                visitor.visit(span, args)?;
+            }
+            let span = match tcx.hir().get_by_def_id(item).ty() {
+                Some(ty) => ty.span,
+                _ => tcx.def_span(item),
+            };
+            visitor.visit(span, tcx.type_of(item).instantiate_identity());
+            for (pred, span) in tcx.predicates_of(item).instantiate_identity(tcx) {
+                visitor.visit(span, pred)?;
+            }}
+        DefKind::Trait => {
+            for (pred, span) in tcx.predicates_of(item).instantiate_identity(tcx) {
+                visitor.visit(span, pred)?;
+            }
+        }
+        DefKind::TraitAlias => {
+            for (pred, span) in tcx.predicates_of(item).instantiate_identity(tcx) {
+                visitor.visit(span, pred)?;
+            }
+        }
+        | DefKind::Variant
+        | DefKind::ForeignTy
+        | DefKind::TyParam
+        | DefKind::ConstParam
+        | DefKind::Ctor(_, _)
+        | DefKind::Field
+        | DefKind::LifetimeParam => {
+            span_bug!(
+                tcx.def_span(item),
+                "{kind:?} has not seen any uses of `walk_types` yet, ping oli-obk if you'd like any help"
+            )
+        }
+        // These don't have any types.
+        | DefKind::ExternCrate
+        | DefKind::ForeignMod
+        | DefKind::Macro(_)
+        | DefKind::GlobalAsm
+        | DefKind::Mod
+        | DefKind::Use => {}
+    }
+    ControlFlow::Continue(())
+}

tests/ui/privacy/associated-item-privacy-trait.rs

@@ -23,7 +23,7 @@ mod priv_trait {
         let _: <Pub as PrivTr>::AssocTy;
         //~^ ERROR associated type `PrivTr::AssocTy` is private
         pub type InSignatureTy = <Pub as PrivTr>::AssocTy;
-        //~^ ERROR trait `PrivTr` is private
+        //~^ ERROR associated type `PrivTr::AssocTy` is private
         pub trait InSignatureTr: PrivTr {}
         //~^ ERROR trait `PrivTr` is private
         impl PrivTr for u8 {}

tests/ui/privacy/associated-item-privacy-trait.stderr

@@ -53,11 +53,11 @@ LL |     priv_trait::mac!();
    |
    = note: this error originates in the macro `priv_trait::mac` (in Nightly builds, run with -Z macro-backtrace for more info)
 
-error: trait `PrivTr` is private
+error: associated type `PrivTr::AssocTy` is private
   --> $DIR/associated-item-privacy-trait.rs:25:34
    |
 LL |         pub type InSignatureTy = <Pub as PrivTr>::AssocTy;
-   |                                  ^^^^^^^^^^^^^^^^^^^^^^^^ private trait
+   |                                  ^^^^^^^^^^^^^^^^^^^^^^^^ private associated type
 ...
 LL |     priv_trait::mac!();
    |     ------------------ in this macro invocation

tests/ui/privacy/private-in-public.rs

@@ -104,8 +104,8 @@ mod aliases_pub {
 
     // This should be OK, but associated type aliases are not substituted yet
     pub fn f3(arg: <Priv as PrivTr>::Assoc) {}
-    //~^ WARNING trait `aliases_pub::PrivTr` is more private than the item `aliases_pub::f3`
+    //~^ WARNING type `aliases_pub::Priv` is more private than the item `aliases_pub::f3`
-    //~| WARNING type `aliases_pub::Priv` is more private than the item `aliases_pub::f3`
+    //~| WARNING associated type `aliases_pub::PrivTr::Assoc` is more private than the item `aliases_pub::f3`
 
     impl PrivUseAlias {
         pub fn f(arg: Priv) {}
@@ -133,8 +133,8 @@ mod aliases_priv {
     pub fn f1(arg: PrivUseAlias) {} //~ WARNING type `Priv1` is more private than the item `aliases_priv::f1`
     pub fn f2(arg: PrivAlias) {} //~ WARNING type `Priv2` is more private than the item `aliases_priv::f2`
     pub fn f3(arg: <Priv as PrivTr>::Assoc) {}
-    //~^ WARNING trait `aliases_priv::PrivTr` is more private than the item `aliases_priv::f3`
+    //~^ WARNING type `aliases_priv::Priv` is more private than the item `aliases_priv::f3`
-    //~| WARNING type `aliases_priv::Priv` is more private than the item `aliases_priv::f3`
+    //~| WARNING associated type `aliases_priv::PrivTr::Assoc` is more private than the item `aliases_priv::f3`
 }
 
 mod aliases_params {

tests/ui/privacy/private-in-public.stderr

@@ -276,17 +276,17 @@ note: but type `impls::Priv` is only usable at visibility `pub(self)`
 LL |     struct Priv;
    |     ^^^^^^^^^^^
 
-warning: trait `aliases_pub::PrivTr` is more private than the item `aliases_pub::f3`
+warning: associated type `aliases_pub::PrivTr::Assoc` is more private than the item `aliases_pub::f3`
   --> $DIR/private-in-public.rs:106:5
    |
 LL |     pub fn f3(arg: <Priv as PrivTr>::Assoc) {}
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ function `aliases_pub::f3` is reachable at visibility `pub(crate)`
    |
-note: but trait `aliases_pub::PrivTr` is only usable at visibility `pub(self)`
+note: but associated type `aliases_pub::PrivTr::Assoc` is only usable at visibility `pub(self)`
-  --> $DIR/private-in-public.rs:100:5
+  --> $DIR/private-in-public.rs:101:9
    |
-LL |     trait PrivTr {
+LL |         type Assoc = m::Pub3;
-   |     ^^^^^^^^^^^^
+   |         ^^^^^^^^^^
 
 warning: type `aliases_pub::Priv` is more private than the item `aliases_pub::f3`
   --> $DIR/private-in-public.rs:106:5
@@ -324,17 +324,17 @@ note: but type `Priv2` is only usable at visibility `pub(self)`
 LL |     struct Priv2;
    |     ^^^^^^^^^^^^
 
-warning: trait `aliases_priv::PrivTr` is more private than the item `aliases_priv::f3`
+warning: associated type `aliases_priv::PrivTr::Assoc` is more private than the item `aliases_priv::f3`
   --> $DIR/private-in-public.rs:135:5
    |
 LL |     pub fn f3(arg: <Priv as PrivTr>::Assoc) {}
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ function `aliases_priv::f3` is reachable at visibility `pub(crate)`
    |
-note: but trait `aliases_priv::PrivTr` is only usable at visibility `pub(self)`
+note: but associated type `aliases_priv::PrivTr::Assoc` is only usable at visibility `pub(self)`
-  --> $DIR/private-in-public.rs:128:5
+  --> $DIR/private-in-public.rs:129:9
    |
-LL |     trait PrivTr {
+LL |         type Assoc = Priv3;
-   |     ^^^^^^^^^^^^
+   |         ^^^^^^^^^^
 
 warning: type `aliases_priv::Priv` is more private than the item `aliases_priv::f3`
   --> $DIR/private-in-public.rs:135:5