Skip to content

Commit fa8fb0d

Browse files
cjgillotcjgillot
committed
Use less HIR in check_private_in_public.
1 parent d80a8cd commit fa8fb0d

File tree

3 files changed

+150
-202
lines changed

3 files changed

+150
-202
lines changed

‎compiler/rustc_privacy/src/lib.rs

+103-155
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ use rustc_data_structures::intern::Interned;
2727
use rustc_hir::def::{DefKind, Res};
2828
use rustc_hir::def_id::{CRATE_DEF_ID, DefId, LocalDefId, LocalModDefId};
2929
use rustc_hir::intravisit::{self, Visitor};
30-
use rustc_hir::{AssocItemKind, ForeignItemKind, ItemId, ItemKind, PatKind};
30+
use rustc_hir::{AssocItemKind, ForeignItemId, ItemId, PatKind};
3131
use rustc_middle::middle::privacy::{EffectiveVisibilities, EffectiveVisibility, Level};
3232
use rustc_middle::query::Providers;
3333
use rustc_middle::ty::print::PrintTraitRefExt as _;
@@ -587,18 +587,13 @@ impl<'tcx> EmbargoVisitor<'tcx> {
587587

588588
DefKind::Struct | DefKind::Union => {
589589
// While structs and unions have type privacy, their fields do not.
590-
let item = self.tcx.hir().expect_item(def_id);
591-
if let hir::ItemKind::Struct(ref struct_def, _)
592-
| hir::ItemKind::Union(ref struct_def, _) = item.kind
593-
{
594-
for field in struct_def.fields() {
595-
let field_vis = self.tcx.local_visibility(field.def_id);
596-
if field_vis.is_accessible_from(module, self.tcx) {
597-
self.reach(field.def_id, macro_ev).ty();
598-
}
590+
let struct_def = self.tcx.adt_def(def_id);
591+
for field in struct_def.non_enum_variant().fields.iter() {
592+
let def_id = field.did.expect_local();
593+
let field_vis = self.tcx.local_visibility(def_id);
594+
if field_vis.is_accessible_from(module, self.tcx) {
595+
self.reach(def_id, macro_ev).ty();
599596
}
600-
} else {
601-
bug!("item {:?} with DefKind {:?}", item, def_kind);
602597
}
603598
}
604599

@@ -1459,15 +1454,16 @@ impl<'tcx> PrivateItemsInPublicInterfacesChecker<'_, 'tcx> {
14591454
fn check_assoc_item(
14601455
&self,
14611456
def_id: LocalDefId,
1462-
assoc_item_kind: AssocItemKind,
1457+
def_kind: DefKind,
14631458
vis: ty::Visibility,
14641459
effective_vis: Option<EffectiveVisibility>,
14651460
) {
14661461
let mut check = self.check(def_id, vis, effective_vis);
14671462

1468-
let (check_ty, is_assoc_ty) = match assoc_item_kind {
1469-
AssocItemKind::Const | AssocItemKind::Fn { .. } => (true, false),
1470-
AssocItemKind::Type => (self.tcx.defaultness(def_id).has_value(), true),
1463+
let (check_ty, is_assoc_ty) = match def_kind {
1464+
DefKind::AssocConst | DefKind::AssocFn => (true, false),
1465+
DefKind::AssocTy => (self.tcx.defaultness(def_id).has_value(), true),
1466+
_ => bug!("wrong def_kind for associated item"),
14711467
};
14721468

14731469
check.in_assoc_ty = is_assoc_ty;
@@ -1501,164 +1497,116 @@ impl<'tcx> PrivateItemsInPublicInterfacesChecker<'_, 'tcx> {
15011497
self.check(def_id, item_visibility, effective_vis).generics().bounds();
15021498
}
15031499
DefKind::Trait => {
1504-
let item = tcx.hir().item(id);
1505-
if let hir::ItemKind::Trait(.., trait_item_refs) = item.kind {
1506-
self.check_unnameable(item.owner_id.def_id, effective_vis);
1500+
self.check_unnameable(def_id, effective_vis);
15071501

1508-
self.check(item.owner_id.def_id, item_visibility, effective_vis)
1509-
.generics()
1510-
.predicates();
1511-
1512-
for trait_item_ref in trait_item_refs {
1513-
self.check_assoc_item(
1514-
trait_item_ref.id.owner_id.def_id,
1515-
trait_item_ref.kind,
1516-
item_visibility,
1517-
effective_vis,
1518-
);
1502+
self.check(def_id, item_visibility, effective_vis).generics().predicates();
15191503

1520-
if let AssocItemKind::Type = trait_item_ref.kind {
1521-
self.check(
1522-
trait_item_ref.id.owner_id.def_id,
1523-
item_visibility,
1524-
effective_vis,
1525-
)
1526-
.bounds();
1527-
}
1504+
for &assoc_id in tcx.associated_item_def_ids(def_id) {
1505+
if tcx.is_impl_trait_in_trait(assoc_id) {
1506+
continue;
1507+
}
1508+
let assoc_id = assoc_id.expect_local();
1509+
let def_kind = tcx.def_kind(assoc_id);
1510+
self.check_assoc_item(assoc_id, def_kind, item_visibility, effective_vis);
1511+
if let DefKind::AssocTy = def_kind {
1512+
self.check(assoc_id, item_visibility, effective_vis).bounds();
15281513
}
15291514
}
15301515
}
15311516
DefKind::TraitAlias => {
15321517
self.check(def_id, item_visibility, effective_vis).generics().predicates();
15331518
}
15341519
DefKind::Enum => {
1535-
let item = tcx.hir().item(id);
1536-
if let hir::ItemKind::Enum(ref def, _) = item.kind {
1537-
self.check_unnameable(item.owner_id.def_id, effective_vis);
1538-
1539-
self.check(item.owner_id.def_id, item_visibility, effective_vis)
1540-
.generics()
1541-
.predicates();
1542-
1543-
for variant in def.variants {
1544-
for field in variant.data.fields() {
1545-
self.check(field.def_id, item_visibility, effective_vis).ty();
1546-
}
1547-
}
1548-
}
1549-
}
1550-
// Subitems of foreign modules have their own publicity.
1551-
DefKind::ForeignMod => {
1552-
let item = tcx.hir().item(id);
1553-
if let hir::ItemKind::ForeignMod { items, .. } = item.kind {
1554-
for foreign_item in items {
1555-
let foreign_item = tcx.hir().foreign_item(foreign_item.id);
1556-
1557-
let ev = self.get(foreign_item.owner_id.def_id);
1558-
let vis = tcx.local_visibility(foreign_item.owner_id.def_id);
1559-
1560-
if let ForeignItemKind::Type = foreign_item.kind {
1561-
self.check_unnameable(foreign_item.owner_id.def_id, ev);
1562-
}
1520+
self.check_unnameable(def_id, effective_vis);
1521+
self.check(def_id, item_visibility, effective_vis).generics().predicates();
15631522

1564-
self.check(foreign_item.owner_id.def_id, vis, ev)
1565-
.generics()
1566-
.predicates()
1567-
.ty();
1568-
}
1523+
let adt = tcx.adt_def(id.owner_id);
1524+
for field in adt.all_fields() {
1525+
self.check(field.did.expect_local(), item_visibility, effective_vis).ty();
15691526
}
15701527
}
15711528
// Subitems of structs and unions have their own publicity.
15721529
DefKind::Struct | DefKind::Union => {
1573-
let item = tcx.hir().item(id);
1574-
if let hir::ItemKind::Struct(ref struct_def, _)
1575-
| hir::ItemKind::Union(ref struct_def, _) = item.kind
1576-
{
1577-
self.check_unnameable(item.owner_id.def_id, effective_vis);
1578-
self.check(item.owner_id.def_id, item_visibility, effective_vis)
1579-
.generics()
1580-
.predicates();
1530+
self.check_unnameable(def_id, effective_vis);
1531+
self.check(def_id, item_visibility, effective_vis).generics().predicates();
15811532

1582-
for field in struct_def.fields() {
1583-
let field_visibility = tcx.local_visibility(field.def_id);
1584-
let field_ev = self.get(field.def_id);
1585-
1586-
self.check(
1587-
field.def_id,
1588-
min(item_visibility, field_visibility, tcx),
1589-
field_ev,
1590-
)
1591-
.ty();
1592-
}
1533+
let adt = tcx.adt_def(id.owner_id);
1534+
for field in adt.all_fields() {
1535+
let visibility = min(item_visibility, field.vis.expect_local(), tcx);
1536+
let field_ev = self.get(field.did.expect_local());
1537+
1538+
self.check(field.did.expect_local(), visibility, field_ev).ty();
15931539
}
15941540
}
1541+
// Subitems of foreign modules have their own publicity.
1542+
DefKind::ForeignMod => {}
15951543
// An inherent impl is public when its type is public
15961544
// Subitems of inherent impls have their own publicity.
15971545
// A trait impl is public when both its type and its trait are public
15981546
// Subitems of trait impls have inherited publicity.
1599-
DefKind::Impl { .. } => {
1600-
let item = tcx.hir().item(id);
1601-
if let hir::ItemKind::Impl(impl_) = item.kind {
1602-
let impl_vis = ty::Visibility::of_impl::<false>(
1603-
item.owner_id.def_id,
1604-
tcx,
1605-
&Default::default(),
1606-
);
1607-
1608-
// We are using the non-shallow version here, unlike when building the
1609-
// effective visisibilities table to avoid large number of false positives.
1610-
// For example in
1611-
//
1612-
// impl From<Priv> for Pub {
1613-
// fn from(_: Priv) -> Pub {...}
1614-
// }
1615-
//
1616-
// lints shouldn't be emitted even if `from` effective visibility
1617-
// is larger than `Priv` nominal visibility and if `Priv` can leak
1618-
// in some scenarios due to type inference.
1619-
let impl_ev = EffectiveVisibility::of_impl::<false>(
1620-
item.owner_id.def_id,
1621-
tcx,
1622-
self.effective_visibilities,
1623-
);
1547+
DefKind::Impl { of_trait } => {
1548+
let impl_vis = ty::Visibility::of_impl::<false>(def_id, tcx, &Default::default());
16241549

1625-
// check that private components do not appear in the generics or predicates of inherent impls
1626-
// this check is intentionally NOT performed for impls of traits, per #90586
1627-
if impl_.of_trait.is_none() {
1628-
self.check(item.owner_id.def_id, impl_vis, Some(impl_ev))
1629-
.generics()
1630-
.predicates();
1550+
// We are using the non-shallow version here, unlike when building the
1551+
// effective visisibilities table to avoid large number of false positives.
1552+
// For example in
1553+
//
1554+
// impl From<Priv> for Pub {
1555+
// fn from(_: Priv) -> Pub {...}
1556+
// }
1557+
//
1558+
// lints shouldn't be emitted even if `from` effective visibility
1559+
// is larger than `Priv` nominal visibility and if `Priv` can leak
1560+
// in some scenarios due to type inference.
1561+
let impl_ev =
1562+
EffectiveVisibility::of_impl::<false>(def_id, tcx, self.effective_visibilities);
1563+
1564+
// check that private components do not appear in the generics or predicates of inherent impls
1565+
// this check is intentionally NOT performed for impls of traits, per #90586
1566+
if !of_trait {
1567+
self.check(def_id, impl_vis, Some(impl_ev)).generics().predicates();
1568+
}
1569+
for &assoc_id in tcx.associated_item_def_ids(def_id) {
1570+
if tcx.is_impl_trait_in_trait(assoc_id) {
1571+
continue;
16311572
}
1632-
for impl_item_ref in impl_.items {
1633-
let impl_item_vis = if impl_.of_trait.is_none() {
1634-
min(
1635-
tcx.local_visibility(impl_item_ref.id.owner_id.def_id),
1636-
impl_vis,
1637-
tcx,
1638-
)
1639-
} else {
1640-
impl_vis
1641-
};
1573+
let assoc_id = assoc_id.expect_local();
1574+
let impl_item_vis = if !of_trait {
1575+
min(tcx.local_visibility(assoc_id), impl_vis, tcx)
1576+
} else {
1577+
impl_vis
1578+
};
16421579

1643-
let impl_item_ev = if impl_.of_trait.is_none() {
1644-
self.get(impl_item_ref.id.owner_id.def_id)
1645-
.map(|ev| ev.min(impl_ev, self.tcx))
1646-
} else {
1647-
Some(impl_ev)
1648-
};
1649-
1650-
self.check_assoc_item(
1651-
impl_item_ref.id.owner_id.def_id,
1652-
impl_item_ref.kind,
1653-
impl_item_vis,
1654-
impl_item_ev,
1655-
);
1656-
}
1580+
let impl_item_ev = if !of_trait {
1581+
self.get(assoc_id).map(|ev| ev.min(impl_ev, self.tcx))
1582+
} else {
1583+
Some(impl_ev)
1584+
};
1585+
1586+
self.check_assoc_item(
1587+
assoc_id,
1588+
tcx.def_kind(assoc_id),
1589+
impl_item_vis,
1590+
impl_item_ev,
1591+
);
16571592
}
16581593
}
16591594
_ => {}
16601595
}
16611596
}
1597+
1598+
fn check_foreign_item(&mut self, id: ForeignItemId) {
1599+
let tcx = self.tcx;
1600+
let def_id = id.owner_id.def_id;
1601+
let item_visibility = tcx.local_visibility(def_id);
1602+
let effective_vis = self.get(def_id);
1603+
1604+
if let DefKind::ForeignTy = self.tcx.def_kind(def_id) {
1605+
self.check_unnameable(def_id, effective_vis);
1606+
}
1607+
1608+
self.check(def_id, item_visibility, effective_vis).generics().predicates().ty();
1609+
}
16621610
}
16631611

16641612
pub fn provide(providers: &mut Providers) {
@@ -1687,16 +1635,12 @@ fn check_mod_privacy(tcx: TyCtxt<'_>, module_def_id: LocalModDefId) {
16871635
if let Some(body_id) = tcx.hir().maybe_body_owned_by(def_id) {
16881636
visitor.visit_nested_body(body_id.id());
16891637
}
1690-
}
16911638

1692-
for id in module.free_items() {
1693-
if let ItemKind::Impl(i) = tcx.hir().item(id).kind {
1694-
if let Some(item) = i.of_trait {
1695-
let trait_ref = tcx.impl_trait_ref(id.owner_id.def_id).unwrap();
1696-
let trait_ref = trait_ref.instantiate_identity();
1697-
visitor.span = item.path.span;
1698-
visitor.visit_def_id(trait_ref.def_id, "trait", &trait_ref.print_only_trait_path());
1699-
}
1639+
if let DefKind::Impl { of_trait: true } = tcx.def_kind(def_id) {
1640+
let trait_ref = tcx.impl_trait_ref(def_id).unwrap();
1641+
let trait_ref = trait_ref.instantiate_identity();
1642+
visitor.span = tcx.hir().expect_item(def_id).expect_impl().of_trait.unwrap().path.span;
1643+
visitor.visit_def_id(trait_ref.def_id, "trait", &trait_ref.print_only_trait_path());
17001644
}
17011645
}
17021646
}
@@ -1787,7 +1731,11 @@ fn check_private_in_public(tcx: TyCtxt<'_>, (): ()) {
17871731
// Check for private types in public interfaces.
17881732
let mut checker = PrivateItemsInPublicInterfacesChecker { tcx, effective_visibilities };
17891733

1790-
for id in tcx.hir().items() {
1734+
let crate_items = tcx.hir_crate_items(());
1735+
for id in crate_items.free_items() {
17911736
checker.check_item(id);
17921737
}
1738+
for id in crate_items.foreign_items() {
1739+
checker.check_foreign_item(id);
1740+
}
17931741
}

‎tests/ui/privacy/associated-item-privacy-trait.stderr

+11-11
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,17 @@ LL | priv_trait::mac!();
7575
|
7676
= note: this error originates in the macro `priv_trait::mac` (in Nightly builds, run with -Z macro-backtrace for more info)
7777

78+
error: trait `PrivTr` is private
79+
--> $DIR/associated-item-privacy-trait.rs:29:14
80+
|
81+
LL | impl PrivTr for u8 {}
82+
| ^^^^^^ private trait
83+
...
84+
LL | priv_trait::mac!();
85+
| ------------------ in this macro invocation
86+
|
87+
= note: this error originates in the macro `priv_trait::mac` (in Nightly builds, run with -Z macro-backtrace for more info)
88+
7889
error: type `priv_signature::Priv` is private
7990
--> $DIR/associated-item-privacy-trait.rs:46:21
8091
|
@@ -317,16 +328,5 @@ LL | priv_parent_substs::mac!();
317328
|
318329
= note: this error originates in the macro `priv_parent_substs::mac` (in Nightly builds, run with -Z macro-backtrace for more info)
319330

320-
error: trait `PrivTr` is private
321-
--> $DIR/associated-item-privacy-trait.rs:29:14
322-
|
323-
LL | impl PrivTr for u8 {}
324-
| ^^^^^^ private trait
325-
...
326-
LL | priv_trait::mac!();
327-
| ------------------ in this macro invocation
328-
|
329-
= note: this error originates in the macro `priv_trait::mac` (in Nightly builds, run with -Z macro-backtrace for more info)
330-
331331
error: aborting due to 30 previous errors
332332

0 commit comments

Comments
 (0)