diff --git a/compiler/rustc_middle/src/query/mod.rs b/compiler/rustc_middle/src/query/mod.rs index f0be70e00dfca..f64483a747dca 100644 --- a/compiler/rustc_middle/src/query/mod.rs +++ b/compiler/rustc_middle/src/query/mod.rs @@ -1149,7 +1149,6 @@ rustc_queries! { desc { "checking effective visibilities" } } query check_private_in_public(_: ()) { - eval_always desc { "checking for private elements in public interfaces" } } diff --git a/compiler/rustc_privacy/src/lib.rs b/compiler/rustc_privacy/src/lib.rs index f67c4cb922cee..0aa61a9616d75 100644 --- a/compiler/rustc_privacy/src/lib.rs +++ b/compiler/rustc_privacy/src/lib.rs @@ -27,7 +27,7 @@ use rustc_data_structures::intern::Interned; use rustc_hir::def::{DefKind, Res}; use rustc_hir::def_id::{CRATE_DEF_ID, DefId, LocalDefId, LocalModDefId}; use rustc_hir::intravisit::{self, Visitor}; -use rustc_hir::{AssocItemKind, ForeignItemKind, ItemId, ItemKind, PatKind}; +use rustc_hir::{AssocItemKind, ForeignItemId, ItemId, PatKind}; use rustc_middle::middle::privacy::{EffectiveVisibilities, EffectiveVisibility, Level}; use rustc_middle::query::Providers; use rustc_middle::ty::print::PrintTraitRefExt as _; @@ -587,18 +587,13 @@ impl<'tcx> EmbargoVisitor<'tcx> { DefKind::Struct | DefKind::Union => { // While structs and unions have type privacy, their fields do not. - let item = self.tcx.hir().expect_item(def_id); - if let hir::ItemKind::Struct(ref struct_def, _) - | hir::ItemKind::Union(ref struct_def, _) = item.kind - { - for field in struct_def.fields() { - let field_vis = self.tcx.local_visibility(field.def_id); - if field_vis.is_accessible_from(module, self.tcx) { - self.reach(field.def_id, macro_ev).ty(); - } + let struct_def = self.tcx.adt_def(def_id); + for field in struct_def.non_enum_variant().fields.iter() { + let def_id = field.did.expect_local(); + let field_vis = self.tcx.local_visibility(def_id); + if field_vis.is_accessible_from(module, self.tcx) { + self.reach(def_id, macro_ev).ty(); } - } else { - bug!("item {:?} with DefKind {:?}", item, def_kind); } } @@ -1459,15 +1454,16 @@ impl<'tcx> PrivateItemsInPublicInterfacesChecker<'_, 'tcx> { fn check_assoc_item( &self, def_id: LocalDefId, - assoc_item_kind: AssocItemKind, + def_kind: DefKind, vis: ty::Visibility, effective_vis: Option, ) { let mut check = self.check(def_id, vis, effective_vis); - let (check_ty, is_assoc_ty) = match assoc_item_kind { - AssocItemKind::Const | AssocItemKind::Fn { .. } => (true, false), - AssocItemKind::Type => (self.tcx.defaultness(def_id).has_value(), true), + let (check_ty, is_assoc_ty) = match def_kind { + DefKind::AssocConst | DefKind::AssocFn => (true, false), + DefKind::AssocTy => (self.tcx.defaultness(def_id).has_value(), true), + _ => bug!("wrong def_kind for associated item"), }; check.in_assoc_ty = is_assoc_ty; @@ -1501,30 +1497,19 @@ impl<'tcx> PrivateItemsInPublicInterfacesChecker<'_, 'tcx> { self.check(def_id, item_visibility, effective_vis).generics().bounds(); } DefKind::Trait => { - let item = tcx.hir().item(id); - if let hir::ItemKind::Trait(.., trait_item_refs) = item.kind { - self.check_unnameable(item.owner_id.def_id, effective_vis); + self.check_unnameable(def_id, effective_vis); - self.check(item.owner_id.def_id, item_visibility, effective_vis) - .generics() - .predicates(); - - for trait_item_ref in trait_item_refs { - self.check_assoc_item( - trait_item_ref.id.owner_id.def_id, - trait_item_ref.kind, - item_visibility, - effective_vis, - ); + self.check(def_id, item_visibility, effective_vis).generics().predicates(); - if let AssocItemKind::Type = trait_item_ref.kind { - self.check( - trait_item_ref.id.owner_id.def_id, - item_visibility, - effective_vis, - ) - .bounds(); - } + for &assoc_id in tcx.associated_item_def_ids(def_id) { + if tcx.is_impl_trait_in_trait(assoc_id) { + continue; + } + let assoc_id = assoc_id.expect_local(); + let def_kind = tcx.def_kind(assoc_id); + self.check_assoc_item(assoc_id, def_kind, item_visibility, effective_vis); + if let DefKind::AssocTy = def_kind { + self.check(assoc_id, item_visibility, effective_vis).bounds(); } } } @@ -1532,133 +1517,96 @@ impl<'tcx> PrivateItemsInPublicInterfacesChecker<'_, 'tcx> { self.check(def_id, item_visibility, effective_vis).generics().predicates(); } DefKind::Enum => { - let item = tcx.hir().item(id); - if let hir::ItemKind::Enum(ref def, _) = item.kind { - self.check_unnameable(item.owner_id.def_id, effective_vis); - - self.check(item.owner_id.def_id, item_visibility, effective_vis) - .generics() - .predicates(); - - for variant in def.variants { - for field in variant.data.fields() { - self.check(field.def_id, item_visibility, effective_vis).ty(); - } - } - } - } - // Subitems of foreign modules have their own publicity. - DefKind::ForeignMod => { - let item = tcx.hir().item(id); - if let hir::ItemKind::ForeignMod { items, .. } = item.kind { - for foreign_item in items { - let foreign_item = tcx.hir().foreign_item(foreign_item.id); - - let ev = self.get(foreign_item.owner_id.def_id); - let vis = tcx.local_visibility(foreign_item.owner_id.def_id); - - if let ForeignItemKind::Type = foreign_item.kind { - self.check_unnameable(foreign_item.owner_id.def_id, ev); - } + self.check_unnameable(def_id, effective_vis); + self.check(def_id, item_visibility, effective_vis).generics().predicates(); - self.check(foreign_item.owner_id.def_id, vis, ev) - .generics() - .predicates() - .ty(); - } + let adt = tcx.adt_def(id.owner_id); + for field in adt.all_fields() { + self.check(field.did.expect_local(), item_visibility, effective_vis).ty(); } } // Subitems of structs and unions have their own publicity. DefKind::Struct | DefKind::Union => { - let item = tcx.hir().item(id); - if let hir::ItemKind::Struct(ref struct_def, _) - | hir::ItemKind::Union(ref struct_def, _) = item.kind - { - self.check_unnameable(item.owner_id.def_id, effective_vis); - self.check(item.owner_id.def_id, item_visibility, effective_vis) - .generics() - .predicates(); + self.check_unnameable(def_id, effective_vis); + self.check(def_id, item_visibility, effective_vis).generics().predicates(); - for field in struct_def.fields() { - let field_visibility = tcx.local_visibility(field.def_id); - let field_ev = self.get(field.def_id); - - self.check( - field.def_id, - min(item_visibility, field_visibility, tcx), - field_ev, - ) - .ty(); - } + let adt = tcx.adt_def(id.owner_id); + for field in adt.all_fields() { + let visibility = min(item_visibility, field.vis.expect_local(), tcx); + let field_ev = self.get(field.did.expect_local()); + + self.check(field.did.expect_local(), visibility, field_ev).ty(); } } + // Subitems of foreign modules have their own publicity. + DefKind::ForeignMod => {} // An inherent impl is public when its type is public // Subitems of inherent impls have their own publicity. // A trait impl is public when both its type and its trait are public // Subitems of trait impls have inherited publicity. - DefKind::Impl { .. } => { - let item = tcx.hir().item(id); - if let hir::ItemKind::Impl(impl_) = item.kind { - let impl_vis = ty::Visibility::of_impl::( - item.owner_id.def_id, - tcx, - &Default::default(), - ); - - // We are using the non-shallow version here, unlike when building the - // effective visisibilities table to avoid large number of false positives. - // For example in - // - // impl From for Pub { - // fn from(_: Priv) -> Pub {...} - // } - // - // lints shouldn't be emitted even if `from` effective visibility - // is larger than `Priv` nominal visibility and if `Priv` can leak - // in some scenarios due to type inference. - let impl_ev = EffectiveVisibility::of_impl::( - item.owner_id.def_id, - tcx, - self.effective_visibilities, - ); + DefKind::Impl { of_trait } => { + let impl_vis = ty::Visibility::of_impl::(def_id, tcx, &Default::default()); - // 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 - if impl_.of_trait.is_none() { - self.check(item.owner_id.def_id, impl_vis, Some(impl_ev)) - .generics() - .predicates(); + // We are using the non-shallow version here, unlike when building the + // effective visisibilities table to avoid large number of false positives. + // For example in + // + // impl From for Pub { + // fn from(_: Priv) -> Pub {...} + // } + // + // lints shouldn't be emitted even if `from` effective visibility + // is larger than `Priv` nominal visibility and if `Priv` can leak + // in some scenarios due to type inference. + let impl_ev = + EffectiveVisibility::of_impl::(def_id, 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 + if !of_trait { + self.check(def_id, impl_vis, Some(impl_ev)).generics().predicates(); + } + for &assoc_id in tcx.associated_item_def_ids(def_id) { + if tcx.is_impl_trait_in_trait(assoc_id) { + continue; } - for impl_item_ref in impl_.items { - let impl_item_vis = if impl_.of_trait.is_none() { - min( - tcx.local_visibility(impl_item_ref.id.owner_id.def_id), - impl_vis, - tcx, - ) - } else { - impl_vis - }; + let assoc_id = assoc_id.expect_local(); + let impl_item_vis = if !of_trait { + min(tcx.local_visibility(assoc_id), impl_vis, tcx) + } else { + impl_vis + }; - let impl_item_ev = if impl_.of_trait.is_none() { - self.get(impl_item_ref.id.owner_id.def_id) - .map(|ev| ev.min(impl_ev, self.tcx)) - } else { - Some(impl_ev) - }; - - self.check_assoc_item( - impl_item_ref.id.owner_id.def_id, - impl_item_ref.kind, - impl_item_vis, - impl_item_ev, - ); - } + let impl_item_ev = if !of_trait { + self.get(assoc_id).map(|ev| ev.min(impl_ev, self.tcx)) + } else { + Some(impl_ev) + }; + + self.check_assoc_item( + assoc_id, + tcx.def_kind(assoc_id), + impl_item_vis, + impl_item_ev, + ); } } _ => {} } } + + fn check_foreign_item(&mut self, id: ForeignItemId) { + let tcx = self.tcx; + let def_id = id.owner_id.def_id; + let item_visibility = tcx.local_visibility(def_id); + let effective_vis = self.get(def_id); + + if let DefKind::ForeignTy = self.tcx.def_kind(def_id) { + self.check_unnameable(def_id, effective_vis); + } + + self.check(def_id, item_visibility, effective_vis).generics().predicates().ty(); + } } pub fn provide(providers: &mut Providers) { @@ -1687,16 +1635,12 @@ fn check_mod_privacy(tcx: TyCtxt<'_>, module_def_id: LocalModDefId) { if let Some(body_id) = tcx.hir().maybe_body_owned_by(def_id) { visitor.visit_nested_body(body_id.id()); } - } - for id in module.free_items() { - if let ItemKind::Impl(i) = tcx.hir().item(id).kind { - if let Some(item) = i.of_trait { - let trait_ref = tcx.impl_trait_ref(id.owner_id.def_id).unwrap(); - let trait_ref = trait_ref.instantiate_identity(); - visitor.span = item.path.span; - visitor.visit_def_id(trait_ref.def_id, "trait", &trait_ref.print_only_trait_path()); - } + if let DefKind::Impl { of_trait: true } = tcx.def_kind(def_id) { + let trait_ref = tcx.impl_trait_ref(def_id).unwrap(); + let trait_ref = trait_ref.instantiate_identity(); + visitor.span = tcx.hir().expect_item(def_id).expect_impl().of_trait.unwrap().path.span; + visitor.visit_def_id(trait_ref.def_id, "trait", &trait_ref.print_only_trait_path()); } } } @@ -1787,7 +1731,11 @@ fn check_private_in_public(tcx: TyCtxt<'_>, (): ()) { // Check for private types in public interfaces. let mut checker = PrivateItemsInPublicInterfacesChecker { tcx, effective_visibilities }; - for id in tcx.hir().items() { + let crate_items = tcx.hir_crate_items(()); + for id in crate_items.free_items() { checker.check_item(id); } + for id in crate_items.foreign_items() { + checker.check_foreign_item(id); + } } diff --git a/tests/ui/privacy/associated-item-privacy-trait.stderr b/tests/ui/privacy/associated-item-privacy-trait.stderr index f79c4cff72fa2..4e9dfa4a83519 100644 --- a/tests/ui/privacy/associated-item-privacy-trait.stderr +++ b/tests/ui/privacy/associated-item-privacy-trait.stderr @@ -75,6 +75,17 @@ 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 + --> $DIR/associated-item-privacy-trait.rs:29:14 + | +LL | impl PrivTr for u8 {} + | ^^^^^^ private trait +... +LL | priv_trait::mac!(); + | ------------------ in this macro invocation + | + = note: this error originates in the macro `priv_trait::mac` (in Nightly builds, run with -Z macro-backtrace for more info) + error: type `priv_signature::Priv` is private --> $DIR/associated-item-privacy-trait.rs:46:21 | @@ -317,16 +328,5 @@ LL | priv_parent_substs::mac!(); | = note: this error originates in the macro `priv_parent_substs::mac` (in Nightly builds, run with -Z macro-backtrace for more info) -error: trait `PrivTr` is private - --> $DIR/associated-item-privacy-trait.rs:29:14 - | -LL | impl PrivTr for u8 {} - | ^^^^^^ private trait -... -LL | priv_trait::mac!(); - | ------------------ in this macro invocation - | - = note: this error originates in the macro `priv_trait::mac` (in Nightly builds, run with -Z macro-backtrace for more info) - error: aborting due to 30 previous errors diff --git a/tests/ui/privacy/private-in-public-warn.stderr b/tests/ui/privacy/private-in-public-warn.stderr index 3743879ffa616..0343002ea3181 100644 --- a/tests/ui/privacy/private-in-public-warn.stderr +++ b/tests/ui/privacy/private-in-public-warn.stderr @@ -84,42 +84,6 @@ note: but type `types::Priv` is only usable at visibility `pub(self)` LL | struct Priv; | ^^^^^^^^^^^ -error: type `types::Priv` is more private than the item `types::ES` - --> $DIR/private-in-public-warn.rs:27:9 - | -LL | pub static ES: Priv; - | ^^^^^^^^^^^^^^^^^^^ static `types::ES` is reachable at visibility `pub(crate)` - | -note: but type `types::Priv` is only usable at visibility `pub(self)` - --> $DIR/private-in-public-warn.rs:9:5 - | -LL | struct Priv; - | ^^^^^^^^^^^ - -error: type `types::Priv` is more private than the item `types::ef1` - --> $DIR/private-in-public-warn.rs:28:9 - | -LL | pub fn ef1(arg: Priv); - | ^^^^^^^^^^^^^^^^^^^^^^ function `types::ef1` is reachable at visibility `pub(crate)` - | -note: but type `types::Priv` is only usable at visibility `pub(self)` - --> $DIR/private-in-public-warn.rs:9:5 - | -LL | struct Priv; - | ^^^^^^^^^^^ - -error: type `types::Priv` is more private than the item `types::ef2` - --> $DIR/private-in-public-warn.rs:29:9 - | -LL | pub fn ef2() -> Priv; - | ^^^^^^^^^^^^^^^^^^^^^ function `types::ef2` is reachable at visibility `pub(crate)` - | -note: but type `types::Priv` is only usable at visibility `pub(self)` - --> $DIR/private-in-public-warn.rs:9:5 - | -LL | struct Priv; - | ^^^^^^^^^^^ - error[E0446]: private type `types::Priv` in public interface --> $DIR/private-in-public-warn.rs:32:9 | @@ -395,6 +359,42 @@ note: but type `Priv2` is only usable at visibility `pub(self)` LL | struct Priv2; | ^^^^^^^^^^^^ +error: type `types::Priv` is more private than the item `types::ES` + --> $DIR/private-in-public-warn.rs:27:9 + | +LL | pub static ES: Priv; + | ^^^^^^^^^^^^^^^^^^^ static `types::ES` is reachable at visibility `pub(crate)` + | +note: but type `types::Priv` is only usable at visibility `pub(self)` + --> $DIR/private-in-public-warn.rs:9:5 + | +LL | struct Priv; + | ^^^^^^^^^^^ + +error: type `types::Priv` is more private than the item `types::ef1` + --> $DIR/private-in-public-warn.rs:28:9 + | +LL | pub fn ef1(arg: Priv); + | ^^^^^^^^^^^^^^^^^^^^^^ function `types::ef1` is reachable at visibility `pub(crate)` + | +note: but type `types::Priv` is only usable at visibility `pub(self)` + --> $DIR/private-in-public-warn.rs:9:5 + | +LL | struct Priv; + | ^^^^^^^^^^^ + +error: type `types::Priv` is more private than the item `types::ef2` + --> $DIR/private-in-public-warn.rs:29:9 + | +LL | pub fn ef2() -> Priv; + | ^^^^^^^^^^^^^^^^^^^^^ function `types::ef2` is reachable at visibility `pub(crate)` + | +note: but type `types::Priv` is only usable at visibility `pub(self)` + --> $DIR/private-in-public-warn.rs:9:5 + | +LL | struct Priv; + | ^^^^^^^^^^^ + warning: bounds on generic parameters in type aliases are not enforced --> $DIR/private-in-public-warn.rs:41:23 |