[PERF] privacy: Visit DefIds once in DefIdVisitorSkeleton

compiler/rustc_driver_impl/src/lib.rs

@@ -276,10 +276,13 @@ pub fn run_compiler(at_args: &[String], callbacks: &mut (dyn Callbacks + Send))
 
     let registered_lints = config.register_lints.is_some();
 
+    let mut crate_name = None;
     interface::run_compiler(config, |compiler| {
         let sess = &compiler.sess;
         let codegen_backend = &*compiler.codegen_backend;
 
+        crate_name = sess.opts.crate_name.clone();
+
         // This is used for early exits unrelated to errors. E.g. when just
         // printing some information without compiling, or exiting immediately
         // after parsing, etc.
@@ -392,7 +395,9 @@ pub fn run_compiler(at_args: &[String], callbacks: &mut (dyn Callbacks + Send))
         if let Some(linker) = linker {
             linker.link(sess, codegen_backend);
         }
-    })
+    });
+
+    eprintln!("{crate_name:?}: {:?}", rustc_privacy::brrr());
 }
 
 fn dump_feature_usage_metrics(tcxt: TyCtxt<'_>, metrics_dir: &Path) {

compiler/rustc_privacy/src/lib.rs

@@ -12,6 +12,8 @@ mod errors;
 use std::fmt;
 use std::marker::PhantomData;
 use std::ops::ControlFlow;
+use std::sync::{Mutex, MutexGuard, OnceLock};
+use std::time::{Duration, Instant};
 
 use errors::{
     FieldIsPrivate, FieldIsPrivateLabel, FromPrivateDependencyInPublicInterface, InPublicInterface,
@@ -20,7 +22,7 @@ use errors::{
 };
 use rustc_ast::MacroDef;
 use rustc_ast::visit::{VisitorResult, try_visit};
-use rustc_data_structures::fx::FxHashSet;
+use rustc_data_structures::fx::{FxHashMap, FxHashSet};
 use rustc_data_structures::intern::Interned;
 use rustc_errors::{MultiSpan, listify};
 use rustc_hir as hir;
@@ -75,15 +77,18 @@ pub trait DefIdVisitor<'tcx> {
     }
 
     fn tcx(&self) -> TyCtxt<'tcx>;
+    /// NOTE: Def-id visiting should be idempotent, because `DefIdVisitorSkeleton` will avoid
+    /// visiting duplicate def-ids. All the current visitors follow this rule.
     fn visit_def_id(&mut self, def_id: DefId, kind: &str, descr: &dyn fmt::Display)
     -> Self::Result;
 
     /// Not overridden, but used to actually visit types and traits.
     fn skeleton(&mut self) -> DefIdVisitorSkeleton<'_, 'tcx, Self> {
         DefIdVisitorSkeleton {
             def_id_visitor: self,
-            visited_opaque_tys: Default::default(),
+            visited_tys: Default::default(),
             dummy: Default::default(),
+            // brrr: Default::default(),
         }
     }
     fn visit(&mut self, ty_fragment: impl TypeVisitable<TyCtxt<'tcx>>) -> Self::Result {
@@ -100,9 +105,85 @@ pub trait DefIdVisitor<'tcx> {
     }
 }
 
