Skip to content

Commit fbe7383

Browse files
Rollup merge of #109704 - petrochenkov:effvisclean, r=jackh726
resolve: Minor improvements to effective visibilities See individual commits.
2 parents 1ffb1af + b3bfeaf commit fbe7383

File tree

5 files changed

+72
-66
lines changed

5 files changed

+72
-66
lines changed

compiler/rustc_middle/src/middle/privacy.rs

+5-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use rustc_data_structures::fx::FxHashMap;
66
use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
77
use rustc_macros::HashStable;
88
use rustc_query_system::ich::StableHashingContext;
9-
use rustc_span::def_id::LocalDefId;
9+
use rustc_span::def_id::{LocalDefId, CRATE_DEF_ID};
1010
use std::hash::Hash;
1111

1212
/// Represents the levels of effective visibility an item can have.
@@ -107,6 +107,10 @@ impl EffectiveVisibilities {
107107
})
108108
}
109109

110+
pub fn update_root(&mut self) {
111+
self.map.insert(CRATE_DEF_ID, EffectiveVisibility::from_vis(Visibility::Public));
112+
}
113+
110114
// FIXME: Share code with `fn update`.
111115
pub fn update_eff_vis(
112116
&mut self,

compiler/rustc_privacy/src/lib.rs

+1
Original file line numberDiff line numberDiff line change
@@ -2149,6 +2149,7 @@ fn effective_visibilities(tcx: TyCtxt<'_>, (): ()) -> &EffectiveVisibilities {
21492149

21502150
let mut check_visitor =
21512151
TestReachabilityVisitor { tcx, effective_visibilities: &visitor.effective_visibilities };
2152+
check_visitor.effective_visibility_diagnostic(CRATE_DEF_ID);
21522153
tcx.hir().visit_all_item_likes_in_crate(&mut check_visitor);
21532154

21542155
tcx.arena.alloc(visitor.effective_visibilities)

compiler/rustc_resolve/src/effective_visibilities.rs

+29-41
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ impl Resolver<'_, '_> {
6161
// For mod items `nearest_normal_mod` returns its argument, but we actually need its parent.
6262
let normal_mod_id = self.nearest_normal_mod(def_id);
6363
if normal_mod_id == def_id {
64-
self.tcx.opt_local_parent(def_id).map_or(Visibility::Public, Visibility::Restricted)
64+
Visibility::Restricted(self.tcx.local_parent(def_id))
6565
} else {
6666
Visibility::Restricted(normal_mod_id)
6767
}
@@ -80,12 +80,11 @@ impl<'r, 'a, 'tcx> EffectiveVisibilitiesVisitor<'r, 'a, 'tcx> {
8080
r,
8181
def_effective_visibilities: Default::default(),
8282
import_effective_visibilities: Default::default(),
83-
current_private_vis: Visibility::Public,
83+
current_private_vis: Visibility::Restricted(CRATE_DEF_ID),
8484
changed: false,
8585
};
8686

87-
visitor.update(CRATE_DEF_ID, CRATE_DEF_ID);
88-
visitor.current_private_vis = Visibility::Restricted(CRATE_DEF_ID);
87+
visitor.def_effective_visibilities.update_root();
8988
visitor.set_bindings_effective_visibilities(CRATE_DEF_ID);
9089

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

126125
for (_, name_resolution) in resolutions.borrow().iter() {
127126
if let Some(mut binding) = name_resolution.borrow().binding() {
128-
if !binding.is_ambiguity() {
129-
// Set the given effective visibility level to `Level::Direct` and
130-
// sets the rest of the `use` chain to `Level::Reexported` until
131-
// we hit the actual exported item.
132-
let mut parent_id = ParentId::Def(module_id);
133-
while let NameBindingKind::Import { binding: nested_binding, .. } = binding.kind
134-
{
135-
let binding_id = ImportId::new_unchecked(binding);
136-
self.update_import(binding_id, parent_id);
137-
138-
parent_id = ParentId::Import(binding_id);
139-
binding = nested_binding;
140-
}
141-
142-
if let Some(def_id) = binding.res().opt_def_id().and_then(|id| id.as_local()) {
143-
self.update_def(def_id, binding.vis.expect_local(), parent_id);
127+
// Set the given effective visibility level to `Level::Direct` and
128+
// sets the rest of the `use` chain to `Level::Reexported` until
129+
// we hit the actual exported item.
130+
//
131+
// If the binding is ambiguous, put the root ambiguity binding and all reexports
132+
// leading to it into the table. They are used by the `ambiguous_glob_reexports`
133+
// lint. For all bindings added to the table this way `is_ambiguity` returns true.
134+
let mut parent_id = ParentId::Def(module_id);
135+
while let NameBindingKind::Import { binding: nested_binding, .. } = binding.kind {
136+
let binding_id = ImportId::new_unchecked(binding);
137+
self.update_import(binding_id, parent_id);
138+
139+
if binding.ambiguity.is_some() {
140+
// Stop at the root ambiguity, further bindings in the chain should not
141+
// be reexported because the root ambiguity blocks any access to them.
142+
// (Those further bindings are most likely not ambiguities themselves.)
143+
break;
144144
}
145-
} else {
146-
// Put the root ambiguity binding and all reexports leading to it into the
147-
// table. They are used by the `ambiguous_glob_reexports` lint. For all
148-
// bindings added to the table here `is_ambiguity` returns true.
149-
let mut parent_id = ParentId::Def(module_id);
150-
while let NameBindingKind::Import { binding: nested_binding, .. } = binding.kind
151-
{
152-
let binding_id = ImportId::new_unchecked(binding);
153-
self.update_import(binding_id, parent_id);
154145

155-
if binding.ambiguity.is_some() {
156-
// Stop at the root ambiguity, further bindings in the chain should not
157-
// be reexported because the root ambiguity blocks any access to them.
158-
// (Those further bindings are most likely not ambiguities themselves.)
159-
break;
160-
}
146+
parent_id = ParentId::Import(binding_id);
147+
binding = nested_binding;
148+
}
161149

162-
parent_id = ParentId::Import(binding_id);
163-
binding = nested_binding;
164-
}
150+
if binding.ambiguity.is_none()
151+
&& let Some(def_id) = binding.res().opt_def_id().and_then(|id| id.as_local()) {
152+
self.update_def(def_id, binding.vis.expect_local(), parent_id);
165153
}
166154
}
167155
}
@@ -213,7 +201,7 @@ impl<'r, 'a, 'tcx> EffectiveVisibilitiesVisitor<'r, 'a, 'tcx> {
213201
);
214202
}
215203

216-
fn update(&mut self, def_id: LocalDefId, parent_id: LocalDefId) {
204+
fn update_field(&mut self, def_id: LocalDefId, parent_id: LocalDefId) {
217205
self.update_def(def_id, self.r.visibilities[&def_id], ParentId::Def(parent_id));
218206
}
219207
}
@@ -245,14 +233,14 @@ impl<'r, 'ast, 'tcx> Visitor<'ast> for EffectiveVisibilitiesVisitor<'ast, 'r, 't
245233
for variant in variants {
246234
let variant_def_id = self.r.local_def_id(variant.id);
247235
for field in variant.data.fields() {
248-
self.update(self.r.local_def_id(field.id), variant_def_id);
236+
self.update_field(self.r.local_def_id(field.id), variant_def_id);
249237
}
250238
}
251239
}
252240

253241
ast::ItemKind::Struct(ref def, _) | ast::ItemKind::Union(ref def, _) => {
254242
for field in def.fields() {
255-
self.update(self.r.local_def_id(field.id), def_id);
243+
self.update_field(self.r.local_def_id(field.id), def_id);
256244
}
257245
}
258246

tests/ui/privacy/effective_visibilities.rs

+1
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
#![rustc_effective_visibility] //~ ERROR Direct: pub, Reexported: pub, Reachable: pub, ReachableThroughImplTrait: pub
12
#![feature(rustc_attrs)]
23

34
#[rustc_effective_visibility]
+36-24
Original file line numberDiff line numberDiff line change
@@ -1,140 +1,152 @@
1+
error: Direct: pub, Reexported: pub, Reachable: pub, ReachableThroughImplTrait: pub
2+
--> $DIR/effective_visibilities.rs:1:1
3+
|
4+
LL | / #![rustc_effective_visibility]
5+
LL | | #![feature(rustc_attrs)]
6+
LL | |
7+
LL | | #[rustc_effective_visibility]
8+
... |
9+
LL | |
10+
LL | | fn main() {}
11+
| |____________^
12+
113
error: Direct: pub(crate), Reexported: pub(crate), Reachable: pub(crate), ReachableThroughImplTrait: pub(crate)
2-
--> $DIR/effective_visibilities.rs:4:1
14+
--> $DIR/effective_visibilities.rs:5:1
315
|
416
LL | mod outer {
517
| ^^^^^^^^^
618

719
error: Direct: pub(crate), Reexported: pub, Reachable: pub, ReachableThroughImplTrait: pub
8-
--> $DIR/effective_visibilities.rs:6:5
20+
--> $DIR/effective_visibilities.rs:7:5
921
|
1022
LL | pub mod inner1 {
1123
| ^^^^^^^^^^^^^^
1224

1325
error: not in the table
14-
--> $DIR/effective_visibilities.rs:9:9
26+
--> $DIR/effective_visibilities.rs:10:9
1527
|
1628
LL | extern "C" {}
1729
| ^^^^^^^^^^^^^
1830

1931
error: Direct: pub(crate), Reexported: pub, Reachable: pub, ReachableThroughImplTrait: pub
20-
--> $DIR/effective_visibilities.rs:12:9
32+
--> $DIR/effective_visibilities.rs:13:9
2133
|
2234
LL | pub trait PubTrait {
2335
| ^^^^^^^^^^^^^^^^^^
2436

2537
error: Direct: pub(self), Reexported: pub(self), Reachable: pub(self), ReachableThroughImplTrait: pub(self)
26-
--> $DIR/effective_visibilities.rs:20:9
38+
--> $DIR/effective_visibilities.rs:21:9
2739
|
2840
LL | struct PrivStruct;
2941
| ^^^^^^^^^^^^^^^^^
3042

3143
error: Direct: pub(self), Reexported: pub(self), Reachable: pub(self), ReachableThroughImplTrait: pub(self)
32-
--> $DIR/effective_visibilities.rs:20:9
44+
--> $DIR/effective_visibilities.rs:21:9
3345
|
3446
LL | struct PrivStruct;
3547
| ^^^^^^^^^^^^^^^^^
3648

3749
error: Direct: pub(crate), Reexported: pub, Reachable: pub, ReachableThroughImplTrait: pub
38-
--> $DIR/effective_visibilities.rs:24:9
50+
--> $DIR/effective_visibilities.rs:25:9
3951
|
4052
LL | pub union PubUnion {
4153
| ^^^^^^^^^^^^^^^^^^
4254

4355
error: Direct: pub(self), Reexported: pub(self), Reachable: pub(self), ReachableThroughImplTrait: pub(self)
44-
--> $DIR/effective_visibilities.rs:26:13
56+
--> $DIR/effective_visibilities.rs:27:13
4557
|
4658
LL | a: u8,
4759
| ^^^^^
4860

4961
error: Direct: pub(crate), Reexported: pub, Reachable: pub, ReachableThroughImplTrait: pub
50-
--> $DIR/effective_visibilities.rs:28:13
62+
--> $DIR/effective_visibilities.rs:29:13
5163
|
5264
LL | pub b: u8,
5365
| ^^^^^^^^^
5466

5567
error: Direct: pub(crate), Reexported: pub, Reachable: pub, ReachableThroughImplTrait: pub
56-
--> $DIR/effective_visibilities.rs:32:9
68+
--> $DIR/effective_visibilities.rs:33:9
5769
|
5870
LL | pub enum Enum {
5971
| ^^^^^^^^^^^^^
6072

6173
error: Direct: pub(crate), Reexported: pub, Reachable: pub, ReachableThroughImplTrait: pub
62-
--> $DIR/effective_visibilities.rs:34:13
74+
--> $DIR/effective_visibilities.rs:35:13
6375
|
6476
LL | A(
6577
| ^
6678

6779
error: Direct: pub(crate), Reexported: pub, Reachable: pub, ReachableThroughImplTrait: pub
68-
--> $DIR/effective_visibilities.rs:34:13
80+
--> $DIR/effective_visibilities.rs:35:13
6981
|
7082
LL | A(
7183
| ^
7284

7385
error: Direct: pub(crate), Reexported: pub, Reachable: pub, ReachableThroughImplTrait: pub
74-
--> $DIR/effective_visibilities.rs:37:17
86+
--> $DIR/effective_visibilities.rs:38:17
7587
|
7688
LL | PubUnion,
7789
| ^^^^^^^^
7890

7991
error: not in the table
80-
--> $DIR/effective_visibilities.rs:43:5
92+
--> $DIR/effective_visibilities.rs:44:5
8193
|
8294
LL | macro_rules! none_macro {
8395
| ^^^^^^^^^^^^^^^^^^^^^^^
8496

8597
error: Direct: pub(self), Reexported: pub, Reachable: pub, ReachableThroughImplTrait: pub
86-
--> $DIR/effective_visibilities.rs:49:5
98+
--> $DIR/effective_visibilities.rs:50:5
8799
|
88100
LL | macro_rules! public_macro {
89101
| ^^^^^^^^^^^^^^^^^^^^^^^^^
90102

91103
error: Direct: pub(crate), Reexported: pub(crate), Reachable: pub, ReachableThroughImplTrait: pub
92-
--> $DIR/effective_visibilities.rs:54:5
104+
--> $DIR/effective_visibilities.rs:55:5
93105
|
94106
LL | pub struct ReachableStruct {
95107
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
96108

97109
error: Direct: pub(crate), Reexported: pub(crate), Reachable: pub, ReachableThroughImplTrait: pub
98-
--> $DIR/effective_visibilities.rs:56:9
110+
--> $DIR/effective_visibilities.rs:57:9
99111
|
100112
LL | pub a: u8,
101113
| ^^^^^^^^^
102114

103115
error: Direct: pub, Reexported: pub, Reachable: pub, ReachableThroughImplTrait: pub
104-
--> $DIR/effective_visibilities.rs:61:9
116+
--> $DIR/effective_visibilities.rs:62:9
105117
|
106118
LL | pub use outer::inner1;
107119
| ^^^^^^^^^^^^^
108120

109121
error: Direct: pub(crate), Reexported: pub, Reachable: pub, ReachableThroughImplTrait: pub
110-
--> $DIR/effective_visibilities.rs:67:5
122+
--> $DIR/effective_visibilities.rs:68:5
111123
|
112124
LL | pub type HalfPublicImport = u8;
113125
| ^^^^^^^^^^^^^^^^^^^^^^^^^
114126

115127
error: Direct: pub(crate), Reexported: pub(crate), Reachable: pub(crate), ReachableThroughImplTrait: pub(crate)
116-
--> $DIR/effective_visibilities.rs:70:5
128+
--> $DIR/effective_visibilities.rs:71:5
117129
|
118130
LL | pub(crate) const HalfPublicImport: u8 = 0;
119131
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
120132

121133
error: Direct: pub, Reexported: pub, Reachable: pub, ReachableThroughImplTrait: pub
122-
--> $DIR/effective_visibilities.rs:74:9
134+
--> $DIR/effective_visibilities.rs:75:9
123135
|
124136
LL | pub use half_public_import::HalfPublicImport;
125137
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
126138

127139
error: Direct: pub(crate), Reexported: pub, Reachable: pub, ReachableThroughImplTrait: pub
128-
--> $DIR/effective_visibilities.rs:14:13
140+
--> $DIR/effective_visibilities.rs:15:13
129141
|
130142
LL | const A: i32;
131143
| ^^^^^^^^^^^^
132144

133145
error: Direct: pub(crate), Reexported: pub, Reachable: pub, ReachableThroughImplTrait: pub
134-
--> $DIR/effective_visibilities.rs:16:13
146+
--> $DIR/effective_visibilities.rs:17:13
135147
|
136148
LL | type B;
137149
| ^^^^^^
138150

139-
error: aborting due to 23 previous errors
151+
error: aborting due to 24 previous errors
140152

0 commit comments

Comments
 (0)