Skip to content

Commit a6b34cd

Browse files
matthiaskrgrmatthiaskrgr
authored
Rollup merge of #101713 - Bryanskiy:AccessLevels, r=petrochenkov
change AccessLevels representation Part of RFC (#48054). This patch implements effective visibility table with basic methods and change AccessLevels table representation according to it. r? ``@petrochenkov``
2 parents 8362321 + d7b9221 commit a6b34cd

File tree

14 files changed

+249
-155
lines changed

14 files changed

+249
-155
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ 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_report_access_level = {$descr}
14+
privacy_report_effective_visibility = {$descr}
1515
1616
privacy_from_private_dep_in_public_interface =
1717
{$kind} `{$descr}` from private dependency '{$krate}' in public interface

‎compiler/rustc_feature/src/builtin_attrs.rs

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

765-
rustc_attr!(TEST, rustc_access_level, Normal, template!(Word), WarnFollowing),
765+
rustc_attr!(TEST, rustc_effective_visibility, Normal, template!(Word), WarnFollowing),
766766
rustc_attr!(TEST, rustc_outlives, Normal, template!(Word), WarnFollowing),
767767
rustc_attr!(TEST, rustc_capture_analysis, Normal, template!(Word), WarnFollowing),
768768
rustc_attr!(TEST, rustc_insignificant_dtor, Normal, template!(Word), WarnFollowing),

‎compiler/rustc_middle/src/middle/privacy.rs

Lines changed: 87 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
//! A pass that checks to make sure private fields and methods aren't used
22
//! outside their scopes. This pass will also generate a set of exported items
33
//! which are available for use externally when compiled as a library.
4-
4+
use crate::ty::Visibility;
55
use rustc_data_structures::fx::FxHashMap;
66
use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
77
use rustc_macros::HashStable;
@@ -27,26 +27,107 @@ pub enum AccessLevel {
2727
Public,
2828
}
2929

30+
#[derive(Clone, Copy, PartialEq, Eq, Debug, HashStable, Default)]
31+
pub struct EffectiveVisibility {
32+
public: Option<Visibility>,
33+
exported: Option<Visibility>,
34+
reachable: Option<Visibility>,
35+
reachable_from_impl_trait: Option<Visibility>,
36+
}
37+
38+
impl EffectiveVisibility {
39+
pub fn get(&self, tag: AccessLevel) -> Option<&Visibility> {
40+
match tag {
41+
AccessLevel::Public => &self.public,
42+
AccessLevel::Exported => &self.exported,
43+
AccessLevel::Reachable => &self.reachable,
44+
AccessLevel::ReachableFromImplTrait => &self.reachable_from_impl_trait,
45+
}
46+
.as_ref()
47+
}
48+
49+
fn get_mut(&mut self, tag: AccessLevel) -> &mut Option<Visibility> {
50+
match tag {
51+
AccessLevel::Public => &mut self.public,
52+
AccessLevel::Exported => &mut self.exported,
53+
AccessLevel::Reachable => &mut self.reachable,
54+
AccessLevel::ReachableFromImplTrait => &mut self.reachable_from_impl_trait,
55+
}
56+
}
57+
58+
pub fn is_public_at_level(&self, tag: AccessLevel) -> bool {
59+
self.get(tag).map_or(false, |vis| vis.is_public())
60+
}
61+
}
62+
3063
/// Holds a map of accessibility levels for reachable HIR nodes.
3164
#[derive(Debug, Clone)]
3265
pub struct AccessLevels<Id = LocalDefId> {
33-
pub map: FxHashMap<Id, AccessLevel>,
66+
map: FxHashMap<Id, EffectiveVisibility>,
3467
}
3568

36-
impl<Id: Hash + Eq> AccessLevels<Id> {
69+
impl<Id: Hash + Eq + Copy> AccessLevels<Id> {
70+
pub fn is_public_at_level(&self, id: Id, tag: AccessLevel) -> bool {
71+
self.get_effective_vis(id)
72+
.map_or(false, |effective_vis| effective_vis.is_public_at_level(tag))
73+
}
74+
3775
/// See `AccessLevel::Reachable`.
3876
pub fn is_reachable(&self, id: Id) -> bool {
39-
self.map.get(&id) >= Some(&AccessLevel::Reachable)
77+
self.is_public_at_level(id, AccessLevel::Reachable)
4078
}
4179

4280
/// See `AccessLevel::Exported`.
4381
pub fn is_exported(&self, id: Id) -> bool {
44-
self.map.get(&id) >= Some(&AccessLevel::Exported)
82+
self.is_public_at_level(id, AccessLevel::Exported)
4583
}
4684

4785
/// See `AccessLevel::Public`.
4886
pub fn is_public(&self, id: Id) -> bool {
49-
self.map.get(&id) >= Some(&AccessLevel::Public)
87+
self.is_public_at_level(id, AccessLevel::Public)
88+
}
89+
90+
pub fn get_access_level(&self, id: Id) -> Option<AccessLevel> {
91+
self.get_effective_vis(id).and_then(|effective_vis| {
92+
for level in [
93+
AccessLevel::Public,
94+
AccessLevel::Exported,
95+
AccessLevel::Reachable,
96+
AccessLevel::ReachableFromImplTrait,
97+
] {
98+
if effective_vis.is_public_at_level(level) {
99+
return Some(level);
100+
}
101+
}
102+
None
103+
})
104+
}
105+
106+
pub fn set_access_level(&mut self, id: Id, tag: AccessLevel) {
107+
let mut effective_vis = self.get_effective_vis(id).copied().unwrap_or_default();
108+
for level in [
109+
AccessLevel::Public,
110+
AccessLevel::Exported,
111+
AccessLevel::Reachable,
112+
AccessLevel::ReachableFromImplTrait,
113+
] {
114+
if level <= tag {
115+
*effective_vis.get_mut(level) = Some(Visibility::Public);
116+
}
117+
}
118+
self.map.insert(id, effective_vis);
119+
}
120+
121+
pub fn get_effective_vis(&self, id: Id) -> Option<&EffectiveVisibility> {
122+
self.map.get(&id)
123+
}
124+
125+
pub fn iter(&self) -> impl Iterator<Item = (&Id, &EffectiveVisibility)> {
126+
self.map.iter()
127+
}
128+
129+
pub fn map_id<OutId: Hash + Eq + Copy>(&self, f: impl Fn(Id) -> OutId) -> AccessLevels<OutId> {
130+
AccessLevels { map: self.map.iter().map(|(k, v)| (f(*k), *v)).collect() }
50131
}
51132
}
52133

‎compiler/rustc_passes/src/dead.rs

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ use rustc_hir::def_id::{DefId, LocalDefId};
1111
use rustc_hir::intravisit::{self, Visitor};
1212
use rustc_hir::{Node, PatKind, TyKind};
1313
use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags;
14-
use rustc_middle::middle::privacy;
14+
use rustc_middle::middle::privacy::AccessLevel;
1515
use rustc_middle::ty::query::Providers;
1616
use rustc_middle::ty::{self, DefIdTree, TyCtxt};
1717
use rustc_session::lint;
@@ -619,13 +619,10 @@ fn create_and_seed_worklist<'tcx>(
619619
// see `MarkSymbolVisitor::struct_constructors`
620620
let mut struct_constructors = Default::default();
621621
let mut worklist = access_levels
622-
.map
623622
.iter()
624-
.filter_map(
625-
|(&id, &level)| {
626-
if level >= privacy::AccessLevel::Reachable { Some(id) } else { None }
627-
},
628-
)
623+
.filter_map(|(&id, effective_vis)| {
624+
effective_vis.is_public_at_level(AccessLevel::Reachable).then_some(id)
625+
})
629626
// Seed entry point
630627
.chain(tcx.entry_fn(()).and_then(|(def_id, _)| def_id.as_local()))
631628
.collect::<Vec<_>>();

‎compiler/rustc_passes/src/reachable.rs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ use rustc_hir::def_id::{DefId, LocalDefId};
1212
use rustc_hir::intravisit::{self, Visitor};
1313
use rustc_hir::Node;
1414
use rustc_middle::middle::codegen_fn_attrs::{CodegenFnAttrFlags, CodegenFnAttrs};
15-
use rustc_middle::middle::privacy;
15+
use rustc_middle::middle::privacy::{self, AccessLevel};
1616
use rustc_middle::ty::query::Providers;
1717
use rustc_middle::ty::{self, DefIdTree, TyCtxt};
1818
use rustc_session::config::CrateType;
@@ -373,7 +373,13 @@ fn reachable_set<'tcx>(tcx: TyCtxt<'tcx>, (): ()) -> FxHashSet<LocalDefId> {
373373
// If other crates link to us, they're going to expect to be able to
374374
// use the lang items, so we need to be sure to mark them as
375375
// exported.
376-
reachable_context.worklist.extend(access_levels.map.keys());
376+
reachable_context.worklist = access_levels
377+
.iter()
378+
.filter_map(|(&id, effective_vis)| {
379+
effective_vis.is_public_at_level(AccessLevel::ReachableFromImplTrait).then_some(id)
380+
})
381+
.collect::<Vec<_>>();
382+
377383
for item in tcx.lang_items().items().iter() {
378384
if let Some(def_id) = *item {
379385
if let Some(def_id) = def_id.as_local() {

‎compiler/rustc_privacy/src/errors.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,8 +76,8 @@ pub struct InPublicInterface<'a> {
7676
}
7777

7878
#[derive(SessionDiagnostic)]
79-
#[diag(privacy::report_access_level)]
80-
pub struct ReportAccessLevel {
79+
#[diag(privacy::report_effective_visibility)]
80+
pub struct ReportEffectiveVisibility {
8181
#[primary_span]
8282
pub span: Span,
8383
pub descr: String,

‎compiler/rustc_privacy/src/lib.rs

Lines changed: 29 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ use std::{cmp, fmt, mem};
4242

4343
use errors::{
4444
FieldIsPrivate, FieldIsPrivateLabel, FromPrivateDependencyInPublicInterface, InPublicInterface,
45-
InPublicInterfaceTraits, ItemIsPrivate, PrivateInPublicLint, ReportAccessLevel,
45+
InPublicInterfaceTraits, ItemIsPrivate, PrivateInPublicLint, ReportEffectiveVisibility,
4646
UnnamedItemIsPrivate,
4747
};
4848

@@ -376,7 +376,7 @@ impl VisibilityLike for Option<AccessLevel> {
376376
// (which require reaching the `DefId`s in them).
377377
const SHALLOW: bool = true;
378378
fn new_min(find: &FindMin<'_, '_, Self>, def_id: LocalDefId) -> Self {
379-
cmp::min(find.access_levels.map.get(&def_id).copied(), find.min)
379+
cmp::min(find.access_levels.get_access_level(def_id), find.min)
380380
}
381381
}
382382

@@ -416,7 +416,7 @@ struct ReachEverythingInTheInterfaceVisitor<'a, 'tcx> {
416416

417417
impl<'tcx> EmbargoVisitor<'tcx> {
418418
fn get(&self, def_id: LocalDefId) -> Option<AccessLevel> {
419-
self.access_levels.map.get(&def_id).copied()
419+
self.access_levels.get_access_level(def_id)
420420
}
421421

422422
fn update_with_hir_id(
@@ -433,7 +433,7 @@ impl<'tcx> EmbargoVisitor<'tcx> {
433433
let old_level = self.get(def_id);
434434
// Accessibility levels can only grow.
435435
if level > old_level {
436-
self.access_levels.map.insert(def_id, level.unwrap());
436+
self.access_levels.set_access_level(def_id, level.unwrap());
437437
self.changed = true;
438438
level
439439
} else {
@@ -914,10 +914,31 @@ pub struct TestReachabilityVisitor<'tcx, 'a> {
914914

915915
impl<'tcx, 'a> TestReachabilityVisitor<'tcx, 'a> {
916916
fn access_level_diagnostic(&mut self, def_id: LocalDefId) {
917-
if self.tcx.has_attr(def_id.to_def_id(), sym::rustc_access_level) {
918-
let access_level = format!("{:?}", self.access_levels.map.get(&def_id));
919-
let span = self.tcx.def_span(def_id.to_def_id());
920-
self.tcx.sess.emit_err(ReportAccessLevel { span, descr: access_level });
917+
let span = self.tcx.def_span(def_id.to_def_id());
918+
if self.tcx.has_attr(def_id.to_def_id(), sym::rustc_effective_visibility) {
919+
let mut error_msg = String::new();
920+
921+
let effective_vis =
922+
self.access_levels.get_effective_vis(def_id).copied().unwrap_or_default();
923+
for level in [
924+
AccessLevel::Public,
925+
AccessLevel::Exported,
926+
AccessLevel::Reachable,
927+
AccessLevel::ReachableFromImplTrait,
928+
] {
929+
let vis_str = match effective_vis.get(level) {
930+
Some(ty::Visibility::Restricted(restricted_id)) => {
931+
format!("pub({})", self.tcx.item_name(restricted_id.to_def_id()))
932+
}
933+
Some(ty::Visibility::Public) => "pub".to_string(),
934+
None => "pub(self)".to_string(),
935+
};
936+
if level != AccessLevel::Public {
937+
error_msg.push_str(", ");
938+
}
939+
error_msg.push_str(&format!("{:?}: {}", level, vis_str));
940+
}
941+
self.tcx.sess.emit_err(ReportEffectiveVisibility { span, descr: error_msg });
921942
}
922943
}
923944
}

‎compiler/rustc_resolve/src/access_levels.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ impl<'r, 'a> AccessLevelsVisitor<'r, 'a> {
4646
/// This will also follow `use` chains (see PrivacyVisitor::set_import_binding_access_level).
4747
fn set_bindings_access_level(&mut self, module_id: LocalDefId) {
4848
assert!(self.r.module_map.contains_key(&&module_id.to_def_id()));
49-
let module_level = self.r.access_levels.map.get(&module_id).copied();
49+
let module_level = self.r.access_levels.get_access_level(module_id);
5050
if !module_level.is_some() {
5151
return;
5252
}
@@ -103,9 +103,9 @@ impl<'r, 'a> AccessLevelsVisitor<'r, 'a> {
103103
def_id: LocalDefId,
104104
access_level: Option<AccessLevel>,
105105
) -> Option<AccessLevel> {
106-
let old_level = self.r.access_levels.map.get(&def_id).copied();
106+
let old_level = self.r.access_levels.get_access_level(def_id);
107107
if old_level < access_level {
108-
self.r.access_levels.map.insert(def_id, access_level.unwrap());
108+
self.r.access_levels.set_access_level(def_id, access_level.unwrap());
109109
self.changed = true;
110110
access_level
111111
} else {
@@ -131,7 +131,7 @@ impl<'r, 'ast> Visitor<'ast> for AccessLevelsVisitor<'ast, 'r> {
131131
// Foreign modules inherit level from parents.
132132
ast::ItemKind::ForeignMod(..) => {
133133
let parent_level =
134-
self.r.access_levels.map.get(&self.r.local_parent(def_id)).copied();
134+
self.r.access_levels.get_access_level(self.r.local_parent(def_id));
135135
self.set_access_level(item.id, parent_level);
136136
}
137137

@@ -151,15 +151,15 @@ impl<'r, 'ast> Visitor<'ast> for AccessLevelsVisitor<'ast, 'r> {
151151
self.set_bindings_access_level(def_id);
152152
for variant in variants {
153153
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();
154+
let variant_level = self.r.access_levels.get_access_level(variant_def_id);
155155
for field in variant.data.fields() {
156156
self.set_access_level(field.id, variant_level);
157157
}
158158
}
159159
}
160160

161161
ast::ItemKind::Struct(ref def, _) | ast::ItemKind::Union(ref def, _) => {
162-
let inherited_level = self.r.access_levels.map.get(&def_id).copied();
162+
let inherited_level = self.r.access_levels.get_access_level(def_id);
163163
for field in def.fields() {
164164
if field.vis.kind.is_pub() {
165165
self.set_access_level(field.id, inherited_level);

‎compiler/rustc_span/src/symbol.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1214,7 +1214,6 @@ symbols! {
12141214
rust_eh_unregister_frames,
12151215
rust_oom,
12161216
rustc,
1217-
rustc_access_level,
12181217
rustc_allocator,
12191218
rustc_allocator_nounwind,
12201219
rustc_allocator_zeroed,
@@ -1242,6 +1241,7 @@ symbols! {
12421241
rustc_dump_program_clauses,
12431242
rustc_dump_user_substs,
12441243
rustc_dump_vtable,
1244+
rustc_effective_visibility,
12451245
rustc_error,
12461246
rustc_evaluate_where_clauses,
12471247
rustc_expected_cgu_reuse,

‎src/librustdoc/core.rs

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ use rustc_hir::intravisit::{self, Visitor};
1010
use rustc_hir::{HirId, Path, TraitCandidate};
1111
use rustc_interface::interface;
1212
use rustc_middle::hir::nested_filter;
13-
use rustc_middle::middle::privacy::AccessLevels;
1413
use rustc_middle::ty::{ParamEnv, Ty, TyCtxt};
1514
use rustc_resolve as resolve;
1615
use rustc_session::config::{self, CrateType, ErrorOutputType};
@@ -364,9 +363,7 @@ pub(crate) fn run_global_ctxt(
364363
.copied()
365364
.filter(|&trait_def_id| tcx.trait_is_auto(trait_def_id))
366365
.collect();
367-
let access_levels = AccessLevels {
368-
map: tcx.privacy_access_levels(()).map.iter().map(|(k, v)| (k.to_def_id(), *v)).collect(),
369-
};
366+
let access_levels = tcx.privacy_access_levels(()).map_id(Into::into);
370367

371368
let mut ctxt = DocContext {
372369
tcx,

‎src/librustdoc/visit_ast.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -230,7 +230,7 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
230230
} else {
231231
// All items need to be handled here in case someone wishes to link
232232
// to them with intra-doc links
233-
self.cx.cache.access_levels.map.insert(did, AccessLevel::Public);
233+
self.cx.cache.access_levels.set_access_level(did, AccessLevel::Public);
234234
}
235235
}
236236
}

‎src/librustdoc/visit_lib.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,10 +38,10 @@ impl<'a, 'tcx> LibEmbargoVisitor<'a, 'tcx> {
3838
fn update(&mut self, did: DefId, level: Option<AccessLevel>) -> Option<AccessLevel> {
3939
let is_hidden = self.tcx.is_doc_hidden(did);
4040

41-
let old_level = self.access_levels.map.get(&did).cloned();
41+
let old_level = self.access_levels.get_access_level(did);
4242
// Accessibility levels can only grow
4343
if level > old_level && !is_hidden {
44-
self.access_levels.map.insert(did, level.unwrap());
44+
self.access_levels.set_access_level(did, level.unwrap());
4545
level
4646
} else {
4747
old_level

0 commit comments

Comments
 (0)