+#[derive(PartialEq, Eq, Hash, Debug)]
+pub enum Ty2 {
+    Bool,
+    Char,
+    Int,
+    Uint,
+    Float,
+    Adt,
+    Foreign,
+    Str,
+    Array,
+    Pat,
+    Slice,
+    RawPtr,
+    Ref,
+    FnDef,
+    FnPtr,
+    UnsafeBinder,
+    Dynamic,
+    Closure,
+    CoroutineClosure,
+    Coroutine,
+    CoroutineWitness,
+    Never,
+    Tuple,
+    Alias,
+    Param,
+    Bound,
+    Placeholder,
+    Infer,
+    Error,
+}
+
+use Ty2::*;
+
+impl Ty2 {
+    fn from_ty(ty: Ty<'_>) -> Ty2 {
+        match ty.kind() {
+            ty::Bool => Bool,
+            ty::Char => Char,
+            ty::Int(..) => Int,
+            ty::Uint(..) => Uint,
+            ty::Float(..) => Float,
+            ty::Adt(..) => Adt,
+            ty::Foreign(..) => Foreign,
+            ty::Str => Str,
+            ty::Array(..) => Array,
+            ty::Pat(..) => Pat,
+            ty::Slice(_) => Slice,
+            ty::RawPtr(..) => RawPtr,
+            ty::Ref(..) => Ref,
+            ty::FnDef(..) => FnDef,
+            ty::FnPtr(..) => FnPtr,
+            ty::UnsafeBinder(..) => UnsafeBinder,
+            ty::Dynamic(..) => Dynamic,
+            ty::Closure(..) => Closure,
+            ty::CoroutineClosure(..) => CoroutineClosure,
+            ty::Coroutine(..) => Coroutine,
+            ty::CoroutineWitness(..) => CoroutineWitness,
+            ty::Never => Never,
+            ty::Tuple(..) => Tuple,
+            ty::Alias(..) => Alias,
+            ty::Param(..) => Param,
+            ty::Bound(..) => Bound,
+            ty::Placeholder(..) => Placeholder,
+            ty::Infer(..) => Infer,
+            ty::Error(..) => Error,
+        }
+    }
+}
+
+pub fn brrr() -> MutexGuard<'static, FxHashMap<Ty2, Duration>> {
+    static MAP: OnceLock<Mutex<FxHashMap<Ty2, Duration>>> = OnceLock::new();
+    MAP.get_or_init(Default::default).lock().unwrap()
+}
+
 pub struct DefIdVisitorSkeleton<'v, 'tcx, V: ?Sized> {
     def_id_visitor: &'v mut V,
-    visited_opaque_tys: FxHashSet<DefId>,
+    visited_tys: FxHashSet<Ty<'tcx>>,
     dummy: PhantomData<TyCtxt<'tcx>>,
 }
 
@@ -169,6 +250,14 @@ where
     }
 }
 
+// impl<V: ?Sized> Drop for DefIdVisitorSkeleton<'_, '_, V> {
+//     fn drop(&mut self) {
+//         if !brrr().is_empty() {
+//             dbg!(brrr());
+//         }
+//     }
+// }
+
 impl<'tcx, V> TypeVisitor<TyCtxt<'tcx>> for DefIdVisitorSkeleton<'_, 'tcx, V>
 where
     V: DefIdVisitor<'tcx> + ?Sized,
@@ -180,6 +269,8 @@ where
     }
 
     fn visit_ty(&mut self, ty: Ty<'tcx>) -> Self::Result {
+        let new = self.visited_tys.insert(ty);
+        let instant = Instant::now();
         let tcx = self.def_id_visitor.tcx();
         // GenericArgs are not visited here because they are visited below
         // in `super_visit_with`.
@@ -258,8 +349,7 @@ where
                 }
             }
             ty::Alias(ty::Opaque, ty::AliasTy { def_id, .. }) => {
-                // Skip repeated `Opaque`s to avoid infinite recursion.
-                if self.visited_opaque_tys.insert(def_id) {
+                if new {
                     // The intent is to treat `impl Trait1 + Trait2` identically to
                     // `dyn Trait1 + Trait2`. Therefore we ignore def-id of the opaque type itself
                     // (it either has no visibility, or its visibility is insignificant, like
@@ -270,33 +360,42 @@ where
                     try_visit!(self.visit_clauses(tcx.explicit_item_bounds(def_id).skip_binder()));
                 }
             }
-            // These types don't have their own def-ids (but may have subcomponents
-            // with def-ids that should be visited recursively).
+            // These types have neither their own def-ids nor subcomponents.
             ty::Bool
             | ty::Char
             | ty::Int(..)
             | ty::Uint(..)
             | ty::Float(..)
             | ty::Str
             | ty::Never
-            | ty::Array(..)
+            | ty::Bound(..)
+            | ty::Param(..) => return V::Result::output(),
+
+            // These types don't have their own def-ids (but may have subcomponents
+            // with def-ids that should be visited recursively).
+            ty::Array(..)
             | ty::Slice(..)
             | ty::Tuple(..)
             | ty::RawPtr(..)
             | ty::Ref(..)
             | ty::Pat(..)
             | ty::FnPtr(..)
             | ty::UnsafeBinder(_)
-            | ty::Param(..)
-            | ty::Bound(..)
             | ty::Error(_)
             | ty::CoroutineWitness(..) => {}
             ty::Placeholder(..) | ty::Infer(..) => {
                 bug!("unexpected type: {:?}", ty)
             }
         }
 
