Skip to content

Commit 7f57787

Browse files
TaKO8KiTaKO8Ki
authored
Rollup merge of #100147 - Bryanskiy:private-in-public, r=petrochenkov
optimization of access level table construction Refactoring which was mentioned in #87487
2 parents 5371128 + d0884c4 commit 7f57787

File tree

10 files changed

+327
-125
lines changed

10 files changed

+327
-125
lines changed

‎compiler/rustc_error_messages/locales/en-US/privacy.ftl

+2
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ privacy_in_public_interface = {$vis_descr} {$kind} `{$descr}` in public interfac
1111
.label = can't leak {$vis_descr} {$kind}
1212
.visibility_label = `{$descr}` declared as {$vis_descr}
1313
14+
privacy_invalid_access_level = {$descr}
15+
1416
privacy_from_private_dep_in_public_interface =
1517
{$kind} `{$descr}` from private dependency '{$krate}' in public interface
1618

‎compiler/rustc_feature/src/builtin_attrs.rs

+1
Original file line numberDiff line numberDiff line change
@@ -762,6 +762,7 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
762762
// Internal attributes, Testing:
763763
// ==========================================================================
764764

765+
rustc_attr!(TEST, rustc_access_level, Normal, template!(Word), WarnFollowing),
765766
rustc_attr!(TEST, rustc_outlives, Normal, template!(Word), WarnFollowing),
766767
rustc_attr!(TEST, rustc_capture_analysis, Normal, template!(Word), WarnFollowing),
767768
rustc_attr!(TEST, rustc_insignificant_dtor, Normal, template!(Word), WarnFollowing),

‎compiler/rustc_privacy/src/errors.rs

+8
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,14 @@ pub struct InPublicInterface<'a> {
7575
pub vis_span: Span,
7676
}
7777

78+
#[derive(SessionDiagnostic)]
79+
#[error(privacy::invalid_access_level)]
80+
pub struct ReportAccessLevel {
81+
#[primary_span]
82+
pub span: Span,
83+
pub descr: String,
84+
}
85+
7886
#[derive(LintDiagnostic)]
7987
#[lint(privacy::from_private_dep_in_public_interface)]
8088
pub struct FromPrivateDependencyInPublicInterface<'a> {

‎compiler/rustc_privacy/src/lib.rs

+61-2
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ use rustc_middle::ty::{self, Const, DefIdTree, GenericParamDefKind};
3030
use rustc_middle::ty::{TraitRef, Ty, TyCtxt, TypeSuperVisitable, TypeVisitable, TypeVisitor};
3131
use rustc_session::lint;
3232
use rustc_span::hygiene::Transparency;
33-
use rustc_span::symbol::{kw, Ident};
33+
use rustc_span::symbol::{kw, sym, Ident};
3434
use rustc_span::Span;
3535

3636
use std::marker::PhantomData;
@@ -39,7 +39,8 @@ use std::{cmp, fmt, mem};
3939

4040
use errors::{
4141
FieldIsPrivate, FieldIsPrivateLabel, FromPrivateDependencyInPublicInterface, InPublicInterface,
42-
InPublicInterfaceTraits, ItemIsPrivate, PrivateInPublicLint, UnnamedItemIsPrivate,
42+
InPublicInterfaceTraits, ItemIsPrivate, PrivateInPublicLint, ReportAccessLevel,
43+
UnnamedItemIsPrivate,
4344
};
4445

4546
////////////////////////////////////////////////////////////////////////////////
@@ -904,6 +905,60 @@ impl<'tcx> DefIdVisitor<'tcx> for ReachEverythingInTheInterfaceVisitor<'_, 'tcx>
904905
}
905906
}
906907

