Skip to content

Commit

Permalink
Rollup merge of rust-lang#109704 - petrochenkov:effvisclean, r=jackh726
Browse files Browse the repository at this point in the history 
resolve: Minor improvements to effective visibilities

See individual commits.
  • Loading branch information
@compiler-errors
compiler-errors committed Mar 30, 2023
2 parents 1ffb1af + b3bfeaf commit fbe7383
Show file tree
Hide file tree
Showing 5 changed files with 72 additions and 66 deletions.
6 changes: 5 additions & 1 deletion compiler/rustc_middle/src/middle/privacy.rs
View file
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
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
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
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
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

0 comments on commit fbe7383

Please sign in to comment.