Skip to content
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.

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

change AccessLevels representation #101713

Merged
merged 1 commit into from
Sep 18, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion compiler/rustc_error_messages/locales/en-US/privacy.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ privacy_in_public_interface = {$vis_descr} {$kind} `{$descr}` in public interfac
.label = can't leak {$vis_descr} {$kind}
.visibility_label = `{$descr}` declared as {$vis_descr}

privacy_report_access_level = {$descr}
privacy_report_effective_visibility = {$descr}

privacy_from_private_dep_in_public_interface =
{$kind} `{$descr}` from private dependency '{$krate}' in public interface
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_feature/src/builtin_attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -756,7 +756,7 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
// Internal attributes, Testing:
// ==========================================================================

rustc_attr!(TEST, rustc_access_level, Normal, template!(Word), WarnFollowing),
rustc_attr!(TEST, rustc_effective_visibility, Normal, template!(Word), WarnFollowing),
rustc_attr!(TEST, rustc_outlives, Normal, template!(Word), WarnFollowing),
rustc_attr!(TEST, rustc_capture_analysis, Normal, template!(Word), WarnFollowing),
rustc_attr!(TEST, rustc_insignificant_dtor, Normal, template!(Word), WarnFollowing),
Expand Down
93 changes: 87 additions & 6 deletions compiler/rustc_middle/src/middle/privacy.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
//! A pass that checks to make sure private fields and methods aren't used
//! outside their scopes. This pass will also generate a set of exported items
//! which are available for use externally when compiled as a library.

use crate::ty::Visibility;
petrochenkov marked this conversation as resolved.
Show resolved Hide resolved
use rustc_data_structures::fx::FxHashMap;
use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
use rustc_macros::HashStable;
Expand All @@ -27,26 +27,107 @@ pub enum AccessLevel {
Public,
}

#[derive(Clone, Copy, PartialEq, Eq, Debug, HashStable, Default)]
pub struct EffectiveVisibility {
public: Option<Visibility>,
exported: Option<Visibility>,
reachable: Option<Visibility>,
reachable_from_impl_trait: Option<Visibility>,
}
petrochenkov marked this conversation as resolved.
Show resolved Hide resolved

impl EffectiveVisibility {
pub fn get(&self, tag: AccessLevel) -> Option<&Visibility> {
match tag {
AccessLevel::Public => &self.public,
AccessLevel::Exported => &self.exported,
AccessLevel::Reachable => &self.reachable,
AccessLevel::ReachableFromImplTrait => &self.reachable_from_impl_trait,
}
petrochenkov marked this conversation as resolved.
Show resolved Hide resolved
.as_ref()
}

fn get_mut(&mut self, tag: AccessLevel) -> &mut Option<Visibility> {
match tag {
AccessLevel::Public => &mut self.public,
AccessLevel::Exported => &mut self.exported,
AccessLevel::Reachable => &mut self.reachable,
AccessLevel::ReachableFromImplTrait => &mut self.reachable_from_impl_trait,
}
}

pub fn is_public_at_level(&self, tag: AccessLevel) -> bool {
self.get(tag).map_or(false, |vis| vis.is_public())
}
}

/// Holds a map of accessibility levels for reachable HIR nodes.
#[derive(Debug, Clone)]
pub struct AccessLevels<Id = LocalDefId> {
pub map: FxHashMap<Id, AccessLevel>,
map: FxHashMap<Id, EffectiveVisibility>,
}

