From cc8354f6e6b74efc85e07a9f684f1c8422a6be2e Mon Sep 17 00:00:00 2001 From: ljedrz Date: Sat, 9 Mar 2019 08:23:35 +0100 Subject: [PATCH 1/3] hir: remove NodeId from Entry & simplify Map --- src/librustc/hir/map/collector.rs | 29 +++---- src/librustc/hir/map/mod.rs | 129 +++++++++++++++--------------- src/librustc/session/mod.rs | 3 - src/librustc/ty/item_path.rs | 6 ++ src/librustc_privacy/lib.rs | 4 +- 5 files changed, 87 insertions(+), 84 deletions(-) diff --git a/src/librustc/hir/map/collector.rs b/src/librustc/hir/map/collector.rs index 9f39d648df1bf..60c755675a59d 100644 --- a/src/librustc/hir/map/collector.rs +++ b/src/librustc/hir/map/collector.rs @@ -8,8 +8,8 @@ use crate::ich::Fingerprint; use crate::middle::cstore::CrateStore; use crate::session::CrateDisambiguator; use crate::session::Session; -use std::iter::repeat; -use syntax::ast::{NodeId, CRATE_NODE_ID}; +use crate::util::nodemap::FxHashMap; +use syntax::ast::NodeId; use syntax::source_map::SourceMap; use syntax_pos::Span; @@ -25,7 +25,7 @@ pub(super) struct NodeCollector<'a, 'hir> { source_map: &'a SourceMap, /// The node map - map: Vec>>, + map: FxHashMap>, /// The parent of this node parent_node: hir::HirId, @@ -146,7 +146,7 @@ impl<'a, 'hir> NodeCollector<'a, 'hir> { let mut collector = NodeCollector { krate, source_map: sess.source_map(), - map: repeat(None).take(sess.current_node_id_count()).collect(), + map: Default::default(), parent_node: hir::CRATE_HIR_ID, current_signature_dep_index: root_mod_sig_dep_index, current_full_dep_index: root_mod_full_dep_index, @@ -158,9 +158,8 @@ impl<'a, 'hir> NodeCollector<'a, 'hir> { hcx, hir_body_nodes, }; - collector.insert_entry(CRATE_NODE_ID, Entry { - parent: CRATE_NODE_ID, - parent_hir: hir::CRATE_HIR_ID, + collector.insert_entry(hir::CRATE_HIR_ID, Entry { + parent: hir::CRATE_HIR_ID, dep_node: root_mod_sig_dep_index, node: Node::Crate, }); @@ -172,7 +171,7 @@ impl<'a, 'hir> NodeCollector<'a, 'hir> { crate_disambiguator: CrateDisambiguator, cstore: &dyn CrateStore, commandline_args_hash: u64) - -> (Vec>>, Svh) + -> (FxHashMap>, Svh) { self.hir_body_nodes.sort_unstable_by_key(|bn| bn.0); @@ -223,15 +222,14 @@ impl<'a, 'hir> NodeCollector<'a, 'hir> { (self.map, svh) } - fn insert_entry(&mut self, id: NodeId, entry: Entry<'hir>) { + fn insert_entry(&mut self, id: HirId, entry: Entry<'hir>) { debug!("hir_map: {:?} => {:?}", id, entry); - self.map[id.as_usize()] = Some(entry); + self.map.insert(id, entry); } fn insert(&mut self, span: Span, hir_id: HirId, node: Node<'hir>) { let entry = Entry { - parent: self.hir_to_node_id[&self.parent_node], - parent_hir: self.parent_node, + parent: self.parent_node, dep_node: if self.currently_in_body { self.current_full_dep_index } else { @@ -240,12 +238,11 @@ impl<'a, 'hir> NodeCollector<'a, 'hir> { node, }; - let node_id = self.hir_to_node_id[&hir_id]; - // Make sure that the DepNode of some node coincides with the HirId // owner of that node. if cfg!(debug_assertions) { - assert_eq!(self.definitions.node_to_hir_id(node_id), hir_id); + let node_id = self.hir_to_node_id[&hir_id]; + assert_eq!(self.definitions.node_to_hir_id(node_id), hir_id); if hir_id.owner != self.current_dep_node_owner { let node_str = match self.definitions.opt_def_index(node_id) { @@ -278,7 +275,7 @@ impl<'a, 'hir> NodeCollector<'a, 'hir> { } } - self.insert_entry(node_id, entry); + self.insert_entry(hir_id, entry); } fn with_parent( diff --git a/src/librustc/hir/map/mod.rs b/src/librustc/hir/map/mod.rs index 86b6805cc9b4c..9cd86b819726b 100644 --- a/src/librustc/hir/map/mod.rs +++ b/src/librustc/hir/map/mod.rs @@ -11,7 +11,7 @@ use crate::middle::cstore::CrateStoreDyn; use rustc_target::spec::abi::Abi; use rustc_data_structures::svh::Svh; -use syntax::ast::{self, Name, NodeId, CRATE_NODE_ID}; +use syntax::ast::{self, Name, NodeId}; use syntax::source_map::Spanned; use syntax::ext::base::MacroKind; use syntax_pos::{Span, DUMMY_SP}; @@ -38,14 +38,13 @@ pub const REGULAR_SPACE: DefIndexAddressSpace = DefIndexAddressSpace::High; /// Represents an entry and its parent `NodeId`. #[derive(Copy, Clone, Debug)] pub struct Entry<'hir> { - parent: NodeId, - parent_hir: HirId, + parent: HirId, dep_node: DepNodeIndex, node: Node<'hir>, } impl<'hir> Entry<'hir> { - fn parent_node(self) -> Option { + fn parent_node(self) -> Option { match self.node { Node::Crate | Node::MacroDef(_) => None, _ => Some(self.parent), @@ -183,7 +182,7 @@ pub struct Map<'hir> { /// /// Also, indexing is pretty quick when you've got a vector and /// plain old integers. - map: Vec>>, + map: FxHashMap>, definitions: &'hir Definitions, @@ -200,7 +199,8 @@ impl<'hir> Map<'hir> { /// read recorded). If the function just returns a DefId or /// NodeId, no actual content was returned, so no read is needed. pub fn read(&self, id: NodeId) { - if let Some(entry) = self.map[id.as_usize()] { + let hir_id = self.node_to_hir_id(id); + if let Some(entry) = self.map.get(&hir_id) { self.dep_graph.read_index(entry.dep_node); } else { bug!("called `HirMap::read()` with invalid `NodeId`: {:?}", id) @@ -242,18 +242,18 @@ impl<'hir> Map<'hir> { #[inline] pub fn local_def_id(&self, node: NodeId) -> DefId { self.opt_local_def_id(node).unwrap_or_else(|| { + let hir_id = self.node_to_hir_id(node); bug!("local_def_id: no entry for `{}`, which has a map of `{:?}`", - node, self.find_entry(node)) + node, self.find_entry(hir_id)) }) } // FIXME(@ljedrz): replace the NodeId variant #[inline] pub fn local_def_id_from_hir_id(&self, hir_id: HirId) -> DefId { - let node_id = self.hir_to_node_id(hir_id); - self.opt_local_def_id(node_id).unwrap_or_else(|| { + self.opt_local_def_id_from_hir_id(hir_id).unwrap_or_else(|| { bug!("local_def_id_from_hir_id: no entry for `{:?}`, which has a map of `{:?}`", - hir_id, self.find_entry(node_id)) + hir_id, self.find_entry(hir_id)) }) } @@ -418,8 +418,8 @@ impl<'hir> Map<'hir> { self.map.len() } - fn find_entry(&self, id: NodeId) -> Option> { - self.map.get(id.as_usize()).cloned().unwrap_or(None) + fn find_entry(&self, id: HirId) -> Option> { + self.map.get(&id).cloned() } pub fn krate(&self) -> &'hir Crate { @@ -451,7 +451,8 @@ impl<'hir> Map<'hir> { } pub fn fn_decl(&self, node_id: ast::NodeId) -> Option { - if let Some(entry) = self.find_entry(node_id) { + let hir_id = self.node_to_hir_id(node_id); + if let Some(entry) = self.find_entry(hir_id) { entry.fn_decl().cloned() } else { bug!("no entry for node_id `{}`", node_id) @@ -468,10 +469,9 @@ impl<'hir> Map<'hir> { /// which this is the body of, i.e., a `fn`, `const` or `static` /// item (possibly associated), a closure, or a `hir::AnonConst`. pub fn body_owner(&self, BodyId { hir_id }: BodyId) -> NodeId { - let node_id = self.hir_to_node_id(hir_id); - let parent = self.get_parent_node(node_id); - assert!(self.map[parent.as_usize()].map_or(false, |e| e.is_body_owner(hir_id))); - parent + let parent = self.get_parent_node_by_hir_id(hir_id); + assert!(self.map.get(&parent).map_or(false, |e| e.is_body_owner(hir_id))); + self.hir_to_node_id(parent) } pub fn body_owner_def_id(&self, id: BodyId) -> DefId { @@ -481,9 +481,10 @@ impl<'hir> Map<'hir> { /// Given a `NodeId`, returns the `BodyId` associated with it, /// if the node is a body owner, otherwise returns `None`. pub fn maybe_body_owned_by(&self, id: NodeId) -> Option { - if let Some(entry) = self.find_entry(id) { + let hir_id = self.node_to_hir_id(id); + if let Some(entry) = self.find_entry(hir_id) { if self.dep_graph.is_fully_enabled() { - let hir_id_owner = self.node_to_hir_id(id).owner; + let hir_id_owner = hir_id.owner; let def_path_hash = self.definitions.def_path_hash(hir_id_owner); self.dep_graph.read(def_path_hash.to_dep_node(DepKind::HirBody)); } @@ -585,17 +586,17 @@ impl<'hir> Map<'hir> { &self.forest.krate.attrs } - pub fn get_module(&self, module: DefId) -> (&'hir Mod, Span, NodeId) + pub fn get_module(&self, module: DefId) -> (&'hir Mod, Span, HirId) { - let node_id = self.as_local_node_id(module).unwrap(); - self.read(node_id); - match self.find_entry(node_id).unwrap().node { + let hir_id = self.as_local_hir_id(module).unwrap(); + self.read_by_hir_id(hir_id); + match self.find_entry(hir_id).unwrap().node { Node::Item(&Item { span, node: ItemKind::Mod(ref m), .. - }) => (m, span, node_id), - Node::Crate => (&self.forest.krate.module, self.forest.krate.span, node_id), + }) => (m, span, hir_id), + Node::Crate => (&self.forest.krate.module, self.forest.krate.span, hir_id), _ => panic!("not a module") } } @@ -672,7 +673,8 @@ impl<'hir> Map<'hir> { /// Retrieves the `Node` corresponding to `id`, returning `None` if cannot be found. pub fn find(&self, id: NodeId) -> Option> { - let result = self.find_entry(id).and_then(|entry| { + let hir_id = self.node_to_hir_id(id); + let result = self.find_entry(hir_id).and_then(|entry| { if let Node::Crate = entry.node { None } else { @@ -680,7 +682,7 @@ impl<'hir> Map<'hir> { } }); if result.is_some() { - self.read(id); + self.read_by_hir_id(hir_id); } result } @@ -702,13 +704,17 @@ impl<'hir> Map<'hir> { /// from a node to the root of the ast (unless you get the same ID back here /// that can happen if the ID is not in the map itself or is just weird). pub fn get_parent_node(&self, id: NodeId) -> NodeId { + let hir_id = self.node_to_hir_id(id); if self.dep_graph.is_fully_enabled() { - let hir_id_owner = self.node_to_hir_id(id).owner; + let hir_id_owner = hir_id.owner; let def_path_hash = self.definitions.def_path_hash(hir_id_owner); self.dep_graph.read(def_path_hash.to_dep_node(DepKind::HirBody)); } - self.find_entry(id).and_then(|x| x.parent_node()).unwrap_or(id) + self.find_entry(hir_id) + .and_then(|x| x.parent_node()) + .map(|x| self.hir_to_node_id(x)) + .unwrap_or(id) } // FIXME(@ljedrz): replace the NodeId variant @@ -745,17 +751,17 @@ impl<'hir> Map<'hir> { /// is not an error, since items in the crate module have the crate root as /// parent. fn walk_parent_nodes(&self, - start_id: NodeId, + start_id: HirId, found: F, bail_early: F2) - -> Result + -> Result where F: Fn(&Node<'hir>) -> bool, F2: Fn(&Node<'hir>) -> bool { let mut id = start_id; loop { - let parent_node = self.get_parent_node(id); - if parent_node == CRATE_NODE_ID { - return Ok(CRATE_NODE_ID); + let parent_node = self.get_parent_node_by_hir_id(id); + if parent_node == CRATE_HIR_ID { + return Ok(CRATE_HIR_ID); } if parent_node == id { return Err(id); @@ -822,10 +828,7 @@ impl<'hir> Map<'hir> { } }; - let node_id = self.hir_to_node_id(id); - self.walk_parent_nodes(node_id, match_fn, match_non_returning_block) - .ok() - .map(|return_node_id| self.node_to_hir_id(return_node_id)) + self.walk_parent_nodes(id, match_fn, match_non_returning_block).ok() } /// Retrieves the `NodeId` for `id`'s parent item, or `id` itself if no @@ -833,7 +836,8 @@ impl<'hir> Map<'hir> { /// in the HIR which is recorded by the map and is an item, either an item /// in a module, trait, or impl. pub fn get_parent(&self, id: NodeId) -> NodeId { - match self.walk_parent_nodes(id, |node| match *node { + let hir_id = self.node_to_hir_id(id); + let parent_hid = match self.walk_parent_nodes(hir_id, |node| match *node { Node::Item(_) | Node::ForeignItem(_) | Node::TraitItem(_) | @@ -842,7 +846,9 @@ impl<'hir> Map<'hir> { }, |_| false) { Ok(id) => id, Err(id) => id, - } + }; + + self.hir_to_node_id(parent_hid) } // FIXME(@ljedrz): replace the NodeId variant @@ -867,13 +873,16 @@ impl<'hir> Map<'hir> { /// Returns the `NodeId` of `id`'s nearest module parent, or `id` itself if no /// module parent is in this map. pub fn get_module_parent_node(&self, id: NodeId) -> NodeId { - match self.walk_parent_nodes(id, |node| match *node { + let hir_id = self.node_to_hir_id(id); + let parent_hid = match self.walk_parent_nodes(hir_id, |node| match *node { Node::Item(&Item { node: ItemKind::Mod(_), .. }) => true, _ => false, }, |_| false) { Ok(id) => id, Err(id) => id, - } + }; + + self.hir_to_node_id(parent_hid) } /// Returns the nearest enclosing scope. A scope is an item or block. @@ -881,14 +890,17 @@ impl<'hir> Map<'hir> { /// and associated types probably shouldn't, for example. Behavior in this /// regard should be expected to be highly unstable. pub fn get_enclosing_scope(&self, id: NodeId) -> Option { - self.walk_parent_nodes(id, |node| match *node { + let hir_id = self.node_to_hir_id(id); + let parent_hid = self.walk_parent_nodes(hir_id, |node| match *node { Node::Item(_) | Node::ForeignItem(_) | Node::TraitItem(_) | Node::ImplItem(_) | Node::Block(_) => true, _ => false, - }, |_| false).ok() + }, |_| false).ok(); + + parent_hid.map(|hid| self.hir_to_node_id(hid)) } pub fn get_parent_did(&self, id: NodeId) -> DefId { @@ -902,16 +914,17 @@ impl<'hir> Map<'hir> { } pub fn get_foreign_abi(&self, id: NodeId) -> Abi { - let parent = self.get_parent(id); + let hir_id = self.node_to_hir_id(id); + let parent = self.get_parent_item(hir_id); if let Some(entry) = self.find_entry(parent) { if let Entry { node: Node::Item(Item { node: ItemKind::ForeignMod(ref nm), .. }), .. } = entry { - self.read(id); // reveals some of the content of a node + self.read_by_hir_id(hir_id); // reveals some of the content of a node return nm.abi; } } - bug!("expected foreign mod or inlined parent, found {}", self.node_to_string(parent)) + bug!("expected foreign mod or inlined parent, found {}", self.hir_to_string(parent)) } // FIXME(@ljedrz): replace the NodeId variant @@ -1055,13 +1068,14 @@ impl<'hir> Map<'hir> { map: self, item_name: parts.last().unwrap(), in_which: &parts[..parts.len() - 1], - idx: CRATE_NODE_ID, + idx: ast::CRATE_NODE_ID, } } pub fn span(&self, id: NodeId) -> Span { - self.read(id); // reveals span from node - match self.find_entry(id).map(|entry| entry.node) { + let hir_id = self.node_to_hir_id(id); + self.read_by_hir_id(hir_id); // reveals span from node + match self.find_entry(hir_id).map(|entry| entry.node) { Some(Node::Item(item)) => item.span, Some(Node::ForeignItem(foreign_item)) => foreign_item.span, Some(Node::TraitItem(trait_method)) => trait_method.span, @@ -1201,7 +1215,8 @@ impl<'a, 'hir> Iterator for NodesMatchingSuffix<'a, 'hir> { return None; } self.idx = NodeId::from_u32(self.idx.as_u32() + 1); - let name = match self.map.find_entry(idx).map(|entry| entry.node) { + let hir_idx = self.map.node_to_hir_id(idx); + let name = match self.map.find_entry(hir_idx).map(|entry| entry.node) { Some(Node::Item(n)) => n.name(), Some(Node::ForeignItem(n)) => n.name(), Some(Node::TraitItem(n)) => n.name(), @@ -1259,18 +1274,6 @@ pub fn map_crate<'hir>(sess: &crate::session::Session, ) }; - if log_enabled!(::log::Level::Debug) { - // This only makes sense for ordered stores; note the - // enumerate to count the number of entries. - let (entries_less_1, _) = map.iter().filter_map(|x| *x).enumerate().last() - .expect("AST map was empty after folding?"); - - let entries = entries_less_1 + 1; - let vector_length = map.len(); - debug!("The AST map has {} entries with a maximum of {}: occupancy {:.1}%", - entries, vector_length, (entries as f64 / vector_length as f64) * 100.); - } - let map = Map { forest, dep_graph: forest.dep_graph.clone(), diff --git a/src/librustc/session/mod.rs b/src/librustc/session/mod.rs index 774bc8b450b59..e058dc1a44cbe 100644 --- a/src/librustc/session/mod.rs +++ b/src/librustc/session/mod.rs @@ -403,9 +403,6 @@ impl Session { pub fn next_node_id(&self) -> NodeId { self.reserve_node_ids(1) } - pub(crate) fn current_node_id_count(&self) -> usize { - self.next_node_id.get().as_u32() as usize - } pub fn diagnostic<'a>(&'a self) -> &'a errors::Handler { &self.parse_sess.span_diagnostic } diff --git a/src/librustc/ty/item_path.rs b/src/librustc/ty/item_path.rs index 26e2705a7a034..3a1bd872542c6 100644 --- a/src/librustc/ty/item_path.rs +++ b/src/librustc/ty/item_path.rs @@ -1,3 +1,4 @@ +use crate::hir; use crate::hir::map::DefPathData; use crate::hir::def_id::{CrateNum, DefId, CRATE_DEF_INDEX, LOCAL_CRATE}; use crate::ty::{self, DefIdTree, Ty, TyCtxt}; @@ -76,6 +77,11 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> { self.item_path_str(self.hir().local_def_id(id)) } + // FIXME(@ljedrz): remove the NodeId variant if possible + pub fn hir_path_str(self, id: hir::HirId) -> String { + self.item_path_str(self.hir().local_def_id_from_hir_id(id)) + } + /// Returns a string identifying this def-id. This string is /// suitable for user output. It always begins with a crate identifier. pub fn absolute_item_path_str(self, def_id: DefId) -> String { diff --git a/src/librustc_privacy/lib.rs b/src/librustc_privacy/lib.rs index d07e89209e7f0..6edb9e36165a4 100644 --- a/src/librustc_privacy/lib.rs +++ b/src/librustc_privacy/lib.rs @@ -1786,8 +1786,8 @@ fn check_mod_privacy<'tcx>(tcx: TyCtxt<'_, 'tcx, 'tcx>, module_def_id: DefId) { current_item: hir::DUMMY_HIR_ID, empty_tables: &empty_tables, }; - let (module, span, node_id) = tcx.hir().get_module(module_def_id); - let hir_id = tcx.hir().node_to_hir_id(node_id); + let (module, span, hir_id) = tcx.hir().get_module(module_def_id); + intravisit::walk_mod(&mut visitor, module, hir_id); // Check privacy of explicitly written types and traits as well as From 3b97954d117ced9a1de21c493efa255600e3c215 Mon Sep 17 00:00:00 2001 From: ljedrz Date: Sat, 9 Mar 2019 08:57:35 +0100 Subject: [PATCH 2/3] hir: make NodeId methods depend on HirId ones --- src/librustc/hir/map/mod.rs | 221 +++++++++++++++++------------------- 1 file changed, 102 insertions(+), 119 deletions(-) diff --git a/src/librustc/hir/map/mod.rs b/src/librustc/hir/map/mod.rs index 9cd86b819726b..64dc198bcdbf5 100644 --- a/src/librustc/hir/map/mod.rs +++ b/src/librustc/hir/map/mod.rs @@ -200,17 +200,16 @@ impl<'hir> Map<'hir> { /// NodeId, no actual content was returned, so no read is needed. pub fn read(&self, id: NodeId) { let hir_id = self.node_to_hir_id(id); - if let Some(entry) = self.map.get(&hir_id) { - self.dep_graph.read_index(entry.dep_node); - } else { - bug!("called `HirMap::read()` with invalid `NodeId`: {:?}", id) - } + self.read_by_hir_id(hir_id); } // FIXME(@ljedrz): replace the NodeId variant pub fn read_by_hir_id(&self, hir_id: HirId) { - let node_id = self.hir_to_node_id(hir_id); - self.read(node_id); + if let Some(entry) = self.map.get(&hir_id) { + self.dep_graph.read_index(entry.dep_node); + } else { + bug!("called `HirMap::read()` with invalid `HirId`: {:?}", hir_id) + } } #[inline] @@ -452,17 +451,16 @@ impl<'hir> Map<'hir> { pub fn fn_decl(&self, node_id: ast::NodeId) -> Option { let hir_id = self.node_to_hir_id(node_id); - if let Some(entry) = self.find_entry(hir_id) { - entry.fn_decl().cloned() - } else { - bug!("no entry for node_id `{}`", node_id) - } + self.fn_decl_by_hir_id(hir_id) } // FIXME(@ljedrz): replace the NodeId variant pub fn fn_decl_by_hir_id(&self, hir_id: HirId) -> Option { - let node_id = self.hir_to_node_id(hir_id); - self.fn_decl(node_id) + if let Some(entry) = self.find_entry(hir_id) { + entry.fn_decl().cloned() + } else { + bug!("no entry for hir_id `{}`", hir_id) + } } /// Returns the `NodeId` that corresponds to the definition of @@ -482,6 +480,11 @@ impl<'hir> Map<'hir> { /// if the node is a body owner, otherwise returns `None`. pub fn maybe_body_owned_by(&self, id: NodeId) -> Option { let hir_id = self.node_to_hir_id(id); + self.maybe_body_owned_by_by_hir_id(hir_id) + } + + // FIXME(@ljedrz): replace the NodeId variant + pub fn maybe_body_owned_by_by_hir_id(&self, hir_id: HirId) -> Option { if let Some(entry) = self.find_entry(hir_id) { if self.dep_graph.is_fully_enabled() { let hir_id_owner = hir_id.owner; @@ -491,16 +494,10 @@ impl<'hir> Map<'hir> { entry.associated_body() } else { - bug!("no entry for id `{}`", id) + bug!("no entry for id `{}`", hir_id) } } - // FIXME(@ljedrz): replace the NodeId variant - pub fn maybe_body_owned_by_by_hir_id(&self, id: HirId) -> Option { - let node_id = self.hir_to_node_id(id); - self.maybe_body_owned_by(node_id) - } - /// Given a body owner's id, returns the `BodyId` associated with it. pub fn body_owned_by(&self, id: HirId) -> BodyId { self.maybe_body_owned_by_by_hir_id(id).unwrap_or_else(|| { @@ -510,7 +507,13 @@ impl<'hir> Map<'hir> { } pub fn body_owner_kind(&self, id: NodeId) -> BodyOwnerKind { - match self.get(id) { + let hir_id = self.node_to_hir_id(id); + self.body_owner_kind_by_hir_id(hir_id) + } + + // FIXME(@ljedrz): replace the NodeId variant + pub fn body_owner_kind_by_hir_id(&self, id: HirId) -> BodyOwnerKind { + match self.get_by_hir_id(id) { Node::Item(&Item { node: ItemKind::Const(..), .. }) | Node::TraitItem(&TraitItem { node: TraitItemKind::Const(..), .. }) | Node::ImplItem(&ImplItem { node: ImplItemKind::Const(..), .. }) | @@ -534,12 +537,6 @@ impl<'hir> Map<'hir> { } } - // FIXME(@ljedrz): replace the NodeId variant - pub fn body_owner_kind_by_hir_id(&self, id: HirId) -> BodyOwnerKind { - let node_id = self.hir_to_node_id(id); - self.body_owner_kind(node_id) - } - pub fn ty_param_owner(&self, id: HirId) -> HirId { match self.get_by_hir_id(id) { Node::Item(&Item { node: ItemKind::Trait(..), .. }) => id, @@ -630,14 +627,15 @@ impl<'hir> Map<'hir> { /// Retrieve the Node corresponding to `id`, panicking if it cannot /// be found. pub fn get(&self, id: NodeId) -> Node<'hir> { - // read recorded by `find` - self.find(id).unwrap_or_else(|| bug!("couldn't find node id {} in the AST map", id)) + let hir_id = self.node_to_hir_id(id); + self.get_by_hir_id(hir_id) } // FIXME(@ljedrz): replace the NodeId variant pub fn get_by_hir_id(&self, id: HirId) -> Node<'hir> { - let node_id = self.hir_to_node_id(id); - self.get(node_id) + // read recorded by `find` + self.find_by_hir_id(id).unwrap_or_else(|| + bug!("couldn't find hir id {} in the HIR map", id)) } pub fn get_if_local(&self, id: DefId) -> Option> { @@ -674,6 +672,11 @@ impl<'hir> Map<'hir> { /// Retrieves the `Node` corresponding to `id`, returning `None` if cannot be found. pub fn find(&self, id: NodeId) -> Option> { let hir_id = self.node_to_hir_id(id); + self.find_by_hir_id(hir_id) + } + + // FIXME(@ljedrz): replace the NodeId variant + pub fn find_by_hir_id(&self, hir_id: HirId) -> Option> { let result = self.find_entry(hir_id).and_then(|entry| { if let Node::Crate = entry.node { None @@ -687,12 +690,6 @@ impl<'hir> Map<'hir> { result } - // FIXME(@ljedrz): replace the NodeId variant - pub fn find_by_hir_id(&self, hir_id: HirId) -> Option> { - let node_id = self.hir_to_node_id(hir_id); - self.find(node_id) - } - /// Similar to `get_parent`; returns the parent node-id, or own `id` if there is /// no parent. Note that the parent may be `CRATE_NODE_ID`, which is not itself /// present in the map -- so passing the return value of get_parent_node to @@ -705,6 +702,12 @@ impl<'hir> Map<'hir> { /// that can happen if the ID is not in the map itself or is just weird). pub fn get_parent_node(&self, id: NodeId) -> NodeId { let hir_id = self.node_to_hir_id(id); + let parent_hir_id = self.get_parent_node_by_hir_id(hir_id); + self.hir_to_node_id(parent_hir_id) + } + + // FIXME(@ljedrz): replace the NodeId variant + pub fn get_parent_node_by_hir_id(&self, hir_id: HirId) -> HirId { if self.dep_graph.is_fully_enabled() { let hir_id_owner = hir_id.owner; let def_path_hash = self.definitions.def_path_hash(hir_id_owner); @@ -713,15 +716,7 @@ impl<'hir> Map<'hir> { self.find_entry(hir_id) .and_then(|x| x.parent_node()) - .map(|x| self.hir_to_node_id(x)) - .unwrap_or(id) - } - - // FIXME(@ljedrz): replace the NodeId variant - pub fn get_parent_node_by_hir_id(&self, id: HirId) -> HirId { - let node_id = self.hir_to_node_id(id); - let parent_node_id = self.get_parent_node(node_id); - self.node_to_hir_id(parent_node_id) + .unwrap_or(hir_id) } /// Check if the node is an argument. An argument is a local variable whose @@ -837,7 +832,13 @@ impl<'hir> Map<'hir> { /// in a module, trait, or impl. pub fn get_parent(&self, id: NodeId) -> NodeId { let hir_id = self.node_to_hir_id(id); - let parent_hid = match self.walk_parent_nodes(hir_id, |node| match *node { + let parent_hir_id = self.get_parent_item(hir_id); + self.hir_to_node_id(parent_hir_id) + } + + // FIXME(@ljedrz): replace the NodeId variant + pub fn get_parent_item(&self, hir_id: HirId) -> HirId { + match self.walk_parent_nodes(hir_id, |node| match *node { Node::Item(_) | Node::ForeignItem(_) | Node::TraitItem(_) | @@ -846,75 +847,65 @@ impl<'hir> Map<'hir> { }, |_| false) { Ok(id) => id, Err(id) => id, - }; - - self.hir_to_node_id(parent_hid) - } - - // FIXME(@ljedrz): replace the NodeId variant - pub fn get_parent_item(&self, id: HirId) -> HirId { - let node_id = self.hir_to_node_id(id); - let parent_node_id = self.get_parent(node_id); - self.node_to_hir_id(parent_node_id) + } } /// Returns the `DefId` of `id`'s nearest module parent, or `id` itself if no /// module parent is in this map. pub fn get_module_parent(&self, id: NodeId) -> DefId { - self.local_def_id(self.get_module_parent_node(id)) + let hir_id = self.node_to_hir_id(id); + self.get_module_parent_by_hir_id(hir_id) } // FIXME(@ljedrz): replace the NodeId variant pub fn get_module_parent_by_hir_id(&self, id: HirId) -> DefId { - let node_id = self.hir_to_node_id(id); - self.get_module_parent(node_id) + self.local_def_id_from_hir_id(self.get_module_parent_node(id)) } - /// Returns the `NodeId` of `id`'s nearest module parent, or `id` itself if no + /// Returns the `HirId` of `id`'s nearest module parent, or `id` itself if no /// module parent is in this map. - pub fn get_module_parent_node(&self, id: NodeId) -> NodeId { - let hir_id = self.node_to_hir_id(id); - let parent_hid = match self.walk_parent_nodes(hir_id, |node| match *node { + pub fn get_module_parent_node(&self, hir_id: HirId) -> HirId { + match self.walk_parent_nodes(hir_id, |node| match *node { Node::Item(&Item { node: ItemKind::Mod(_), .. }) => true, _ => false, }, |_| false) { Ok(id) => id, Err(id) => id, - }; - - self.hir_to_node_id(parent_hid) + } } /// Returns the nearest enclosing scope. A scope is an item or block. /// FIXME: it is not clear to me that all items qualify as scopes -- statics /// and associated types probably shouldn't, for example. Behavior in this /// regard should be expected to be highly unstable. - pub fn get_enclosing_scope(&self, id: NodeId) -> Option { - let hir_id = self.node_to_hir_id(id); - let parent_hid = self.walk_parent_nodes(hir_id, |node| match *node { + pub fn get_enclosing_scope(&self, hir_id: HirId) -> Option { + self.walk_parent_nodes(hir_id, |node| match *node { Node::Item(_) | Node::ForeignItem(_) | Node::TraitItem(_) | Node::ImplItem(_) | Node::Block(_) => true, _ => false, - }, |_| false).ok(); - - parent_hid.map(|hid| self.hir_to_node_id(hid)) + }, |_| false).ok() } pub fn get_parent_did(&self, id: NodeId) -> DefId { - self.local_def_id(self.get_parent(id)) + let hir_id = self.node_to_hir_id(id); + self.get_parent_did_by_hir_id(hir_id) } // FIXME(@ljedrz): replace the NodeId variant pub fn get_parent_did_by_hir_id(&self, id: HirId) -> DefId { - let node_id = self.hir_to_node_id(id); - self.get_parent_did(node_id) + self.local_def_id_from_hir_id(self.get_parent_item(id)) } pub fn get_foreign_abi(&self, id: NodeId) -> Abi { let hir_id = self.node_to_hir_id(id); + self.get_foreign_abi_by_hir_id(hir_id) + } + + // FIXME(@ljedrz): replace the NodeId variant + pub fn get_foreign_abi_by_hir_id(&self, hir_id: HirId) -> Abi { let parent = self.get_parent_item(hir_id); if let Some(entry) = self.find_entry(parent) { if let Entry { @@ -927,17 +918,9 @@ impl<'hir> Map<'hir> { bug!("expected foreign mod or inlined parent, found {}", self.hir_to_string(parent)) } - // FIXME(@ljedrz): replace the NodeId variant - pub fn get_foreign_abi_by_hir_id(&self, id: HirId) -> Abi { - let node_id = self.hir_to_node_id(id); - self.get_foreign_abi(node_id) - } - pub fn expect_item(&self, id: NodeId) -> &'hir Item { - match self.find(id) { // read recorded by `find` - Some(Node::Item(item)) => item, - _ => bug!("expected item, found {}", self.node_to_string(id)) - } + let hir_id = self.node_to_hir_id(id); + self.expect_item_by_hir_id(hir_id) } // FIXME(@ljedrz): replace the NodeId variant @@ -992,21 +975,27 @@ impl<'hir> Map<'hir> { } pub fn expect_expr(&self, id: NodeId) -> &'hir Expr { - match self.find(id) { // read recorded by find - Some(Node::Expr(expr)) => expr, - _ => bug!("expected expr, found {}", self.node_to_string(id)) - } + let hir_id = self.node_to_hir_id(id); + self.expect_expr_by_hir_id(hir_id) } // FIXME(@ljedrz): replace the NodeId variant pub fn expect_expr_by_hir_id(&self, id: HirId) -> &'hir Expr { - let node_id = self.hir_to_node_id(id); - self.expect_expr(node_id) + match self.find_by_hir_id(id) { // read recorded by find + Some(Node::Expr(expr)) => expr, + _ => bug!("expected expr, found {}", self.hir_to_string(id)) + } } /// Returns the name associated with the given NodeId's AST. pub fn name(&self, id: NodeId) -> Name { - match self.get(id) { + let hir_id = self.node_to_hir_id(id); + self.name_by_hir_id(hir_id) + } + + // FIXME(@ljedrz): replace the NodeId variant + pub fn name_by_hir_id(&self, id: HirId) -> Name { + match self.get_by_hir_id(id) { Node::Item(i) => i.ident.name, Node::ForeignItem(fi) => fi.ident.name, Node::ImplItem(ii) => ii.ident.name, @@ -1016,22 +1005,22 @@ impl<'hir> Map<'hir> { Node::Lifetime(lt) => lt.name.ident().name, Node::GenericParam(param) => param.name.ident().name, Node::Binding(&Pat { node: PatKind::Binding(_, _, l, _), .. }) => l.name, - Node::StructCtor(_) => self.name(self.get_parent(id)), - _ => bug!("no name for {}", self.node_to_string(id)) + Node::StructCtor(_) => self.name_by_hir_id(self.get_parent_item(id)), + _ => bug!("no name for {}", self.hir_to_string(id)) } } - // FIXME(@ljedrz): replace the NodeId variant - pub fn name_by_hir_id(&self, id: HirId) -> Name { - let node_id = self.hir_to_node_id(id); - self.name(node_id) - } - /// Given a node ID, get a list of attributes associated with the AST /// corresponding to the Node ID pub fn attrs(&self, id: NodeId) -> &'hir [ast::Attribute] { - self.read(id); // reveals attributes on the node - let attrs = match self.find(id) { + let hir_id = self.node_to_hir_id(id); + self.attrs_by_hir_id(hir_id) + } + + // FIXME(@ljedrz): replace the NodeId variant + pub fn attrs_by_hir_id(&self, id: HirId) -> &'hir [ast::Attribute] { + self.read_by_hir_id(id); // reveals attributes on the node + let attrs = match self.find_by_hir_id(id) { Some(Node::Item(i)) => Some(&i.attrs[..]), Some(Node::ForeignItem(fi)) => Some(&fi.attrs[..]), Some(Node::TraitItem(ref ti)) => Some(&ti.attrs[..]), @@ -1043,18 +1032,12 @@ impl<'hir> Map<'hir> { Some(Node::GenericParam(param)) => Some(¶m.attrs[..]), // unit/tuple structs take the attributes straight from // the struct definition. - Some(Node::StructCtor(_)) => return self.attrs(self.get_parent(id)), + Some(Node::StructCtor(_)) => return self.attrs_by_hir_id(self.get_parent_item(id)), _ => None }; attrs.unwrap_or(&[]) } - // FIXME(@ljedrz): replace the NodeId variant - pub fn attrs_by_hir_id(&self, id: HirId) -> &'hir [ast::Attribute] { - let node_id = self.hir_to_node_id(id); - self.attrs(node_id) - } - /// Returns an iterator that yields the node id's with paths that /// match `parts`. (Requires `parts` is non-empty.) /// @@ -1074,6 +1057,11 @@ impl<'hir> Map<'hir> { pub fn span(&self, id: NodeId) -> Span { let hir_id = self.node_to_hir_id(id); + self.span_by_hir_id(hir_id) + } + + // FIXME(@ljedrz): replace the NodeId variant + pub fn span_by_hir_id(&self, hir_id: HirId) -> Span { self.read_by_hir_id(hir_id); // reveals span from node match self.find_entry(hir_id).map(|entry| entry.node) { Some(Node::Item(item)) => item.span, @@ -1091,7 +1079,8 @@ impl<'hir> Map<'hir> { Some(Node::Binding(pat)) => pat.span, Some(Node::Pat(pat)) => pat.span, Some(Node::Block(block)) => block.span, - Some(Node::StructCtor(_)) => self.expect_item(self.get_parent(id)).span, + Some(Node::StructCtor(_)) => self.expect_item_by_hir_id( + self.get_parent_item(hir_id)).span, Some(Node::Lifetime(lifetime)) => lifetime.span, Some(Node::GenericParam(param)) => param.span, Some(Node::Visibility(&Spanned { @@ -1101,16 +1090,10 @@ impl<'hir> Map<'hir> { Some(Node::Local(local)) => local.span, Some(Node::MacroDef(macro_def)) => macro_def.span, Some(Node::Crate) => self.forest.krate.span, - None => bug!("hir::map::Map::span: id not in map: {:?}", id), + None => bug!("hir::map::Map::span: id not in map: {:?}", hir_id), } } - // FIXME(@ljedrz): replace the NodeId variant - pub fn span_by_hir_id(&self, id: HirId) -> Span { - let node_id = self.hir_to_node_id(id); - self.span(node_id) - } - pub fn span_if_local(&self, id: DefId) -> Option { self.as_local_node_id(id).map(|id| self.span(id)) } From 12c55fd2ffe067f754aee82f112a8d809fb5586e Mon Sep 17 00:00:00 2001 From: ljedrz Date: Sat, 9 Mar 2019 09:22:08 +0100 Subject: [PATCH 3/3] doc: some HirIdification --- .../passes/collect_intra_doc_links.rs | 18 +++++++++--------- src/librustdoc/visit_ast.rs | 18 +++++++++--------- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index c346714ab485a..c8ac59ccb2597 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -49,7 +49,7 @@ enum PathKind { struct LinkCollector<'a, 'tcx: 'a, 'rcx: 'a> { cx: &'a DocContext<'a, 'tcx, 'rcx>, - mod_ids: Vec, + mod_ids: Vec, is_nightly_build: bool, } @@ -69,7 +69,7 @@ impl<'a, 'tcx, 'rcx> LinkCollector<'a, 'tcx, 'rcx> { path_str: &str, is_val: bool, current_item: &Option, - parent_id: Option) + parent_id: Option) -> Result<(Def, Option), ()> { let cx = self.cx; @@ -232,11 +232,11 @@ impl<'a, 'tcx, 'rcx> DocFolder for LinkCollector<'a, 'tcx, 'rcx> { }; // FIXME: get the resolver to work with non-local resolve scopes. - let parent_node = self.cx.as_local_node_id(item.def_id).and_then(|node_id| { + let parent_node = self.cx.as_local_hir_id(item.def_id).and_then(|hir_id| { // FIXME: this fails hard for impls in non-module scope, but is necessary for the // current `resolve()` implementation. - match self.cx.tcx.hir().get_module_parent_node(node_id) { - id if id != node_id => Some(id), + match self.cx.tcx.hir().get_module_parent_node(hir_id) { + id if id != hir_id => Some(id), _ => None, } }); @@ -255,9 +255,9 @@ impl<'a, 'tcx, 'rcx> DocFolder for LinkCollector<'a, 'tcx, 'rcx> { } } else { match parent_node.or(self.mod_ids.last().cloned()) { - Some(parent) if parent != ast::CRATE_NODE_ID => { + Some(parent) if parent != hir::CRATE_HIR_ID => { // FIXME: can we pull the parent module's name from elsewhere? - Some(self.cx.tcx.hir().name(parent).to_string()) + Some(self.cx.tcx.hir().name_by_hir_id(parent).to_string()) } _ => None, } @@ -274,7 +274,7 @@ impl<'a, 'tcx, 'rcx> DocFolder for LinkCollector<'a, 'tcx, 'rcx> { }; if item.is_mod() && item.attrs.inner_docs { - self.mod_ids.push(self.cx.tcx.hir().hir_to_node_id(item_hir_id.unwrap())); + self.mod_ids.push(item_hir_id.unwrap()); } let cx = self.cx; @@ -421,7 +421,7 @@ impl<'a, 'tcx, 'rcx> DocFolder for LinkCollector<'a, 'tcx, 'rcx> { } if item.is_mod() && !item.attrs.inner_docs { - self.mod_ids.push(self.cx.tcx.hir().hir_to_node_id(item_hir_id.unwrap())); + self.mod_ids.push(item_hir_id.unwrap()); } if item.is_mod() { diff --git a/src/librustdoc/visit_ast.rs b/src/librustdoc/visit_ast.rs index c1bd1d83a5b00..3174c65173cfe 100644 --- a/src/librustdoc/visit_ast.rs +++ b/src/librustdoc/visit_ast.rs @@ -31,7 +31,7 @@ pub struct RustdocVisitor<'a, 'tcx: 'a, 'rcx: 'a> { pub module: Module, pub attrs: hir::HirVec, pub cx: &'a core::DocContext<'a, 'tcx, 'rcx>, - view_item_stack: FxHashSet, + view_item_stack: FxHashSet, inlining: bool, /// Are the current module and all of its parents public? inside_public_path: bool, @@ -44,7 +44,7 @@ impl<'a, 'tcx, 'rcx> RustdocVisitor<'a, 'tcx, 'rcx> { ) -> RustdocVisitor<'a, 'tcx, 'rcx> { // If the root is re-exported, terminate all recursion. let mut stack = FxHashSet::default(); - stack.insert(ast::CRATE_NODE_ID); + stack.insert(hir::CRATE_HIR_ID); RustdocVisitor { module: Module::new(None), attrs: hir::HirVec::new(), @@ -269,13 +269,13 @@ impl<'a, 'tcx, 'rcx> RustdocVisitor<'a, 'tcx, 'rcx> { om: &mut Module, please_inline: bool) -> bool { - fn inherits_doc_hidden(cx: &core::DocContext<'_, '_, '_>, mut node: ast::NodeId) -> bool { + fn inherits_doc_hidden(cx: &core::DocContext<'_, '_, '_>, mut node: hir::HirId) -> bool { while let Some(id) = cx.tcx.hir().get_enclosing_scope(node) { node = id; if cx.tcx.hir().attrs(node).lists("doc").has_word("hidden") { return true; } - if node == ast::CRATE_NODE_ID { + if node == hir::CRATE_HIR_ID { break; } } @@ -324,21 +324,21 @@ impl<'a, 'tcx, 'rcx> RustdocVisitor<'a, 'tcx, 'rcx> { return false } - let def_node_id = match tcx.hir().as_local_node_id(def_did) { + let def_hir_id = match tcx.hir().as_local_hir_id(def_did) { Some(n) => n, None => return false }; let is_private = !self.cx.renderinfo.borrow().access_levels.is_public(def_did); - let is_hidden = inherits_doc_hidden(self.cx, def_node_id); + let is_hidden = inherits_doc_hidden(self.cx, def_hir_id); // Only inline if requested or if the item would otherwise be stripped. if (!please_inline && !is_private && !is_hidden) || is_no_inline { return false } - if !self.view_item_stack.insert(def_node_id) { return false } + if !self.view_item_stack.insert(def_hir_id) { return false } - let ret = match tcx.hir().get(def_node_id) { + let ret = match tcx.hir().get_by_hir_id(def_hir_id) { Node::Item(&hir::Item { node: hir::ItemKind::Mod(ref m), .. }) if glob => { let prev = mem::replace(&mut self.inlining, true); for i in &m.item_ids { @@ -371,7 +371,7 @@ impl<'a, 'tcx, 'rcx> RustdocVisitor<'a, 'tcx, 'rcx> { } _ => false, }; - self.view_item_stack.remove(&def_node_id); + self.view_item_stack.remove(&def_hir_id); ret }