Auto merge of #147486 - petrochenkov:optpriv, r=<try>

[PERF] privacy: Visit DefIds once in DefIdVisitorSkeleton

Author: rust-bors[bot]

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_def_ids: 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_def_ids: FxHashSet<DefId>,
     dummy: PhantomData<TyCtxt<'tcx>>,
 }
 
@@ -112,11 +114,13 @@ where
 {
     fn visit_trait(&mut self, trait_ref: TraitRef<'tcx>) -> V::Result {
         let TraitRef { def_id, args, .. } = trait_ref;
-        try_visit!(self.def_id_visitor.visit_def_id(
-            def_id,
-            "trait",
-            &trait_ref.print_only_trait_path()
-        ));
+        if self.visited_def_ids.insert(def_id) {
+            try_visit!(self.def_id_visitor.visit_def_id(
+                def_id,
+                "trait",
+                &trait_ref.print_only_trait_path()
+            ));
+        }
         if V::SHALLOW { V::Result::output() } else { args.visit_with(self) }
     }
 
@@ -190,7 +194,9 @@ where
             | ty::Closure(def_id, ..)
             | ty::CoroutineClosure(def_id, ..)
             | ty::Coroutine(def_id, ..) => {
-                try_visit!(self.def_id_visitor.visit_def_id(def_id, "type", &ty));
+                if self.visited_def_ids.insert(def_id) {
+                    try_visit!(self.def_id_visitor.visit_def_id(def_id, "type", &ty));
+                }
                 if V::SHALLOW {
                     return V::Result::output();
                 }
@@ -221,15 +227,17 @@ where
                     return V::Result::output();
                 }
 
-                try_visit!(self.def_id_visitor.visit_def_id(
-                    data.def_id,
-                    match kind {
-                        ty::Inherent | ty::Projection => "associated type",
-                        ty::Free => "type alias",
-                        ty::Opaque => unreachable!(),
-                    },
-                    &LazyDefPathStr { def_id: data.def_id, tcx },
-                ));
+                if self.visited_def_ids.insert(data.def_id) {
+                    try_visit!(self.def_id_visitor.visit_def_id(
+                        data.def_id,
+                        match kind {
+                            ty::Inherent | ty::Projection => "associated type",
+                            ty::Free => "type alias",
+                            ty::Opaque => unreachable!(),
+                        },
+                        &LazyDefPathStr { def_id: data.def_id, tcx },
+                    ));
+                }
 
                 // This will also visit args if necessary, so we don't need to recurse.
                 return if V::SHALLOW {
@@ -254,12 +262,14 @@ where
                         }
                     };
                     let ty::ExistentialTraitRef { def_id, .. } = trait_ref;
-                    try_visit!(self.def_id_visitor.visit_def_id(def_id, "trait", &trait_ref));
+                    if self.visited_def_ids.insert(def_id) {
+                        try_visit!(self.def_id_visitor.visit_def_id(def_id, "trait", &trait_ref));
+                    }
                 }
             }
             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 self.visited_def_ids.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
@@ -923,7 +933,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 +951,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 +1014,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 +1152,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 +1411,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`.