From f756304655b8604327f7a906c9e55b74a5fedfd1 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Mon, 10 Mar 2025 18:24:54 +0300 Subject: [PATCH] privacy: Visit types and traits in impls in type privacy lints --- compiler/rustc_privacy/src/lib.rs | 40 ++++++++++++++----- tests/ui/privacy/pub-priv-dep/pub-priv1.rs | 7 ++-- .../ui/privacy/pub-priv-dep/pub-priv1.stderr | 22 +++++++++- 3 files changed, 55 insertions(+), 14 deletions(-) diff --git a/compiler/rustc_privacy/src/lib.rs b/compiler/rustc_privacy/src/lib.rs index ed00d97c31d..643b82f4753 100644 --- a/compiler/rustc_privacy/src/lib.rs +++ b/compiler/rustc_privacy/src/lib.rs @@ -72,7 +72,9 @@ impl<'tcx> fmt::Display for LazyDefPathStr<'tcx> { pub trait DefIdVisitor<'tcx> { type Result: VisitorResult = (); const SHALLOW: bool = false; - const SKIP_ASSOC_TYS: bool = false; + fn skip_assoc_tys(&self) -> bool { + false + } fn tcx(&self) -> TyCtxt<'tcx>; fn visit_def_id(&mut self, def_id: DefId, kind: &str, descr: &dyn fmt::Display) @@ -213,7 +215,7 @@ where } } ty::Alias(kind @ (ty::Inherent | ty::Weak | ty::Projection), data) => { - if V::SKIP_ASSOC_TYS { + if self.def_id_visitor.skip_assoc_tys() { // Visitors searching for minimal visibility/reachability want to // conservatively approximate associated types like `Type::Alias` // as visible/reachable even if `Type` is private. @@ -324,7 +326,9 @@ impl<'a, 'tcx, VL: VisibilityLike, const SHALLOW: bool> DefIdVisitor<'tcx> for FindMin<'a, 'tcx, VL, SHALLOW> { const SHALLOW: bool = SHALLOW; - const SKIP_ASSOC_TYS: bool = true; + fn skip_assoc_tys(&self) -> bool { + true + } fn tcx(&self) -> TyCtxt<'tcx> { self.tcx } @@ -342,7 +346,7 @@ trait VisibilityLike: Sized { def_id: LocalDefId, ) -> Self; - // Returns an over-approximation (`SKIP_ASSOC_TYS` = true) of visibility due to + // Returns an over-approximation (`skip_assoc_tys()` = true) of visibility due to // associated types for which we can't determine visibility precisely. fn of_impl( def_id: LocalDefId, @@ -1352,6 +1356,7 @@ struct SearchInterfaceForPrivateItemsVisitor<'tcx> { required_effective_vis: Option, in_assoc_ty: bool, in_primary_interface: bool, + skip_assoc_tys: bool, } impl SearchInterfaceForPrivateItemsVisitor<'_> { @@ -1398,6 +1403,14 @@ impl SearchInterfaceForPrivateItemsVisitor<'_> { self } + fn trait_ref(&mut self) -> &mut Self { + self.in_primary_interface = true; + if let Some(trait_ref) = self.tcx.impl_trait_ref(self.item_def_id) { + let _ = self.visit_trait(trait_ref.instantiate_identity()); + } + self + } + fn check_def_id(&mut self, def_id: DefId, kind: &str, descr: &dyn fmt::Display) -> bool { if self.leaks_private_dep(def_id) { self.tcx.emit_node_span_lint( @@ -1495,6 +1508,9 @@ impl SearchInterfaceForPrivateItemsVisitor<'_> { impl<'tcx> DefIdVisitor<'tcx> for SearchInterfaceForPrivateItemsVisitor<'tcx> { type Result = ControlFlow<()>; + fn skip_assoc_tys(&self) -> bool { + self.skip_assoc_tys + } fn tcx(&self) -> TyCtxt<'tcx> { self.tcx } @@ -1531,6 +1547,7 @@ impl<'tcx> PrivateItemsInPublicInterfacesChecker<'_, 'tcx> { required_effective_vis, in_assoc_ty: false, in_primary_interface: true, + skip_assoc_tys: false, } } @@ -1726,13 +1743,18 @@ impl<'tcx> PrivateItemsInPublicInterfacesChecker<'_, 'tcx> { self.effective_visibilities, ); - // check that private components do not appear in the generics or predicates of inherent impls - // this check is intentionally NOT performed for impls of traits, per #90586 + let mut check = self.check(item.owner_id.def_id, impl_vis, Some(impl_ev)); + // Generics and predicates of trait impls are intentionally not checked + // for private components (#90586). if impl_.of_trait.is_none() { - self.check(item.owner_id.def_id, impl_vis, Some(impl_ev)) - .generics() - .predicates(); + check.generics().predicates(); } + // Skip checking private components in associated types, due to lack of full + // normalization they produce very ridiculous false positives. + // FIXME: Remove this when full normalization is implemented. + check.skip_assoc_tys = true; + check.ty().trait_ref(); + for impl_item_ref in impl_.items { let impl_item_vis = if impl_.of_trait.is_none() { min( diff --git a/tests/ui/privacy/pub-priv-dep/pub-priv1.rs b/tests/ui/privacy/pub-priv-dep/pub-priv1.rs index 112eaf528be..877029f3de3 100644 --- a/tests/ui/privacy/pub-priv-dep/pub-priv1.rs +++ b/tests/ui/privacy/pub-priv-dep/pub-priv1.rs @@ -77,15 +77,14 @@ pub type Alias = OtherType; pub struct PublicWithPrivateImpl; -// FIXME: This should trigger. -// See https://github.com/rust-lang/rust/issues/71043 impl OtherTrait for PublicWithPrivateImpl {} +//~^ ERROR trait `OtherTrait` from private dependency 'priv_dep' in public interface pub trait PubTraitOnPrivate {} -// FIXME: This should trigger. -// See https://github.com/rust-lang/rust/issues/71043 impl PubTraitOnPrivate for OtherType {} +//~^ 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 { #[allow(exported_private_dependencies)] diff --git a/tests/ui/privacy/pub-priv-dep/pub-priv1.stderr b/tests/ui/privacy/pub-priv-dep/pub-priv1.stderr index 53d461a5774..adfe13424cd 100644 --- a/tests/ui/privacy/pub-priv-dep/pub-priv1.stderr +++ b/tests/ui/privacy/pub-priv-dep/pub-priv1.stderr @@ -70,5 +70,25 @@ error: type `OtherType` from private dependency 'priv_dep' in public interface LL | pub type Alias = OtherType; | ^^^^^^^^^^^^^^ -error: aborting due to 11 previous errors +error: trait `OtherTrait` from private dependency 'priv_dep' in public interface + --> $DIR/pub-priv1.rs:80:1 + | +LL | impl OtherTrait for PublicWithPrivateImpl {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: type `OtherType` from private dependency 'priv_dep' in public interface + --> $DIR/pub-priv1.rs:85:1 + | +LL | impl PubTraitOnPrivate for OtherType {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: type `OtherType` from private dependency 'priv_dep' in public interface + --> $DIR/pub-priv1.rs:85:1 + | +LL | impl PubTraitOnPrivate for OtherType {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` + +error: aborting due to 14 previous errors