impl<Id: Hash + Eq> AccessLevels<Id> {
impl<Id: Hash + Eq + Copy> AccessLevels<Id> {
pub fn is_public_at_level(&self, id: Id, tag: AccessLevel) -> bool {
self.get_effective_vis(id)
.map_or(false, |effective_vis| effective_vis.is_public_at_level(tag))
}

/// See `AccessLevel::Reachable`.
pub fn is_reachable(&self, id: Id) -> bool {
self.map.get(&id) >= Some(&AccessLevel::Reachable)
self.is_public_at_level(id, AccessLevel::Reachable)
}

/// See `AccessLevel::Exported`.
pub fn is_exported(&self, id: Id) -> bool {
self.map.get(&id) >= Some(&AccessLevel::Exported)
self.is_public_at_level(id, AccessLevel::Exported)
}

/// See `AccessLevel::Public`.
pub fn is_public(&self, id: Id) -> bool {
self.map.get(&id) >= Some(&AccessLevel::Public)
self.is_public_at_level(id, AccessLevel::Public)
}

pub fn get_access_level(&self, id: Id) -> Option<AccessLevel> {
self.get_effective_vis(id).and_then(|effective_vis| {
for level in [
AccessLevel::Public,
AccessLevel::Exported,
AccessLevel::Reachable,
AccessLevel::ReachableFromImplTrait,
] {
if effective_vis.is_public_at_level(level) {
return Some(level);
}
}
None
})
}

pub fn set_access_level(&mut self, id: Id, tag: AccessLevel) {
let mut effective_vis = self.get_effective_vis(id).copied().unwrap_or_default();
for level in [
AccessLevel::Public,
AccessLevel::Exported,
AccessLevel::Reachable,
AccessLevel::ReachableFromImplTrait,
] {
if level <= tag {
*effective_vis.get_mut(level) = Some(Visibility::Public);
}
}
self.map.insert(id, effective_vis);
}

pub fn get_effective_vis(&self, id: Id) -> Option<&EffectiveVisibility> {
self.map.get(&id)
}

pub fn iter(&self) -> impl Iterator<Item = (&Id, &EffectiveVisibility)> {
self.map.iter()
}

pub fn map_id<OutId: Hash + Eq + Copy>(&self, f: impl Fn(Id) -> OutId) -> AccessLevels<OutId> {
AccessLevels { map: self.map.iter().map(|(k, v)| (f(*k), *v)).collect() }
}
}

Expand Down
11 changes: 4 additions & 7 deletions compiler/rustc_passes/src/dead.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use rustc_hir::def_id::{DefId, LocalDefId};
use rustc_hir::intravisit::{self, Visitor};
use rustc_hir::{Node, PatKind, TyKind};
use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags;
use rustc_middle::middle::privacy;
use rustc_middle::middle::privacy::AccessLevel;
use rustc_middle::ty::query::Providers;
use rustc_middle::ty::{self, DefIdTree, TyCtxt};
use rustc_session::lint;
Expand Down Expand Up @@ -619,13 +619,10 @@ fn create_and_seed_worklist<'tcx>(
// see `MarkSymbolVisitor::struct_constructors`
let mut struct_constructors = Default::default();
let mut worklist = access_levels
.map
.iter()
.filter_map(
|(&id, &level)| {
if level >= privacy::AccessLevel::Reachable { Some(id) } else { None }
},
)
.filter_map(|(&id, effective_vis)| {
effective_vis.is_public_at_level(AccessLevel::Reachable).then_some(id)
})
// Seed entry point
.chain(tcx.entry_fn(()).and_then(|(def_id, _)| def_id.as_local()))
.collect::<Vec<_>>();
Expand Down
10 changes: 8 additions & 2 deletions compiler/rustc_passes/src/reachable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use rustc_hir::def_id::{DefId, LocalDefId};
use rustc_hir::intravisit::{self, Visitor};
use rustc_hir::Node;
use rustc_middle::middle::codegen_fn_attrs::{CodegenFnAttrFlags, CodegenFnAttrs};
use rustc_middle::middle::privacy;
use rustc_middle::middle::privacy::{self, AccessLevel};
use rustc_middle::ty::query::Providers;
use rustc_middle::ty::{self, DefIdTree, TyCtxt};
use rustc_session::config::CrateType;
Expand Down Expand Up @@ -373,7 +373,13 @@ fn reachable_set<'tcx>(tcx: TyCtxt<'tcx>, (): ()) -> FxHashSet<LocalDefId> {
// If other crates link to us, they're going to expect to be able to
// use the lang items, so we need to be sure to mark them as
// exported.
reachable_context.worklist.extend(access_levels.map.keys());
reachable_context.worklist = access_levels
.iter()
.filter_map(|(&id, effective_vis)| {
effective_vis.is_public_at_level(AccessLevel::ReachableFromImplTrait).then_some(id)
})
.collect::<Vec<_>>();

for item in tcx.lang_items().items().iter() {
if let Some(def_id) = *item {
if let Some(def_id) = def_id.as_local() {
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_privacy/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,8 @@ pub struct InPublicInterface<'a> {
}

#[derive(SessionDiagnostic)]
#[diag(privacy::report_access_level)]
pub struct ReportAccessLevel {
#[diag(privacy::report_effective_visibility)]
pub struct ReportEffectiveVisibility {
#[primary_span]
pub span: Span,
pub descr: String,
Expand Down
37 changes: 29 additions & 8 deletions compiler/rustc_privacy/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ use std::{cmp, fmt, mem};

use errors::{
FieldIsPrivate, FieldIsPrivateLabel, FromPrivateDependencyInPublicInterface, InPublicInterface,
InPublicInterfaceTraits, ItemIsPrivate, PrivateInPublicLint, ReportAccessLevel,
InPublicInterfaceTraits, ItemIsPrivate, PrivateInPublicLint, ReportEffectiveVisibility,
UnnamedItemIsPrivate,
};

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

Expand Down Expand Up @@ -417,7 +417,7 @@ struct ReachEverythingInTheInterfaceVisitor<'a, 'tcx> {

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

fn update_with_hir_id(
Expand All @@ -434,7 +434,7 @@ impl<'tcx> EmbargoVisitor<'tcx> {
let old_level = self.get(def_id);
// Accessibility levels can only grow.
if level > old_level {
self.access_levels.map.insert(def_id, level.unwrap());
self.access_levels.set_access_level(def_id, level.unwrap());
self.changed = true;
level
} else {
Expand Down Expand Up @@ -915,10 +915,31 @@ pub struct TestReachabilityVisitor<'tcx, 'a> {

impl<'tcx, 'a> TestReachabilityVisitor<'tcx, 'a> {
fn access_level_diagnostic(&mut self, def_id: LocalDefId) {
if self.tcx.has_attr(def_id.to_def_id(), sym::rustc_access_level) {
let access_level = format!("{:?}", self.access_levels.map.get(&def_id));
let span = self.tcx.def_span(def_id.to_def_id());
self.tcx.sess.emit_err(ReportAccessLevel { span, descr: access_level });
let span = self.tcx.def_span(def_id.to_def_id());
if self.tcx.has_attr(def_id.to_def_id(), sym::rustc_effective_visibility) {
let mut error_msg = String::new();

let effective_vis =
self.access_levels.get_effective_vis(def_id).copied().unwrap_or_default();
for level in [
AccessLevel::Public,
AccessLevel::Exported,
AccessLevel::Reachable,
AccessLevel::ReachableFromImplTrait,
] {
let vis_str = match effective_vis.get(level) {
Some(ty::Visibility::Restricted(restricted_id)) => {
format!("pub({})", self.tcx.item_name(restricted_id.to_def_id()))
}
Some(ty::Visibility::Public) => "pub".to_string(),
None => "pub(self)".to_string(),
};
if level != AccessLevel::Public {
error_msg.push_str(", ");
}
error_msg.push_str(&format!("{:?}: {}", level, vis_str));
}
self.tcx.sess.emit_err(ReportEffectiveVisibility { span, descr: error_msg });
}
}
}
Expand Down
12 changes: 6 additions & 6 deletions compiler/rustc_resolve/src/access_levels.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ impl<'r, 'a> AccessLevelsVisitor<'r, 'a> {
/// This will also follow `use` chains (see PrivacyVisitor::set_import_binding_access_level).
fn set_bindings_access_level(&mut self, module_id: LocalDefId) {
assert!(self.r.module_map.contains_key(&&module_id.to_def_id()));
let module_level = self.r.access_levels.map.get(&module_id).copied();
let module_level = self.r.access_levels.get_access_level(module_id);
if !module_level.is_some() {
return;
}
Expand Down Expand Up @@ -103,9 +103,9 @@ impl<'r, 'a> AccessLevelsVisitor<'r, 'a> {
def_id: LocalDefId,
access_level: Option<AccessLevel>,
) -> Option<AccessLevel> {
let old_level = self.r.access_levels.map.get(&def_id).copied();
let old_level = self.r.access_levels.get_access_level(def_id);
if old_level < access_level {
self.r.access_levels.map.insert(def_id, access_level.unwrap());
self.r.access_levels.set_access_level(def_id, access_level.unwrap());
self.changed = true;
access_level
} else {
Expand All @@ -131,7 +131,7 @@ impl<'r, 'ast> Visitor<'ast> for AccessLevelsVisitor<'ast, 'r> {
// Foreign modules inherit level from parents.
ast::ItemKind::ForeignMod(..) => {
let parent_level =
self.r.access_levels.map.get(&self.r.local_parent(def_id)).copied();
self.r.access_levels.get_access_level(self.r.local_parent(def_id));
self.set_access_level(item.id, parent_level);
}

Expand All @@ -151,15 +151,15 @@ impl<'r, 'ast> Visitor<'ast> for AccessLevelsVisitor<'ast, 'r> {
self.set_bindings_access_level(def_id);
for variant in variants {
let variant_def_id = self.r.local_def_id(variant.id);
let variant_level = self.r.access_levels.map.get(&variant_def_id).copied();
let variant_level = self.r.access_levels.get_access_level(variant_def_id);
for field in variant.data.fields() {
self.set_access_level(field.id, variant_level);
}
}
}

ast::ItemKind::Struct(ref def, _) | ast::ItemKind::Union(ref def, _) => {
let inherited_level = self.r.access_levels.map.get(&def_id).copied();
let inherited_level = self.r.access_levels.get_access_level(def_id);
for field in def.fields() {
if field.vis.kind.is_pub() {
self.set_access_level(field.id, inherited_level);
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_span/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1211,7 +1211,6 @@ symbols! {
rust_eh_unregister_frames,
rust_oom,
rustc,
rustc_access_level,
rustc_allocator,
rustc_allocator_nounwind,
rustc_allocator_zeroed,
Expand Down Expand Up @@ -1239,6 +1238,7 @@ symbols! {
rustc_dump_program_clauses,
rustc_dump_user_substs,
rustc_dump_vtable,
rustc_effective_visibility,
rustc_error,
rustc_evaluate_where_clauses,
rustc_expected_cgu_reuse,
Expand Down
5 changes: 1 addition & 4 deletions src/librustdoc/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ use rustc_hir::intravisit::{self, Visitor};
use rustc_hir::{HirId, Path, TraitCandidate};
use rustc_interface::interface;
use rustc_middle::hir::nested_filter;
use rustc_middle::middle::privacy::AccessLevels;
use rustc_middle::ty::{ParamEnv, Ty, TyCtxt};
use rustc_resolve as resolve;
use rustc_session::config::{self, CrateType, ErrorOutputType};
Expand Down Expand Up @@ -364,9 +363,7 @@ pub(crate) fn run_global_ctxt(
.copied()
.filter(|&trait_def_id| tcx.trait_is_auto(trait_def_id))
.collect();
let access_levels = AccessLevels {
map: tcx.privacy_access_levels(()).map.iter().map(|(k, v)| (k.to_def_id(), *v)).collect(),
};
let access_levels = tcx.privacy_access_levels(()).map_id(Into::into);

let mut ctxt = DocContext {
tcx,
Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/visit_ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
} else {
// All items need to be handled here in case someone wishes to link
// to them with intra-doc links
self.cx.cache.access_levels.map.insert(did, AccessLevel::Public);
self.cx.cache.access_levels.set_access_level(did, AccessLevel::Public);
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/librustdoc/visit_lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,10 @@ impl<'a, 'tcx> LibEmbargoVisitor<'a, 'tcx> {
fn update(&mut self, did: DefId, level: Option<AccessLevel>) -> Option<AccessLevel> {
let is_hidden = self.tcx.is_doc_hidden(did);

let old_level = self.access_levels.map.get(&did).cloned();
let old_level = self.access_levels.get_access_level(did);
// Accessibility levels can only grow
if level > old_level && !is_hidden {
self.access_levels.map.insert(did, level.unwrap());
self.access_levels.set_access_level(did, level.unwrap());
level
} else {
old_level
Expand Down
Loading