Skip to content

resolve: Minor improvements to effective visibilities #109704

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Mar 31, 2023
Merged

resolve: Minor improvements to effective visibilities #109704

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4695ddf
privacy: Support `#![rustc_effective_visibility]` on the crate root
petrochenkov Mar 28, 2023
ede21e8
effvis: Merge two similar code paths
petrochenkov Mar 28, 2023
b3bfeaf
effvis: Stop considering crate root its own parent
petrochenkov Mar 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion compiler/rustc_middle/src/middle/privacy.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use rustc_data_structures::fx::FxHashMap;
use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
use rustc_macros::HashStable;
use rustc_query_system::ich::StableHashingContext;
use rustc_span::def_id::LocalDefId;
use rustc_span::def_id::{LocalDefId, CRATE_DEF_ID};
use std::hash::Hash;

/// Represents the levels of effective visibility an item can have.
Expand Down Expand Up @@ -107,6 +107,10 @@ impl EffectiveVisibilities {
})
}

pub fn update_root(&mut self) {
self.map.insert(CRATE_DEF_ID, EffectiveVisibility::from_vis(Visibility::Public));
}

// FIXME: Share code with `fn update`.
pub fn update_eff_vis(
&mut self,
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_privacy/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2149,6 +2149,7 @@ fn effective_visibilities(tcx: TyCtxt<'_>, (): ()) -> &EffectiveVisibilities {

let mut check_visitor =
TestReachabilityVisitor { tcx, effective_visibilities: &visitor.effective_visibilities };
check_visitor.effective_visibility_diagnostic(CRATE_DEF_ID);
tcx.hir().visit_all_item_likes_in_crate(&mut check_visitor);

tcx.arena.alloc(visitor.effective_visibilities)
Expand Down
70 changes: 29 additions & 41 deletions compiler/rustc_resolve/src/effective_visibilities.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ impl Resolver<'_, '_> {
// For mod items `nearest_normal_mod` returns its argument, but we actually need its parent.
let normal_mod_id = self.nearest_normal_mod(def_id);
if normal_mod_id == def_id {
self.tcx.opt_local_parent(def_id).map_or(Visibility::Public, Visibility::Restricted)
Visibility::Restricted(self.tcx.local_parent(def_id))
} else {
Visibility::Restricted(normal_mod_id)
}
Expand All @@ -80,12 +80,11 @@ impl<'r, 'a, 'tcx> EffectiveVisibilitiesVisitor<'r, 'a, 'tcx> {
r,
def_effective_visibilities: Default::default(),
import_effective_visibilities: Default::default(),
current_private_vis: Visibility::Public,
current_private_vis: Visibility::Restricted(CRATE_DEF_ID),
changed: false,
};

visitor.update(CRATE_DEF_ID, CRATE_DEF_ID);
visitor.current_private_vis = Visibility::Restricted(CRATE_DEF_ID);
visitor.def_effective_visibilities.update_root();
visitor.set_bindings_effective_visibilities(CRATE_DEF_ID);

while visitor.changed {
Expand Down Expand Up @@ -125,43 +124,32 @@ impl<'r, 'a, 'tcx> EffectiveVisibilitiesVisitor<'r, 'a, 'tcx> {

for (_, name_resolution) in resolutions.borrow().iter() {
if let Some(mut binding) = name_resolution.borrow().binding() {
if !binding.is_ambiguity() {
// Set the given effective visibility level to `Level::Direct` and
// sets the rest of the `use` chain to `Level::Reexported` until
// we hit the actual exported item.
let mut parent_id = ParentId::Def(module_id);
while let NameBindingKind::Import { binding: nested_binding, .. } = binding.kind
{
let binding_id = ImportId::new_unchecked(binding);
self.update_import(binding_id, parent_id);

parent_id = ParentId::Import(binding_id);
binding = nested_binding;
}

if let Some(def_id) = binding.res().opt_def_id().and_then(|id| id.as_local()) {
self.update_def(def_id, binding.vis.expect_local(), parent_id);
// Set the given effective visibility level to `Level::Direct` and
// sets the rest of the `use` chain to `Level::Reexported` until
// we hit the actual exported item.
//
// If the binding is ambiguous, put the root ambiguity binding and all reexports
// leading to it into the table. They are used by the `ambiguous_glob_reexports`
// lint. For all bindings added to the table this way `is_ambiguity` returns true.
let mut parent_id = ParentId::Def(module_id);
while let NameBindingKind::Import { binding: nested_binding, .. } = binding.kind {
let binding_id = ImportId::new_unchecked(binding);
self.update_import(binding_id, parent_id);

if binding.ambiguity.is_some() {
// Stop at the root ambiguity, further bindings in the chain should not
// be reexported because the root ambiguity blocks any access to them.
// (Those further bindings are most likely not ambiguities themselves.)
break;
}
} else {
// Put the root ambiguity binding and all reexports leading to it into the
// table. They are used by the `ambiguous_glob_reexports` lint. For all
// bindings added to the table here `is_ambiguity` returns true.
let mut parent_id = ParentId::Def(module_id);
while let NameBindingKind::Import { binding: nested_binding, .. } = binding.kind
{
let binding_id = ImportId::new_unchecked(binding);
self.update_import(binding_id, parent_id);

if binding.ambiguity.is_some() {
// Stop at the root ambiguity, further bindings in the chain should not
// be reexported because the root ambiguity blocks any access to them.
// (Those further bindings are most likely not ambiguities themselves.)
break;
}
parent_id = ParentId::Import(binding_id);
binding = nested_binding;
}

parent_id = ParentId::Import(binding_id);
binding = nested_binding;
}
if binding.ambiguity.is_none()
&& let Some(def_id) = binding.res().opt_def_id().and_then(|id| id.as_local()) {
self.update_def(def_id, binding.vis.expect_local(), parent_id);
}
}
}
Expand Down Expand Up @@ -213,7 +201,7 @@ impl<'r, 'a, 'tcx> EffectiveVisibilitiesVisitor<'r, 'a, 'tcx> {
);
}

fn update(&mut self, def_id: LocalDefId, parent_id: LocalDefId) {
fn update_field(&mut self, def_id: LocalDefId, parent_id: LocalDefId) {
self.update_def(def_id, self.r.visibilities[&def_id], ParentId::Def(parent_id));
}
}
Expand Down Expand Up @@ -245,14 +233,14 @@ impl<'r, 'ast, 'tcx> Visitor<'ast> for EffectiveVisibilitiesVisitor<'ast, 'r, 't
for variant in variants {
let variant_def_id = self.r.local_def_id(variant.id);
for field in variant.data.fields() {
self.update(self.r.local_def_id(field.id), variant_def_id);
self.update_field(self.r.local_def_id(field.id), variant_def_id);
}
}
}

ast::ItemKind::Struct(ref def, _) | ast::ItemKind::Union(ref def, _) => {
for field in def.fields() {
self.update(self.r.local_def_id(field.id), def_id);
self.update_field(self.r.local_def_id(field.id), def_id);
}
}

Expand Down
1 change: 1 addition & 0 deletions tests/ui/privacy/effective_visibilities.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#![rustc_effective_visibility] //~ ERROR Direct: pub, Reexported: pub, Reachable: pub, ReachableThroughImplTrait: pub
#![feature(rustc_attrs)]

#[rustc_effective_visibility]
Expand Down
60 changes: 36 additions & 24 deletions tests/ui/privacy/effective_visibilities.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,140 +1,152 @@
error: Direct: pub, Reexported: pub, Reachable: pub, ReachableThroughImplTrait: pub
--> $DIR/effective_visibilities.rs:1:1
|
LL | / #![rustc_effective_visibility]
LL | | #![feature(rustc_attrs)]
LL | |
LL | | #[rustc_effective_visibility]
... |
LL | |
LL | | fn main() {}
| |____________^

error: Direct: pub(crate), Reexported: pub(crate), Reachable: pub(crate), ReachableThroughImplTrait: pub(crate)
--> $DIR/effective_visibilities.rs:4:1
--> $DIR/effective_visibilities.rs:5:1
|
LL | mod outer {
| ^^^^^^^^^

error: Direct: pub(crate), Reexported: pub, Reachable: pub, ReachableThroughImplTrait: pub
--> $DIR/effective_visibilities.rs:6:5
--> $DIR/effective_visibilities.rs:7:5
|
LL | pub mod inner1 {
| ^^^^^^^^^^^^^^

error: not in the table
--> $DIR/effective_visibilities.rs:9:9
--> $DIR/effective_visibilities.rs:10:9
|
LL | extern "C" {}
| ^^^^^^^^^^^^^

error: Direct: pub(crate), Reexported: pub, Reachable: pub, ReachableThroughImplTrait: pub
--> $DIR/effective_visibilities.rs:12:9
--> $DIR/effective_visibilities.rs:13:9
|
LL | pub trait PubTrait {
| ^^^^^^^^^^^^^^^^^^

error: Direct: pub(self), Reexported: pub(self), Reachable: pub(self), ReachableThroughImplTrait: pub(self)
--> $DIR/effective_visibilities.rs:20:9
--> $DIR/effective_visibilities.rs:21:9
|
LL | struct PrivStruct;
| ^^^^^^^^^^^^^^^^^

error: Direct: pub(self), Reexported: pub(self), Reachable: pub(self), ReachableThroughImplTrait: pub(self)
--> $DIR/effective_visibilities.rs:20:9
--> $DIR/effective_visibilities.rs:21:9
|
LL | struct PrivStruct;
| ^^^^^^^^^^^^^^^^^

error: Direct: pub(crate), Reexported: pub, Reachable: pub, ReachableThroughImplTrait: pub
--> $DIR/effective_visibilities.rs:24:9
--> $DIR/effective_visibilities.rs:25:9
|
LL | pub union PubUnion {
| ^^^^^^^^^^^^^^^^^^

error: Direct: pub(self), Reexported: pub(self), Reachable: pub(self), ReachableThroughImplTrait: pub(self)
--> $DIR/effective_visibilities.rs:26:13
--> $DIR/effective_visibilities.rs:27:13
|
LL | a: u8,
| ^^^^^

error: Direct: pub(crate), Reexported: pub, Reachable: pub, ReachableThroughImplTrait: pub
--> $DIR/effective_visibilities.rs:28:13
--> $DIR/effective_visibilities.rs:29:13
|
LL | pub b: u8,
| ^^^^^^^^^

error: Direct: pub(crate), Reexported: pub, Reachable: pub, ReachableThroughImplTrait: pub
--> $DIR/effective_visibilities.rs:32:9
--> $DIR/effective_visibilities.rs:33:9
|
LL | pub enum Enum {
| ^^^^^^^^^^^^^

error: Direct: pub(crate), Reexported: pub, Reachable: pub, ReachableThroughImplTrait: pub
--> $DIR/effective_visibilities.rs:34:13
--> $DIR/effective_visibilities.rs:35:13
|
LL | A(
| ^

error: Direct: pub(crate), Reexported: pub, Reachable: pub, ReachableThroughImplTrait: pub
--> $DIR/effective_visibilities.rs:34:13
--> $DIR/effective_visibilities.rs:35:13
|
LL | A(
| ^

error: Direct: pub(crate), Reexported: pub, Reachable: pub, ReachableThroughImplTrait: pub
--> $DIR/effective_visibilities.rs:37:17
--> $DIR/effective_visibilities.rs:38:17
|
LL | PubUnion,
| ^^^^^^^^

error: not in the table
--> $DIR/effective_visibilities.rs:43:5
--> $DIR/effective_visibilities.rs:44:5
|
LL | macro_rules! none_macro {
| ^^^^^^^^^^^^^^^^^^^^^^^

error: Direct: pub(self), Reexported: pub, Reachable: pub, ReachableThroughImplTrait: pub
--> $DIR/effective_visibilities.rs:49:5
--> $DIR/effective_visibilities.rs:50:5
|
LL | macro_rules! public_macro {
| ^^^^^^^^^^^^^^^^^^^^^^^^^

error: Direct: pub(crate), Reexported: pub(crate), Reachable: pub, ReachableThroughImplTrait: pub
--> $DIR/effective_visibilities.rs:54:5
--> $DIR/effective_visibilities.rs:55:5
|
LL | pub struct ReachableStruct {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^

error: Direct: pub(crate), Reexported: pub(crate), Reachable: pub, ReachableThroughImplTrait: pub
--> $DIR/effective_visibilities.rs:56:9
--> $DIR/effective_visibilities.rs:57:9
|
LL | pub a: u8,
| ^^^^^^^^^

error: Direct: pub, Reexported: pub, Reachable: pub, ReachableThroughImplTrait: pub
--> $DIR/effective_visibilities.rs:61:9
--> $DIR/effective_visibilities.rs:62:9
|
LL | pub use outer::inner1;
| ^^^^^^^^^^^^^

error: Direct: pub(crate), Reexported: pub, Reachable: pub, ReachableThroughImplTrait: pub
--> $DIR/effective_visibilities.rs:67:5
--> $DIR/effective_visibilities.rs:68:5
|
LL | pub type HalfPublicImport = u8;
| ^^^^^^^^^^^^^^^^^^^^^^^^^

error: Direct: pub(crate), Reexported: pub(crate), Reachable: pub(crate), ReachableThroughImplTrait: pub(crate)
--> $DIR/effective_visibilities.rs:70:5
--> $DIR/effective_visibilities.rs:71:5
|
LL | pub(crate) const HalfPublicImport: u8 = 0;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: Direct: pub, Reexported: pub, Reachable: pub, ReachableThroughImplTrait: pub
--> $DIR/effective_visibilities.rs:74:9
--> $DIR/effective_visibilities.rs:75:9
|
LL | pub use half_public_import::HalfPublicImport;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: Direct: pub(crate), Reexported: pub, Reachable: pub, ReachableThroughImplTrait: pub
--> $DIR/effective_visibilities.rs:14:13
--> $DIR/effective_visibilities.rs:15:13
|
LL | const A: i32;
| ^^^^^^^^^^^^

error: Direct: pub(crate), Reexported: pub, Reachable: pub, ReachableThroughImplTrait: pub
--> $DIR/effective_visibilities.rs:16:13
--> $DIR/effective_visibilities.rs:17:13
|
LL | type B;
| ^^^^^^

error: aborting due to 23 previous errors
error: aborting due to 24 previous errors