diff --git a/compiler/rustc_error_messages/locales/en-US/privacy.ftl b/compiler/rustc_error_messages/locales/en-US/privacy.ftl index 223092a74bd97..da987152ff660 100644 --- a/compiler/rustc_error_messages/locales/en-US/privacy.ftl +++ b/compiler/rustc_error_messages/locales/en-US/privacy.ftl @@ -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 diff --git a/compiler/rustc_feature/src/builtin_attrs.rs b/compiler/rustc_feature/src/builtin_attrs.rs index 0487270b52a9a..0c88379d49899 100644 --- a/compiler/rustc_feature/src/builtin_attrs.rs +++ b/compiler/rustc_feature/src/builtin_attrs.rs @@ -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), diff --git a/compiler/rustc_privacy/src/errors.rs b/compiler/rustc_privacy/src/errors.rs index 1e423ddb7102c..63f83f8965ec5 100644 --- a/compiler/rustc_privacy/src/errors.rs +++ b/compiler/rustc_privacy/src/errors.rs @@ -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> { diff --git a/compiler/rustc_privacy/src/lib.rs b/compiler/rustc_privacy/src/lib.rs index 5d562f18a8158..075a1411f02b4 100644 --- a/compiler/rustc_privacy/src/lib.rs +++ b/compiler/rustc_privacy/src/lib.rs @@ -30,7 +30,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; @@ -39,7 +39,8 @@ use std::{cmp, fmt, mem}; use errors::{ FieldIsPrivate, FieldIsPrivateLabel, FromPrivateDependencyInPublicInterface, InPublicInterface, - InPublicInterfaceTraits, ItemIsPrivate, PrivateInPublicLint, UnnamedItemIsPrivate, + InPublicInterfaceTraits, ItemIsPrivate, PrivateInPublicLint, ReportAccessLevel, + UnnamedItemIsPrivate, }; //////////////////////////////////////////////////////////////////////////////// @@ -904,6 +905,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, @@ -2042,6 +2097,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) } diff --git a/compiler/rustc_resolve/src/access_levels.rs b/compiler/rustc_resolve/src/access_levels.rs index 3fba923d9fdf4..c98f7f0ccef44 100644 --- a/compiler/rustc_resolve/src/access_levels.rs +++ b/compiler/rustc_resolve/src/access_levels.rs @@ -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, 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)); - - if let Some(exports) = self.r.reexport_map.get(&module_id) { - let pub_exports = exports - .iter() - .filter(|ex| ex.vis == Visibility::Public) - .cloned() - .collect::>(); - - 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) - } + let module = self.r.get_module(module_id.to_def_id()).unwrap(); + let resolutions = self.r.resolutions(module); + + 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); + } - // 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)); + // 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)); } + } - let orig_level = std::mem::replace(&mut self.prev_level, access_level); + ast::ItemKind::Mod(..) => { + 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!(), } } } diff --git a/compiler/rustc_resolve/src/imports.rs b/compiler/rustc_resolve/src/imports.rs index c2491c6ebdec0..9a2c39fab72fd 100644 --- a/compiler/rustc_resolve/src/imports.rs +++ b/compiler/rustc_resolve/src/imports.rs @@ -1135,24 +1135,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, + }); } }); diff --git a/compiler/rustc_resolve/src/lib.rs b/compiler/rustc_resolve/src/lib.rs index 66090c96d1ee2..74f73bbd85cd3 100644 --- a/compiler/rustc_resolve/src/lib.rs +++ b/compiler/rustc_resolve/src/lib.rs @@ -2021,6 +2021,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> { + // 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 { diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index 6eca7dc52b26a..f854395ff8149 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -1206,6 +1206,7 @@ symbols! { rust_eh_unregister_frames, rust_oom, rustc, + rustc_access_level, rustc_allocator, rustc_allocator_nounwind, rustc_allocator_zeroed, diff --git a/src/test/ui/privacy/access_levels.rs b/src/test/ui/privacy/access_levels.rs new file mode 100644 index 0000000000000..d51d2b57267b6 --- /dev/null +++ b/src/test/ui/privacy/access_levels.rs @@ -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() {} diff --git a/src/test/ui/privacy/access_levels.stderr b/src/test/ui/privacy/access_levels.stderr new file mode 100644 index 0000000000000..f326293c384a5 --- /dev/null +++ b/src/test/ui/privacy/access_levels.stderr @@ -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 +