908+
////////////////////////////////////////////////////////////////////////////////
909+
/// Visitor, used for AccessLevels table checking
910+
////////////////////////////////////////////////////////////////////////////////
911+
pub struct TestReachabilityVisitor<'tcx> {
912+
tcx: TyCtxt<'tcx>,
913+
access_levels: &'tcx AccessLevels,
914+
}
915+
916+
impl<'tcx> TestReachabilityVisitor<'tcx> {
917+
fn access_level_diagnostic(&mut self, def_id: LocalDefId) {
918+
if self.tcx.has_attr(def_id.to_def_id(), sym::rustc_access_level) {
919+
let access_level = format!("{:?}", self.access_levels.map.get(&def_id));
920+
let span = self.tcx.def_span(def_id.to_def_id());
921+
self.tcx.sess.emit_err(ReportAccessLevel { span, descr: access_level });
922+
}
923+
}
924+
}
925+
926+
impl<'tcx> Visitor<'tcx> for TestReachabilityVisitor<'tcx> {
927+
fn visit_item(&mut self, item: &'tcx hir::Item<'tcx>) {
928+
self.access_level_diagnostic(item.def_id);
929+
930+
match item.kind {
931+
hir::ItemKind::Enum(ref def, _) => {
932+
for variant in def.variants.iter() {
933+
let variant_id = self.tcx.hir().local_def_id(variant.id);
934+
self.access_level_diagnostic(variant_id);
935+
for field in variant.data.fields() {
936+
let def_id = self.tcx.hir().local_def_id(field.hir_id);
937+
self.access_level_diagnostic(def_id);
938+
}
939+
}
940+
}
941+
hir::ItemKind::Struct(ref def, _) | hir::ItemKind::Union(ref def, _) => {
942+
for field in def.fields() {
943+
let def_id = self.tcx.hir().local_def_id(field.hir_id);
944+
self.access_level_diagnostic(def_id);
945+
}
946+
}
947+
_ => {}
948+
}
949+
}
950+
951+
fn visit_trait_item(&mut self, item: &'tcx hir::TraitItem<'tcx>) {
952+
self.access_level_diagnostic(item.def_id);
953+
}
954+
fn visit_impl_item(&mut self, item: &'tcx hir::ImplItem<'tcx>) {
955+
self.access_level_diagnostic(item.def_id);
956+
}
957+
fn visit_foreign_item(&mut self, item: &'tcx hir::ForeignItem<'tcx>) {
958+
self.access_level_diagnostic(item.def_id);
959+
}
960+
}
961+
907962
//////////////////////////////////////////////////////////////////////////////////////
908963
/// Name privacy visitor, checks privacy and reports violations.
909964
/// Most of name privacy checks are performed during the main resolution phase,
@@ -2043,6 +2098,10 @@ fn privacy_access_levels(tcx: TyCtxt<'_>, (): ()) -> &AccessLevels {
20432098
}
20442099
}
20452100

2101+
let mut check_visitor =
2102+
TestReachabilityVisitor { tcx, access_levels: &tcx.resolutions(()).access_levels };
2103+
tcx.hir().visit_all_item_likes_in_crate(&mut check_visitor);
2104+
20462105
tcx.arena.alloc(visitor.access_levels)
20472106
}
20482107

‎compiler/rustc_resolve/src/access_levels.rs

+53-105
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,21 @@
1+
use crate::imports::ImportKind;
2+
use crate::NameBinding;
3+
use crate::NameBindingKind;
4+
use crate::Resolver;
15
use rustc_ast::ast;
26
use rustc_ast::visit;
37
use rustc_ast::visit::Visitor;
48
use rustc_ast::Crate;
59
use rustc_ast::EnumDef;
6-
use rustc_ast::ForeignMod;
710
use rustc_ast::NodeId;
811
use rustc_hir::def_id::LocalDefId;
912
use rustc_hir::def_id::CRATE_DEF_ID;
1013
use rustc_middle::middle::privacy::AccessLevel;
11-
use rustc_middle::ty::Visibility;
14+
use rustc_middle::ty::DefIdTree;
1215
use rustc_span::sym;
1316

14-
use crate::imports::ImportKind;
15-
use crate::BindingKey;
16-
use crate::NameBinding;
17-
use crate::NameBindingKind;
18-
use crate::Resolver;
19-
2017
pub struct AccessLevelsVisitor<'r, 'a> {
2118
r: &'r mut Resolver<'a>,
22-
prev_level: Option<AccessLevel>,
2319
changed: bool,
2420
}
2521