-        if V::SHALLOW { V::Result::output() } else { ty.super_visit_with(self) }
+        let result = if V::SHALLOW { V::Result::output() } else { ty.super_visit_with(self) };
+
+        if !new {
+            let duration = instant.elapsed();
+            *brrr().entry(Ty2::from_ty(ty)).or_default() += duration;
+        }
+
+        result
     }
 
     fn visit_const(&mut self, c: Const<'tcx>) -> Self::Result {
@@ -923,7 +1022,7 @@ impl<'tcx> NamePrivacyVisitor<'tcx> {
 
     // Checks that a field in a struct constructor (expression or pattern) is accessible.
     fn check_field(
-        &mut self,
+        &self,
         hir_id: hir::HirId,    // ID of the field use
         use_ctxt: Span,        // syntax context of the field name at the use site
         def: ty::AdtDef<'tcx>, // definition of the struct or enum
@@ -941,7 +1040,7 @@ impl<'tcx> NamePrivacyVisitor<'tcx> {
 
     // Checks that a field in a struct constructor (expression or pattern) is accessible.
     fn emit_unreachable_field_error(
-        &mut self,
+        &self,
         fields: Vec<(Symbol, Span, bool /* field is present */)>,
         def: ty::AdtDef<'tcx>, // definition of the struct or enum
         update_syntax: Option<Span>,
@@ -1004,7 +1103,7 @@ impl<'tcx> NamePrivacyVisitor<'tcx> {
     }
 
     fn check_expanded_fields(
-        &mut self,
+        &self,
         adt: ty::AdtDef<'tcx>,
         variant: &'tcx ty::VariantDef,
         fields: &[hir::ExprField<'tcx>],
@@ -1142,7 +1241,7 @@ impl<'tcx> TypePrivacyVisitor<'tcx> {
         result.is_break()
     }
 
-    fn check_def_id(&mut self, def_id: DefId, kind: &str, descr: &dyn fmt::Display) -> bool {
+    fn check_def_id(&self, def_id: DefId, kind: &str, descr: &dyn fmt::Display) -> bool {
         let is_error = !self.item_is_accessible(def_id);
         if is_error {
             self.tcx.dcx().emit_err(ItemIsPrivate { span: self.span, kind, descr: descr.into() });
@@ -1401,7 +1500,7 @@ impl SearchInterfaceForPrivateItemsVisitor<'_> {
         self
     }
 
-    fn check_def_id(&mut self, def_id: DefId, kind: &str, descr: &dyn fmt::Display) -> bool {
+    fn check_def_id(&self, def_id: DefId, kind: &str, descr: &dyn fmt::Display) -> bool {
         if self.leaks_private_dep(def_id) {
             self.tcx.emit_node_span_lint(
                 lint::builtin::EXPORTED_PRIVATE_DEPENDENCIES,

tests/ui/const-generics/generic_const_exprs/eval-privacy.rs

@@ -16,7 +16,6 @@ where
 {
     type AssocTy = Const<{ my_const_fn(U) }>;
     //~^ ERROR private type
-    //~| ERROR private type
     fn assoc_fn() -> Self::AssocTy {
         Const
     }

tests/ui/const-generics/generic_const_exprs/eval-privacy.stderr

@@ -7,17 +7,6 @@ LL |     type AssocTy = Const<{ my_const_fn(U) }>;
 LL | const fn my_const_fn(val: u8) -> u8 {
    | ----------------------------------- `fn(u8) -> u8 {my_const_fn}` declared as private
 
-error[E0446]: private type `fn(u8) -> u8 {my_const_fn}` in public interface
-  --> $DIR/eval-privacy.rs:17:5
-   |
-LL |     type AssocTy = Const<{ my_const_fn(U) }>;
-   |     ^^^^^^^^^^^^ can't leak private type
-...
-LL | const fn my_const_fn(val: u8) -> u8 {
-   | ----------------------------------- `fn(u8) -> u8 {my_const_fn}` declared as private
-   |
-   = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
-
-error: aborting due to 2 previous errors
+error: aborting due to 1 previous error
 
 For more information about this error, try `rustc --explain E0446`.

tests/ui/privacy/pub-priv-dep/pub-priv1.rs

@@ -145,7 +145,6 @@ impl From<PublicWithStdImpl> for OtherType {
     //~| ERROR type `OtherType` from private dependency 'priv_dep' in public interface
     fn from(val: PublicWithStdImpl) -> Self { Self }
     //~^ ERROR type `OtherType` from private dependency 'priv_dep' in public interface
-    //~| ERROR type `OtherType` from private dependency 'priv_dep' in public interface
 }
 
 pub struct AllowedPrivType {

tests/ui/privacy/pub-priv-dep/pub-priv1.stderr

@@ -11,55 +11,55 @@ LL | #![deny(exported_private_dependencies)]
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 error: macro `m` from private dependency 'priv_dep' is re-exported
-  --> $DIR/pub-priv1.rs:156:9
+  --> $DIR/pub-priv1.rs:155:9
    |
 LL | pub use priv_dep::m;
    |         ^^^^^^^^^^^
 
 error: macro `fn_like` from private dependency 'pm' is re-exported
-  --> $DIR/pub-priv1.rs:158:9
+  --> $DIR/pub-priv1.rs:157:9
    |
 LL | pub use pm::fn_like;
    |         ^^^^^^^^^^^
 
 error: derive macro `PmDerive` from private dependency 'pm' is re-exported
-  --> $DIR/pub-priv1.rs:160:9
+  --> $DIR/pub-priv1.rs:159:9
    |
 LL | pub use pm::PmDerive;
    |         ^^^^^^^^^^^^
 
 error: attribute macro `pm_attr` from private dependency 'pm' is re-exported
-  --> $DIR/pub-priv1.rs:162:9
+  --> $DIR/pub-priv1.rs:161:9
    |
 LL | pub use pm::pm_attr;
    |         ^^^^^^^^^^^
 
 error: variant `V1` from private dependency 'priv_dep' is re-exported
-  --> $DIR/pub-priv1.rs:165:9
+  --> $DIR/pub-priv1.rs:164:9
    |
 LL | pub use priv_dep::E::V1;
    |         ^^^^^^^^^^^^^^^
 
 error: type alias `Unit` from private dependency 'priv_dep' is re-exported
-  --> $DIR/pub-priv1.rs:168:9
+  --> $DIR/pub-priv1.rs:167:9
    |
 LL | pub use priv_dep::Unit;
    |         ^^^^^^^^^^^^^^
 
 error: type alias `PubPub` from private dependency 'priv_dep' is re-exported
-  --> $DIR/pub-priv1.rs:170:9
+  --> $DIR/pub-priv1.rs:169:9
    |
 LL | pub use priv_dep::PubPub;
    |         ^^^^^^^^^^^^^^^^
 
 error: type alias `PubPriv` from private dependency 'priv_dep' is re-exported
-  --> $DIR/pub-priv1.rs:172:9
+  --> $DIR/pub-priv1.rs:171:9
    |
 LL | pub use priv_dep::PubPriv;
    |         ^^^^^^^^^^^^^^^^^
 
 error: struct `Renamed` from private dependency 'priv_dep' is re-exported
-  --> $DIR/pub-priv1.rs:174:9
+  --> $DIR/pub-priv1.rs:173:9
    |
 LL | pub use priv_dep::OtherType as Renamed;
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -248,13 +248,5 @@ error: type `OtherType` from private dependency 'priv_dep' in public interface
 LL |     fn from(val: PublicWithStdImpl) -> Self { Self }
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
-error: type `OtherType` from private dependency 'priv_dep' in public interface
-  --> $DIR/pub-priv1.rs:146:5
-   |
-LL |     fn from(val: PublicWithStdImpl) -> Self { Self }
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-   |
-   = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
-
-error: aborting due to 41 previous errors
+error: aborting due to 40 previous errors