diff --git a/src/librustc/lint/context.rs b/src/librustc/lint/context.rs index 798f4866d34..74a61f700e3 100644 --- a/src/librustc/lint/context.rs +++ b/src/librustc/lint/context.rs @@ -25,7 +25,7 @@ //! for all lint attributes. use self::TargetLint::*; -use middle::privacy::ExportedItems; +use middle::privacy::AccessLevels; use middle::ty::{self, Ty}; use session::{early_error, Session}; use lint::{Level, LevelSource, Lint, LintId, LintArray, LintPass}; @@ -277,8 +277,8 @@ pub struct LateContext<'a, 'tcx: 'a> { /// The crate being checked. pub krate: &'a hir::Crate, - /// Items exported from the crate being checked. - pub exported_items: &'a ExportedItems, + /// Items accessible from the crate being checked. + pub access_levels: &'a AccessLevels, /// The store of registered lints. lints: LintStore, @@ -564,7 +564,7 @@ impl<'a> EarlyContext<'a> { impl<'a, 'tcx> LateContext<'a, 'tcx> { fn new(tcx: &'a ty::ctxt<'tcx>, krate: &'a hir::Crate, - exported_items: &'a ExportedItems) -> LateContext<'a, 'tcx> { + access_levels: &'a AccessLevels) -> LateContext<'a, 'tcx> { // We want to own the lint store, so move it out of the session. let lint_store = mem::replace(&mut *tcx.sess.lint_store.borrow_mut(), LintStore::new()); @@ -572,7 +572,7 @@ impl<'a, 'tcx> LateContext<'a, 'tcx> { LateContext { tcx: tcx, krate: krate, - exported_items: exported_items, + access_levels: access_levels, lints: lint_store, level_stack: vec![], node_levels: RefCell::new(FnvHashMap()), @@ -1014,10 +1014,9 @@ impl LateLintPass for GatherNodeLevels { /// Perform lint checking on a crate. /// /// Consumes the `lint_store` field of the `Session`. -pub fn check_crate(tcx: &ty::ctxt, - exported_items: &ExportedItems) { +pub fn check_crate(tcx: &ty::ctxt, access_levels: &AccessLevels) { let krate = tcx.map.krate(); - let mut cx = LateContext::new(tcx, krate, exported_items); + let mut cx = LateContext::new(tcx, krate, access_levels); // Visit the whole crate. cx.with_lint_attrs(&krate.attrs, |cx| { diff --git a/src/librustc/middle/dead.rs b/src/librustc/middle/dead.rs index 6dfddce9bfe..acd635874e3 100644 --- a/src/librustc/middle/dead.rs +++ b/src/librustc/middle/dead.rs @@ -19,7 +19,6 @@ use rustc_front::intravisit::{self, Visitor}; use middle::{def, pat_util, privacy, ty}; use middle::def_id::{DefId}; use lint; -use util::nodemap::NodeSet; use std::collections::HashSet; use syntax::{ast, codemap}; @@ -370,25 +369,10 @@ impl<'v> Visitor<'v> for LifeSeeder { } fn create_and_seed_worklist(tcx: &ty::ctxt, - exported_items: &privacy::ExportedItems, - reachable_symbols: &NodeSet, + access_levels: &privacy::AccessLevels, krate: &hir::Crate) -> Vec { let mut worklist = Vec::new(); - - // Preferably, we would only need to seed the worklist with reachable - // symbols. However, since the set of reachable symbols differs - // depending on whether a crate is built as bin or lib, and we want - // the warning to be consistent, we also seed the worklist with - // exported symbols. - for id in exported_items { - worklist.push(*id); - } - for id in reachable_symbols { - // Reachable variants can be dead, because we warn about - // variants never constructed, not variants never used. - if let Some(ast_map::NodeVariant(..)) = tcx.map.find(*id) { - continue; - } + for (id, _) in &access_levels.map { worklist.push(*id); } @@ -408,12 +392,10 @@ fn create_and_seed_worklist(tcx: &ty::ctxt, } fn find_live(tcx: &ty::ctxt, - exported_items: &privacy::ExportedItems, - reachable_symbols: &NodeSet, + access_levels: &privacy::AccessLevels, krate: &hir::Crate) -> Box> { - let worklist = create_and_seed_worklist(tcx, exported_items, - reachable_symbols, krate); + let worklist = create_and_seed_worklist(tcx, access_levels, krate); let mut symbol_visitor = MarkSymbolVisitor::new(tcx, worklist); symbol_visitor.mark_live_symbols(); symbol_visitor.live_symbols @@ -607,12 +589,9 @@ impl<'a, 'tcx, 'v> Visitor<'v> for DeadVisitor<'a, 'tcx> { } } -pub fn check_crate(tcx: &ty::ctxt, - exported_items: &privacy::ExportedItems, - reachable_symbols: &NodeSet) { +pub fn check_crate(tcx: &ty::ctxt, access_levels: &privacy::AccessLevels) { let krate = tcx.map.krate(); - let live_symbols = find_live(tcx, exported_items, - reachable_symbols, krate); + let live_symbols = find_live(tcx, access_levels, krate); let mut visitor = DeadVisitor { tcx: tcx, live_symbols: live_symbols }; intravisit::walk_crate(&mut visitor, krate); } diff --git a/src/librustc/middle/privacy.rs b/src/librustc/middle/privacy.rs index 4a1be3bba7b..f464ea58c2d 100644 --- a/src/librustc/middle/privacy.rs +++ b/src/librustc/middle/privacy.rs @@ -17,20 +17,54 @@ pub use self::ImportUse::*; pub use self::LastPrivate::*; use middle::def_id::DefId; -use util::nodemap::{DefIdSet, NodeSet}; +use util::nodemap::{DefIdSet, FnvHashMap}; -/// A set of AST nodes exported by the crate. -pub type ExportedItems = NodeSet; +use std::hash::Hash; +use syntax::ast::NodeId; + +// Accessibility levels, sorted in ascending order +#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] +pub enum AccessLevel { + // Exported items + items participating in various kinds of public interfaces, + // but not directly nameable. For example, if function `fn f() -> T {...}` is + // public, then type `T` is exported. Its values can be obtained by other crates + // even if the type itseld is not nameable. + // FIXME: Mostly unimplemented. Only `type` aliases export items currently. + Reachable, + // Public items + items accessible to other crates with help of `pub use` reexports + Exported, + // Items accessible to other crates directly, without help of reexports + Public, +} + +// Accessibility levels for reachable HIR nodes +#[derive(Clone)] +pub struct AccessLevels { + pub map: FnvHashMap +} + +impl AccessLevels { + pub fn is_reachable(&self, id: Id) -> bool { + self.map.contains_key(&id) + } + pub fn is_exported(&self, id: Id) -> bool { + self.map.get(&id) >= Some(&AccessLevel::Exported) + } + pub fn is_public(&self, id: Id) -> bool { + self.map.get(&id) >= Some(&AccessLevel::Public) + } +} + +impl Default for AccessLevels { + fn default() -> Self { + AccessLevels { map: Default::default() } + } +} /// A set containing all exported definitions from external crates. /// The set does not contain any entries from local crates. pub type ExternalExports = DefIdSet; -/// A set of AST nodes that are fully public in the crate. This map is used for -/// documentation purposes (reexporting a private struct inlines the doc, -/// reexporting a public struct doesn't inline the doc). -pub type PublicItems = NodeSet; - #[derive(Copy, Clone, Debug)] pub enum LastPrivate { LastMod(PrivateDep), diff --git a/src/librustc/middle/reachable.rs b/src/librustc/middle/reachable.rs index 86237a2321a..d146ad2d800 100644 --- a/src/librustc/middle/reachable.rs +++ b/src/librustc/middle/reachable.rs @@ -329,7 +329,7 @@ impl<'a, 'tcx> ReachableContext<'a, 'tcx> { // trait items are used from inlinable code through method call syntax or UFCS, or their // trait is a lang item. struct CollectPrivateImplItemsVisitor<'a> { - exported_items: &'a privacy::ExportedItems, + access_levels: &'a privacy::AccessLevels, worklist: &'a mut Vec, } @@ -337,7 +337,7 @@ impl<'a, 'v> Visitor<'v> for CollectPrivateImplItemsVisitor<'a> { fn visit_item(&mut self, item: &hir::Item) { // We need only trait impls here, not inherent impls, and only non-exported ones if let hir::ItemImpl(_, _, _, Some(_), _, ref impl_items) = item.node { - if !self.exported_items.contains(&item.id) { + if !self.access_levels.is_reachable(item.id) { for impl_item in impl_items { self.worklist.push(impl_item.id); } @@ -347,7 +347,7 @@ impl<'a, 'v> Visitor<'v> for CollectPrivateImplItemsVisitor<'a> { } pub fn find_reachable(tcx: &ty::ctxt, - exported_items: &privacy::ExportedItems) + access_levels: &privacy::AccessLevels) -> NodeSet { let mut reachable_context = ReachableContext::new(tcx); @@ -357,7 +357,7 @@ pub fn find_reachable(tcx: &ty::ctxt, // 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. - for id in exported_items { + for (id, _) in &access_levels.map { reachable_context.worklist.push(*id); } for (_, item) in tcx.lang_items.items() { @@ -369,7 +369,7 @@ pub fn find_reachable(tcx: &ty::ctxt, } { let mut collect_private_impl_items = CollectPrivateImplItemsVisitor { - exported_items: exported_items, + access_levels: access_levels, worklist: &mut reachable_context.worklist, }; tcx.map.krate().visit_all_items(&mut collect_private_impl_items); diff --git a/src/librustc/middle/stability.rs b/src/librustc/middle/stability.rs index 2ba66807d4e..0ef2ac723fc 100644 --- a/src/librustc/middle/stability.rs +++ b/src/librustc/middle/stability.rs @@ -19,7 +19,7 @@ use metadata::cstore::LOCAL_CRATE; use middle::def; use middle::def_id::{CRATE_DEF_INDEX, DefId}; use middle::ty; -use middle::privacy::PublicItems; +use middle::privacy::AccessLevels; use metadata::csearch; use syntax::parse::token::InternedString; use syntax::codemap::{Span, DUMMY_SP}; @@ -73,7 +73,7 @@ struct Annotator<'a, 'tcx: 'a> { tcx: &'a ty::ctxt<'tcx>, index: &'a mut Index<'tcx>, parent: Option<&'tcx Stability>, - export_map: &'a PublicItems, + access_levels: &'a AccessLevels, in_trait_impl: bool, in_enum: bool, } @@ -143,7 +143,7 @@ impl<'a, 'tcx: 'a> Annotator<'a, 'tcx> { } else { debug!("annotate: not found, parent = {:?}", self.parent); let mut is_error = kind == AnnotationKind::Required && - self.export_map.contains(&id) && + self.access_levels.is_reachable(id) && !self.tcx.sess.opts.test; if let Some(stab) = self.parent { if stab.level.is_unstable() { @@ -266,12 +266,12 @@ impl<'a, 'tcx, 'v> Visitor<'v> for Annotator<'a, 'tcx> { impl<'tcx> Index<'tcx> { /// Construct the stability index for a crate being compiled. - pub fn build(&mut self, tcx: &ty::ctxt<'tcx>, krate: &'tcx Crate, export_map: &PublicItems) { + pub fn build(&mut self, tcx: &ty::ctxt<'tcx>, krate: &Crate, access_levels: &AccessLevels) { let mut annotator = Annotator { tcx: tcx, index: self, parent: None, - export_map: export_map, + access_levels: access_levels, in_trait_impl: false, in_enum: false, }; diff --git a/src/librustc/middle/ty/mod.rs b/src/librustc/middle/ty/mod.rs index 42bba0c8aef..501781627fb 100644 --- a/src/librustc/middle/ty/mod.rs +++ b/src/librustc/middle/ty/mod.rs @@ -104,14 +104,12 @@ pub const INITIAL_DISCRIMINANT_VALUE: Disr = 0; /// produced by the driver and fed to trans and later passes. pub struct CrateAnalysis<'a> { pub export_map: ExportMap, - pub exported_items: middle::privacy::ExportedItems, - pub public_items: middle::privacy::PublicItems, + pub access_levels: middle::privacy::AccessLevels, pub reachable: NodeSet, pub name: &'a str, pub glob_map: Option, } - #[derive(Copy, Clone)] pub enum DtorKind { NoDtor, diff --git a/src/librustc_driver/driver.rs b/src/librustc_driver/driver.rs index f8ac2759e85..cb345ac517e 100644 --- a/src/librustc_driver/driver.rs +++ b/src/librustc_driver/driver.rs @@ -746,7 +746,7 @@ pub fn phase_3_run_analysis_passes<'tcx, F, R>(sess: &'tcx Session, "const checking", || middle::check_const::check_crate(tcx)); - let (exported_items, public_items) = + let access_levels = time(time_passes, "privacy checking", || { rustc_privacy::check_crate(tcx, &export_map, @@ -755,7 +755,7 @@ pub fn phase_3_run_analysis_passes<'tcx, F, R>(sess: &'tcx Session, // Do not move this check past lint time(time_passes, "stability index", || { - tcx.stability.borrow_mut().build(tcx, krate, &exported_items) + tcx.stability.borrow_mut().build(tcx, krate, &access_levels) }); time(time_passes, @@ -807,12 +807,10 @@ pub fn phase_3_run_analysis_passes<'tcx, F, R>(sess: &'tcx Session, let reachable_map = time(time_passes, "reachability checking", - || reachable::find_reachable(tcx, &exported_items)); + || reachable::find_reachable(tcx, &access_levels)); time(time_passes, "death checking", || { - middle::dead::check_crate(tcx, - &exported_items, - &reachable_map) + middle::dead::check_crate(tcx, &access_levels); }); let ref lib_features_used = @@ -827,7 +825,7 @@ pub fn phase_3_run_analysis_passes<'tcx, F, R>(sess: &'tcx Session, time(time_passes, "lint checking", - || lint::check_crate(tcx, &exported_items)); + || lint::check_crate(tcx, &access_levels)); // The above three passes generate errors w/o aborting tcx.sess.abort_if_errors(); @@ -836,8 +834,7 @@ pub fn phase_3_run_analysis_passes<'tcx, F, R>(sess: &'tcx Session, mir_map, ty::CrateAnalysis { export_map: export_map, - exported_items: exported_items, - public_items: public_items, + access_levels: access_levels, reachable: reachable_map, name: name, glob_map: glob_map, diff --git a/src/librustc_lint/builtin.rs b/src/librustc_lint/builtin.rs index d24c336dd3f..1fd4ee72cec 100644 --- a/src/librustc_lint/builtin.rs +++ b/src/librustc_lint/builtin.rs @@ -301,8 +301,8 @@ impl MissingDoc { // Only check publicly-visible items, using the result from the privacy pass. // It's an option so the crate root can also use this function (it doesn't // have a NodeId). - if let Some(ref id) = id { - if !cx.exported_items.contains(id) { + if let Some(id) = id { + if !cx.access_levels.is_exported(id) { return; } } @@ -470,7 +470,7 @@ impl LintPass for MissingCopyImplementations { impl LateLintPass for MissingCopyImplementations { fn check_item(&mut self, cx: &LateContext, item: &hir::Item) { - if !cx.exported_items.contains(&item.id) { + if !cx.access_levels.is_reachable(item.id) { return; } let (def, ty) = match item.node { @@ -534,7 +534,7 @@ impl LintPass for MissingDebugImplementations { impl LateLintPass for MissingDebugImplementations { fn check_item(&mut self, cx: &LateContext, item: &hir::Item) { - if !cx.exported_items.contains(&item.id) { + if !cx.access_levels.is_reachable(item.id) { return; } @@ -987,7 +987,7 @@ impl LateLintPass for InvalidNoMangleItems { match it.node { hir::ItemFn(..) => { if attr::contains_name(&it.attrs, "no_mangle") && - !cx.exported_items.contains(&it.id) { + !cx.access_levels.is_reachable(it.id) { let msg = format!("function {} is marked #[no_mangle], but not exported", it.name); cx.span_lint(PRIVATE_NO_MANGLE_FNS, it.span, &msg); @@ -995,7 +995,7 @@ impl LateLintPass for InvalidNoMangleItems { }, hir::ItemStatic(..) => { if attr::contains_name(&it.attrs, "no_mangle") && - !cx.exported_items.contains(&it.id) { + !cx.access_levels.is_reachable(it.id) { let msg = format!("static {} is marked #[no_mangle], but not exported", it.name); cx.span_lint(PRIVATE_NO_MANGLE_STATICS, it.span, &msg); diff --git a/src/librustc_privacy/lib.rs b/src/librustc_privacy/lib.rs index d11880ecca1..1ca8eeadfe5 100644 --- a/src/librustc_privacy/lib.rs +++ b/src/librustc_privacy/lib.rs @@ -32,6 +32,7 @@ extern crate rustc_front; use self::PrivacyResult::*; use self::FieldName::*; +use std::cmp; use std::mem::replace; use rustc_front::hir; @@ -39,12 +40,13 @@ use rustc_front::intravisit::{self, Visitor}; use rustc::middle::def; use rustc::middle::def_id::DefId; +use rustc::middle::privacy::{AccessLevel, AccessLevels}; use rustc::middle::privacy::ImportUse::*; use rustc::middle::privacy::LastPrivate::*; use rustc::middle::privacy::PrivateDep::*; -use rustc::middle::privacy::{ExternalExports, ExportedItems, PublicItems}; +use rustc::middle::privacy::ExternalExports; use rustc::middle::ty::{self, Ty}; -use rustc::util::nodemap::{NodeMap, NodeSet}; +use rustc::util::nodemap::NodeMap; use rustc::front::map as ast_map; use syntax::ast; @@ -159,64 +161,57 @@ struct EmbargoVisitor<'a, 'tcx: 'a> { tcx: &'a ty::ctxt<'tcx>, export_map: &'a def::ExportMap, - // This flag is an indicator of whether the previous item in the - // hierarchical chain was exported or not. This is the indicator of whether - // children should be exported as well. Note that this can flip from false - // to true if a reexported module is entered (or an action similar). - prev_exported: bool, - - // This is a list of all exported items in the AST. An exported item is any - // function/method/item which is usable by external crates. This essentially - // means that the result is "public all the way down", but the "path down" - // may jump across private boundaries through reexport statements or type aliases. - exported_items: ExportedItems, - - // Items that are directly public without help of reexports or type aliases. - // These two fields are closely related to one another in that they are only - // used for generation of the `public_items` set, not for privacy checking at - // all. Invariant: at any moment public items are a subset of exported items. - public_items: PublicItems, - prev_public: bool, + // Accessibility levels for reachable nodes + access_levels: AccessLevels, + // Previous accessibility level, None means unreachable + prev_level: Option, + // Have something changed in the level map? + changed: bool, } impl<'a, 'tcx> EmbargoVisitor<'a, 'tcx> { - // Returns tuple (is_public, is_exported) for a type - fn is_public_exported_ty(&self, ty: &hir::Ty) -> (bool, bool) { + fn ty_level(&self, ty: &hir::Ty) -> Option { if let hir::TyPath(..) = ty.node { match self.tcx.def_map.borrow().get(&ty.id).unwrap().full_def() { def::DefPrimTy(..) | def::DefSelfTy(..) | def::DefTyParam(..) => { - (true, true) + Some(AccessLevel::Public) } def => { if let Some(node_id) = self.tcx.map.as_local_node_id(def.def_id()) { - (self.public_items.contains(&node_id), - self.exported_items.contains(&node_id)) + self.get(node_id) } else { - (true, true) + Some(AccessLevel::Public) } } } } else { - (true, true) + Some(AccessLevel::Public) } } - // Returns tuple (is_public, is_exported) for a trait - fn is_public_exported_trait(&self, trait_ref: &hir::TraitRef) -> (bool, bool) { + fn trait_level(&self, trait_ref: &hir::TraitRef) -> Option { let did = self.tcx.trait_ref_to_def_id(trait_ref); if let Some(node_id) = self.tcx.map.as_local_node_id(did) { - (self.public_items.contains(&node_id), self.exported_items.contains(&node_id)) + self.get(node_id) } else { - (true, true) + Some(AccessLevel::Public) } } - fn maybe_insert_id(&mut self, id: ast::NodeId) { - if self.prev_public { - self.public_items.insert(id); - } - if self.prev_exported { - self.exported_items.insert(id); + fn get(&self, id: ast::NodeId) -> Option { + self.access_levels.map.get(&id).cloned() + } + + // Updates node level and returns the updated level + fn update(&mut self, id: ast::NodeId, level: Option) -> Option { + let old_level = self.get(id); + // Accessibility levels can only grow + if level > old_level { + self.access_levels.map.insert(id, level.unwrap()); + self.changed = true; + level + } else { + old_level } } } @@ -227,187 +222,126 @@ impl<'a, 'tcx, 'v> Visitor<'v> for EmbargoVisitor<'a, 'tcx> { fn visit_nested_item(&mut self, item: hir::ItemId) { self.visit_item(self.tcx.map.expect_item(item.id)) } + fn visit_item(&mut self, item: &hir::Item) { - let orig_all_public = self.prev_public; - let orig_all_exported = self.prev_exported; - match item.node { - // impls/extern blocks do not break the "public chain" because they - // cannot have visibility qualifiers on them anyway. Impls are also not - // added to public/exported sets based on inherited publicity. - hir::ItemImpl(..) | hir::ItemDefaultImpl(..) => {} + let inherited_item_level = match item.node { + // Impls inherit level from their types and traits + hir::ItemImpl(_, _, _, None, ref ty, _) => { + self.ty_level(&ty) + } + hir::ItemImpl(_, _, _, Some(ref trait_ref), ref ty, _) => { + cmp::min(self.ty_level(&ty), self.trait_level(trait_ref)) + } + hir::ItemDefaultImpl(_, ref trait_ref) => { + self.trait_level(trait_ref) + } + // Foreign mods inherit level from parents hir::ItemForeignMod(..) => { - self.maybe_insert_id(item.id); + self.prev_level } - - // Private by default, hence we only retain the "public chain" if - // `pub` is explicitly listed. + // Other `pub` items inherit levels from parents _ => { - self.prev_public = self.prev_public && item.vis == hir::Public; - self.prev_exported = (self.prev_exported && item.vis == hir::Public) || - self.exported_items.contains(&item.id); - - self.maybe_insert_id(item.id); + if item.vis == hir::Public { self.prev_level } else { None } } - } + }; + // Update id of the item itself + let item_level = self.update(item.id, inherited_item_level); + + // Update ids of nested things match item.node { - // Enum variants inherit from their parent, so if the enum is - // public all variants are public hir::ItemEnum(ref def, _) => { for variant in &def.variants { - self.maybe_insert_id(variant.node.data.id()); + let variant_level = self.update(variant.node.data.id(), item_level); for field in variant.node.data.fields() { - // Variant fields are always public - self.maybe_insert_id(field.node.id); + self.update(field.node.id, variant_level); } } } - - // Inherent impls for public/exported types and their public items are public/exported - hir::ItemImpl(_, _, _, None, ref ty, ref impl_items) => { - let (public_ty, exported_ty) = self.is_public_exported_ty(&ty); - - if public_ty { - self.public_items.insert(item.id); - } - if exported_ty { - self.exported_items.insert(item.id); - } - + hir::ItemImpl(_, _, _, None, _, ref impl_items) => { for impl_item in impl_items { if impl_item.vis == hir::Public { - if public_ty { - self.public_items.insert(impl_item.id); - } - if exported_ty { - self.exported_items.insert(impl_item.id); - } + self.update(impl_item.id, item_level); } } } - - // Trait impl and its items are public/exported if both the self type and the trait - // of this impl are public/exported - hir::ItemImpl(_, _, _, Some(ref trait_ref), ref ty, ref impl_items) => { - let (public_ty, exported_ty) = self.is_public_exported_ty(&ty); - let (public_trait, exported_trait) = self.is_public_exported_trait(trait_ref); - - if public_ty && public_trait { - self.public_items.insert(item.id); - } - if exported_ty && exported_trait { - self.exported_items.insert(item.id); - } - + hir::ItemImpl(_, _, _, Some(_), _, ref impl_items) => { for impl_item in impl_items { - if public_ty && public_trait { - self.public_items.insert(impl_item.id); - } - if exported_ty && exported_trait { - self.exported_items.insert(impl_item.id); - } + self.update(impl_item.id, item_level); } } - - // Default trait impls are public/exported for public/exported traits - hir::ItemDefaultImpl(_, ref trait_ref) => { - let (public_trait, exported_trait) = self.is_public_exported_trait(trait_ref); - - if public_trait { - self.public_items.insert(item.id); - } - if exported_trait { - self.exported_items.insert(item.id); - } - } - - // Default methods on traits are all public/exported so long as the trait - // is public/exported hir::ItemTrait(_, _, _, ref trait_items) => { for trait_item in trait_items { - self.maybe_insert_id(trait_item.id); + self.update(trait_item.id, item_level); } } - - // Struct constructors are public if the struct is all public. hir::ItemStruct(ref def, _) => { if !def.is_struct() { - self.maybe_insert_id(def.id()); + self.update(def.id(), item_level); } for field in def.fields() { - // Struct fields can be public or private, so lets check if field.node.kind.visibility() == hir::Public { - self.maybe_insert_id(field.node.id); + self.update(field.node.id, item_level); } } } - - hir::ItemTy(ref ty, _) if self.prev_exported => { + hir::ItemForeignMod(ref foreign_mod) => { + for foreign_item in &foreign_mod.items { + if foreign_item.vis == hir::Public { + self.update(foreign_item.id, item_level); + } + } + } + hir::ItemTy(ref ty, _) if item_level.is_some() => { if let hir::TyPath(..) = ty.node { match self.tcx.def_map.borrow().get(&ty.id).unwrap().full_def() { def::DefPrimTy(..) | def::DefSelfTy(..) | def::DefTyParam(..) => {}, def => { if let Some(node_id) = self.tcx.map.as_local_node_id(def.def_id()) { - self.exported_items.insert(node_id); + self.update(node_id, Some(AccessLevel::Reachable)); } } } } } - - hir::ItemForeignMod(ref foreign_mod) => { - for foreign_item in &foreign_mod.items { - let public = self.prev_public && foreign_item.vis == hir::Public; - let exported = (self.prev_exported && foreign_item.vis == hir::Public) || - self.exported_items.contains(&foreign_item.id); - - if public { - self.public_items.insert(foreign_item.id); - } - if exported { - self.exported_items.insert(foreign_item.id); - } - } - } - _ => {} } + let orig_level = self.prev_level; + self.prev_level = item_level; + intravisit::walk_item(self, item); - self.prev_public = orig_all_public; - self.prev_exported = orig_all_exported; + self.prev_level = orig_level; } fn visit_block(&mut self, b: &'v hir::Block) { - let orig_all_public = replace(&mut self.prev_public, false); - let orig_all_exported = replace(&mut self.prev_exported, false); + let orig_level = replace(&mut self.prev_level, None); - // Blocks can have exported and public items, for example impls, but they always - // start as non-public and non-exported regardless of publicity of a function, + // Blocks can have public items, for example impls, but they always + // start as completely private regardless of publicity of a function, // constant, type, field, etc. in which this block resides intravisit::walk_block(self, b); - self.prev_public = orig_all_public; - self.prev_exported = orig_all_exported; + self.prev_level = orig_level; } fn visit_mod(&mut self, m: &hir::Mod, _sp: Span, id: ast::NodeId) { // This code is here instead of in visit_item so that the // crate module gets processed as well. - if self.prev_exported { - assert!(self.export_map.contains_key(&id), "wut {}", id); - for export in self.export_map.get(&id).unwrap() { + if self.prev_level.is_some() { + for export in self.export_map.get(&id).expect("module isn't found in export map") { if let Some(node_id) = self.tcx.map.as_local_node_id(export.def_id) { - self.exported_items.insert(node_id); + self.update(node_id, Some(AccessLevel::Exported)); } } } - intravisit::walk_mod(self, m) + + intravisit::walk_mod(self, m); } fn visit_macro_def(&mut self, md: &'v hir::MacroDef) { - self.maybe_insert_id(md.id); + self.update(md.id, Some(AccessLevel::Public)); } } @@ -1169,8 +1103,7 @@ impl<'a, 'tcx> SanePrivacyVisitor<'a, 'tcx> { struct VisiblePrivateTypesVisitor<'a, 'tcx: 'a> { tcx: &'a ty::ctxt<'tcx>, - exported_items: &'a ExportedItems, - public_items: &'a PublicItems, + access_levels: &'a AccessLevels, in_variant: bool, } @@ -1210,7 +1143,7 @@ impl<'a, 'tcx> VisiblePrivateTypesVisitor<'a, 'tcx> { fn trait_is_public(&self, trait_id: ast::NodeId) -> bool { // FIXME: this would preferably be using `exported_items`, but all // traits are exported currently (see `EmbargoVisitor.exported_trait`) - self.public_items.contains(&trait_id) + self.access_levels.is_public(trait_id) } fn check_ty_param_bound(&self, @@ -1226,7 +1159,7 @@ impl<'a, 'tcx> VisiblePrivateTypesVisitor<'a, 'tcx> { } fn item_is_public(&self, id: &ast::NodeId, vis: hir::Visibility) -> bool { - self.exported_items.contains(id) || vis == hir::Public + self.access_levels.is_reachable(*id) || vis == hir::Public } } @@ -1332,7 +1265,7 @@ impl<'a, 'tcx, 'v> Visitor<'v> for VisiblePrivateTypesVisitor<'a, 'tcx> { match impl_item.node { hir::ImplItemKind::Const(..) | hir::ImplItemKind::Method(..) => { - self.exported_items.contains(&impl_item.id) + self.access_levels.is_reachable(impl_item.id) } hir::ImplItemKind::Type(_) => false, } @@ -1461,7 +1394,7 @@ impl<'a, 'tcx, 'v> Visitor<'v> for VisiblePrivateTypesVisitor<'a, 'tcx> { } fn visit_foreign_item(&mut self, item: &hir::ForeignItem) { - if self.exported_items.contains(&item.id) { + if self.access_levels.is_reachable(item.id) { intravisit::walk_foreign_item(self, item) } } @@ -1479,7 +1412,7 @@ impl<'a, 'tcx, 'v> Visitor<'v> for VisiblePrivateTypesVisitor<'a, 'tcx> { } fn visit_variant(&mut self, v: &hir::Variant, g: &hir::Generics, item_id: ast::NodeId) { - if self.exported_items.contains(&v.node.data.id()) { + if self.access_levels.is_reachable(v.node.data.id()) { self.in_variant = true; intravisit::walk_variant(self, v, g, item_id); self.in_variant = false; @@ -1509,7 +1442,7 @@ impl<'a, 'tcx, 'v> Visitor<'v> for VisiblePrivateTypesVisitor<'a, 'tcx> { pub fn check_crate(tcx: &ty::ctxt, export_map: &def::ExportMap, external_exports: ExternalExports) - -> (ExportedItems, PublicItems) { + -> AccessLevels { let krate = tcx.map.krate(); // Sanity check to make sure that all privacy usage and controls are @@ -1544,33 +1477,31 @@ pub fn check_crate(tcx: &ty::ctxt, // items which are reachable from external crates based on visibility. let mut visitor = EmbargoVisitor { tcx: tcx, - exported_items: NodeSet(), - public_items: NodeSet(), export_map: export_map, - prev_exported: true, - prev_public: true, + access_levels: Default::default(), + prev_level: Some(AccessLevel::Public), + changed: false, }; - visitor.exported_items.insert(ast::CRATE_NODE_ID); - visitor.public_items.insert(ast::CRATE_NODE_ID); loop { - let before = (visitor.exported_items.len(), visitor.public_items.len()); intravisit::walk_crate(&mut visitor, krate); - let after = (visitor.exported_items.len(), visitor.public_items.len()); - if after == before { + if visitor.changed { + visitor.changed = false; + } else { break } } + visitor.update(ast::CRATE_NODE_ID, Some(AccessLevel::Public)); - let EmbargoVisitor { exported_items, public_items, .. } = visitor; + let EmbargoVisitor { access_levels, .. } = visitor; { let mut visitor = VisiblePrivateTypesVisitor { tcx: tcx, - exported_items: &exported_items, - public_items: &public_items, + access_levels: &access_levels, in_variant: false, }; intravisit::walk_crate(&mut visitor, krate); } - return (exported_items, public_items); + + access_levels } diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index f9a138f8fd7..f20f5dbbc4e 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -13,10 +13,10 @@ use rustc_lint; use rustc_driver::{driver, target_features}; use rustc::session::{self, config}; use rustc::middle::def_id::DefId; +use rustc::middle::privacy::AccessLevels; use rustc::middle::ty; use rustc::front::map as hir_map; use rustc::lint; -use rustc::util::nodemap::DefIdSet; use rustc_trans::back::link; use rustc_resolve as resolve; use rustc_front::lowering::{lower_crate, LoweringContext}; @@ -77,8 +77,7 @@ impl<'b, 'tcx> DocContext<'b, 'tcx> { } pub struct CrateAnalysis { - pub exported_items: DefIdSet, - pub public_items: DefIdSet, + pub access_levels: AccessLevels, pub external_paths: ExternalPaths, pub external_typarams: RefCell>>, pub inlined: RefCell>>, @@ -147,18 +146,15 @@ pub fn run_core(search_paths: SearchPaths, cfgs: Vec, externs: Externs, &name, resolve::MakeGlobMap::No, |tcx, _, analysis| { - let ty::CrateAnalysis { exported_items, public_items, .. } = analysis; + let ty::CrateAnalysis { access_levels, .. } = analysis; // Convert from a NodeId set to a DefId set since we don't always have easy access // to the map from defid -> nodeid - let exported_items: DefIdSet = - exported_items.into_iter() - .map(|n| tcx.map.local_def_id(n)) - .collect(); - let public_items: DefIdSet = - public_items.into_iter() - .map(|n| tcx.map.local_def_id(n)) - .collect(); + let access_levels = AccessLevels { + map: access_levels.map.into_iter() + .map(|(k, v)| (tcx.map.local_def_id(k), v)) + .collect() + }; let ctxt = DocContext { map: &tcx.map, @@ -174,8 +170,7 @@ pub fn run_core(search_paths: SearchPaths, cfgs: Vec, externs: Externs, debug!("crate: {:?}", ctxt.map.krate()); let mut analysis = CrateAnalysis { - exported_items: exported_items, - public_items: public_items, + access_levels: access_levels, external_paths: RefCell::new(None), external_typarams: RefCell::new(None), inlined: RefCell::new(None), diff --git a/src/librustdoc/html/render.rs b/src/librustdoc/html/render.rs index 49228ac91f6..3a802b37473 100644 --- a/src/librustdoc/html/render.rs +++ b/src/librustdoc/html/render.rs @@ -56,8 +56,8 @@ use serialize::json::{self, ToJson}; use syntax::{abi, ast}; use rustc::metadata::cstore::LOCAL_CRATE; use rustc::middle::def_id::{CRATE_DEF_INDEX, DefId}; +use rustc::middle::privacy::AccessLevels; use rustc::middle::stability; -use rustc::util::nodemap::DefIdSet; use rustc_front::hir; use clean::{self, SelfTy}; @@ -244,7 +244,7 @@ pub struct Cache { search_index: Vec, privmod: bool, remove_priv: bool, - public_items: DefIdSet, + access_levels: AccessLevels, deref_trait_did: Option, // In rare case where a structure is defined in one module but implemented @@ -415,8 +415,8 @@ pub fn run(mut krate: clean::Crate, // Crawl the crate to build various caches used for the output let analysis = ::ANALYSISKEY.with(|a| a.clone()); let analysis = analysis.borrow(); - let public_items = analysis.as_ref().map(|a| a.public_items.clone()); - let public_items = public_items.unwrap_or(DefIdSet()); + let access_levels = analysis.as_ref().map(|a| a.access_levels.clone()); + let access_levels = access_levels.unwrap_or(Default::default()); let paths: HashMap, ItemType)> = analysis.as_ref().map(|a| { let paths = a.external_paths.borrow_mut().take().unwrap(); @@ -435,7 +435,7 @@ pub fn run(mut krate: clean::Crate, primitive_locations: HashMap::new(), remove_priv: cx.passes.contains("strip-private"), privmod: false, - public_items: public_items, + access_levels: access_levels, orphan_methods: Vec::new(), traits: mem::replace(&mut krate.external_traits, HashMap::new()), deref_trait_did: analysis.as_ref().and_then(|a| a.deref_trait_did), @@ -1053,7 +1053,7 @@ impl DocFolder for Cache { if !self.paths.contains_key(&item.def_id) || !item.def_id.is_local() || - self.public_items.contains(&item.def_id) + self.access_levels.is_public(item.def_id) { self.paths.insert(item.def_id, (self.stack.clone(), shortty(&item))); diff --git a/src/librustdoc/passes.rs b/src/librustdoc/passes.rs index a09ca95dcea..d011ba08a9c 100644 --- a/src/librustdoc/passes.rs +++ b/src/librustdoc/passes.rs @@ -8,6 +8,8 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +use rustc::middle::def_id::DefId; +use rustc::middle::privacy::AccessLevels; use rustc::util::nodemap::DefIdSet; use std::cmp; use std::string::String; @@ -96,13 +98,13 @@ pub fn strip_private(mut krate: clean::Crate) -> plugins::PluginResult { let analysis = super::ANALYSISKEY.with(|a| a.clone()); let analysis = analysis.borrow(); let analysis = analysis.as_ref().unwrap(); - let exported_items = analysis.exported_items.clone(); + let access_levels = analysis.access_levels.clone(); // strip all private items { let mut stripper = Stripper { retained: &mut retained, - exported_items: &exported_items, + access_levels: &access_levels, }; krate = stripper.fold_crate(krate); } @@ -117,7 +119,7 @@ pub fn strip_private(mut krate: clean::Crate) -> plugins::PluginResult { struct Stripper<'a> { retained: &'a mut DefIdSet, - exported_items: &'a DefIdSet, + access_levels: &'a AccessLevels, } impl<'a> fold::DocFolder for Stripper<'a> { @@ -130,18 +132,14 @@ impl<'a> fold::DocFolder for Stripper<'a> { clean::VariantItem(..) | clean::MethodItem(..) | clean::ForeignFunctionItem(..) | clean::ForeignStaticItem(..) => { if i.def_id.is_local() { - if !self.exported_items.contains(&i.def_id) { - return None; - } - // Traits are in exported_items even when they're totally private. - if i.is_trait() && i.visibility != Some(hir::Public) { + if !self.access_levels.is_exported(i.def_id) { return None; } } } clean::ConstantItem(..) => { - if i.def_id.is_local() && !self.exported_items.contains(&i.def_id) { + if i.def_id.is_local() && !self.access_levels.is_exported(i.def_id) { return None; } } @@ -168,7 +166,7 @@ impl<'a> fold::DocFolder for Stripper<'a> { clean::ImplItem(clean::Impl{ for_: clean::ResolvedPath{ did, .. }, .. }) => { - if did.is_local() && !self.exported_items.contains(&did) { + if did.is_local() && !self.access_levels.is_exported(did) { return None; } } diff --git a/src/librustdoc/visit_ast.rs b/src/librustdoc/visit_ast.rs index 36ef110fba0..150464404a2 100644 --- a/src/librustdoc/visit_ast.rs +++ b/src/librustdoc/visit_ast.rs @@ -214,7 +214,7 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { let analysis = match self.analysis { Some(analysis) => analysis, None => return false }; - if !please_inline && analysis.public_items.contains(&def) { + if !please_inline && analysis.access_levels.is_public(def) { return false } if !self.view_item_stack.insert(def_node_id) { return false }