Rollup merge of #100147 - Bryanskiy:private-in-public, r=petrochenkov

optimization of access level table construction

Refactoring which was mentioned in #87487
This commit is contained in:
Guillaume Gomez 2022-09-02 11:34:48 +02:00 committed by GitHub
commit 1aaf9aec95
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 326 additions and 125 deletions

View File

@ -11,6 +11,8 @@ 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_from_private_dep_in_public_interface =
{$kind} `{$descr}` from private dependency '{$krate}' in public interface

View File

@ -758,6 +758,7 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
// Internal attributes, Testing:
// ==========================================================================
rustc_attr!(TEST, rustc_access_level, 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),

View File

@ -75,6 +75,14 @@ pub struct InPublicInterface<'a> {
pub vis_span: Span,
}
#[derive(SessionDiagnostic)]
#[diag(privacy::report_access_level)]
pub struct ReportAccessLevel {
#[primary_span]
pub span: Span,
pub descr: String,
}
#[derive(LintDiagnostic)]
#[diag(privacy::from_private_dep_in_public_interface)]
pub struct FromPrivateDependencyInPublicInterface<'a> {

View File

@ -33,7 +33,7 @@ use rustc_middle::ty::{self, Const, DefIdTree, GenericParamDefKind};
use rustc_middle::ty::{TraitRef, Ty, TyCtxt, TypeSuperVisitable, TypeVisitable, TypeVisitor};
use rustc_session::lint;
use rustc_span::hygiene::Transparency;
use rustc_span::symbol::{kw, Ident};
use rustc_span::symbol::{kw, sym, Ident};
use rustc_span::Span;
use std::marker::PhantomData;
@ -42,7 +42,8 @@ use std::{cmp, fmt, mem};
use errors::{
FieldIsPrivate, FieldIsPrivateLabel, FromPrivateDependencyInPublicInterface, InPublicInterface,
InPublicInterfaceTraits, ItemIsPrivate, PrivateInPublicLint, UnnamedItemIsPrivate,
InPublicInterfaceTraits, ItemIsPrivate, PrivateInPublicLint, ReportAccessLevel,
UnnamedItemIsPrivate,
};
////////////////////////////////////////////////////////////////////////////////
@ -907,6 +908,60 @@ impl<'tcx> DefIdVisitor<'tcx> for ReachEverythingInTheInterfaceVisitor<'_, 'tcx>
}
}
////////////////////////////////////////////////////////////////////////////////
/// Visitor, used for AccessLevels table checking
////////////////////////////////////////////////////////////////////////////////
pub struct TestReachabilityVisitor<'tcx, 'a> {
tcx: TyCtxt<'tcx>,
access_levels: &'a AccessLevels,
}
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 });
}
}
}
impl<'tcx, 'a> Visitor<'tcx> for TestReachabilityVisitor<'tcx, 'a> {
fn visit_item(&mut self, item: &'tcx hir::Item<'tcx>) {
self.access_level_diagnostic(item.def_id);
match item.kind {
hir::ItemKind::Enum(ref def, _) => {
for variant in def.variants.iter() {
let variant_id = self.tcx.hir().local_def_id(variant.id);
self.access_level_diagnostic(variant_id);
for field in variant.data.fields() {
let def_id = self.tcx.hir().local_def_id(field.hir_id);
self.access_level_diagnostic(def_id);
}
}
}
hir::ItemKind::Struct(ref def, _) | hir::ItemKind::Union(ref def, _) => {
for field in def.fields() {
let def_id = self.tcx.hir().local_def_id(field.hir_id);
self.access_level_diagnostic(def_id);
}
}
_ => {}
}
}
fn visit_trait_item(&mut self, item: &'tcx hir::TraitItem<'tcx>) {
self.access_level_diagnostic(item.def_id);
}
fn visit_impl_item(&mut self, item: &'tcx hir::ImplItem<'tcx>) {
self.access_level_diagnostic(item.def_id);
}
fn visit_foreign_item(&mut self, item: &'tcx hir::ForeignItem<'tcx>) {
self.access_level_diagnostic(item.def_id);
}
}
//////////////////////////////////////////////////////////////////////////////////////
/// Name privacy visitor, checks privacy and reports violations.
/// Most of name privacy checks are performed during the main resolution phase,
@ -2045,6 +2100,9 @@ fn privacy_access_levels(tcx: TyCtxt<'_>, (): ()) -> &AccessLevels {
}
}
let mut check_visitor = TestReachabilityVisitor { tcx, access_levels: &visitor.access_levels };
tcx.hir().visit_all_item_likes_in_crate(&mut check_visitor);
tcx.arena.alloc(visitor.access_levels)
}