@@ -28,11 +24,10 @@ impl<'r, 'a> AccessLevelsVisitor<'r, 'a> {
2824
/// For now, this doesn't resolve macros (FIXME) and cannot resolve Impl, as we
2925
/// need access to a TyCtxt for that.
3026
pub fn compute_access_levels<'c>(r: &'r mut Resolver<'a>, krate: &'c Crate) {
31-
let mut visitor =
32-
AccessLevelsVisitor { r, changed: false, prev_level: Some(AccessLevel::Public) };
27+
let mut visitor = AccessLevelsVisitor { r, changed: false };
3328

3429
visitor.set_access_level_def_id(CRATE_DEF_ID, Some(AccessLevel::Public));
35-
visitor.set_exports_access_level(CRATE_DEF_ID);
30+
visitor.set_bindings_access_level(CRATE_DEF_ID);
3631

3732
while visitor.changed {
3833
visitor.reset();
@@ -44,15 +39,17 @@ impl<'r, 'a> AccessLevelsVisitor<'r, 'a> {
4439

4540
fn reset(&mut self) {
4641
self.changed = false;
47-
self.prev_level = Some(AccessLevel::Public);
4842
}
4943

50-
/// Update the access level of the exports of the given module accordingly. The module access
44+
/// Update the access level of the bindings in the given module accordingly. The module access
5145
/// level has to be Exported or Public.
5246
/// This will also follow `use` chains (see PrivacyVisitor::set_import_binding_access_level).
53-
fn set_exports_access_level(&mut self, module_id: LocalDefId) {
47+
fn set_bindings_access_level(&mut self, module_id: LocalDefId) {
5448
assert!(self.r.module_map.contains_key(&&module_id.to_def_id()));
55-
49+
let module_level = self.r.access_levels.map.get(&module_id).copied();
50+
if !module_level.is_some() {
51+
return;
52+
}
5653
// Set the given binding access level to `AccessLevel::Public` and
5754
// sets the rest of the `use` chain to `AccessLevel::Exported` until
5855
// we hit the actual exported item.
@@ -72,28 +69,20 @@ impl<'r, 'a> AccessLevelsVisitor<'r, 'a> {
7269
}
7370
};
7471

75-
let module_level = self.r.access_levels.map.get(&module_id).copied();
76-
assert!(module_level >= Some(AccessLevel::Exported));
77-
78-
if let Some(exports) = self.r.reexport_map.get(&module_id) {
79-
let pub_exports = exports
80-
.iter()
81-
.filter(|ex| ex.vis == Visibility::Public)
82-
.cloned()
83-
.collect::<Vec<_>>();
84-
85-
let module = self.r.get_module(module_id.to_def_id()).unwrap();
86-
for export in pub_exports.into_iter() {
87-
if let Some(export_def_id) = export.res.opt_def_id().and_then(|id| id.as_local()) {
88-
self.set_access_level_def_id(export_def_id, Some(AccessLevel::Exported));
89-
}
90-
91-
if let Some(ns) = export.res.ns() {
92-
let key = BindingKey { ident: export.ident, ns, disambiguator: 0 };
93-
let name_res = self.r.resolution(module, key);
94-
if let Some(binding) = name_res.borrow().binding() {
95-
set_import_binding_access_level(self, binding, module_level)
96-
}
72+
let module = self.r.get_module(module_id.to_def_id()).unwrap();
73+
let resolutions = self.r.resolutions(module);
74+
75+
for (.., name_resolution) in resolutions.borrow().iter() {
76+
if let Some(binding) = name_resolution.borrow().binding() && binding.vis.is_public() {
77+
let access_level = match self.r.is_reexport(binding) {
78+
Some(_) => {
79+
set_import_binding_access_level(self, binding, module_level);
80+
Some(AccessLevel::Exported)
81+
},
82+
None => module_level,
83+
};
84+
if let Some(def_id) = binding.res().opt_def_id().and_then(|id| id.as_local()) {
85+
self.set_access_level_def_id(def_id, access_level);
9786
}
9887
}
9988
}
@@ -127,97 +116,59 @@ impl<'r, 'a> AccessLevelsVisitor<'r, 'a> {
127116

128117
impl<'r, 'ast> Visitor<'ast> for AccessLevelsVisitor<'ast, 'r> {
129118
fn visit_item(&mut self, item: &'ast ast::Item) {
130-
let inherited_item_level = match item.kind {
119+
let def_id = self.r.local_def_id(item.id);
120+
// Set access level of nested items.
121+
// If it's a mod, also make the visitor walk all of its items
122+
match item.kind {
131123
// Resolved in rustc_privacy when types are available
132124
ast::ItemKind::Impl(..) => return,
133125

134-
// Only exported `macro_rules!` items are public, but they always are
135-
ast::ItemKind::MacroDef(ref macro_def) if macro_def.macro_rules => {
136-
let is_macro_export =
137-
item.attrs.iter().any(|attr| attr.has_name(sym::macro_export));
138-
if is_macro_export { Some(AccessLevel::Public) } else { None }
139-
}
140-
141-
// Foreign modules inherit level from parents.
142-
ast::ItemKind::ForeignMod(..) => self.prev_level,
143-
144-
// Other `pub` items inherit levels from parents.
145-
ast::ItemKind::ExternCrate(..)
146-
| ast::ItemKind::Use(..)
147-
| ast::ItemKind::Static(..)
148-
| ast::ItemKind::Const(..)
149-
| ast::ItemKind::Fn(..)
150-
| ast::ItemKind::Mod(..)
151-
| ast::ItemKind::GlobalAsm(..)
152-
| ast::ItemKind::TyAlias(..)
153-
| ast::ItemKind::Enum(..)
154-
| ast::ItemKind::Struct(..)
155-
| ast::ItemKind::Union(..)
156-
| ast::ItemKind::Trait(..)
157-
| ast::ItemKind::TraitAlias(..)
158-
| ast::ItemKind::MacroDef(..) => {
159-
if item.vis.kind.is_pub() {
160-
self.prev_level
161-
} else {
162-
None
163-
}
164-
}
165-
166126
// Should be unreachable at this stage
167127
ast::ItemKind::MacCall(..) => panic!(
168128
"ast::ItemKind::MacCall encountered, this should not anymore appear at this stage"
169129
),
170-
};
171130

172-
let access_level = self.set_access_level(item.id, inherited_item_level);
131+
// Foreign modules inherit level from parents.
132+
ast::ItemKind::ForeignMod(..) => {
133+
let parent_level =
134+
self.r.access_levels.map.get(&self.r.local_parent(def_id)).copied();
135+
self.set_access_level(item.id, parent_level);
136+
}
173137

174-
// Set access level of nested items.
175-
// If it's a mod, also make the visitor walk all of its items
176-
match item.kind {
177-
ast::ItemKind::Mod(..) => {
178-
if access_level.is_some() {
179-
self.set_exports_access_level(self.r.local_def_id(item.id));
138+
// Only exported `macro_rules!` items are public, but they always are
139+
ast::ItemKind::MacroDef(ref macro_def) if macro_def.macro_rules => {
140+
if item.attrs.iter().any(|attr| attr.has_name(sym::macro_export)) {
141+
self.set_access_level(item.id, Some(AccessLevel::Public));
180142
}
143+
}
181144

182-
let orig_level = std::mem::replace(&mut self.prev_level, access_level);
145+
ast::ItemKind::Mod(..) => {
146+
self.set_bindings_access_level(def_id);
183147
visit::walk_item(self, item);
184-
self.prev_level = orig_level;
185148
}
186149

187-
ast::ItemKind::ForeignMod(ForeignMod { ref items, .. }) => {
188-
for nested in items {
189-
if nested.vis.kind.is_pub() {
190-
self.set_access_level(nested.id, access_level);
191-
}
192-
}
193-
}
194150
ast::ItemKind::Enum(EnumDef { ref variants }, _) => {
151+
self.set_bindings_access_level(def_id);
195152
for variant in variants {
196-
let variant_level = self.set_access_level(variant.id, access_level);
197-
if let Some(ctor_id) = variant.data.ctor_id() {
198-
self.set_access_level(ctor_id, access_level);
199-
}
200-
153+
let variant_def_id = self.r.local_def_id(variant.id);
154+
let variant_level = self.r.access_levels.map.get(&variant_def_id).copied();
201155
for field in variant.data.fields() {
202156
self.set_access_level(field.id, variant_level);
203157
}
204158
}
205159
}
206-
ast::ItemKind::Struct(ref def, _) | ast::ItemKind::Union(ref def, _) => {
207-
if let Some(ctor_id) = def.ctor_id() {
208-
self.set_access_level(ctor_id, access_level);
209-
}
210160

161+
ast::ItemKind::Struct(ref def, _) | ast::ItemKind::Union(ref def, _) => {
162+
let inherited_level = self.r.access_levels.map.get(&def_id).copied();
211163
for field in def.fields() {
212164
if field.vis.kind.is_pub() {
213-
self.set_access_level(field.id, access_level);
165+
self.set_access_level(field.id, inherited_level);
214166
}
215167
}
216168
}
217-
ast::ItemKind::Trait(ref trait_kind) => {
218-
for nested in trait_kind.items.iter() {
219-
self.set_access_level(nested.id, access_level);
220-
}
169+
170+
ast::ItemKind::Trait(..) => {
171+
self.set_bindings_access_level(def_id);
221172
}
222173

223174
ast::ItemKind::ExternCrate(..)
@@ -229,9 +180,6 @@ impl<'r, 'ast> Visitor<'ast> for AccessLevelsVisitor<'ast, 'r> {
229180
| ast::ItemKind::TraitAlias(..)
230181
| ast::ItemKind::MacroDef(..)
231182
| ast::ItemKind::Fn(..) => return,
232-
233-
// Unreachable kinds
234-
ast::ItemKind::Impl(..) | ast::ItemKind::MacCall(..) => unreachable!(),
235183
}
236184
}
237185
}

0 commit comments

Comments
 (0)