[PERF] privacy: Visit DefIds once in DefIdVisitorSkeleton

compiler/rustc_privacy/src/lib.rs

@@ -75,14 +75,16 @@ 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(),
         }
     }
@@ -102,7 +104,7 @@ pub trait DefIdVisitor<'tcx> {
 
 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>>,
 }
 
@@ -190,6 +192,9 @@ where
             | ty::Closure(def_id, ..)
             | ty::CoroutineClosure(def_id, ..)
             | ty::Coroutine(def_id, ..) => {
+                if !self.visited_tys.insert(ty) {
+                    return V::Result::output();
+                }
                 try_visit!(self.def_id_visitor.visit_def_id(def_id, "type", &ty));
                 if V::SHALLOW {
                     return V::Result::output();
@@ -212,6 +217,9 @@ where
                 }
             }
             ty::Alias(kind @ (ty::Inherent | ty::Free | ty::Projection), data) => {
+                if !self.visited_tys.insert(ty) {
+                    return V::Result::output();
+                }
                 if self.def_id_visitor.skip_assoc_tys() {
                     // Visitors searching for minimal visibility/reachability want to
                     // conservatively approximate associated types like `Type::Alias`
@@ -243,6 +251,9 @@ where
                 };
             }
             ty::Dynamic(predicates, ..) => {
+                if !self.visited_tys.insert(ty) {
+                    return V::Result::output();
+                }
                 // All traits in the list are considered the "primary" part of the type
                 // and are visited by shallow visitors.
                 for predicate in predicates {
@@ -258,39 +269,45 @@ 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) {
-                    // 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
-                    // visibilities of type aliases) and recurse into bounds instead to go
-                    // through the trait list (default type visitor doesn't visit those traits).
-                    // All traits in the list are considered the "primary" part of the type
-                    // and are visited by shallow visitors.
-                    try_visit!(self.visit_clauses(tcx.explicit_item_bounds(def_id).skip_binder()));
+                if !self.visited_tys.insert(ty) {
+                    return V::Result::output();
                 }
+                // 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
+                // visibilities of type aliases) and recurse into bounds instead to go
+                // through the trait list (default type visitor doesn't visit those traits).
+                // All traits in the list are considered the "primary" part of the type
+                // and are visited by shallow visitors.
+                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::CoroutineWitness(..) => {
+                if !self.visited_tys.insert(ty) {
+                    return V::Result::output();
+                }
+            }
             ty::Placeholder(..) | ty::Infer(..) => {
                 bug!("unexpected type: {:?}", ty)
             }
@@ -923,7 +940,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 +958,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 +1021,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 +1159,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 +1418,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
 

tests/ui/privacy/where-priv-type.rs

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

tests/ui/privacy/where-priv-type.stderr

@@ -77,17 +77,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/where-priv-type.rs:66: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; 5 warnings emitted
+error: aborting due to 1 previous error; 5 warnings emitted
 
 For more information about this error, try `rustc --explain E0446`.