From 87b407136b7b0ba60af3a6c282b5bef15282d76d Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Tue, 13 Dec 2016 17:48:52 -0500 Subject: [PATCH 01/11] Definitions: Split out NodeId <-> DefIndex mapping --- src/librustc/hir/map/definitions.rs | 44 ++++++++++++++--------------- 1 file changed, 21 insertions(+), 23 deletions(-) diff --git a/src/librustc/hir/map/definitions.rs b/src/librustc/hir/map/definitions.rs index a684563512da6..72c1ff9ee2efc 100644 --- a/src/librustc/hir/map/definitions.rs +++ b/src/librustc/hir/map/definitions.rs @@ -18,12 +18,20 @@ use syntax::symbol::{Symbol, InternedString}; use ty::TyCtxt; use util::nodemap::NodeMap; +//! For each definition, we track the following data. A definition +//! here is defined somewhat circularly as "something with a def-id", +//! but it generally corresponds to things like structs, enums, etc. +//! There are also some rather random cases (like const initializer +//! expressions) that are mostly just leftovers. + + /// The definition table containing node definitions #[derive(Clone)] pub struct Definitions { - data: Vec, + data: Vec, key_map: FxHashMap, - node_map: NodeMap, + node_to_def_index: NodeMap, + def_index_to_node: Vec, } /// A unique identifier that we can use to lookup a definition @@ -50,19 +58,6 @@ pub struct DisambiguatedDefPathData { pub disambiguator: u32 } -/// For each definition, we track the following data. A definition -/// here is defined somewhat circularly as "something with a def-id", -/// but it generally corresponds to things like structs, enums, etc. -/// There are also some rather random cases (like const initializer -/// expressions) that are mostly just leftovers. -#[derive(Clone, Debug)] -pub struct DefData { - pub key: DefKey, - - /// Local ID within the HIR. - pub node_id: ast::NodeId, -} - #[derive(Clone, Debug, PartialEq, Eq, Hash, RustcEncodable, RustcDecodable)] pub struct DefPath { /// the path leading from the crate root to the item @@ -221,7 +216,8 @@ impl Definitions { Definitions { data: vec![], key_map: FxHashMap(), - node_map: NodeMap(), + node_to_def_index: NodeMap(), + def_index_to_node: vec![], } } @@ -248,7 +244,7 @@ impl Definitions { } pub fn opt_def_index(&self, node: ast::NodeId) -> Option { - self.node_map.get(&node).cloned() + self.node_to_def_index.get(&node).cloned() } pub fn opt_local_def_id(&self, node: ast::NodeId) -> Option { @@ -262,7 +258,8 @@ impl Definitions { pub fn as_local_node_id(&self, def_id: DefId) -> Option { if def_id.krate == LOCAL_CRATE { assert!(def_id.index.as_usize() < self.data.len()); - Some(self.data[def_id.index.as_usize()].node_id) + // Some(self.data[def_id.index.as_usize()].node_id) + Some(self.def_index_to_node[def_id.index.as_usize()]) } else { None } @@ -277,11 +274,11 @@ impl Definitions { debug!("create_def_with_parent(parent={:?}, node_id={:?}, data={:?})", parent, node_id, data); - assert!(!self.node_map.contains_key(&node_id), + assert!(!self.node_to_def_index.contains_key(&node_id), "adding a def'n for node-id {:?} and data {:?} but a previous def'n exists: {:?}", node_id, data, - self.data[self.node_map[&node_id].as_usize()]); + self.data[self.node_to_def_index[&node_id].as_usize()]); assert!(parent.is_some() ^ match data { DefPathData::CrateRoot | DefPathData::InlinedRoot(_) => true, @@ -306,9 +303,10 @@ impl Definitions { // Create the definition. let index = DefIndex::new(self.data.len()); - self.data.push(DefData { key: key.clone(), node_id: node_id }); - debug!("create_def_with_parent: node_map[{:?}] = {:?}", node_id, index); - self.node_map.insert(node_id, index); + self.data.push(DefData { key: key.clone() }); + self.def_index_to_node.push(node_id); + debug!("create_def_with_parent: node_to_def_index[{:?}] = {:?}", node_id, index); + self.node_to_def_index.insert(node_id, index); debug!("create_def_with_parent: key_map[{:?}] = {:?}", key, index); self.key_map.insert(key, index); From 8e34a8e4223a744bcc4fb2d77c06fb6c183dcc26 Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Wed, 14 Dec 2016 11:27:39 -0500 Subject: [PATCH 02/11] Definitions: Extract DefPath interning into its own data structure. --- src/librustc/hir/map/definitions.rs | 108 ++++++++++++++++++++++------ 1 file changed, 85 insertions(+), 23 deletions(-) diff --git a/src/librustc/hir/map/definitions.rs b/src/librustc/hir/map/definitions.rs index 72c1ff9ee2efc..159e80d7d3975 100644 --- a/src/librustc/hir/map/definitions.rs +++ b/src/librustc/hir/map/definitions.rs @@ -8,9 +8,19 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. + +//! For each definition, we track the following data. A definition +//! here is defined somewhat circularly as "something with a def-id", +//! but it generally corresponds to things like structs, enums, etc. +//! There are also some rather random cases (like const initializer +//! expressions) that are mostly just leftovers. + + + use hir::def_id::{CrateNum, DefId, DefIndex, LOCAL_CRATE}; use rustc_data_structures::fx::FxHashMap; use rustc_data_structures::stable_hasher::StableHasher; +use serialize::{Encodable, Decodable, Encoder, Decoder}; use std::fmt::Write; use std::hash::{Hash, Hasher}; use syntax::ast; @@ -18,18 +28,72 @@ use syntax::symbol::{Symbol, InternedString}; use ty::TyCtxt; use util::nodemap::NodeMap; -//! For each definition, we track the following data. A definition -//! here is defined somewhat circularly as "something with a def-id", -//! but it generally corresponds to things like structs, enums, etc. -//! There are also some rather random cases (like const initializer -//! expressions) that are mostly just leftovers. +#[derive(Clone)] +pub struct DefPathTable { + index_to_key: Vec, + key_to_index: FxHashMap, +} + +impl DefPathTable { + fn insert(&mut self, key: DefKey) -> DefIndex { + let index = DefIndex::new(self.index_to_key.len()); + debug!("DefPathTable::insert() - {:?} <-> {:?}", key, index); + self.index_to_key.push(key.clone()); + self.key_to_index.insert(key, index); + index + } + + #[inline(always)] + pub fn def_key(&self, index: DefIndex) -> DefKey { + self.index_to_key[index.as_usize()].clone() + } + + #[inline(always)] + pub fn def_index_for_def_key(&self, key: &DefKey) -> Option { + self.key_to_index.get(key).cloned() + } + + #[inline(always)] + pub fn contains_key(&self, key: &DefKey) -> bool { + self.key_to_index.contains_key(key) + } + + /// Returns the path from the crate root to `index`. The root + /// nodes are not included in the path (i.e., this will be an + /// empty vector for the crate root). For an inlined item, this + /// will be the path of the item in the external crate (but the + /// path will begin with the path to the external crate). + pub fn def_path(&self, index: DefIndex) -> DefPath { + DefPath::make(LOCAL_CRATE, index, |p| self.def_key(p)) + } +} + + +impl Encodable for DefPathTable { + fn encode(&self, s: &mut S) -> Result<(), S::Error> { + self.index_to_key.encode(s) + } +} + +impl Decodable for DefPathTable { + fn decode(d: &mut D) -> Result { + let index_to_key: Vec = Decodable::decode(d)?; + let key_to_index = index_to_key.iter() + .enumerate() + .map(|(index, key)| (key.clone(), DefIndex::new(index))) + .collect(); + Ok(DefPathTable { + index_to_key: index_to_key, + key_to_index: key_to_index, + }) + } +} /// The definition table containing node definitions #[derive(Clone)] pub struct Definitions { - data: Vec, - key_map: FxHashMap, + table: DefPathTable, node_to_def_index: NodeMap, def_index_to_node: Vec, } @@ -214,8 +278,10 @@ impl Definitions { /// Create new empty definition map. pub fn new() -> Definitions { Definitions { - data: vec![], - key_map: FxHashMap(), + table: DefPathTable { + index_to_key: vec![], + key_to_index: FxHashMap(), + }, node_to_def_index: NodeMap(), def_index_to_node: vec![], } @@ -223,15 +289,15 @@ impl Definitions { /// Get the number of definitions. pub fn len(&self) -> usize { - self.data.len() + self.def_index_to_node.len() } pub fn def_key(&self, index: DefIndex) -> DefKey { - self.data[index.as_usize()].key.clone() + self.table.def_key(index) } pub fn def_index_for_def_key(&self, key: DefKey) -> Option { - self.key_map.get(&key).cloned() + self.table.def_index_for_def_key(&key) } /// Returns the path from the crate root to `index`. The root @@ -257,8 +323,7 @@ impl Definitions { pub fn as_local_node_id(&self, def_id: DefId) -> Option { if def_id.krate == LOCAL_CRATE { - assert!(def_id.index.as_usize() < self.data.len()); - // Some(self.data[def_id.index.as_usize()].node_id) + assert!(def_id.index.as_usize() < self.def_index_to_node.len()); Some(self.def_index_to_node[def_id.index.as_usize()]) } else { None @@ -278,7 +343,7 @@ impl Definitions { "adding a def'n for node-id {:?} and data {:?} but a previous def'n exists: {:?}", node_id, data, - self.data[self.node_to_def_index[&node_id].as_usize()]); + self.table.def_key(self.node_to_def_index[&node_id])); assert!(parent.is_some() ^ match data { DefPathData::CrateRoot | DefPathData::InlinedRoot(_) => true, @@ -295,21 +360,18 @@ impl Definitions { } }; - while self.key_map.contains_key(&key) { + while self.table.contains_key(&key) { key.disambiguated_data.disambiguator += 1; } debug!("create_def_with_parent: after disambiguation, key = {:?}", key); // Create the definition. - let index = DefIndex::new(self.data.len()); - self.data.push(DefData { key: key.clone() }); - self.def_index_to_node.push(node_id); - debug!("create_def_with_parent: node_to_def_index[{:?}] = {:?}", node_id, index); + let index = self.table.insert(key); + debug!("create_def_with_parent: def_index_to_node[{:?} <-> {:?}", index, node_id); self.node_to_def_index.insert(node_id, index); - debug!("create_def_with_parent: key_map[{:?}] = {:?}", key, index); - self.key_map.insert(key, index); - + assert_eq!(index.as_usize(), self.def_index_to_node.len()); + self.def_index_to_node.push(node_id); index } From aed0cdbfd21984b144d685131e0070cfa811fb49 Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Fri, 16 Dec 2016 12:48:54 -0500 Subject: [PATCH 03/11] definitions: Don't allocate DefIds for inlined HIR --- src/librustc/hir/map/collector.rs | 4 -- src/librustc/hir/map/def_collector.rs | 21 +--------- src/librustc/hir/map/definitions.rs | 57 ++------------------------- src/librustc/hir/map/mod.rs | 13 +----- src/librustc/ty/item_path.rs | 5 --- src/librustc_metadata/astencode.rs | 10 ----- src/librustc_metadata/decoder.rs | 5 +-- 7 files changed, 6 insertions(+), 109 deletions(-) diff --git a/src/librustc/hir/map/collector.rs b/src/librustc/hir/map/collector.rs index c46c8f044e0ff..45988886a608a 100644 --- a/src/librustc/hir/map/collector.rs +++ b/src/librustc/hir/map/collector.rs @@ -11,7 +11,6 @@ use super::*; use hir::intravisit::{Visitor, NestedVisitorMap}; -use hir::def_id::DefId; use middle::cstore::InlinedItem; use std::iter::repeat; use syntax::ast::{NodeId, CRATE_NODE_ID}; @@ -47,8 +46,6 @@ impl<'ast> NodeCollector<'ast> { pub fn extend(krate: &'ast Crate, parent: &'ast InlinedItem, parent_node: NodeId, - parent_def_path: DefPath, - parent_def_id: DefId, map: Vec>) -> NodeCollector<'ast> { let mut collector = NodeCollector { @@ -58,7 +55,6 @@ impl<'ast> NodeCollector<'ast> { ignore_nested_items: true }; - assert_eq!(parent_def_path.krate, parent_def_id.krate); collector.insert_entry(parent_node, RootInlinedParent(parent)); collector diff --git a/src/librustc/hir/map/def_collector.rs b/src/librustc/hir/map/def_collector.rs index eb5a89f320e7b..8c303a96b0ced 100644 --- a/src/librustc/hir/map/def_collector.rs +++ b/src/librustc/hir/map/def_collector.rs @@ -12,7 +12,7 @@ use hir::map::definitions::*; use hir; use hir::intravisit::{self, Visitor, NestedVisitorMap}; -use hir::def_id::{CRATE_DEF_INDEX, DefId, DefIndex}; +use hir::def_id::{CRATE_DEF_INDEX, DefIndex}; use middle::cstore::InlinedItem; @@ -47,25 +47,6 @@ impl<'a> DefCollector<'a> { } } - pub fn extend(parent_node: NodeId, - parent_def_path: DefPath, - parent_def_id: DefId, - definitions: &'a mut Definitions) - -> Self { - let mut collector = DefCollector::new(definitions); - - assert_eq!(parent_def_path.krate, parent_def_id.krate); - let root_path = Box::new(InlinedRootPath { - data: parent_def_path.data, - def_id: parent_def_id, - }); - - let def = collector.create_def(parent_node, DefPathData::InlinedRoot(root_path)); - collector.parent_def = Some(def); - - collector - } - pub fn collect_root(&mut self) { let root = self.create_def_with_parent(None, CRATE_NODE_ID, DefPathData::CrateRoot); assert_eq!(root, CRATE_DEF_INDEX); diff --git a/src/librustc/hir/map/definitions.rs b/src/librustc/hir/map/definitions.rs index 159e80d7d3975..d90a9ef7d80d3 100644 --- a/src/librustc/hir/map/definitions.rs +++ b/src/librustc/hir/map/definitions.rs @@ -57,15 +57,6 @@ impl DefPathTable { pub fn contains_key(&self, key: &DefKey) -> bool { self.key_to_index.contains_key(key) } - - /// Returns the path from the crate root to `index`. The root - /// nodes are not included in the path (i.e., this will be an - /// empty vector for the crate root). For an inlined item, this - /// will be the path of the item in the external crate (but the - /// path will begin with the path to the external crate). - pub fn def_path(&self, index: DefIndex) -> DefPath { - DefPath::make(LOCAL_CRATE, index, |p| self.def_key(p)) - } } @@ -136,12 +127,11 @@ impl DefPath { self.krate == LOCAL_CRATE } - pub fn make(start_krate: CrateNum, + pub fn make(krate: CrateNum, start_index: DefIndex, mut get_key: FN) -> DefPath where FN: FnMut(DefIndex) -> DefKey { - let mut krate = start_krate; let mut data = vec![]; let mut index = Some(start_index); loop { @@ -154,13 +144,6 @@ impl DefPath { assert!(key.parent.is_none()); break; } - DefPathData::InlinedRoot(ref p) => { - assert!(key.parent.is_none()); - assert!(!p.def_id.is_local()); - data.extend(p.data.iter().cloned().rev()); - krate = p.def_id.krate; - break; - } _ => { data.push(key.disambiguated_data); index = key.parent; @@ -203,31 +186,6 @@ impl DefPath { } } -/// Root of an inlined item. We track the `DefPath` of the item within -/// the original crate but also its def-id. This is kind of an -/// augmented version of a `DefPath` that includes a `DefId`. This is -/// all sort of ugly but the hope is that inlined items will be going -/// away soon anyway. -/// -/// Some of the constraints that led to the current approach: -/// -/// - I don't want to have a `DefId` in the main `DefPath` because -/// that gets serialized for incr. comp., and when reloaded the -/// `DefId` is no longer valid. I'd rather maintain the invariant -/// that every `DefId` is valid, and a potentially outdated `DefId` is -/// represented as a `DefPath`. -/// - (We don't serialize def-paths from inlined items, so it's ok to have one here.) -/// - We need to be able to extract the def-id from inline items to -/// make the symbol name. In theory we could retrace it from the -/// data, but the metadata doesn't have the required indices, and I -/// don't want to write the code to create one just for this. -/// - It may be that we don't actually need `data` at all. We'll have -/// to see about that. -#[derive(Clone, Debug, PartialEq, Eq, Hash, RustcEncodable, RustcDecodable)] -pub struct InlinedRootPath { - pub data: Vec, - pub def_id: DefId, -} #[derive(Clone, Debug, PartialEq, Eq, Hash, RustcEncodable, RustcDecodable)] pub enum DefPathData { @@ -235,9 +193,7 @@ pub enum DefPathData { // they are treated specially by the `def_path` function. /// The crate root (marker) CrateRoot, - /// An inlined root - InlinedRoot(Box), - + // Catch-all for random DefId things like DUMMY_NODE_ID Misc, @@ -345,10 +301,7 @@ impl Definitions { data, self.table.def_key(self.node_to_def_index[&node_id])); - assert!(parent.is_some() ^ match data { - DefPathData::CrateRoot | DefPathData::InlinedRoot(_) => true, - _ => false, - }); + assert!(parent.is_some() ^ (data == DefPathData::CrateRoot)); // Find a unique DefKey. This basically means incrementing the disambiguator // until we get no match. @@ -393,7 +346,6 @@ impl DefPathData { Impl | CrateRoot | - InlinedRoot(_) | Misc | ClosureExpr | StructCtor | @@ -420,9 +372,6 @@ impl DefPathData { // note that this does not show up in user printouts CrateRoot => "{{root}}", - // note that this does not show up in user printouts - InlinedRoot(_) => "{{inlined-root}}", - Impl => "{{impl}}", Misc => "{{?}}", ClosureExpr => "{{closure}}", diff --git a/src/librustc/hir/map/mod.rs b/src/librustc/hir/map/mod.rs index 117edcf14a1d1..569d697f374a1 100644 --- a/src/librustc/hir/map/mod.rs +++ b/src/librustc/hir/map/mod.rs @@ -13,7 +13,7 @@ use self::MapEntry::*; use self::collector::NodeCollector; pub use self::def_collector::{DefCollector, MacroInvocationData}; pub use self::definitions::{Definitions, DefKey, DefPath, DefPathData, - DisambiguatedDefPathData, InlinedRootPath}; + DisambiguatedDefPathData}; use dep_graph::{DepGraph, DepNode}; @@ -945,8 +945,6 @@ pub fn map_crate<'ast>(forest: &'ast mut Forest, /// Used for items loaded from external crate that are being inlined into this /// crate. pub fn map_decoded_item<'ast>(map: &Map<'ast>, - parent_def_path: DefPath, - parent_def_id: DefId, ii: InlinedItem, ii_parent_id: NodeId) -> &'ast InlinedItem { @@ -954,18 +952,9 @@ pub fn map_decoded_item<'ast>(map: &Map<'ast>, let ii = map.forest.inlined_items.alloc(ii); - let defs = &mut *map.definitions.borrow_mut(); - let mut def_collector = DefCollector::extend(ii_parent_id, - parent_def_path.clone(), - parent_def_id, - defs); - def_collector.walk_item(ii, map.krate()); - let mut collector = NodeCollector::extend(map.krate(), ii, ii_parent_id, - parent_def_path, - parent_def_id, mem::replace(&mut *map.map.borrow_mut(), vec![])); ii.visit(&mut collector); *map.map.borrow_mut() = collector.map; diff --git a/src/librustc/ty/item_path.rs b/src/librustc/ty/item_path.rs index 440a3916786fa..0e4c14029e9b9 100644 --- a/src/librustc/ty/item_path.rs +++ b/src/librustc/ty/item_path.rs @@ -160,11 +160,6 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> { self.push_krate_path(buffer, def_id.krate); } - DefPathData::InlinedRoot(ref root_path) => { - assert!(key.parent.is_none()); - self.push_item_path(buffer, root_path.def_id); - } - DefPathData::Impl => { self.push_impl_path(buffer, def_id); } diff --git a/src/librustc_metadata/astencode.rs b/src/librustc_metadata/astencode.rs index 6598b7dcc527f..926c44824ce48 100644 --- a/src/librustc_metadata/astencode.rs +++ b/src/librustc_metadata/astencode.rs @@ -102,8 +102,6 @@ impl<'a, 'b, 'tcx> Visitor<'tcx> for SideTableEncodingIdVisitor<'a, 'b, 'tcx> { /// ast-map. pub fn decode_inlined_item<'a, 'tcx>(cdata: &CrateMetadata, tcx: TyCtxt<'a, 'tcx, 'tcx>, - parent_def_path: ast_map::DefPath, - parent_did: DefId, ast: Ast<'tcx>, orig_did: DefId) -> &'tcx InlinedItem { @@ -120,17 +118,9 @@ pub fn decode_inlined_item<'a, 'tcx>(cdata: &CrateMetadata, let ii = ast.item.decode((cdata, tcx, id_ranges)); let item_node_id = tcx.sess.next_node_id(); let ii = ast_map::map_decoded_item(&tcx.map, - parent_def_path, - parent_did, ii, item_node_id); - let inlined_did = tcx.map.local_def_id(item_node_id); - let ty = tcx.item_type(orig_did); - let generics = tcx.item_generics(orig_did); - tcx.item_types.borrow_mut().insert(inlined_did, ty); - tcx.generics.borrow_mut().insert(inlined_did, generics); - for (id, entry) in ast.side_tables.decode((cdata, tcx, id_ranges)) { match entry { TableEntry::TypeRelativeDef(def) => { diff --git a/src/librustc_metadata/decoder.rs b/src/librustc_metadata/decoder.rs index 539c05b71bb1c..d487d2e6da679 100644 --- a/src/librustc_metadata/decoder.rs +++ b/src/librustc_metadata/decoder.rs @@ -839,12 +839,9 @@ impl<'a, 'tcx> CrateMetadata { if self.is_proc_macro(id) { return None; } let item_doc = self.entry(id); let item_did = self.local_def_id(id); - let parent_def_id = self.local_def_id(self.def_key(id).parent.unwrap()); - let mut parent_def_path = self.def_path(id).unwrap(); - parent_def_path.data.pop(); item_doc.ast.map(|ast| { let ast = ast.decode(self); - decode_inlined_item(self, tcx, parent_def_path, parent_def_id, ast, item_did) + decode_inlined_item(self, tcx, ast, item_did) }) } From ed5a88e3d03a57cb390662e44611bb0d33c28a4b Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Fri, 16 Dec 2016 12:51:36 -0500 Subject: [PATCH 04/11] definitions: Store DefPath data in separate table in metadata --- src/librustc/hir/map/definitions.rs | 6 +++- src/librustc/hir/map/mod.rs | 6 +++- src/librustc_metadata/creader.rs | 2 +- src/librustc_metadata/cstore.rs | 4 +-- src/librustc_metadata/cstore_impl.rs | 2 +- src/librustc_metadata/decoder.rs | 49 ++++++++-------------------- src/librustc_metadata/encoder.rs | 32 ++++++++---------- src/librustc_metadata/schema.rs | 2 +- 8 files changed, 41 insertions(+), 62 deletions(-) diff --git a/src/librustc/hir/map/definitions.rs b/src/librustc/hir/map/definitions.rs index d90a9ef7d80d3..ef574b61aa7e9 100644 --- a/src/librustc/hir/map/definitions.rs +++ b/src/librustc/hir/map/definitions.rs @@ -193,7 +193,7 @@ pub enum DefPathData { // they are treated specially by the `def_path` function. /// The crate root (marker) CrateRoot, - + // Catch-all for random DefId things like DUMMY_NODE_ID Misc, @@ -243,6 +243,10 @@ impl Definitions { } } + pub fn def_path_table(&self) -> &DefPathTable { + &self.table + } + /// Get the number of definitions. pub fn len(&self) -> usize { self.def_index_to_node.len() diff --git a/src/librustc/hir/map/mod.rs b/src/librustc/hir/map/mod.rs index 569d697f374a1..e5c5b43081167 100644 --- a/src/librustc/hir/map/mod.rs +++ b/src/librustc/hir/map/mod.rs @@ -29,7 +29,7 @@ use hir::*; use hir::print as pprust; use arena::TypedArena; -use std::cell::RefCell; +use std::cell::{RefCell, Ref}; use std::io; use std::mem; @@ -395,6 +395,10 @@ impl<'ast> Map<'ast> { self.definitions.borrow().len() } + pub fn definitions(&self) -> Ref { + self.definitions.borrow() + } + pub fn def_key(&self, def_id: DefId) -> DefKey { assert!(def_id.is_local()); self.definitions.borrow().def_key(def_id.index) diff --git a/src/librustc_metadata/creader.rs b/src/librustc_metadata/creader.rs index d36242537b8e5..9eed5cb8fe8c4 100644 --- a/src/librustc_metadata/creader.rs +++ b/src/librustc_metadata/creader.rs @@ -300,7 +300,7 @@ impl<'a> CrateLoader<'a> { let mut cmeta = cstore::CrateMetadata { name: name, extern_crate: Cell::new(None), - key_map: metadata.load_key_map(crate_root.index), + def_path_table: crate_root.def_path_table.decode(&metadata), proc_macros: crate_root.macro_derive_registrar.map(|_| { self.load_derive_macros(&crate_root, dylib.clone().map(|p| p.0), span) }), diff --git a/src/librustc_metadata/cstore.rs b/src/librustc_metadata/cstore.rs index 7700ebde18133..7ec847d24cfa3 100644 --- a/src/librustc_metadata/cstore.rs +++ b/src/librustc_metadata/cstore.rs @@ -16,7 +16,7 @@ use schema; use rustc::dep_graph::DepGraph; use rustc::hir::def_id::{CRATE_DEF_INDEX, LOCAL_CRATE, CrateNum, DefIndex, DefId}; -use rustc::hir::map::DefKey; +use rustc::hir::map::definitions::DefPathTable; use rustc::hir::svh::Svh; use rustc::middle::cstore::{DepKind, ExternCrate}; use rustc_back::PanicStrategy; @@ -78,7 +78,7 @@ pub struct CrateMetadata { /// hashmap, which gives the reverse mapping. This allows us to /// quickly retrace a `DefPath`, which is needed for incremental /// compilation support. - pub key_map: FxHashMap, + pub def_path_table: DefPathTable, pub dep_kind: Cell, pub source: CrateSource, diff --git a/src/librustc_metadata/cstore_impl.rs b/src/librustc_metadata/cstore_impl.rs index 1a1bb1432eec1..7305b23951590 100644 --- a/src/librustc_metadata/cstore_impl.rs +++ b/src/librustc_metadata/cstore_impl.rs @@ -341,7 +341,7 @@ impl<'tcx> CrateStore<'tcx> for cstore::CStore { def: DefKey) -> Option { let cdata = self.get_crate_data(cnum); - cdata.key_map.get(&def).cloned() + cdata.def_path_table.def_index_for_def_key(&def) } /// Returns the `DefKey` for a given `DefId`. This indicates the diff --git a/src/librustc_metadata/decoder.rs b/src/librustc_metadata/decoder.rs index d487d2e6da679..cf550f03d8eb8 100644 --- a/src/librustc_metadata/decoder.rs +++ b/src/librustc_metadata/decoder.rs @@ -12,12 +12,10 @@ use astencode::decode_inlined_item; use cstore::{self, CrateMetadata, MetadataBlob, NativeLibrary}; -use index::Index; use schema::*; use rustc::hir::map as hir_map; use rustc::hir::map::{DefKey, DefPathData}; -use rustc::util::nodemap::FxHashMap; use rustc::hir; use rustc::hir::intravisit::IdRange; @@ -456,14 +454,6 @@ impl<'a, 'tcx> MetadataBlob { Lazy::with_position(pos).decode(self) } - /// Go through each item in the metadata and create a map from that - /// item's def-key to the item's DefIndex. - pub fn load_key_map(&self, index: LazySeq) -> FxHashMap { - index.iter_enumerated(self.raw_bytes()) - .map(|(index, item)| (item.decode(self).def_key.decode(self), index)) - .collect() - } - pub fn list_crate_metadata(&self, out: &mut io::Write) -> io::Result<()> { write!(out, "=External Dependencies=\n")?; let root = self.get_root(); @@ -543,9 +533,8 @@ impl<'a, 'tcx> CrateMetadata { } } - fn item_name(&self, item: &Entry<'tcx>) -> ast::Name { - item.def_key - .decode(self) + fn item_name(&self, item_index: DefIndex) -> ast::Name { + self.def_key(item_index) .disambiguated_data .data .get_opt_name() @@ -594,12 +583,12 @@ impl<'a, 'tcx> CrateMetadata { (ty::VariantDef { did: self.local_def_id(data.struct_ctor.unwrap_or(index)), - name: self.item_name(item), + name: self.item_name(index), fields: item.children.decode(self).map(|index| { let f = self.entry(index); ty::FieldDef { did: self.local_def_id(index), - name: self.item_name(&f), + name: self.item_name(index), vis: f.visibility } }).collect(), @@ -771,7 +760,7 @@ impl<'a, 'tcx> CrateMetadata { if let Some(def) = self.get_def(child_index) { callback(def::Export { def: def, - name: self.item_name(&self.entry(child_index)), + name: self.item_name(child_index), }); } } @@ -783,7 +772,7 @@ impl<'a, 'tcx> CrateMetadata { _ => {} } - let def_key = child.def_key.decode(self); + let def_key = self.def_key(child_index); if let (Some(def), Some(name)) = (self.get_def(child_index), def_key.disambiguated_data.data.get_opt_name()) { callback(def::Export { @@ -886,7 +875,7 @@ impl<'a, 'tcx> CrateMetadata { pub fn get_associated_item(&self, id: DefIndex) -> Option { let item = self.entry(id); let parent_and_name = || { - let def_key = item.def_key.decode(self); + let def_key = self.def_key(id); (self.local_def_id(def_key.parent.unwrap()), def_key.disambiguated_data.data.get_opt_name().unwrap()) }; @@ -963,7 +952,7 @@ impl<'a, 'tcx> CrateMetadata { // we assume that someone passing in a tuple struct ctor is actually wanting to // look at the definition let mut item = self.entry(node_id); - let def_key = item.def_key.decode(self); + let def_key = self.def_key(node_id); if def_key.disambiguated_data.data == DefPathData::StructCtor { item = self.entry(def_key.parent.unwrap()); } @@ -974,7 +963,7 @@ impl<'a, 'tcx> CrateMetadata { self.entry(id) .children .decode(self) - .map(|index| self.item_name(&self.entry(index))) + .map(|index| self.item_name(index)) .collect() } @@ -1036,7 +1025,7 @@ impl<'a, 'tcx> CrateMetadata { } pub fn get_trait_of_item(&self, id: DefIndex) -> Option { - self.entry(id).def_key.decode(self).parent.and_then(|parent_index| { + self.def_key(id).parent.and_then(|parent_index| { match self.entry(parent_index).kind { EntryKind::Trait(_) => Some(self.local_def_id(parent_index)), _ => None, @@ -1082,7 +1071,7 @@ impl<'a, 'tcx> CrateMetadata { pub fn get_macro(&self, id: DefIndex) -> (ast::Name, MacroDef) { let entry = self.entry(id); match entry.kind { - EntryKind::MacroDef(macro_def) => (self.item_name(&entry), macro_def.decode(self)), + EntryKind::MacroDef(macro_def) => (self.item_name(id), macro_def.decode(self)), _ => bug!(), } } @@ -1135,20 +1124,8 @@ impl<'a, 'tcx> CrateMetadata { } } - pub fn def_key(&self, id: DefIndex) -> hir_map::DefKey { - debug!("def_key: id={:?}", id); - if self.is_proc_macro(id) { - let name = self.proc_macros.as_ref().unwrap()[id.as_usize() - 1].0; - hir_map::DefKey { - parent: Some(CRATE_DEF_INDEX), - disambiguated_data: hir_map::DisambiguatedDefPathData { - data: hir_map::DefPathData::MacroDef(name.as_str()), - disambiguator: 0, - }, - } - } else { - self.entry(id).def_key.decode(self) - } + pub fn def_key(&self, index: DefIndex) -> DefKey { + self.def_path_table.def_key(index) } // Returns the path leading to the thing with this `id`. Note that diff --git a/src/librustc_metadata/encoder.rs b/src/librustc_metadata/encoder.rs index 443f3fbaa6e41..cf032013ac962 100644 --- a/src/librustc_metadata/encoder.rs +++ b/src/librustc_metadata/encoder.rs @@ -16,6 +16,7 @@ use rustc::middle::cstore::{InlinedItemRef, LinkMeta}; use rustc::middle::cstore::{LinkagePreference, NativeLibrary}; use rustc::hir::def; use rustc::hir::def_id::{CrateNum, CRATE_DEF_INDEX, DefIndex, DefId}; +use rustc::hir::map::definitions::DefPathTable; use rustc::middle::dependency_format::Linkage; use rustc::middle::lang_items; use rustc::mir; @@ -233,13 +234,6 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { Ok(()) } - /// For every DefId that we create a metadata item for, we include a - /// serialized copy of its DefKey, which allows us to recreate a path. - fn encode_def_key(&mut self, def_id: DefId) -> Lazy { - let tcx = self.tcx; - self.lazy(&tcx.map.def_key(def_id)) - } - fn encode_item_variances(&mut self, def_id: DefId) -> LazySeq { let tcx = self.tcx; self.lazy_seq(tcx.item_variances(def_id).iter().cloned()) @@ -276,7 +270,6 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { kind: EntryKind::Variant(self.lazy(&data)), visibility: enum_vis.simplify(), span: self.lazy(&tcx.def_span(def_id)), - def_key: self.encode_def_key(def_id), attributes: self.encode_attributes(&tcx.get_attrs(def_id)), children: self.lazy_seq(variant.fields.iter().map(|f| { assert!(f.did.is_local()); @@ -315,7 +308,6 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { kind: EntryKind::Mod(self.lazy(&data)), visibility: vis.simplify(), span: self.lazy(&md.inner), - def_key: self.encode_def_key(def_id), attributes: self.encode_attributes(attrs), children: self.lazy_seq(md.item_ids.iter().map(|item_id| { tcx.map.local_def_id(item_id.id).index @@ -396,7 +388,6 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { kind: EntryKind::Field, visibility: field.vis.simplify(), span: self.lazy(&tcx.def_span(def_id)), - def_key: self.encode_def_key(def_id), attributes: self.encode_attributes(&variant_data.fields()[field_index].attrs), children: LazySeq::empty(), stability: self.encode_stability(def_id), @@ -430,7 +421,6 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { kind: EntryKind::Struct(self.lazy(&data)), visibility: struct_vis.simplify(), span: self.lazy(&tcx.def_span(def_id)), - def_key: self.encode_def_key(def_id), attributes: LazySeq::empty(), children: LazySeq::empty(), stability: self.encode_stability(def_id), @@ -497,7 +487,6 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { kind: kind, visibility: trait_item.vis.simplify(), span: self.lazy(&ast_item.span), - def_key: self.encode_def_key(def_id), attributes: self.encode_attributes(&ast_item.attrs), children: LazySeq::empty(), stability: self.encode_stability(def_id), @@ -587,7 +576,6 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { kind: kind, visibility: impl_item.vis.simplify(), span: self.lazy(&ast_item.span), - def_key: self.encode_def_key(def_id), attributes: self.encode_attributes(&ast_item.attrs), children: LazySeq::empty(), stability: self.encode_stability(def_id), @@ -750,7 +738,6 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { kind: kind, visibility: item.vis.simplify(), span: self.lazy(&item.span), - def_key: self.encode_def_key(def_id), attributes: self.encode_attributes(&item.attrs), children: match item.node { hir::ItemForeignMod(ref fm) => { @@ -858,14 +845,12 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { /// Serialize the text of exported macros fn encode_info_for_macro_def(&mut self, macro_def: &hir::MacroDef) -> Entry<'tcx> { - let def_id = self.tcx.map.local_def_id(macro_def.id); Entry { kind: EntryKind::MacroDef(self.lazy(&MacroDef { body: ::syntax::print::pprust::tts_to_string(¯o_def.body) })), visibility: ty::Visibility::Public, span: self.lazy(¯o_def.span), - def_key: self.encode_def_key(def_id), attributes: self.encode_attributes(¯o_def.attrs), children: LazySeq::empty(), @@ -967,7 +952,6 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { kind: kind, visibility: nitem.vis.simplify(), span: self.lazy(&nitem.span), - def_key: self.encode_def_key(def_id), attributes: self.encode_attributes(&nitem.attrs), children: LazySeq::empty(), stability: self.encode_stability(def_id), @@ -1050,7 +1034,6 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { kind: EntryKind::Type, visibility: ty::Visibility::Public, span: self.lazy(&tcx.def_span(def_id)), - def_key: self.encode_def_key(def_id), attributes: LazySeq::empty(), children: LazySeq::empty(), stability: None, @@ -1079,7 +1062,6 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { kind: EntryKind::Closure(self.lazy(&data)), visibility: ty::Visibility::Public, span: self.lazy(&tcx.def_span(def_id)), - def_key: self.encode_def_key(def_id), attributes: self.encode_attributes(&tcx.get_attrs(def_id)), children: LazySeq::empty(), stability: None, @@ -1179,6 +1161,11 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { }) .map(|filemap| &**filemap)) } + + fn encode_def_path_table(&mut self) -> Lazy { + let definitions = self.tcx.map.definitions(); + self.lazy(definitions.def_path_table()) + } } struct ImplVisitor<'a, 'tcx: 'a> { @@ -1276,6 +1263,11 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { let codemap = self.encode_codemap(); let codemap_bytes = self.position() - i; + // Encode DefPathTable + i = self.position(); + let def_path_table = self.encode_def_path_table(); + let def_path_table_bytes = self.position() - i; + // Encode the def IDs of impls, for coherence checking. i = self.position(); let impls = self.encode_impls(); @@ -1321,6 +1313,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { lang_items_missing: lang_items_missing, native_libraries: native_libraries, codemap: codemap, + def_path_table: def_path_table, impls: impls, exported_symbols: exported_symbols, index: index, @@ -1343,6 +1336,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { println!(" codemap bytes: {}", codemap_bytes); println!(" impl bytes: {}", impl_bytes); println!(" exp. symbols bytes: {}", exported_symbols_bytes); + println!(" def-path table bytes: {}", def_path_table_bytes); println!(" item bytes: {}", item_bytes); println!(" index bytes: {}", index_bytes); println!(" zero bytes: {}", zero_bytes); diff --git a/src/librustc_metadata/schema.rs b/src/librustc_metadata/schema.rs index f92051cbf1943..0b6606a00d3c0 100644 --- a/src/librustc_metadata/schema.rs +++ b/src/librustc_metadata/schema.rs @@ -179,6 +179,7 @@ pub struct CrateRoot { pub lang_items_missing: LazySeq, pub native_libraries: LazySeq, pub codemap: LazySeq, + pub def_path_table: Lazy, pub impls: LazySeq, pub exported_symbols: LazySeq, pub index: LazySeq, @@ -202,7 +203,6 @@ pub struct Entry<'tcx> { pub kind: EntryKind<'tcx>, pub visibility: ty::Visibility, pub span: Lazy, - pub def_key: Lazy, pub attributes: LazySeq, pub children: LazySeq, pub stability: Option>, From e0d26294cd2d1034accf7b7b317c877a4a9b28af Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Fri, 16 Dec 2016 16:27:29 -0500 Subject: [PATCH 05/11] definitions: Add some timing stats for DefPathTable decoding. --- src/librustc/session/mod.rs | 5 +++++ src/librustc_metadata/creader.rs | 7 ++++++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/librustc/session/mod.rs b/src/librustc/session/mod.rs index 91765e68ae6e1..36a887e062273 100644 --- a/src/librustc/session/mod.rs +++ b/src/librustc/session/mod.rs @@ -136,6 +136,8 @@ pub struct PerfStats { pub incr_comp_bytes_hashed: Cell, // The accumulated time spent on computing symbol hashes pub symbol_hash_time: Cell, + // The accumulated time spent decoding def path tables from metadata + pub decode_def_path_tables_time: Cell, } impl Session { @@ -501,6 +503,8 @@ impl Session { self.perf_stats.incr_comp_hashes_count.get()); println!("Total time spent computing symbol hashes: {}", duration_to_secs_str(self.perf_stats.symbol_hash_time.get())); + println!("Total time spent decoding DefPath tables: {}", + duration_to_secs_str(self.perf_stats.decode_def_path_tables_time.get())); } } @@ -635,6 +639,7 @@ pub fn build_session_(sopts: config::Options, incr_comp_hashes_count: Cell::new(0), incr_comp_bytes_hashed: Cell::new(0), symbol_hash_time: Cell::new(Duration::from_secs(0)), + decode_def_path_tables_time: Cell::new(Duration::from_secs(0)), }, code_stats: RefCell::new(CodeStats::new()), }; diff --git a/src/librustc_metadata/creader.rs b/src/librustc_metadata/creader.rs index 9eed5cb8fe8c4..a9af4118c5957 100644 --- a/src/librustc_metadata/creader.rs +++ b/src/librustc_metadata/creader.rs @@ -22,6 +22,7 @@ use rustc_back::PanicStrategy; use rustc::session::search_paths::PathKind; use rustc::middle; use rustc::middle::cstore::{CrateStore, validate_crate_name, ExternCrate}; +use rustc::util::common::record_time; use rustc::util::nodemap::FxHashSet; use rustc::middle::cstore::NativeLibrary; use rustc::hir::map::Definitions; @@ -297,10 +298,14 @@ impl<'a> CrateLoader<'a> { let cnum_map = self.resolve_crate_deps(root, &crate_root, &metadata, cnum, span, dep_kind); + let def_path_table = record_time(&self.sess.perf_stats.decode_def_path_tables_time, || { + crate_root.def_path_table.decode(&metadata) + }); + let mut cmeta = cstore::CrateMetadata { name: name, extern_crate: Cell::new(None), - def_path_table: crate_root.def_path_table.decode(&metadata), + def_path_table: def_path_table, proc_macros: crate_root.macro_derive_registrar.map(|_| { self.load_derive_macros(&crate_root, dylib.clone().map(|p| p.0), span) }), From ea733b589c2c12b1b635d18ec033122eaa3e0d06 Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Fri, 16 Dec 2016 17:23:29 -0500 Subject: [PATCH 06/11] No need to store Definitions in RefCell within HIR map --- src/librustc/hir/map/mod.rs | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/librustc/hir/map/mod.rs b/src/librustc/hir/map/mod.rs index e5c5b43081167..7b827b0a7e5a9 100644 --- a/src/librustc/hir/map/mod.rs +++ b/src/librustc/hir/map/mod.rs @@ -29,7 +29,7 @@ use hir::*; use hir::print as pprust; use arena::TypedArena; -use std::cell::{RefCell, Ref}; +use std::cell::RefCell; use std::io; use std::mem; @@ -221,7 +221,7 @@ pub struct Map<'ast> { /// plain old integers. map: RefCell>>, - definitions: RefCell, + definitions: Definitions, /// All NodeIds that are numerically greater or equal to this value come /// from inlined items. @@ -392,16 +392,16 @@ impl<'ast> Map<'ast> { } pub fn num_local_def_ids(&self) -> usize { - self.definitions.borrow().len() + self.definitions.len() } - pub fn definitions(&self) -> Ref { - self.definitions.borrow() + pub fn definitions(&self) -> &Definitions { + &self.definitions } pub fn def_key(&self, def_id: DefId) -> DefKey { assert!(def_id.is_local()); - self.definitions.borrow().def_key(def_id.index) + self.definitions.def_key(def_id.index) } pub fn def_path_from_id(&self, id: NodeId) -> Option { @@ -412,11 +412,11 @@ impl<'ast> Map<'ast> { pub fn def_path(&self, def_id: DefId) -> DefPath { assert!(def_id.is_local()); - self.definitions.borrow().def_path(def_id.index) + self.definitions.def_path(def_id.index) } pub fn def_index_for_def_key(&self, def_key: DefKey) -> Option { - self.definitions.borrow().def_index_for_def_key(def_key) + self.definitions.def_index_for_def_key(def_key) } pub fn local_def_id(&self, node: NodeId) -> DefId { @@ -427,11 +427,11 @@ impl<'ast> Map<'ast> { } pub fn opt_local_def_id(&self, node: NodeId) -> Option { - self.definitions.borrow().opt_local_def_id(node) + self.definitions.opt_local_def_id(node) } pub fn as_local_node_id(&self, def_id: DefId) -> Option { - self.definitions.borrow().as_local_node_id(def_id) + self.definitions.as_local_node_id(def_id) } fn entry_count(&self) -> usize { @@ -940,7 +940,7 @@ pub fn map_crate<'ast>(forest: &'ast mut Forest, forest: forest, dep_graph: forest.dep_graph.clone(), map: RefCell::new(map), - definitions: RefCell::new(definitions), + definitions: definitions, local_node_id_watermark: local_node_id_watermark, local_def_id_watermark: local_def_id_watermark, } From 72f95aac1ba1891fcf13b568514520205e3c848b Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Fri, 16 Dec 2016 17:24:27 -0500 Subject: [PATCH 07/11] Move retrace_path() implementation to DefPathTable --- src/librustc/hir/map/definitions.rs | 33 +++++++++++++++++-- src/librustc/middle/cstore.rs | 27 +++++++-------- src/librustc/ty/context.rs | 49 ++++++---------------------- src/librustc_metadata/cstore_impl.rs | 19 ++++++----- 4 files changed, 65 insertions(+), 63 deletions(-) diff --git a/src/librustc/hir/map/definitions.rs b/src/librustc/hir/map/definitions.rs index ef574b61aa7e9..16b8880b36fea 100644 --- a/src/librustc/hir/map/definitions.rs +++ b/src/librustc/hir/map/definitions.rs @@ -15,8 +15,6 @@ //! There are also some rather random cases (like const initializer //! expressions) that are mostly just leftovers. - - use hir::def_id::{CrateNum, DefId, DefIndex, LOCAL_CRATE}; use rustc_data_structures::fx::FxHashMap; use rustc_data_structures::stable_hasher::StableHasher; @@ -57,6 +55,37 @@ impl DefPathTable { pub fn contains_key(&self, key: &DefKey) -> bool { self.key_to_index.contains_key(key) } + + pub fn retrace_path(&self, + path_data: &[DisambiguatedDefPathData]) + -> Option { + let root_key = DefKey { + parent: None, + disambiguated_data: DisambiguatedDefPathData { + data: DefPathData::CrateRoot, + disambiguator: 0, + }, + }; + + let root_index = self.key_to_index + .get(&root_key) + .expect("no root key?") + .clone(); + + debug!("retrace_path: root_index={:?}", root_index); + + let mut index = root_index; + for data in path_data { + let key = DefKey { parent: Some(index), disambiguated_data: data.clone() }; + debug!("retrace_path: key={:?}", key); + match self.key_to_index.get(&key) { + Some(&i) => index = i, + None => return None, + } + } + + Some(index) + } } diff --git a/src/librustc/middle/cstore.rs b/src/librustc/middle/cstore.rs index f2be97c832370..88fac105bf55a 100644 --- a/src/librustc/middle/cstore.rs +++ b/src/librustc/middle/cstore.rs @@ -25,7 +25,7 @@ use hir::def::{self, Def}; use hir::def_id::{CrateNum, DefId, DefIndex}; use hir::map as hir_map; -use hir::map::definitions::{Definitions, DefKey}; +use hir::map::definitions::{Definitions, DefKey, DisambiguatedDefPathData}; use hir::svh::Svh; use middle::lang_items; use ty::{self, Ty, TyCtxt}; @@ -336,11 +336,11 @@ pub trait CrateStore<'tcx> { fn is_no_builtins(&self, cnum: CrateNum) -> bool; // resolve - fn def_index_for_def_key(&self, - cnum: CrateNum, - def: DefKey) - -> Option; - fn def_key(&self, def: DefId) -> hir_map::DefKey; + fn retrace_path(&self, + cnum: CrateNum, + path_data: &[DisambiguatedDefPathData]) + -> Option; + fn def_key(&self, def: DefId) -> DefKey; fn relative_def_path(&self, def: DefId) -> Option; fn struct_field_names(&self, def: DefId) -> Vec; fn item_children(&self, did: DefId) -> Vec; @@ -442,12 +442,6 @@ impl<'tcx> CrateStore<'tcx> for DummyCrateStore { // trait info fn implementations_of_trait(&self, filter: Option) -> Vec { vec![] } - fn def_index_for_def_key(&self, - cnum: CrateNum, - def: DefKey) - -> Option { - None - } // impl info fn associated_item_def_ids(&self, def_id: DefId) -> Vec @@ -508,7 +502,14 @@ impl<'tcx> CrateStore<'tcx> for DummyCrateStore { fn is_no_builtins(&self, cnum: CrateNum) -> bool { bug!("is_no_builtins") } // resolve - fn def_key(&self, def: DefId) -> hir_map::DefKey { bug!("def_key") } + fn retrace_path(&self, + cnum: CrateNum, + path_data: &[DisambiguatedDefPathData]) + -> Option { + None + } + + fn def_key(&self, def: DefId) -> DefKey { bug!("def_key") } fn relative_def_path(&self, def: DefId) -> Option { bug!("relative_def_path") } diff --git a/src/librustc/ty/context.rs b/src/librustc/ty/context.rs index 4854a14f733f5..001d18b95cac0 100644 --- a/src/librustc/ty/context.rs +++ b/src/librustc/ty/context.rs @@ -15,9 +15,9 @@ use session::Session; use middle; use hir::TraitMap; use hir::def::Def; -use hir::def_id::{CrateNum, DefId, DefIndex, LOCAL_CRATE}; +use hir::def_id::{CrateNum, DefId, LOCAL_CRATE}; use hir::map as ast_map; -use hir::map::{DefKey, DefPathData, DisambiguatedDefPathData}; +use hir::map::DisambiguatedDefPathData; use middle::free_region::FreeRegionMap; use middle::region::RegionMaps; use middle::resolve_lifetime; @@ -627,50 +627,21 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> { } } - /// Given a def-key `key` and a crate `krate`, finds the def-index - /// that `krate` assigned to `key`. This `DefIndex` will always be - /// relative to `krate`. - /// - /// Returns `None` if there is no `DefIndex` with that key. - pub fn def_index_for_def_key(self, krate: CrateNum, key: DefKey) - -> Option { - if krate == LOCAL_CRATE { - self.map.def_index_for_def_key(key) - } else { - self.sess.cstore.def_index_for_def_key(krate, key) - } - } - pub fn retrace_path(self, krate: CrateNum, path_data: &[DisambiguatedDefPathData]) -> Option { debug!("retrace_path(path={:?}, krate={:?})", path_data, self.crate_name(krate)); - let root_key = DefKey { - parent: None, - disambiguated_data: DisambiguatedDefPathData { - data: DefPathData::CrateRoot, - disambiguator: 0, - }, - }; - - let root_index = self.def_index_for_def_key(krate, root_key) - .expect("no root key?"); - - debug!("retrace_path: root_index={:?}", root_index); - - let mut index = root_index; - for data in path_data { - let key = DefKey { parent: Some(index), disambiguated_data: data.clone() }; - debug!("retrace_path: key={:?}", key); - match self.def_index_for_def_key(krate, key) { - Some(i) => index = i, - None => return None, - } + if krate == LOCAL_CRATE { + self.map + .definitions() + .def_path_table() + .retrace_path(path_data) + .map(|def_index| DefId { krate: krate, index: def_index }) + } else { + self.sess.cstore.retrace_path(krate, path_data) } - - Some(DefId { krate: krate, index: index }) } pub fn type_parameter_def(self, diff --git a/src/librustc_metadata/cstore_impl.rs b/src/librustc_metadata/cstore_impl.rs index 7305b23951590..bcb30982e1f70 100644 --- a/src/librustc_metadata/cstore_impl.rs +++ b/src/librustc_metadata/cstore_impl.rs @@ -22,8 +22,7 @@ use rustc::ty::{self, Ty, TyCtxt}; use rustc::hir::def_id::{CrateNum, DefId, DefIndex, CRATE_DEF_INDEX, LOCAL_CRATE}; use rustc::dep_graph::DepNode; -use rustc::hir::map as hir_map; -use rustc::hir::map::DefKey; +use rustc::hir::map::{DefKey, DefPath, DisambiguatedDefPathData}; use rustc::mir::Mir; use rustc::util::nodemap::{NodeSet, DefIdMap}; use rustc_back::PanicStrategy; @@ -336,18 +335,20 @@ impl<'tcx> CrateStore<'tcx> for cstore::CStore { self.get_crate_data(cnum).is_no_builtins() } - fn def_index_for_def_key(&self, - cnum: CrateNum, - def: DefKey) - -> Option { + fn retrace_path(&self, + cnum: CrateNum, + path: &[DisambiguatedDefPathData]) + -> Option { let cdata = self.get_crate_data(cnum); - cdata.def_path_table.def_index_for_def_key(&def) + cdata.def_path_table + .retrace_path(&path) + .map(|index| DefId { krate: cnum, index: index }) } /// Returns the `DefKey` for a given `DefId`. This indicates the /// parent `DefId` as well as some idea of what kind of data the /// `DefId` refers to. - fn def_key(&self, def: DefId) -> hir_map::DefKey { + fn def_key(&self, def: DefId) -> DefKey { // Note: loading the def-key (or def-path) for a def-id is not // a *read* of its metadata. This is because the def-id is // really just an interned shorthand for a def-path, which is the @@ -357,7 +358,7 @@ impl<'tcx> CrateStore<'tcx> for cstore::CStore { self.get_crate_data(def.krate).def_key(def.index) } - fn relative_def_path(&self, def: DefId) -> Option { + fn relative_def_path(&self, def: DefId) -> Option { // See `Note` above in `def_key()` for why this read is // commented out: // From 70944c2b5f81d2e2cbf3fc811ea1ca8330ad8d8c Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Fri, 16 Dec 2016 18:52:47 -0500 Subject: [PATCH 08/11] No need to have tcx::opt_def_path() now that we store all DefPaths --- src/librustc/hir/def_id.rs | 4 +--- src/librustc/middle/cstore.rs | 4 ++-- src/librustc/ty/mod.rs | 35 ++++------------------------ src/librustc_metadata/cstore_impl.rs | 2 +- src/librustc_metadata/decoder.rs | 17 ++++---------- 5 files changed, 13 insertions(+), 49 deletions(-) diff --git a/src/librustc/hir/def_id.rs b/src/librustc/hir/def_id.rs index d3771b1755b16..cbf162cc1366e 100644 --- a/src/librustc/hir/def_id.rs +++ b/src/librustc/hir/def_id.rs @@ -120,9 +120,7 @@ impl fmt::Debug for DefId { ty::tls::with_opt(|opt_tcx| { if let Some(tcx) = opt_tcx { - if let Some(def_path) = tcx.opt_def_path(*self) { - write!(f, " => {}", def_path.to_string(tcx))?; - } + write!(f, " => {}", tcx.def_path(*self).to_string(tcx))?; } Ok(()) })?; diff --git a/src/librustc/middle/cstore.rs b/src/librustc/middle/cstore.rs index 88fac105bf55a..9f9c6fd87aa85 100644 --- a/src/librustc/middle/cstore.rs +++ b/src/librustc/middle/cstore.rs @@ -341,7 +341,7 @@ pub trait CrateStore<'tcx> { path_data: &[DisambiguatedDefPathData]) -> Option; fn def_key(&self, def: DefId) -> DefKey; - fn relative_def_path(&self, def: DefId) -> Option; + fn def_path(&self, def: DefId) -> hir_map::DefPath; fn struct_field_names(&self, def: DefId) -> Vec; fn item_children(&self, did: DefId) -> Vec; fn load_macro(&self, did: DefId, sess: &Session) -> LoadedMacro; @@ -510,7 +510,7 @@ impl<'tcx> CrateStore<'tcx> for DummyCrateStore { } fn def_key(&self, def: DefId) -> DefKey { bug!("def_key") } - fn relative_def_path(&self, def: DefId) -> Option { + fn def_path(&self, def: DefId) -> hir_map::DefPath { bug!("relative_def_path") } fn struct_field_names(&self, def: DefId) -> Vec { bug!("struct_field_names") } diff --git a/src/librustc/ty/mod.rs b/src/librustc/ty/mod.rs index df12c252907a5..f8dee95f338f8 100644 --- a/src/librustc/ty/mod.rs +++ b/src/librustc/ty/mod.rs @@ -2241,40 +2241,13 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> { /// Convert a `DefId` into its fully expanded `DefPath` (every /// `DefId` is really just an interned def-path). /// - /// Note that if `id` is not local to this crate -- or is - /// inlined into this crate -- the result will be a non-local - /// `DefPath`. - /// - /// This function is only safe to use when you are sure that the - /// full def-path is accessible. Examples that are known to be - /// safe are local def-ids or items; see `opt_def_path` for more - /// details. + /// Note that if `id` is not local to this crate, the result will + // be a non-local `DefPath`. pub fn def_path(self, id: DefId) -> ast_map::DefPath { - self.opt_def_path(id).unwrap_or_else(|| { - bug!("could not load def-path for {:?}", id) - }) - } - - /// Convert a `DefId` into its fully expanded `DefPath` (every - /// `DefId` is really just an interned def-path). - /// - /// When going across crates, we do not save the full info for - /// every cross-crate def-id, and hence we may not always be able - /// to create a def-path. Therefore, this returns - /// `Option` to cover that possibility. It will always - /// return `Some` for local def-ids, however, as well as for - /// items. The problems arise with "minor" def-ids like those - /// associated with a pattern, `impl Trait`, or other internal - /// detail to a fn. - /// - /// Note that if `id` is not local to this crate -- or is - /// inlined into this crate -- the result will be a non-local - /// `DefPath`. - pub fn opt_def_path(self, id: DefId) -> Option { if id.is_local() { - Some(self.map.def_path(id)) + self.map.def_path(id) } else { - self.sess.cstore.relative_def_path(id) + self.sess.cstore.def_path(id) } } diff --git a/src/librustc_metadata/cstore_impl.rs b/src/librustc_metadata/cstore_impl.rs index bcb30982e1f70..8a29f9d627688 100644 --- a/src/librustc_metadata/cstore_impl.rs +++ b/src/librustc_metadata/cstore_impl.rs @@ -358,7 +358,7 @@ impl<'tcx> CrateStore<'tcx> for cstore::CStore { self.get_crate_data(def.krate).def_key(def.index) } - fn relative_def_path(&self, def: DefId) -> Option { + fn def_path(&self, def: DefId) -> DefPath { // See `Note` above in `def_key()` for why this read is // commented out: // diff --git a/src/librustc_metadata/decoder.rs b/src/librustc_metadata/decoder.rs index cf550f03d8eb8..853a49dffc7b5 100644 --- a/src/librustc_metadata/decoder.rs +++ b/src/librustc_metadata/decoder.rs @@ -14,8 +14,7 @@ use astencode::decode_inlined_item; use cstore::{self, CrateMetadata, MetadataBlob, NativeLibrary}; use schema::*; -use rustc::hir::map as hir_map; -use rustc::hir::map::{DefKey, DefPathData}; +use rustc::hir::map::{DefKey, DefPath, DefPathData}; use rustc::hir; use rustc::hir::intravisit::IdRange; @@ -567,7 +566,7 @@ impl<'a, 'tcx> CrateMetadata { ty::TraitDef::new(self.local_def_id(item_id), data.unsafety, data.paren_sugar, - self.def_path(item_id).unwrap().deterministic_hash(tcx)) + self.def_path(item_id).deterministic_hash(tcx)) } fn get_variant(&self, @@ -1128,16 +1127,10 @@ impl<'a, 'tcx> CrateMetadata { self.def_path_table.def_key(index) } - // Returns the path leading to the thing with this `id`. Note that - // some def-ids don't wind up in the metadata, so `def_path` sometimes - // returns `None` - pub fn def_path(&self, id: DefIndex) -> Option { + // Returns the path leading to the thing with this `id`. + pub fn def_path(&self, id: DefIndex) -> DefPath { debug!("def_path(id={:?})", id); - if self.is_proc_macro(id) || self.maybe_entry(id).is_some() { - Some(hir_map::DefPath::make(self.cnum, id, |parent| self.def_key(parent))) - } else { - None - } + DefPath::make(self.cnum, id, |parent| self.def_path_table.def_key(parent)) } /// Imports the codemap from an external crate into the codemap of the crate From 45571f34aaaa904ef58484aad41233f815ccace9 Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Fri, 16 Dec 2016 19:12:37 -0500 Subject: [PATCH 09/11] definitions: Add some documentation. --- src/librustc/hir/map/definitions.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/librustc/hir/map/definitions.rs b/src/librustc/hir/map/definitions.rs index 16b8880b36fea..4f64670f48279 100644 --- a/src/librustc/hir/map/definitions.rs +++ b/src/librustc/hir/map/definitions.rs @@ -8,7 +8,6 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. - //! For each definition, we track the following data. A definition //! here is defined somewhat circularly as "something with a def-id", //! but it generally corresponds to things like structs, enums, etc. @@ -26,6 +25,10 @@ use syntax::symbol::{Symbol, InternedString}; use ty::TyCtxt; use util::nodemap::NodeMap; +/// The DefPathTable maps DefIndexes to DefKeys and vice versa. +/// Internally the DefPathTable holds a tree of DefKeys, where each DefKey +/// stores the DefIndex of its parent. +/// There is one DefPathTable for each crate. #[derive(Clone)] pub struct DefPathTable { index_to_key: Vec, @@ -110,7 +113,9 @@ impl Decodable for DefPathTable { } -/// The definition table containing node definitions +/// The definition table containing node definitions. +/// It holds the DefPathTable for local DefIds/DefPaths and it also stores a +/// mapping from NodeIds to local DefIds. #[derive(Clone)] pub struct Definitions { table: DefPathTable, From 77e659a6d324bd6f096832899f5f77ae5c5d734e Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Fri, 16 Dec 2016 20:11:59 -0500 Subject: [PATCH 10/11] Remove some more things that were only needed for inlined-HIR DefIds --- src/librustc/dep_graph/README.md | 6 +- src/librustc/dep_graph/visit.rs | 2 - src/librustc/hir/map/def_collector.rs | 192 ----------------------- src/librustc/hir/map/mod.rs | 12 -- src/librustc_incremental/persist/hash.rs | 5 - 5 files changed, 2 insertions(+), 215 deletions(-) diff --git a/src/librustc/dep_graph/README.md b/src/librustc/dep_graph/README.md index 48f5b7ea2595d..d2b94db689bc4 100644 --- a/src/librustc/dep_graph/README.md +++ b/src/librustc/dep_graph/README.md @@ -418,7 +418,7 @@ to see something like: Hir(foo) -> Collect(bar) Collect(bar) -> TypeckItemBody(bar) - + That first edge looks suspicious to you. So you set `RUST_FORBID_DEP_GRAPH_EDGE` to `Hir&foo -> Collect&bar`, re-run, and then observe the backtrace. Voila, bug fixed! @@ -440,6 +440,4 @@ To achieve this, the HIR map will detect if the def-id originates in an inlined node and add a dependency to a suitable `MetaData` node instead. If you are reading a HIR node and are not sure if it may be inlined or not, you can use `tcx.map.read(node_id)` and it will detect -whether the node is inlined or not and do the right thing. You can -also use `tcx.map.is_inlined_def_id()` and -`tcx.map.is_inlined_node_id()` to test. +whether the node is inlined or not and do the right thing. diff --git a/src/librustc/dep_graph/visit.rs b/src/librustc/dep_graph/visit.rs index 600732fc6f70b..f6a22e47cf212 100644 --- a/src/librustc/dep_graph/visit.rs +++ b/src/librustc/dep_graph/visit.rs @@ -40,7 +40,6 @@ pub fn visit_all_item_likes_in_krate<'a, 'tcx, V, F>(tcx: TyCtxt<'a, 'tcx, 'tcx> let task_id = (self.dep_node_fn)(item_def_id); let _task = self.tcx.dep_graph.in_task(task_id.clone()); debug!("Started task {:?}", task_id); - assert!(!self.tcx.map.is_inlined_def_id(item_def_id)); self.tcx.dep_graph.read(DepNode::Hir(item_def_id)); self.visitor.visit_item(i); debug!("Ended task {:?}", task_id); @@ -51,7 +50,6 @@ pub fn visit_all_item_likes_in_krate<'a, 'tcx, V, F>(tcx: TyCtxt<'a, 'tcx, 'tcx> let task_id = (self.dep_node_fn)(impl_item_def_id); let _task = self.tcx.dep_graph.in_task(task_id.clone()); debug!("Started task {:?}", task_id); - assert!(!self.tcx.map.is_inlined_def_id(impl_item_def_id)); self.tcx.dep_graph.read(DepNode::Hir(impl_item_def_id)); self.visitor.visit_impl_item(i); debug!("Ended task {:?}", task_id); diff --git a/src/librustc/hir/map/def_collector.rs b/src/librustc/hir/map/def_collector.rs index 8c303a96b0ced..256aee342a3fc 100644 --- a/src/librustc/hir/map/def_collector.rs +++ b/src/librustc/hir/map/def_collector.rs @@ -9,13 +9,8 @@ // except according to those terms. use hir::map::definitions::*; - -use hir; -use hir::intravisit::{self, Visitor, NestedVisitorMap}; use hir::def_id::{CRATE_DEF_INDEX, DefIndex}; -use middle::cstore::InlinedItem; - use syntax::ast::*; use syntax::ext::hygiene::Mark; use syntax::visit; @@ -23,9 +18,6 @@ use syntax::symbol::{Symbol, keywords}; /// Creates def ids for nodes in the HIR. pub struct DefCollector<'a> { - // If we are walking HIR (c.f., AST), we need to keep a reference to the - // crate. - hir_crate: Option<&'a hir::Crate>, definitions: &'a mut Definitions, parent_def: Option, pub visit_macro_invoc: Option<&'a mut FnMut(MacroInvocationData)>, @@ -40,7 +32,6 @@ pub struct MacroInvocationData { impl<'a> DefCollector<'a> { pub fn new(definitions: &'a mut Definitions) -> Self { DefCollector { - hir_crate: None, definitions: definitions, parent_def: None, visit_macro_invoc: None, @@ -51,13 +42,6 @@ impl<'a> DefCollector<'a> { let root = self.create_def_with_parent(None, CRATE_NODE_ID, DefPathData::CrateRoot); assert_eq!(root, CRATE_DEF_INDEX); self.parent_def = Some(root); - - self.create_def_with_parent(Some(CRATE_DEF_INDEX), DUMMY_NODE_ID, DefPathData::Misc); - } - - pub fn walk_item(&mut self, ii: &'a InlinedItem, krate: &'a hir::Crate) { - self.hir_crate = Some(krate); - ii.visit(self); } fn create_def(&mut self, node_id: NodeId, data: DefPathData) -> DefIndex { @@ -95,16 +79,6 @@ impl<'a> DefCollector<'a> { self.create_def(expr.id, DefPathData::Initializer); } - fn visit_hir_const_integer(&mut self, expr: &hir::Expr) { - // FIXME(eddyb) Closures should have separate - // function definition IDs and expression IDs. - if let hir::ExprClosure(..) = expr.node { - return; - } - - self.create_def(expr.id, DefPathData::Initializer); - } - fn visit_macro_invoc(&mut self, id: NodeId, const_integer: bool) { if let Some(ref mut visit) = self.visit_macro_invoc { visit(MacroInvocationData { @@ -305,169 +279,3 @@ impl<'a> visit::Visitor<'a> for DefCollector<'a> { } } } - -// We walk the HIR rather than the AST when reading items from metadata. -impl<'ast> Visitor<'ast> for DefCollector<'ast> { - fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'ast> { - // note however that we override `visit_body` below - NestedVisitorMap::None - } - - fn visit_body(&mut self, id: hir::ExprId) { - if let Some(krate) = self.hir_crate { - self.visit_expr(krate.expr(id)); - } - } - - fn visit_item(&mut self, i: &'ast hir::Item) { - debug!("visit_item: {:?}", i); - - // Pick the def data. This need not be unique, but the more - // information we encapsulate into - let def_data = match i.node { - hir::ItemDefaultImpl(..) | hir::ItemImpl(..) => - DefPathData::Impl, - hir::ItemEnum(..) | hir::ItemStruct(..) | hir::ItemUnion(..) | - hir::ItemTrait(..) | hir::ItemExternCrate(..) | hir::ItemMod(..) | - hir::ItemForeignMod(..) | hir::ItemTy(..) => - DefPathData::TypeNs(i.name.as_str()), - hir::ItemStatic(..) | hir::ItemConst(..) | hir::ItemFn(..) => - DefPathData::ValueNs(i.name.as_str()), - hir::ItemUse(..) => DefPathData::Misc, - }; - let def = self.create_def(i.id, def_data); - - self.with_parent(def, |this| { - match i.node { - hir::ItemEnum(ref enum_definition, _) => { - for v in &enum_definition.variants { - let variant_def_index = - this.create_def(v.node.data.id(), - DefPathData::EnumVariant(v.node.name.as_str())); - - this.with_parent(variant_def_index, |this| { - for field in v.node.data.fields() { - this.create_def(field.id, - DefPathData::Field(field.name.as_str())); - } - if let Some(ref expr) = v.node.disr_expr { - this.visit_hir_const_integer(expr); - } - }); - } - } - hir::ItemStruct(ref struct_def, _) | - hir::ItemUnion(ref struct_def, _) => { - // If this is a tuple-like struct, register the constructor. - if !struct_def.is_struct() { - this.create_def(struct_def.id(), - DefPathData::StructCtor); - } - - for field in struct_def.fields() { - this.create_def(field.id, DefPathData::Field(field.name.as_str())); - } - } - _ => {} - } - intravisit::walk_item(this, i); - }); - } - - fn visit_foreign_item(&mut self, foreign_item: &'ast hir::ForeignItem) { - let def = self.create_def(foreign_item.id, - DefPathData::ValueNs(foreign_item.name.as_str())); - - self.with_parent(def, |this| { - intravisit::walk_foreign_item(this, foreign_item); - }); - } - - fn visit_generics(&mut self, generics: &'ast hir::Generics) { - for ty_param in generics.ty_params.iter() { - self.create_def(ty_param.id, DefPathData::TypeParam(ty_param.name.as_str())); - } - - intravisit::walk_generics(self, generics); - } - - fn visit_trait_item(&mut self, ti: &'ast hir::TraitItem) { - let def_data = match ti.node { - hir::MethodTraitItem(..) | hir::ConstTraitItem(..) => - DefPathData::ValueNs(ti.name.as_str()), - hir::TypeTraitItem(..) => DefPathData::TypeNs(ti.name.as_str()), - }; - - let def = self.create_def(ti.id, def_data); - self.with_parent(def, |this| { - if let hir::ConstTraitItem(_, Some(ref expr)) = ti.node { - this.create_def(expr.id, DefPathData::Initializer); - } - - intravisit::walk_trait_item(this, ti); - }); - } - - fn visit_impl_item(&mut self, ii: &'ast hir::ImplItem) { - let def_data = match ii.node { - hir::ImplItemKind::Method(..) | hir::ImplItemKind::Const(..) => - DefPathData::ValueNs(ii.name.as_str()), - hir::ImplItemKind::Type(..) => DefPathData::TypeNs(ii.name.as_str()), - }; - - let def = self.create_def(ii.id, def_data); - self.with_parent(def, |this| { - if let hir::ImplItemKind::Const(_, ref expr) = ii.node { - this.create_def(expr.id, DefPathData::Initializer); - } - - intravisit::walk_impl_item(this, ii); - }); - } - - fn visit_pat(&mut self, pat: &'ast hir::Pat) { - let parent_def = self.parent_def; - - if let hir::PatKind::Binding(_, _, name, _) = pat.node { - let def = self.create_def(pat.id, DefPathData::Binding(name.node.as_str())); - self.parent_def = Some(def); - } - - intravisit::walk_pat(self, pat); - self.parent_def = parent_def; - } - - fn visit_expr(&mut self, expr: &'ast hir::Expr) { - let parent_def = self.parent_def; - - if let hir::ExprRepeat(_, ref count) = expr.node { - self.visit_hir_const_integer(count); - } - - if let hir::ExprClosure(..) = expr.node { - let def = self.create_def(expr.id, DefPathData::ClosureExpr); - self.parent_def = Some(def); - } - - intravisit::walk_expr(self, expr); - self.parent_def = parent_def; - } - - fn visit_ty(&mut self, ty: &'ast hir::Ty) { - if let hir::TyArray(_, ref length) = ty.node { - self.visit_hir_const_integer(length); - } - if let hir::TyImplTrait(..) = ty.node { - self.create_def(ty.id, DefPathData::ImplTrait); - } - intravisit::walk_ty(self, ty); - } - - fn visit_lifetime_def(&mut self, def: &'ast hir::LifetimeDef) { - self.create_def(def.lifetime.id, DefPathData::LifetimeDef(def.lifetime.name.as_str())); - } - - fn visit_macro_def(&mut self, macro_def: &'ast hir::MacroDef) { - self.create_def(macro_def.id, DefPathData::MacroDef(macro_def.name.as_str())); - } -} diff --git a/src/librustc/hir/map/mod.rs b/src/librustc/hir/map/mod.rs index 7b827b0a7e5a9..4546f6d8c27e6 100644 --- a/src/librustc/hir/map/mod.rs +++ b/src/librustc/hir/map/mod.rs @@ -226,17 +226,9 @@ pub struct Map<'ast> { /// All NodeIds that are numerically greater or equal to this value come /// from inlined items. local_node_id_watermark: NodeId, - - /// All def-indices that are numerically greater or equal to this value come - /// from inlined items. - local_def_id_watermark: usize, } impl<'ast> Map<'ast> { - pub fn is_inlined_def_id(&self, id: DefId) -> bool { - id.is_local() && id.index.as_usize() >= self.local_def_id_watermark - } - pub fn is_inlined_node_id(&self, id: NodeId) -> bool { id >= self.local_node_id_watermark } @@ -262,7 +254,6 @@ impl<'ast> Map<'ast> { EntryItem(_, item) => { assert_eq!(id, item.id); let def_id = self.local_def_id(id); - assert!(!self.is_inlined_def_id(def_id)); if let Some(last_id) = last_expr { // The body of the item may have a separate dep node @@ -278,7 +269,6 @@ impl<'ast> Map<'ast> { EntryImplItem(_, item) => { let def_id = self.local_def_id(id); - assert!(!self.is_inlined_def_id(def_id)); if let Some(last_id) = last_expr { // The body of the item may have a separate dep node @@ -934,7 +924,6 @@ pub fn map_crate<'ast>(forest: &'ast mut Forest, } let local_node_id_watermark = NodeId::new(map.len()); - let local_def_id_watermark = definitions.len(); Map { forest: forest, @@ -942,7 +931,6 @@ pub fn map_crate<'ast>(forest: &'ast mut Forest, map: RefCell::new(map), definitions: definitions, local_node_id_watermark: local_node_id_watermark, - local_def_id_watermark: local_def_id_watermark, } } diff --git a/src/librustc_incremental/persist/hash.rs b/src/librustc_incremental/persist/hash.rs index e5203ea02b45a..799cb6c5e3d8c 100644 --- a/src/librustc_incremental/persist/hash.rs +++ b/src/librustc_incremental/persist/hash.rs @@ -66,11 +66,6 @@ impl<'a, 'tcx> HashContext<'a, 'tcx> { def_id, self.tcx.item_path_str(def_id)); - assert!(!self.tcx.map.is_inlined_def_id(def_id), - "cannot hash HIR for inlined def-id {:?} => {:?}", - def_id, - self.tcx.item_path_str(def_id)); - Some(self.incremental_hashes_map[dep_node]) } From 3a82b0da3d501f5b7f532e26b4738ac0e53fab13 Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Mon, 19 Dec 2016 14:24:33 -0500 Subject: [PATCH 11/11] Don't try get local DefId of imported macro in rustdoc. --- src/librustdoc/clean/mod.rs | 2 +- src/librustdoc/doctree.rs | 4 +++- src/librustdoc/visit_ast.rs | 8 ++++---- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index 28ca92f5db6f6..123516dc89d74 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -2801,7 +2801,7 @@ impl Clean for doctree::Macro { visibility: Some(Public), stability: self.stab.clean(cx), deprecation: self.depr.clean(cx), - def_id: cx.tcx.map.local_def_id(self.id), + def_id: self.def_id, inner: MacroItem(Macro { source: format!("macro_rules! {} {{\n{}}}", name, diff --git a/src/librustdoc/doctree.rs b/src/librustdoc/doctree.rs index 21fc135eaadae..31e10fbd3b7d3 100644 --- a/src/librustdoc/doctree.rs +++ b/src/librustdoc/doctree.rs @@ -233,9 +233,11 @@ pub struct DefaultImpl { pub whence: Span, } +// For Macro we store the DefId instead of the NodeId, since we also create +// these imported macro_rules (which only have a DUMMY_NODE_ID). pub struct Macro { pub name: Name, - pub id: ast::NodeId, + pub def_id: hir::def_id::DefId, pub attrs: hir::HirVec, pub whence: Span, pub matchers: hir::HirVec, diff --git a/src/librustdoc/visit_ast.rs b/src/librustdoc/visit_ast.rs index 4087b9a761f97..29e874c5ab526 100644 --- a/src/librustdoc/visit_ast.rs +++ b/src/librustdoc/visit_ast.rs @@ -85,7 +85,7 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { None); // attach the crate's exported macros to the top-level module: let macro_exports: Vec<_> = - krate.exported_macros.iter().map(|def| self.visit_macro(def)).collect(); + krate.exported_macros.iter().map(|def| self.visit_local_macro(def)).collect(); self.module.macros.extend(macro_exports); self.module.is_crate = true; } @@ -210,7 +210,7 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { // FIXME(jseyfried) merge with `self.visit_macro()` let matchers = def.body.chunks(4).map(|arm| arm[0].get_span()).collect(); om.macros.push(Macro { - id: def.id, + def_id: def_id, attrs: def.attrs.clone().into(), name: def.ident.name, whence: def.span, @@ -513,12 +513,12 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { } // convert each exported_macro into a doc item - fn visit_macro(&self, def: &hir::MacroDef) -> Macro { + fn visit_local_macro(&self, def: &hir::MacroDef) -> Macro { // Extract the spans of all matchers. They represent the "interface" of the macro. let matchers = def.body.chunks(4).map(|arm| arm[0].get_span()).collect(); Macro { - id: def.id, + def_id: self.cx.tcx.map.local_def_id(def.id), attrs: def.attrs.clone(), name: def.name, whence: def.span,