View File

@ -1,25 +1,21 @@
use crate::imports::ImportKind;
use crate::NameBinding;
use crate::NameBindingKind;
use crate::Resolver;
use rustc_ast::ast;
use rustc_ast::visit;
use rustc_ast::visit::Visitor;
use rustc_ast::Crate;
use rustc_ast::EnumDef;
use rustc_ast::ForeignMod;
use rustc_ast::NodeId;
use rustc_hir::def_id::LocalDefId;
use rustc_hir::def_id::CRATE_DEF_ID;
use rustc_middle::middle::privacy::AccessLevel;
use rustc_middle::ty::Visibility;
use rustc_middle::ty::DefIdTree;
use rustc_span::sym;
use crate::imports::ImportKind;
use crate::BindingKey;
use crate::NameBinding;
use crate::NameBindingKind;
use crate::Resolver;
pub struct AccessLevelsVisitor<'r, 'a> {
r: &'r mut Resolver<'a>,
prev_level: Option<AccessLevel>,
changed: bool,
}
@ -28,11 +24,10 @@ impl<'r, 'a> AccessLevelsVisitor<'r, 'a> {
/// For now, this doesn't resolve macros (FIXME) and cannot resolve Impl, as we
/// need access to a TyCtxt for that.
pub fn compute_access_levels<'c>(r: &'r mut Resolver<'a>, krate: &'c Crate) {
let mut visitor =
AccessLevelsVisitor { r, changed: false, prev_level: Some(AccessLevel::Public) };
let mut visitor = AccessLevelsVisitor { r, changed: false };
visitor.set_access_level_def_id(CRATE_DEF_ID, Some(AccessLevel::Public));
visitor.set_exports_access_level(CRATE_DEF_ID);
visitor.set_bindings_access_level(CRATE_DEF_ID);
while visitor.changed {
visitor.reset();
@ -44,15 +39,17 @@ impl<'r, 'a> AccessLevelsVisitor<'r, 'a> {
fn reset(&mut self) {
self.changed = false;
self.prev_level = Some(AccessLevel::Public);
}
/// Update the access level of the exports of the given module accordingly. The module access
/// Update the access level of the bindings in the given module accordingly. The module access
/// level has to be Exported or Public.
/// This will also follow `use` chains (see PrivacyVisitor::set_import_binding_access_level).
fn set_exports_access_level(&mut self, module_id: LocalDefId) {
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();
if !module_level.is_some() {
return;
}
// Set the given binding access level to `AccessLevel::Public` and
// sets the rest of the `use` chain to `AccessLevel::Exported` until
// we hit the actual exported item.
@ -72,28 +69,20 @@ impl<'r, 'a> AccessLevelsVisitor<'r, 'a> {
}
};
let module_level = self.r.access_levels.map.get(&module_id).copied();
assert!(module_level >= Some(AccessLevel::Exported));
let module = self.r.get_module(module_id.to_def_id()).unwrap();
let resolutions = self.r.resolutions(module);
if let Some(exports) = self.r.reexport_map.get(&module_id) {
let pub_exports = exports
.iter()
.filter(|ex| ex.vis == Visibility::Public)
.cloned()
.collect::<Vec<_>>();
let module = self.r.get_module(module_id.to_def_id()).unwrap();
for export in pub_exports.into_iter() {
if let Some(export_def_id) = export.res.opt_def_id().and_then(|id| id.as_local()) {
self.set_access_level_def_id(export_def_id, Some(AccessLevel::Exported));
}
if let Some(ns) = export.res.ns() {
let key = BindingKey { ident: export.ident, ns, disambiguator: 0 };
let name_res = self.r.resolution(module, key);
if let Some(binding) = name_res.borrow().binding() {
set_import_binding_access_level(self, binding, module_level)
}
for (.., name_resolution) in resolutions.borrow().iter() {
if let Some(binding) = name_resolution.borrow().binding() && binding.vis.is_public() && !binding.is_ambiguity() {
let access_level = match binding.is_import() {
true => {
set_import_binding_access_level(self, binding, module_level);
Some(AccessLevel::Exported)
},
false => module_level,
};
if let Some(def_id) = binding.res().opt_def_id().and_then(|id| id.as_local()) {
self.set_access_level_def_id(def_id, access_level);
}
}
}
@ -127,97 +116,59 @@ impl<'r, 'a> AccessLevelsVisitor<'r, 'a> {
impl<'r, 'ast> Visitor<'ast> for AccessLevelsVisitor<'ast, 'r> {
fn visit_item(&mut self, item: &'ast ast::Item) {
let inherited_item_level = match item.kind {
let def_id = self.r.local_def_id(item.id);
// Set access level of nested items.
// If it's a mod, also make the visitor walk all of its items
match item.kind {
// Resolved in rustc_privacy when types are available
ast::ItemKind::Impl(..) => return,
// Only exported `macro_rules!` items are public, but they always are
ast::ItemKind::MacroDef(ref macro_def) if macro_def.macro_rules => {
let is_macro_export =
item.attrs.iter().any(|attr| attr.has_name(sym::macro_export));
if is_macro_export { Some(AccessLevel::Public) } else { None }
}
// Foreign modules inherit level from parents.
ast::ItemKind::ForeignMod(..) => self.prev_level,
// Other `pub` items inherit levels from parents.
ast::ItemKind::ExternCrate(..)
| ast::ItemKind::Use(..)
| ast::ItemKind::Static(..)
| ast::ItemKind::Const(..)
| ast::ItemKind::Fn(..)
| ast::ItemKind::Mod(..)
| ast::ItemKind::GlobalAsm(..)
| ast::ItemKind::TyAlias(..)
| ast::ItemKind::Enum(..)
| ast::ItemKind::Struct(..)
| ast::ItemKind::Union(..)
| ast::ItemKind::Trait(..)
| ast::ItemKind::TraitAlias(..)
| ast::ItemKind::MacroDef(..) => {
if item.vis.kind.is_pub() {
self.prev_level
} else {
None
}
}
// Should be unreachable at this stage
ast::ItemKind::MacCall(..) => panic!(
"ast::ItemKind::MacCall encountered, this should not anymore appear at this stage"
),
};
let access_level = self.set_access_level(item.id, inherited_item_level);
// 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.set_access_level(item.id, parent_level);
}
// Only exported `macro_rules!` items are public, but they always are
ast::ItemKind::MacroDef(ref macro_def) if macro_def.macro_rules => {
if item.attrs.iter().any(|attr| attr.has_name(sym::macro_export)) {
self.set_access_level(item.id, Some(AccessLevel::Public));
}
}
// Set access level of nested items.
// If it's a mod, also make the visitor walk all of its items
match item.kind {
ast::ItemKind::Mod(..) => {
if access_level.is_some() {
self.set_exports_access_level(self.r.local_def_id(item.id));
}
let orig_level = std::mem::replace(&mut self.prev_level, access_level);
self.set_bindings_access_level(def_id);
visit::walk_item(self, item);
self.prev_level = orig_level;
}
ast::ItemKind::ForeignMod(ForeignMod { ref items, .. }) => {
for nested in items {
if nested.vis.kind.is_pub() {
self.set_access_level(nested.id, access_level);
}
}
}
ast::ItemKind::Enum(EnumDef { ref variants }, _) => {
self.set_bindings_access_level(def_id);
for variant in variants {
let variant_level = self.set_access_level(variant.id, access_level);
if let Some(ctor_id) = variant.data.ctor_id() {
self.set_access_level(ctor_id, access_level);
}
let variant_def_id = self.r.local_def_id(variant.id);
let variant_level = self.r.access_levels.map.get(&variant_def_id).copied();
for field in variant.data.fields() {
self.set_access_level(field.id, variant_level);
}
}
}
ast::ItemKind::Struct(ref def, _) | ast::ItemKind::Union(ref def, _) => {
if let Some(ctor_id) = def.ctor_id() {
self.set_access_level(ctor_id, access_level);
}
ast::ItemKind::Struct(ref def, _) | ast::ItemKind::Union(ref def, _) => {
let inherited_level = self.r.access_levels.map.get(&def_id).copied();
for field in def.fields() {
if field.vis.kind.is_pub() {
self.set_access_level(field.id, access_level);
self.set_access_level(field.id, inherited_level);
}
}
}
ast::ItemKind::Trait(ref trait_kind) => {
for nested in trait_kind.items.iter() {
self.set_access_level(nested.id, access_level);
}
ast::ItemKind::Trait(..) => {
self.set_bindings_access_level(def_id);
}
ast::ItemKind::ExternCrate(..)
@ -229,9 +180,6 @@ impl<'r, 'ast> Visitor<'ast> for AccessLevelsVisitor<'ast, 'r> {
| ast::ItemKind::TraitAlias(..)
| ast::ItemKind::MacroDef(..)
| ast::ItemKind::Fn(..) => return,
// Unreachable kinds
ast::ItemKind::Impl(..) | ast::ItemKind::MacCall(..) => unreachable!(),
}
}
}

View File

@ -1133,24 +1133,15 @@ impl<'a, 'b> ImportResolver<'a, 'b> {
if let Some(def_id) = module.opt_def_id() {
let mut reexports = Vec::new();
module.for_each_child(self.r, |_, ident, _, binding| {
// FIXME: Consider changing the binding inserted by `#[macro_export] macro_rules`
// into the crate root to actual `NameBindingKind::Import`.
if binding.is_import()
|| matches!(binding.kind, NameBindingKind::Res(_, _is_macro_export @ true))
{
let res = binding.res().expect_non_local();
// Ambiguous imports are treated as errors at this point and are
// not exposed to other crates (see #36837 for more details).
if res != def::Res::Err && !binding.is_ambiguity() {
reexports.push(ModChild {
ident,
res,
vis: binding.vis,
span: binding.span,
macro_rules: false,
});
}
module.for_each_child(self.r, |this, ident, _, binding| {
if let Some(res) = this.is_reexport(binding) {
reexports.push(ModChild {
ident,
res,
vis: binding.vis,
span: binding.span,
macro_rules: false,
});
}
});

View File

@ -2020,6 +2020,24 @@ impl<'a> Resolver<'a> {
}
self.main_def = Some(MainDefinition { res, is_import, span });
}
// Items that go to reexport table encoded to metadata and visible through it to other crates.
fn is_reexport(&self, binding: &NameBinding<'a>) -> Option<def::Res<!>> {
// FIXME: Consider changing the binding inserted by `#[macro_export] macro_rules`
// into the crate root to actual `NameBindingKind::Import`.
if binding.is_import()
|| matches!(binding.kind, NameBindingKind::Res(_, _is_macro_export @ true))
{
let res = binding.res().expect_non_local();
// Ambiguous imports are treated as errors at this point and are
// not exposed to other crates (see #36837 for more details).
if res != def::Res::Err && !binding.is_ambiguity() {
return Some(res);
}
}
return None;
}
}
fn names_to_string(names: &[Symbol]) -> String {

View File

@ -1209,6 +1209,7 @@ symbols! {
rust_eh_unregister_frames,
rust_oom,
rustc,
rustc_access_level,
rustc_allocator,
rustc_allocator_nounwind,
rustc_allocator_zeroed,

View File

@ -0,0 +1,49 @@
#![feature(rustc_attrs)]
#[rustc_access_level] mod outer { //~ ERROR None
#[rustc_access_level] pub mod inner { //~ ERROR Some(Exported)
#[rustc_access_level]
extern "C" { //~ ERROR Some(Exported)
#[rustc_access_level] static a: u8; //~ ERROR None
#[rustc_access_level] pub fn b(); //~ ERROR Some(Exported)
}
#[rustc_access_level]
pub trait Trait { //~ ERROR Some(Exported)
#[rustc_access_level] const A: i32; //~ ERROR Some(Exported)
#[rustc_access_level] type B; //~ ERROR Some(Exported)
}
#[rustc_access_level]
pub struct Struct { //~ ERROR Some(Exported)
#[rustc_access_level] a: u8, //~ ERROR None
#[rustc_access_level] pub b: u8, //~ ERROR Some(Exported)
}
#[rustc_access_level]
pub union Union { //~ ERROR Some(Exported)
#[rustc_access_level] a: u8, //~ ERROR None
#[rustc_access_level] pub b: u8, //~ ERROR Some(Exported)
}
#[rustc_access_level]
pub enum Enum { //~ ERROR Some(Exported)
#[rustc_access_level] A( //~ ERROR Some(Exported)
#[rustc_access_level] Struct, //~ ERROR Some(Exported)
#[rustc_access_level] Union, //~ ERROR Some(Exported)
),
}
}
#[rustc_access_level] macro_rules! none_macro { //~ ERROR None
() => {};
}
#[macro_export]
#[rustc_access_level] macro_rules! public_macro { //~ ERROR Some(Public)
() => {};
}
}
pub use outer::inner;
fn main() {}

View File

@ -0,0 +1,125 @@
error: None
--> $DIR/access_levels.rs:3:23
|
LL | #[rustc_access_level] mod outer {
| ^^^^^^^^^
error: Some(Exported)
--> $DIR/access_levels.rs:4:27
|
LL | #[rustc_access_level] pub mod inner {
| ^^^^^^^^^^^^^
error: Some(Exported)
--> $DIR/access_levels.rs:6:9
|
LL | / extern "C" {
LL | | #[rustc_access_level] static a: u8;
LL | | #[rustc_access_level] pub fn b();
LL | | }
| |_________^
error: Some(Exported)
--> $DIR/access_levels.rs:11:9
|
LL | pub trait Trait {
| ^^^^^^^^^^^^^^^
error: Some(Exported)
--> $DIR/access_levels.rs:17:9
|
LL | pub struct Struct {
| ^^^^^^^^^^^^^^^^^
error: None
--> $DIR/access_levels.rs:18:35
|
LL | #[rustc_access_level] a: u8,
| ^^^^^
error: Some(Exported)
--> $DIR/access_levels.rs:19:35
|
LL | #[rustc_access_level] pub b: u8,
| ^^^^^^^^^
error: Some(Exported)
--> $DIR/access_levels.rs:23:9
|
LL | pub union Union {
| ^^^^^^^^^^^^^^^
error: None
--> $DIR/access_levels.rs:24:35
|
LL | #[rustc_access_level] a: u8,
| ^^^^^
error: Some(Exported)
--> $DIR/access_levels.rs:25:35
|
LL | #[rustc_access_level] pub b: u8,
| ^^^^^^^^^
error: Some(Exported)
--> $DIR/access_levels.rs:29:9
|
LL | pub enum Enum {
| ^^^^^^^^^^^^^
error: Some(Exported)
--> $DIR/access_levels.rs:30:35
|
LL | #[rustc_access_level] A(
| ^
error: Some(Exported)
--> $DIR/access_levels.rs:31:39
|
LL | #[rustc_access_level] Struct,
| ^^^^^^
error: Some(Exported)
--> $DIR/access_levels.rs:32:39
|
LL | #[rustc_access_level] Union,
| ^^^^^
error: None
--> $DIR/access_levels.rs:37:27
|
LL | #[rustc_access_level] macro_rules! none_macro {
| ^^^^^^^^^^^^^^^^^^^^^^^
error: Some(Public)
--> $DIR/access_levels.rs:42:27
|
LL | #[rustc_access_level] macro_rules! public_macro {
| ^^^^^^^^^^^^^^^^^^^^^^^^^
error: Some(Exported)
--> $DIR/access_levels.rs:12:35
|
LL | #[rustc_access_level] const A: i32;
| ^^^^^^^^^^^^
error: Some(Exported)
--> $DIR/access_levels.rs:13:35
|
LL | #[rustc_access_level] type B;
| ^^^^^^
error: None
--> $DIR/access_levels.rs:7:35
|
LL | #[rustc_access_level] static a: u8;
| ^^^^^^^^^^^^
error: Some(Exported)
--> $DIR/access_levels.rs:8:35
|
LL | #[rustc_access_level] pub fn b();
| ^^^^^^^^^^
error: aborting due to 20 previous errors