Auto merge of #59042 - ljedrz:HirIdification_rework_map, r=Zoxc

HirIdification: rework Map

The next iteration of HirIdification (#57578).

- remove `NodeId` from `Entry`
- change `Map::map` to an `FxHashMap<HirId, Entry>`
- base the `NodeId` `Map` methods on `HirId` ones (reverses the current state)
- HirIdify `librustdoc` a little bit (some `NodeId` `Map` methods were converted to work on `HirId`s)

The second change might have performance implications, so I'd do a perf run to be sure it's fine; it simplifies the codebase and shouldn't have an impact as long as the `Map` searches are cached (which is now possible thanks to using `HirId`s).

r? @Zoxc

src/librustc/hir/map/collector.rs

@@ -8,8 +8,8 @@
 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<Option<Entry<'hir>>>,
+    map: FxHashMap<HirId, Entry<'hir>>,
     /// The parent of this node
     parent_node: hir::HirId,
 
@@ -145,7 +145,8 @@ pub(super) fn root(sess: &'a Session,
         let mut collector = NodeCollector {
             krate,
             source_map: sess.source_map(),
-            map: repeat(None).take(sess.current_node_id_count()).collect(),
+            map: FxHashMap::with_capacity_and_hasher(sess.current_node_id_count(),
+                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,
@@ -157,9 +158,8 @@ pub(super) fn root(sess: &'a Session,
             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,
         });
@@ -171,7 +171,7 @@ pub(super) fn finalize_and_compute_crate_hash(mut self,
                                                   crate_disambiguator: CrateDisambiguator,
                                                   cstore: &dyn CrateStore,
                                                   commandline_args_hash: u64)
-                                                  -> (Vec<Option<Entry<'hir>>>, Svh)
+                                                  -> (FxHashMap<HirId, Entry<'hir>>, Svh)
     {
         self.hir_body_nodes.sort_unstable_by_key(|bn| bn.0);
 
@@ -222,15 +222,14 @@ pub(super) fn finalize_and_compute_crate_hash(mut self,
         (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 {
@@ -239,11 +238,10 @@ fn insert(&mut self, span: Span, hir_id: HirId, node: Node<'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) {
+            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 {
@@ -277,7 +275,7 @@ fn insert(&mut self, span: Span, hir_id: HirId, node: Node<'hir>) {
             }
         }
 
-        self.insert_entry(node_id, entry);
+        self.insert_entry(hir_id, entry);
     }
 
     fn with_parent<F: FnOnce(&mut Self)>(

src/librustc_privacy/lib.rs

@@ -1827,6 +1827,7 @@ fn check_mod_privacy<'tcx>(tcx: TyCtxt<'_, 'tcx, 'tcx>, module_def_id: DefId) {
         empty_tables: &empty_tables,
     };
     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

src/librustdoc/core.rs

@@ -166,15 +166,6 @@ pub fn next_def_id(&self, crate_num: CrateNum) -> DefId {
 
     /// Like the function of the same name on the HIR map, but skips calling it on fake DefIds.
     /// (This avoids a slice-index-out-of-bounds panic.)
-    pub fn as_local_node_id(&self, def_id: DefId) -> Option<ast::NodeId> {
-        if self.all_fake_def_ids.borrow().contains(&def_id) {
-            None
-        } else {
-            self.tcx.hir().as_local_node_id(def_id)
-        }
-    }
-
-    // FIXME(@ljedrz): remove the NodeId variant
     pub fn as_local_hir_id(&self, def_id: DefId) -> Option<HirId> {
         if self.all_fake_def_ids.borrow().contains(&def_id) {
             None

src/librustdoc/passes/collect_intra_doc_links.rs

@@ -38,7 +38,7 @@ pub fn collect_intra_doc_links(krate: Crate, cx: &DocContext<'_>) -> Crate {
 
 struct LinkCollector<'a, 'tcx> {
     cx: &'a DocContext<'tcx>,
-    mod_ids: Vec<ast::NodeId>,
+    mod_ids: Vec<hir::HirId>,
 }
 
 impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
@@ -55,7 +55,7 @@ fn resolve(&self,
                path_str: &str,
                ns: Namespace,
                current_item: &Option<String>,
-               parent_id: Option<ast::NodeId>)
+               parent_id: Option<hir::HirId>)
         -> Result<(Def, Option<String>), ()>
     {
         let cx = self.cx;
@@ -64,8 +64,9 @@ fn resolve(&self,
         // path.
         if let Some(id) = parent_id.or(self.mod_ids.last().cloned()) {
             // FIXME: `with_scope` requires the `NodeId` of a module.
+            let node_id = cx.tcx.hir().hir_to_node_id(id);
             let result = cx.enter_resolver(|resolver| {
-                resolver.with_scope(id, |resolver| {
+                resolver.with_scope(node_id, |resolver| {
                     resolver.resolve_str_path_error(DUMMY_SP, &path_str, ns == ValueNS)
                 })
             });
@@ -127,7 +128,8 @@ fn resolve(&self,
             }
 
             // FIXME: `with_scope` requires the `NodeId` of a module.
-            let ty = cx.enter_resolver(|resolver| resolver.with_scope(id, |resolver| {
+            let node_id = cx.tcx.hir().hir_to_node_id(id);
+            let ty = cx.enter_resolver(|resolver| resolver.with_scope(node_id, |resolver| {
                     resolver.resolve_str_path_error(DUMMY_SP, &path, false)
             }))?;
             match ty.def {
@@ -216,11 +218,11 @@ fn fold_item(&mut self, mut item: Item) -> Option<Item> {
         };
 
         // 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,
             }
         });
@@ -239,9 +241,9 @@ fn fold_item(&mut self, mut item: Item) -> Option<Item> {
                     }
                 } 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,
                     }
@@ -258,7 +260,7 @@ fn fold_item(&mut self, mut item: Item) -> Option<Item> {
         };
 
         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;
@@ -392,7 +394,7 @@ fn fold_item(&mut self, mut item: Item) -> Option<Item> {
         }
 
         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() {

src/librustdoc/visit_ast.rs

@@ -31,7 +31,7 @@ pub struct RustdocVisitor<'a, 'tcx> {
     pub module: Module,
     pub attrs: hir::HirVec<ast::Attribute>,
     pub cx: &'a core::DocContext<'tcx>,
-    view_item_stack: FxHashSet<ast::NodeId>,
+    view_item_stack: FxHashSet<hir::HirId>,
     inlining: bool,
     /// Are the current module and all of its parents public?
     inside_public_path: bool,
@@ -44,7 +44,7 @@ pub fn new(
     ) -> RustdocVisitor<'a, 'tcx> {
         // 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(),
@@ -271,13 +271,13 @@ fn maybe_inline_local(&mut self,
                           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") {
+                if cx.tcx.hir().attrs_by_hir_id(node).lists("doc").has_word("hidden") {
                     return true;
                 }
-                if node == ast::CRATE_NODE_ID {
+                if node == hir::CRATE_HIR_ID {
                     break;
                 }
             }
@@ -326,21 +326,21 @@ fn inherits_doc_hidden(cx: &core::DocContext<'_>, mut node: ast::NodeId) -> bool
             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 {
@@ -373,7 +373,7 @@ fn inherits_doc_hidden(cx: &core::DocContext<'_>, mut node: ast::NodeId) -> bool
             }
             _ => false,
         };
-        self.view_item_stack.remove(&def_node_id);
+        self.view_item_stack.remove(&def_hir_id);
         ret
     }