From 41f1e189ee2b078838fcd2f7550d14e9f1fd5e45 Mon Sep 17 00:00:00 2001 From: Jeffrey Seyfried Date: Tue, 20 Dec 2016 08:32:15 +0000 Subject: [PATCH] Use `DefId`s instead of `NodeId`s for `pub(restricted)` visibilities. --- src/librustc/ty/mod.rs | 54 +++++++------- src/librustc_metadata/decoder.rs | 10 +-- src/librustc_metadata/encoder.rs | 46 +++--------- src/librustc_metadata/schema.rs | 2 +- src/librustc_privacy/lib.rs | 32 ++++----- src/librustc_resolve/build_reduced_graph.rs | 41 +++++------ src/librustc_resolve/lib.rs | 78 ++++++++++----------- src/librustc_resolve/macros.rs | 4 +- src/librustc_resolve/resolve_imports.rs | 23 +++--- src/librustc_typeck/check/method/mod.rs | 2 +- src/librustc_typeck/check/method/probe.rs | 2 +- src/librustc_typeck/check/mod.rs | 4 +- 12 files changed, 137 insertions(+), 161 deletions(-) diff --git a/src/librustc/ty/mod.rs b/src/librustc/ty/mod.rs index 4e175e50194..89e976598bf 100644 --- a/src/librustc/ty/mod.rs +++ b/src/librustc/ty/mod.rs @@ -219,26 +219,18 @@ pub enum Visibility { /// Visible everywhere (including in other crates). Public, /// Visible only in the given crate-local module. - Restricted(NodeId), + Restricted(DefId), /// Not visible anywhere in the local crate. This is the visibility of private external items. - PrivateExternal, + Invisible, } -pub trait NodeIdTree { - fn is_descendant_of(&self, node: NodeId, ancestor: NodeId) -> bool; +pub trait DefIdTree: Copy { + fn parent(self, id: DefId) -> Option; } -impl<'a> NodeIdTree for ast_map::Map<'a> { - fn is_descendant_of(&self, node: NodeId, ancestor: NodeId) -> bool { - let mut node_ancestor = node; - while node_ancestor != ancestor { - let node_ancestor_parent = self.get_module_parent(node_ancestor); - if node_ancestor_parent == node_ancestor { - return false; - } - node_ancestor = node_ancestor_parent; - } - true +impl<'a, 'gcx, 'tcx> DefIdTree for TyCtxt<'a, 'gcx, 'tcx> { + fn parent(self, id: DefId) -> Option { + self.def_key(id).parent.map(|index| DefId { index: index, ..id }) } } @@ -246,36 +238,46 @@ impl Visibility { pub fn from_hir(visibility: &hir::Visibility, id: NodeId, tcx: TyCtxt) -> Self { match *visibility { hir::Public => Visibility::Public, - hir::Visibility::Crate => Visibility::Restricted(ast::CRATE_NODE_ID), + hir::Visibility::Crate => Visibility::Restricted(DefId::local(CRATE_DEF_INDEX)), hir::Visibility::Restricted { ref path, .. } => match path.def { // If there is no resolution, `resolve` will have already reported an error, so // assume that the visibility is public to avoid reporting more privacy errors. Def::Err => Visibility::Public, - def => Visibility::Restricted(tcx.map.as_local_node_id(def.def_id()).unwrap()), + def => Visibility::Restricted(def.def_id()), }, - hir::Inherited => Visibility::Restricted(tcx.map.get_module_parent(id)), + hir::Inherited => { + Visibility::Restricted(tcx.map.local_def_id(tcx.map.get_module_parent(id))) + } } } /// Returns true if an item with this visibility is accessible from the given block. - pub fn is_accessible_from(self, block: NodeId, tree: &T) -> bool { + pub fn is_accessible_from(self, mut module: DefId, tree: T) -> bool { let restriction = match self { // Public items are visible everywhere. Visibility::Public => return true, // Private items from other crates are visible nowhere. - Visibility::PrivateExternal => return false, + Visibility::Invisible => return false, // Restricted items are visible in an arbitrary local module. + Visibility::Restricted(other) if other.krate != module.krate => return false, Visibility::Restricted(module) => module, }; - tree.is_descendant_of(block, restriction) + while module != restriction { + match tree.parent(module) { + Some(parent) => module = parent, + None => return false, + } + } + + true } /// Returns true if this visibility is at least as accessible as the given visibility - pub fn is_at_least(self, vis: Visibility, tree: &T) -> bool { + pub fn is_at_least(self, vis: Visibility, tree: T) -> bool { let vis_restriction = match vis { Visibility::Public => return self == Visibility::Public, - Visibility::PrivateExternal => return true, + Visibility::Invisible => return true, Visibility::Restricted(module) => module, }; @@ -1779,7 +1781,7 @@ impl<'a, 'gcx, 'tcx> FieldDef { block: Option, tcx: TyCtxt<'a, 'gcx, 'tcx>, substs: &'tcx Substs<'tcx>) -> bool { - block.map_or(true, |b| self.vis.is_accessible_from(b, &tcx.map)) && + block.map_or(true, |b| tcx.vis_is_accessible_from(self.vis, b)) && self.ty(tcx, substs).is_uninhabited_recurse(visited, block, tcx) } } @@ -2266,6 +2268,10 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> { } } + pub fn vis_is_accessible_from(self, vis: Visibility, block: NodeId) -> bool { + vis.is_accessible_from(self.map.local_def_id(self.map.get_module_parent(block)), self) + } + pub fn item_name(self, id: DefId) -> ast::Name { if let Some(id) = self.map.as_local_node_id(id) { self.map.name(id) diff --git a/src/librustc_metadata/decoder.rs b/src/librustc_metadata/decoder.rs index 853a49dffc7..93c79046580 100644 --- a/src/librustc_metadata/decoder.rs +++ b/src/librustc_metadata/decoder.rs @@ -588,7 +588,7 @@ impl<'a, 'tcx> CrateMetadata { ty::FieldDef { did: self.local_def_id(index), name: self.item_name(index), - vis: f.visibility + vis: f.visibility.decode(self) } }).collect(), disr_val: ConstInt::Infer(data.disr), @@ -678,7 +678,7 @@ impl<'a, 'tcx> CrateMetadata { pub fn get_visibility(&self, id: DefIndex) -> ty::Visibility { match self.is_proc_macro(id) { true => ty::Visibility::Public, - false => self.entry(id).visibility, + false => self.entry(id).visibility.decode(self), } } @@ -885,7 +885,7 @@ impl<'a, 'tcx> CrateMetadata { ty::AssociatedItem { name: name, kind: ty::AssociatedKind::Const, - vis: item.visibility, + vis: item.visibility.decode(self), defaultness: container.defaultness(), def_id: self.local_def_id(id), container: container.with_def_id(parent), @@ -898,7 +898,7 @@ impl<'a, 'tcx> CrateMetadata { ty::AssociatedItem { name: name, kind: ty::AssociatedKind::Method, - vis: item.visibility, + vis: item.visibility.decode(self), defaultness: data.container.defaultness(), def_id: self.local_def_id(id), container: data.container.with_def_id(parent), @@ -910,7 +910,7 @@ impl<'a, 'tcx> CrateMetadata { ty::AssociatedItem { name: name, kind: ty::AssociatedKind::Type, - vis: item.visibility, + vis: item.visibility.decode(self), defaultness: container.defaultness(), def_id: self.local_def_id(id), container: container.with_def_id(parent), diff --git a/src/librustc_metadata/encoder.rs b/src/librustc_metadata/encoder.rs index cf032013ac9..bc0a64b9a51 100644 --- a/src/librustc_metadata/encoder.rs +++ b/src/librustc_metadata/encoder.rs @@ -268,7 +268,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { Entry { kind: EntryKind::Variant(self.lazy(&data)), - visibility: enum_vis.simplify(), + visibility: self.lazy(&ty::Visibility::from_hir(enum_vis, enum_id, tcx)), span: self.lazy(&tcx.def_span(def_id)), attributes: self.encode_attributes(&tcx.get_attrs(def_id)), children: self.lazy_seq(variant.fields.iter().map(|f| { @@ -306,7 +306,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { Entry { kind: EntryKind::Mod(self.lazy(&data)), - visibility: vis.simplify(), + visibility: self.lazy(&ty::Visibility::from_hir(vis, id, tcx)), span: self.lazy(&md.inner), attributes: self.encode_attributes(attrs), children: self.lazy_seq(md.item_ids.iter().map(|item_id| { @@ -327,30 +327,6 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { } } -trait Visibility { - fn simplify(&self) -> ty::Visibility; -} - -impl Visibility for hir::Visibility { - fn simplify(&self) -> ty::Visibility { - if *self == hir::Public { - ty::Visibility::Public - } else { - ty::Visibility::PrivateExternal - } - } -} - -impl Visibility for ty::Visibility { - fn simplify(&self) -> ty::Visibility { - if *self == ty::Visibility::Public { - ty::Visibility::Public - } else { - ty::Visibility::PrivateExternal - } - } -} - impl<'a, 'b, 'tcx> IndexBuilder<'a, 'b, 'tcx> { fn encode_fields(&mut self, adt_def_id: DefId) { let def = self.tcx.lookup_adt_def(adt_def_id); @@ -386,7 +362,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { Entry { kind: EntryKind::Field, - visibility: field.vis.simplify(), + visibility: self.lazy(&field.vis), span: self.lazy(&tcx.def_span(def_id)), attributes: self.encode_attributes(&variant_data.fields()[field_index].attrs), children: LazySeq::empty(), @@ -419,7 +395,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { Entry { kind: EntryKind::Struct(self.lazy(&data)), - visibility: struct_vis.simplify(), + visibility: self.lazy(&ty::Visibility::from_hir(struct_vis, struct_id, tcx)), span: self.lazy(&tcx.def_span(def_id)), attributes: LazySeq::empty(), children: LazySeq::empty(), @@ -485,7 +461,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { Entry { kind: kind, - visibility: trait_item.vis.simplify(), + visibility: self.lazy(&trait_item.vis), span: self.lazy(&ast_item.span), attributes: self.encode_attributes(&ast_item.attrs), children: LazySeq::empty(), @@ -574,7 +550,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { Entry { kind: kind, - visibility: impl_item.vis.simplify(), + visibility: self.lazy(&impl_item.vis), span: self.lazy(&ast_item.span), attributes: self.encode_attributes(&ast_item.attrs), children: LazySeq::empty(), @@ -736,7 +712,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { Entry { kind: kind, - visibility: item.vis.simplify(), + visibility: self.lazy(&ty::Visibility::from_hir(&item.vis, item.id, tcx)), span: self.lazy(&item.span), attributes: self.encode_attributes(&item.attrs), children: match item.node { @@ -849,7 +825,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { kind: EntryKind::MacroDef(self.lazy(&MacroDef { body: ::syntax::print::pprust::tts_to_string(¯o_def.body) })), - visibility: ty::Visibility::Public, + visibility: self.lazy(&ty::Visibility::Public), span: self.lazy(¯o_def.span), attributes: self.encode_attributes(¯o_def.attrs), @@ -950,7 +926,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { Entry { kind: kind, - visibility: nitem.vis.simplify(), + visibility: self.lazy(&ty::Visibility::from_hir(&nitem.vis, nitem.id, tcx)), span: self.lazy(&nitem.span), attributes: self.encode_attributes(&nitem.attrs), children: LazySeq::empty(), @@ -1032,7 +1008,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { let tcx = self.tcx; Entry { kind: EntryKind::Type, - visibility: ty::Visibility::Public, + visibility: self.lazy(&ty::Visibility::Public), span: self.lazy(&tcx.def_span(def_id)), attributes: LazySeq::empty(), children: LazySeq::empty(), @@ -1060,7 +1036,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { Entry { kind: EntryKind::Closure(self.lazy(&data)), - visibility: ty::Visibility::Public, + visibility: self.lazy(&ty::Visibility::Public), span: self.lazy(&tcx.def_span(def_id)), attributes: self.encode_attributes(&tcx.get_attrs(def_id)), children: LazySeq::empty(), diff --git a/src/librustc_metadata/schema.rs b/src/librustc_metadata/schema.rs index 0b6606a00d3..2bd5a9ea59d 100644 --- a/src/librustc_metadata/schema.rs +++ b/src/librustc_metadata/schema.rs @@ -201,7 +201,7 @@ pub struct TraitImpls { #[derive(RustcEncodable, RustcDecodable)] pub struct Entry<'tcx> { pub kind: EntryKind<'tcx>, - pub visibility: ty::Visibility, + pub visibility: Lazy, pub span: Lazy, pub attributes: LazySeq, pub children: LazySeq, diff --git a/src/librustc_privacy/lib.rs b/src/librustc_privacy/lib.rs index 145b9176f6b..ca3e4e1c762 100644 --- a/src/librustc_privacy/lib.rs +++ b/src/librustc_privacy/lib.rs @@ -28,7 +28,7 @@ extern crate syntax_pos; use rustc::dep_graph::DepNode; use rustc::hir::{self, PatKind}; use rustc::hir::def::{self, Def, CtorKind}; -use rustc::hir::def_id::DefId; +use rustc::hir::def_id::{CRATE_DEF_INDEX, DefId}; use rustc::hir::intravisit::{self, Visitor, NestedVisitorMap}; use rustc::hir::itemlikevisit::DeepVisitor; use rustc::hir::pat_util::EnumerateAndAdjustIterator; @@ -391,7 +391,7 @@ impl<'b, 'a, 'tcx> TypeVisitor<'tcx> for ReachEverythingInTheInterfaceVisitor<'b struct PrivacyVisitor<'a, 'tcx: 'a> { tcx: TyCtxt<'a, 'tcx, 'tcx>, - curitem: ast::NodeId, + curitem: DefId, in_foreign: bool, } @@ -401,12 +401,12 @@ impl<'a, 'tcx> PrivacyVisitor<'a, 'tcx> { Some(node_id) => ty::Visibility::from_hir(&self.tcx.map.expect_item(node_id).vis, node_id, self.tcx), None => self.tcx.sess.cstore.visibility(did), - }.is_accessible_from(self.curitem, &self.tcx.map) + }.is_accessible_from(self.curitem, self.tcx) } // Checks that a field is in scope. fn check_field(&mut self, span: Span, def: &'tcx ty::AdtDef, field: &'tcx ty::FieldDef) { - if !def.is_enum() && !field.vis.is_accessible_from(self.curitem, &self.tcx.map) { + if !def.is_enum() && !field.vis.is_accessible_from(self.curitem, self.tcx) { struct_span_err!(self.tcx.sess, span, E0451, "field `{}` of {} `{}` is private", field.name, def.variant_descr(), self.tcx.item_path_str(def.did)) .span_label(span, &format!("field `{}` is private", field.name)) @@ -437,7 +437,7 @@ impl<'a, 'tcx> Visitor<'tcx> for PrivacyVisitor<'a, 'tcx> { } fn visit_item(&mut self, item: &'tcx hir::Item) { - let orig_curitem = replace(&mut self.curitem, item.id); + let orig_curitem = replace(&mut self.curitem, self.tcx.map.local_def_id(item.id)); intravisit::walk_item(self, item); self.curitem = orig_curitem; } @@ -474,7 +474,7 @@ impl<'a, 'tcx> Visitor<'tcx> for PrivacyVisitor<'a, 'tcx> { if let Def::StructCtor(_, CtorKind::Fn) = path.def { let adt_def = self.tcx.expect_variant_def(path.def); let private_indexes = adt_def.fields.iter().enumerate().filter(|&(_, field)| { - !field.vis.is_accessible_from(self.curitem, &self.tcx.map) + !field.vis.is_accessible_from(self.curitem, self.tcx) }).map(|(i, _)| i).collect::>(); if !private_indexes.is_empty() { @@ -940,7 +940,7 @@ impl<'a, 'tcx: 'a> TypeVisitor<'tcx> for SearchInterfaceForPrivateItemsVisitor<' ty::TyAdt(adt, _) => Some(adt.did), ty::TyDynamic(ref obj, ..) => obj.principal().map(|p| p.def_id()), ty::TyProjection(ref proj) => { - if self.required_visibility == ty::Visibility::PrivateExternal { + if self.required_visibility == ty::Visibility::Invisible { // Conservatively approximate the whole type alias as public without // recursing into its components when determining impl publicity. // For example, `impl ::Alias {...}` may be a public impl @@ -961,10 +961,10 @@ impl<'a, 'tcx: 'a> TypeVisitor<'tcx> for SearchInterfaceForPrivateItemsVisitor<' let item = self.tcx.map.expect_item(node_id); let vis = ty::Visibility::from_hir(&item.vis, node_id, self.tcx); - if !vis.is_at_least(self.min_visibility, &self.tcx.map) { + if !vis.is_at_least(self.min_visibility, self.tcx) { self.min_visibility = vis; } - if !vis.is_at_least(self.required_visibility, &self.tcx.map) { + if !vis.is_at_least(self.required_visibility, self.tcx) { if self.tcx.sess.features.borrow().pub_restricted || self.has_old_errors { let mut err = struct_span_err!(self.tcx.sess, self.span, E0446, "private type `{}` in public interface", ty); @@ -996,10 +996,10 @@ impl<'a, 'tcx: 'a> TypeVisitor<'tcx> for SearchInterfaceForPrivateItemsVisitor<' let item = self.tcx.map.expect_item(node_id); let vis = ty::Visibility::from_hir(&item.vis, node_id, self.tcx); - if !vis.is_at_least(self.min_visibility, &self.tcx.map) { + if !vis.is_at_least(self.min_visibility, self.tcx) { self.min_visibility = vis; } - if !vis.is_at_least(self.required_visibility, &self.tcx.map) { + if !vis.is_at_least(self.required_visibility, self.tcx) { if self.tcx.sess.features.borrow().pub_restricted || self.has_old_errors { struct_span_err!(self.tcx.sess, self.span, E0445, "private trait `{}` in public interface", trait_ref) @@ -1071,7 +1071,7 @@ impl<'a, 'tcx> Visitor<'tcx> for PrivateItemsInPublicInterfacesVisitor<'a, 'tcx> fn visit_item(&mut self, item: &'tcx hir::Item) { let tcx = self.tcx; let min = |vis1: ty::Visibility, vis2| { - if vis1.is_at_least(vis2, &tcx.map) { vis2 } else { vis1 } + if vis1.is_at_least(vis2, tcx) { vis2 } else { vis1 } }; let item_visibility = ty::Visibility::from_hir(&item.vis, item.id, tcx); @@ -1137,8 +1137,8 @@ impl<'a, 'tcx> Visitor<'tcx> for PrivateItemsInPublicInterfacesVisitor<'a, 'tcx> // An inherent impl is public when its type is public // Subitems of inherent impls have their own publicity hir::ItemImpl(.., None, _, ref impl_item_refs) => { - let ty_vis = self.check(item.id, ty::Visibility::PrivateExternal) - .item_type().min_visibility; + let ty_vis = + self.check(item.id, ty::Visibility::Invisible).item_type().min_visibility; self.check(item.id, ty_vis).generics().predicates(); for impl_item_ref in impl_item_refs { @@ -1156,7 +1156,7 @@ impl<'a, 'tcx> Visitor<'tcx> for PrivateItemsInPublicInterfacesVisitor<'a, 'tcx> // A trait impl is public when both its type and its trait are public // Subitems of trait impls have inherited publicity hir::ItemImpl(.., Some(_), _, ref impl_item_refs) => { - let vis = self.check(item.id, ty::Visibility::PrivateExternal) + let vis = self.check(item.id, ty::Visibility::Invisible) .item_type().impl_trait_ref().min_visibility; self.check(item.id, vis).generics().predicates(); for impl_item_ref in impl_item_refs { @@ -1203,7 +1203,7 @@ pub fn check_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, // Use the parent map to check the privacy of everything let mut visitor = PrivacyVisitor { - curitem: ast::DUMMY_NODE_ID, + curitem: DefId::local(CRATE_DEF_INDEX), in_foreign: false, tcx: tcx, }; diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs index 09f438953ec..758b93aed67 100644 --- a/src/librustc_resolve/build_reduced_graph.rs +++ b/src/librustc_resolve/build_reduced_graph.rs @@ -229,7 +229,7 @@ impl<'a> Resolver<'a> { ViewPathGlob(_) => { let subclass = GlobImport { is_prelude: is_prelude, - max_vis: Cell::new(ty::Visibility::PrivateExternal), + max_vis: Cell::new(ty::Visibility::Invisible), }; self.add_import_directive( module_path, subclass, view_path.span, item.id, vis, expansion, @@ -265,16 +265,16 @@ impl<'a> Resolver<'a> { ItemKind::Mod(..) if item.ident == keywords::Invalid.ident() => {} // Crate root ItemKind::Mod(..) => { - let def = Def::Mod(self.definitions.local_def_id(item.id)); + let def_id = self.definitions.local_def_id(item.id); + let module_kind = ModuleKind::Def(Def::Mod(def_id), ident.name); let module = self.arenas.alloc_module(ModuleData { no_implicit_prelude: parent.no_implicit_prelude || { attr::contains_name(&item.attrs, "no_implicit_prelude") }, - normal_ancestor_id: Some(item.id), - ..ModuleData::new(Some(parent), ModuleKind::Def(def, ident.name)) + ..ModuleData::new(Some(parent), module_kind, def_id) }); self.define(parent, ident, TypeNS, (module, vis, sp, expansion)); - self.module_map.insert(item.id, module); + self.module_map.insert(def_id, module); // Descend into the module. self.current_module = module; @@ -305,7 +305,8 @@ impl<'a> Resolver<'a> { ItemKind::Enum(ref enum_definition, _) => { let def = Def::Enum(self.definitions.local_def_id(item.id)); - let module = self.new_module(parent, ModuleKind::Def(def, ident.name), true); + let module_kind = ModuleKind::Def(def, ident.name); + let module = self.new_module(parent, module_kind, parent.normal_ancestor_id); self.define(parent, ident, TypeNS, (module, vis, sp, expansion)); for variant in &(*enum_definition).variants { @@ -355,8 +356,8 @@ impl<'a> Resolver<'a> { let def_id = self.definitions.local_def_id(item.id); // Add all the items within to a new module. - let module = - self.new_module(parent, ModuleKind::Def(Def::Trait(def_id), ident.name), true); + let module_kind = ModuleKind::Def(Def::Trait(def_id), ident.name); + let module = self.new_module(parent, module_kind, parent.normal_ancestor_id); self.define(parent, ident, TypeNS, (module, vis, sp, expansion)); self.current_module = module; } @@ -404,15 +405,10 @@ impl<'a> Resolver<'a> { fn build_reduced_graph_for_block(&mut self, block: &Block) { let parent = self.current_module; if self.block_needs_anonymous_module(block) { - let block_id = block.id; - - debug!("(building reduced graph for block) creating a new anonymous module for block \ - {}", - block_id); - - let new_module = self.new_module(parent, ModuleKind::Block(block_id), true); - self.module_map.insert(block_id, new_module); - self.current_module = new_module; // Descend into the block. + let module = + self.new_module(parent, ModuleKind::Block(block.id), parent.normal_ancestor_id); + self.block_map.insert(block.id, module); + self.current_module = module; // Descend into the block. } } @@ -429,7 +425,7 @@ impl<'a> Resolver<'a> { match def { Def::Mod(..) | Def::Enum(..) => { - let module = self.new_module(parent, ModuleKind::Def(def, ident.name), false); + let module = self.new_module(parent, ModuleKind::Def(def, ident.name), def_id); self.define(parent, ident, TypeNS, (module, vis, DUMMY_SP, Mark::root())); } Def::Variant(..) => { @@ -446,7 +442,8 @@ impl<'a> Resolver<'a> { self.define(parent, ident, ValueNS, (def, vis, DUMMY_SP, Mark::root())); } Def::Trait(..) => { - let module = self.new_module(parent, ModuleKind::Def(def, ident.name), false); + let module_kind = ModuleKind::Def(def, ident.name); + let module = self.new_module(parent, module_kind, parent.normal_ancestor_id); self.define(parent, ident, TypeNS, (module, vis, DUMMY_SP, Mark::root())); // If this is a trait, add all the trait item names to the trait info. @@ -497,12 +494,10 @@ impl<'a> Resolver<'a> { let def_id = DefId { krate: cnum, index: CRATE_DEF_INDEX }; let name = self.session.cstore.crate_name(cnum); let macros_only = self.session.cstore.dep_kind(cnum).macros_only(); + let module_kind = ModuleKind::Def(Def::Mod(def_id), name); let arenas = self.arenas; *self.extern_crate_roots.entry((cnum, macros_only)).or_insert_with(|| { - arenas.alloc_module(ModuleData { - populated: Cell::new(false), - ..ModuleData::new(None, ModuleKind::Def(Def::Mod(def_id), name)) - }) + arenas.alloc_module(ModuleData::new(None, module_kind, def_id)) }) } diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index dbeb8fe63b6..d7f237ea095 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -43,14 +43,13 @@ use rustc::middle::cstore::CrateLoader; use rustc::session::Session; use rustc::lint; use rustc::hir::def::*; -use rustc::hir::def_id::{CrateNum, CRATE_DEF_INDEX, DefId}; +use rustc::hir::def_id::{CrateNum, CRATE_DEF_INDEX, LOCAL_CRATE, DefId}; use rustc::ty; use rustc::hir::{Freevar, FreevarMap, TraitCandidate, TraitMap, GlobMap}; use rustc::util::nodemap::{NodeMap, NodeSet, FxHashMap, FxHashSet}; use syntax::ext::hygiene::{Mark, SyntaxContext}; -use syntax::ast::{self, FloatTy}; -use syntax::ast::{CRATE_NODE_ID, Name, NodeId, Ident, SpannedIdent, IntTy, UintTy}; +use syntax::ast::{self, Name, NodeId, Ident, SpannedIdent, FloatTy, IntTy, UintTy}; use syntax::ext::base::SyntaxExtension; use syntax::ext::base::Determinacy::{Determined, Undetermined}; use syntax::symbol::{Symbol, keywords}; @@ -771,8 +770,8 @@ pub struct ModuleData<'a> { parent: Option>, kind: ModuleKind, - // The node id of the closest normal module (`mod`) ancestor (including this module). - normal_ancestor_id: Option, + // The def id of the closest normal module (`mod`) ancestor (including this module). + normal_ancestor_id: DefId, resolutions: RefCell>>>, legacy_macro_resolutions: RefCell>, @@ -798,11 +797,11 @@ pub struct ModuleData<'a> { pub type Module<'a> = &'a ModuleData<'a>; impl<'a> ModuleData<'a> { - fn new(parent: Option>, kind: ModuleKind) -> Self { + fn new(parent: Option>, kind: ModuleKind, normal_ancestor_id: DefId) -> Self { ModuleData { parent: parent, kind: kind, - normal_ancestor_id: None, + normal_ancestor_id: normal_ancestor_id, resolutions: RefCell::new(FxHashMap()), legacy_macro_resolutions: RefCell::new(Vec::new()), macro_resolutions: RefCell::new(Vec::new()), @@ -811,7 +810,7 @@ impl<'a> ModuleData<'a> { glob_importers: RefCell::new(Vec::new()), globs: RefCell::new((Vec::new())), traits: RefCell::new(None), - populated: Cell::new(true), + populated: Cell::new(normal_ancestor_id.is_local()), } } @@ -848,7 +847,7 @@ impl<'a> ModuleData<'a> { } fn is_local(&self) -> bool { - self.normal_ancestor_id.is_some() + self.normal_ancestor_id.is_local() } } @@ -1063,7 +1062,7 @@ pub struct Resolver<'a> { pub export_map: ExportMap, pub trait_map: TraitMap, - // A map from nodes to modules, both normal (`mod`) modules and anonymous modules. + // A map from nodes to anonymous modules. // Anonymous modules are pseudo-modules that are implicitly created around items // contained within blocks. // @@ -1077,7 +1076,8 @@ pub struct Resolver<'a> { // // There will be an anonymous module created around `g` with the ID of the // entry block for `f`. - module_map: NodeMap>, + block_map: NodeMap>, + module_map: FxHashMap>, extern_crate_roots: FxHashMap<(CrateNum, bool /* MacrosOnly? */), Module<'a>>, pub make_glob_map: bool, @@ -1153,15 +1153,12 @@ impl<'a> ResolverArenas<'a> { } } -impl<'a> ty::NodeIdTree for Resolver<'a> { - fn is_descendant_of(&self, mut node: NodeId, ancestor: NodeId) -> bool { - while node != ancestor { - node = match self.module_map[&node].parent { - Some(parent) => parent.normal_ancestor_id.unwrap(), - None => return false, - } - } - true +impl<'a, 'b: 'a> ty::DefIdTree for &'a Resolver<'b> { + fn parent(self, id: DefId) -> Option { + match id.krate { + LOCAL_CRATE => self.definitions.def_key(id.index).parent, + _ => self.session.cstore.def_key(id).parent, + }.map(|index| DefId { index: index, ..id }) } } @@ -1202,14 +1199,14 @@ impl<'a> Resolver<'a> { crate_loader: &'a mut CrateLoader, arenas: &'a ResolverArenas<'a>) -> Resolver<'a> { - let root_def = Def::Mod(DefId::local(CRATE_DEF_INDEX)); + let root_def_id = DefId::local(CRATE_DEF_INDEX); + let root_module_kind = ModuleKind::Def(Def::Mod(root_def_id), keywords::Invalid.name()); let graph_root = arenas.alloc_module(ModuleData { - normal_ancestor_id: Some(CRATE_NODE_ID), no_implicit_prelude: attr::contains_name(&krate.attrs, "no_implicit_prelude"), - ..ModuleData::new(None, ModuleKind::Def(root_def, keywords::Invalid.name())) + ..ModuleData::new(None, root_module_kind, root_def_id) }); - let mut module_map = NodeMap(); - module_map.insert(CRATE_NODE_ID, graph_root); + let mut module_map = FxHashMap(); + module_map.insert(DefId::local(CRATE_DEF_INDEX), graph_root); let mut definitions = Definitions::new(); DefCollector::new(&mut definitions).collect_root(); @@ -1254,6 +1251,7 @@ impl<'a> Resolver<'a> { export_map: NodeMap(), trait_map: NodeMap(), module_map: module_map, + block_map: NodeMap(), extern_crate_roots: FxHashMap(), make_glob_map: make_glob_map == MakeGlobMap::Yes, @@ -1324,12 +1322,9 @@ impl<'a> Resolver<'a> { self.crate_loader.postprocess(krate); } - fn new_module(&self, parent: Module<'a>, kind: ModuleKind, local: bool) -> Module<'a> { - self.arenas.alloc_module(ModuleData { - normal_ancestor_id: if local { self.current_module.normal_ancestor_id } else { None }, - populated: Cell::new(local), - ..ModuleData::new(Some(parent), kind) - }) + fn new_module(&self, parent: Module<'a>, kind: ModuleKind, normal_ancestor_id: DefId) + -> Module<'a> { + self.arenas.alloc_module(ModuleData::new(Some(parent), kind, normal_ancestor_id)) } fn record_use(&mut self, ident: Ident, ns: Namespace, binding: &'a NameBinding<'a>, span: Span) @@ -1462,6 +1457,7 @@ impl<'a> Resolver<'a> { fn with_scope(&mut self, id: NodeId, f: F) where F: FnOnce(&mut Resolver) { + let id = self.definitions.local_def_id(id); let module = self.module_map.get(&id).cloned(); // clones a reference if let Some(module) = module { // Move down in the graph. @@ -1958,7 +1954,7 @@ impl<'a> Resolver<'a> { debug!("(resolving block) entering block"); // Move down in the graph, if there's an anonymous module rooted here. let orig_module = self.current_module; - let anonymous_module = self.module_map.get(&block.id).cloned(); // clones a reference + let anonymous_module = self.block_map.get(&block.id).cloned(); // clones a reference let mut num_macro_definition_ribs = 0; if let Some(anonymous_module) = anonymous_module { @@ -2334,13 +2330,13 @@ impl<'a> Resolver<'a> { let ns = if is_last { opt_ns.unwrap_or(TypeNS) } else { TypeNS }; if i == 0 && ns == TypeNS && ident.name == keywords::SelfValue.name() { - module = Some(self.module_map[&self.current_module.normal_ancestor_id.unwrap()]); + module = Some(self.module_map[&self.current_module.normal_ancestor_id]); continue } else if allow_super && ns == TypeNS && ident.name == keywords::Super.name() { let current_module = if i == 0 { self.current_module } else { module.unwrap() }; - let self_module = self.module_map[¤t_module.normal_ancestor_id.unwrap()]; + let self_module = self.module_map[¤t_module.normal_ancestor_id]; if let Some(parent) = self_module.parent { - module = Some(self.module_map[&parent.normal_ancestor_id.unwrap()]); + module = Some(self.module_map[&parent.normal_ancestor_id]); continue } else { let msg = "There are too many initial `super`s.".to_string(); @@ -3000,10 +2996,12 @@ impl<'a> Resolver<'a> { fn resolve_visibility(&mut self, vis: &ast::Visibility) -> ty::Visibility { let (segments, span, id) = match *vis { ast::Visibility::Public => return ty::Visibility::Public, - ast::Visibility::Crate(_) => return ty::Visibility::Restricted(ast::CRATE_NODE_ID), + ast::Visibility::Crate(_) => { + return ty::Visibility::Restricted(DefId::local(CRATE_DEF_INDEX)); + } ast::Visibility::Restricted { ref path, id } => (&path.segments, path.span, id), ast::Visibility::Inherited => { - return ty::Visibility::Restricted(self.current_module.normal_ancestor_id.unwrap()); + return ty::Visibility::Restricted(self.current_module.normal_ancestor_id); } }; @@ -3012,7 +3010,7 @@ impl<'a> Resolver<'a> { let vis = match self.resolve_path(&path, None, Some(span)) { PathResult::Module(module) => { path_resolution = PathResolution::new(module.def().unwrap()); - ty::Visibility::Restricted(module.normal_ancestor_id.unwrap()) + ty::Visibility::Restricted(module.normal_ancestor_id) } PathResult::Failed(msg, _) => { self.session.span_err(span, &format!("failed to resolve module path. {}", msg)); @@ -3029,11 +3027,11 @@ impl<'a> Resolver<'a> { } fn is_accessible(&self, vis: ty::Visibility) -> bool { - vis.is_accessible_from(self.current_module.normal_ancestor_id.unwrap(), self) + vis.is_accessible_from(self.current_module.normal_ancestor_id, self) } fn is_accessible_from(&self, vis: ty::Visibility, module: Module<'a>) -> bool { - vis.is_accessible_from(module.normal_ancestor_id.unwrap(), self) + vis.is_accessible_from(module.normal_ancestor_id, self) } fn report_errors(&mut self) { diff --git a/src/librustc_resolve/macros.rs b/src/librustc_resolve/macros.rs index b0f72d9f6e8..44cc580ad12 100644 --- a/src/librustc_resolve/macros.rs +++ b/src/librustc_resolve/macros.rs @@ -88,7 +88,7 @@ impl<'a> base::Resolver for Resolver<'a> { fn get_module_scope(&mut self, id: ast::NodeId) -> Mark { let mark = Mark::fresh(); - let module = self.module_map[&id]; + let module = self.module_map[&self.definitions.local_def_id(id)]; self.invocations.insert(mark, self.arenas.alloc_invocation_data(InvocationData { module: Cell::new(module), def_index: module.def_id().unwrap().index, @@ -154,7 +154,7 @@ impl<'a> base::Resolver for Resolver<'a> { let binding = self.arenas.alloc_name_binding(NameBinding { kind: NameBindingKind::Def(Def::Macro(def_id)), span: DUMMY_SP, - vis: ty::Visibility::PrivateExternal, + vis: ty::Visibility::Invisible, expansion: Mark::root(), }); self.builtin_macros.insert(ident.name, binding); diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs index f62974b30d1..62dcb7e9e2f 100644 --- a/src/librustc_resolve/resolve_imports.rs +++ b/src/librustc_resolve/resolve_imports.rs @@ -19,6 +19,7 @@ use {resolve_error, ResolutionError}; use rustc::ty; use rustc::lint::builtin::PRIVATE_IN_PUBLIC; +use rustc::hir::def_id::DefId; use rustc::hir::def::*; use syntax::ast::{Ident, NodeId}; @@ -274,7 +275,7 @@ impl<'a> Resolver<'a> { // Given a binding and an import directive that resolves to it, // return the corresponding binding defined by the import directive. - pub fn import(&mut self, binding: &'a NameBinding<'a>, directive: &'a ImportDirective<'a>) + pub fn import(&self, binding: &'a NameBinding<'a>, directive: &'a ImportDirective<'a>) -> &'a NameBinding<'a> { let vis = if binding.pseudo_vis().is_at_least(directive.vis.get(), self) || !directive.is_glob() && binding.is_extern_crate() { // c.f. `PRIVATE_IN_PUBLIC` @@ -317,7 +318,7 @@ impl<'a> Resolver<'a> { resolution.shadows_glob = Some(binding); } else if binding.def() != old_binding.def() { resolution.binding = Some(this.ambiguity(old_binding, binding)); - } else if !old_binding.vis.is_at_least(binding.vis, this) { + } else if !old_binding.vis.is_at_least(binding.vis, &*this) { // We are glob-importing the same item but with greater visibility. resolution.binding = Some(binding); } @@ -340,7 +341,7 @@ impl<'a> Resolver<'a> { }) } - pub fn ambiguity(&mut self, b1: &'a NameBinding<'a>, b2: &'a NameBinding<'a>) + pub fn ambiguity(&self, b1: &'a NameBinding<'a>, b2: &'a NameBinding<'a>) -> &'a NameBinding<'a> { self.arenas.alloc_name_binding(NameBinding { kind: NameBindingKind::Ambiguity { b1: b1, b2: b2, legacy: false }, @@ -415,9 +416,9 @@ impl<'a, 'b: 'a> ::std::ops::DerefMut for ImportResolver<'a, 'b> { } } -impl<'a, 'b: 'a> ty::NodeIdTree for ImportResolver<'a, 'b> { - fn is_descendant_of(&self, node: NodeId, ancestor: NodeId) -> bool { - self.resolver.is_descendant_of(node, ancestor) +impl<'a, 'b: 'a> ty::DefIdTree for &'a ImportResolver<'a, 'b> { + fn parent(self, id: DefId) -> Option { + self.resolver.parent(id) } } @@ -490,7 +491,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { let vis = directive.vis.get(); // For better failure detection, pretend that the import will not define any names // while resolving its module path. - directive.vis.set(ty::Visibility::PrivateExternal); + directive.vis.set(ty::Visibility::Invisible); let result = self.resolve_path(&directive.module_path, None, None); directive.vis.set(vis); @@ -580,8 +581,8 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { } GlobImport { is_prelude, ref max_vis } => { if !is_prelude && - max_vis.get() != ty::Visibility::PrivateExternal && // Allow empty globs. - !max_vis.get().is_at_least(directive.vis.get(), self) { + max_vis.get() != ty::Visibility::Invisible && // Allow empty globs. + !max_vis.get().is_at_least(directive.vis.get(), &*self) { let msg = "A non-empty glob must import something with the glob's visibility"; self.session.span_err(directive.span, msg); } @@ -644,7 +645,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { self.per_ns(|this, ns| { if let Ok(binding) = result[ns].get() { let vis = directive.vis.get(); - if !binding.pseudo_vis().is_at_least(vis, this) { + if !binding.pseudo_vis().is_at_least(vis, &*this) { reexport_error = Some((ns, binding)); } else { any_successful_reexport = true; @@ -752,7 +753,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { match binding.kind { NameBindingKind::Import { binding: orig_binding, directive, .. } => { if ns == TypeNS && orig_binding.is_variant() && - !orig_binding.vis.is_at_least(binding.vis, self) { + !orig_binding.vis.is_at_least(binding.vis, &*self) { let msg = format!("variant `{}` is private, and cannot be reexported \ (error E0364), consider declaring its enum as `pub`", ident); diff --git a/src/librustc_typeck/check/method/mod.rs b/src/librustc_typeck/check/method/mod.rs index 6353b45200b..9b8e77301e5 100644 --- a/src/librustc_typeck/check/method/mod.rs +++ b/src/librustc_typeck/check/method/mod.rs @@ -344,7 +344,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { self.tcx.check_stability(def.def_id(), expr_id, span); if let probe::InherentImplPick = pick.kind { - if !pick.item.vis.is_accessible_from(self.body_id, &self.tcx.map) { + if !self.tcx.vis_is_accessible_from(pick.item.vis, self.body_id) { let msg = format!("{} `{}` is private", def.kind_name(), method_name); self.tcx.sess.span_err(span, &msg); } diff --git a/src/librustc_typeck/check/method/probe.rs b/src/librustc_typeck/check/method/probe.rs index 06158366f5c..1962534c397 100644 --- a/src/librustc_typeck/check/method/probe.rs +++ b/src/librustc_typeck/check/method/probe.rs @@ -503,7 +503,7 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> { continue } - if !item.vis.is_accessible_from(self.body_id, &self.tcx.map) { + if !self.tcx.vis_is_accessible_from(item.vis, self.body_id) { self.private_candidate = Some(item.def()); continue } diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index 3086721852d..a2dceed8d26 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -3009,7 +3009,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { debug!("struct named {:?}", base_t); if let Some(field) = base_def.struct_variant().find_field_named(field.node) { let field_ty = self.field_ty(expr.span, field, substs); - if field.vis.is_accessible_from(self.body_id, &self.tcx().map) { + if self.tcx.vis_is_accessible_from(field.vis, self.body_id) { autoderef.finalize(lvalue_pref, Some(base)); self.write_autoderef_adjustment(base.id, autoderefs, base_t); @@ -3116,7 +3116,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { base_def.struct_variant().fields.get(idx.node).and_then(|field| { let field_ty = self.field_ty(expr.span, field, substs); private_candidate = Some((base_def.did, field_ty)); - if field.vis.is_accessible_from(self.body_id, &self.tcx().map) { + if self.tcx.vis_is_accessible_from(field.vis, self.body_id) { self.tcx.check_stability(field.did, expr.id, expr.span); Some(field_ty) } else {