Rollup merge of rust-lang#73291 - marmeladema:hir-id-ification-fix, r=petrochenkov

Pre-compute `LocalDefId` <-> `HirId` mappings and remove `NodeId` <-> `HirId` conversion APIs

cc rust-lang#50928

I don't know who is exactly the best person to review this.

r? @petrochenkov

Author: Manishearth

src/librustc_ast_lowering/lib.rs

@@ -205,6 +205,8 @@ pub trait Resolver {
     fn lint_buffer(&mut self) -> &mut LintBuffer;
 
     fn next_node_id(&mut self) -> NodeId;
+
+    fn trait_map(&self) -> &NodeMap<Vec<hir::TraitCandidate>>;
 }
 
 type NtToTokenstream = fn(&Nonterminal, &ParseSess, Span) -> TokenStream;
@@ -557,6 +559,13 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
         let proc_macros =
             c.proc_macros.iter().map(|id| self.node_id_to_hir_id[*id].unwrap()).collect();
 
+        let trait_map = self
+            .resolver
+            .trait_map()
+            .iter()
+            .map(|(&k, v)| (self.node_id_to_hir_id[k].unwrap(), v.clone()))
+            .collect();
+
         self.resolver.definitions().init_node_id_to_hir_id_mapping(self.node_id_to_hir_id);
 
         hir::Crate {
@@ -571,6 +580,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
             trait_impls: self.trait_impls,
             modules: self.modules,
             proc_macros,
+            trait_map,
         }
     }
 

src/librustc_hir/definitions.rs

@@ -81,14 +81,13 @@ pub struct Definitions {
 
     def_id_to_span: IndexVec<LocalDefId, Span>,
 
-    // FIXME(eddyb) don't go through `ast::NodeId` to convert between `HirId`
-    // and `LocalDefId` - ideally all `LocalDefId`s would be HIR owners.
     node_id_to_def_id: FxHashMap<ast::NodeId, LocalDefId>,
     def_id_to_node_id: IndexVec<LocalDefId, ast::NodeId>,
 
-    pub(super) node_id_to_hir_id: IndexVec<ast::NodeId, Option<hir::HirId>>,
-    /// The reverse mapping of `node_id_to_hir_id`.
-    pub(super) hir_id_to_node_id: FxHashMap<hir::HirId, ast::NodeId>,
+    // FIXME(eddyb) ideally all `LocalDefId`s would be HIR owners.
+    pub(super) def_id_to_hir_id: IndexVec<LocalDefId, Option<hir::HirId>>,
+    /// The reverse mapping of `def_id_to_hir_id`.
+    pub(super) hir_id_to_def_id: FxHashMap<hir::HirId, LocalDefId>,
 
     /// If `ExpnId` is an ID of some macro expansion,
     /// then `DefId` is the normal module (`mod`) in which the expanded macro was defined.
@@ -327,47 +326,27 @@ impl Definitions {
 
     #[inline]
     pub fn local_def_id(&self, node: ast::NodeId) -> LocalDefId {
-        self.opt_local_def_id(node).unwrap_or_else(|| {
-            panic!("no entry for node id: `{:?}` / `{:?}`", node, self.opt_node_id_to_hir_id(node))
-        })
+        self.opt_local_def_id(node).unwrap_or_else(|| panic!("no entry for node id: `{:?}`", node))
     }
 
     #[inline]
     pub fn as_local_hir_id(&self, def_id: LocalDefId) -> hir::HirId {
         self.local_def_id_to_hir_id(def_id)
     }
 
-    #[inline]
-    pub fn hir_id_to_node_id(&self, hir_id: hir::HirId) -> ast::NodeId {
-        self.hir_id_to_node_id[&hir_id]
-    }
-
-    #[inline]
-    pub fn node_id_to_hir_id(&self, node_id: ast::NodeId) -> hir::HirId {
-        self.node_id_to_hir_id[node_id].unwrap()
-    }
-
-    #[inline]
-    pub fn opt_node_id_to_hir_id(&self, node_id: ast::NodeId) -> Option<hir::HirId> {
-        self.node_id_to_hir_id[node_id]
-    }
-
     #[inline]
     pub fn local_def_id_to_hir_id(&self, id: LocalDefId) -> hir::HirId {
-        let node_id = self.def_id_to_node_id[id];
-        self.node_id_to_hir_id[node_id].unwrap()
+        self.def_id_to_hir_id[id].unwrap()
     }
 
     #[inline]
     pub fn opt_local_def_id_to_hir_id(&self, id: LocalDefId) -> Option<hir::HirId> {
-        let node_id = self.def_id_to_node_id[id];
-        self.node_id_to_hir_id[node_id]
+        self.def_id_to_hir_id[id]
     }
 
     #[inline]
     pub fn opt_hir_id_to_local_def_id(&self, hir_id: hir::HirId) -> Option<LocalDefId> {
-        let node_id = self.hir_id_to_node_id(hir_id);
-        self.opt_local_def_id(node_id)
+        self.hir_id_to_def_id.get(&hir_id).copied()
     }
 
     /// Retrieves the span of the given `DefId` if `DefId` is in the local crate.
@@ -477,16 +456,24 @@ impl Definitions {
         mapping: IndexVec<ast::NodeId, Option<hir::HirId>>,
     ) {
         assert!(
-            self.node_id_to_hir_id.is_empty(),
-            "trying to initialize `NodeId` -> `HirId` mapping twice"
+            self.def_id_to_hir_id.is_empty(),
+            "trying to initialize `LocalDefId` <-> `HirId` mappings twice"
         );
-        self.node_id_to_hir_id = mapping;
 
-        // Build the reverse mapping of `node_id_to_hir_id`.
-        self.hir_id_to_node_id = self
-            .node_id_to_hir_id
-            .iter_enumerated()
-            .filter_map(|(node_id, &hir_id)| hir_id.map(|hir_id| (hir_id, node_id)))
+        self.def_id_to_hir_id = self
+            .def_id_to_node_id
+            .iter()
+            .map(|&node_id| mapping.get(node_id).and_then(|&hir_id| hir_id))
+            .collect();
+
+        // Build the reverse mapping of `def_id_to_hir_id`.
+        self.hir_id_to_def_id = mapping
+            .into_iter_enumerated()
+            .filter_map(|(node_id, hir_id)| {
+                hir_id.and_then(|hir_id| {
+                    self.node_id_to_def_id.get(&node_id).map(|&def_id| (hir_id, def_id))
+                })
+            })
             .collect();
     }
 

src/librustc_hir/hir.rs

@@ -639,6 +639,8 @@ pub struct Crate<'hir> {
     /// A list of proc macro HirIds, written out in the order in which
     /// they are declared in the static array generated by proc_macro_harness.
     pub proc_macros: Vec<HirId>,
+
+    pub trait_map: BTreeMap<HirId, Vec<TraitCandidate>>,
 }
 
 impl Crate<'hir> {
@@ -2651,7 +2653,7 @@ pub type CaptureModeMap = NodeMap<CaptureBy>;
 // The TraitCandidate's import_ids is empty if the trait is defined in the same module, and
 // has length > 0 if the trait is found through an chain of imports, starting with the
 // import/use statement in the scope where the trait is used.
-#[derive(Clone, Debug)]
+#[derive(RustcEncodable, RustcDecodable, Clone, Debug)]
 pub struct TraitCandidate {
     pub def_id: DefId,
     pub import_ids: SmallVec<[LocalDefId; 1]>,

src/librustc_middle/hir/map/collector.rs

@@ -117,6 +117,7 @@ impl<'a, 'hir> NodeCollector<'a, 'hir> {
                 body_ids: _,
                 modules: _,
                 proc_macros: _,
+                trait_map: _,
             } = *krate;
 
             hash_body(&mut hcx, root_mod_def_path_hash, item, &mut hir_body_nodes)
@@ -241,11 +242,8 @@ impl<'a, 'hir> NodeCollector<'a, 'hir> {
         // Make sure that the DepNode of some node coincides with the HirId
         // owner of that node.
         if cfg!(debug_assertions) {
-            let node_id = self.definitions.hir_id_to_node_id(hir_id);
-            assert_eq!(self.definitions.node_id_to_hir_id(node_id), hir_id);
-
             if hir_id.owner != self.current_dep_node_owner {
-                let node_str = match self.definitions.opt_local_def_id(node_id) {
+                let node_str = match self.definitions.opt_hir_id_to_local_def_id(hir_id) {
                     Some(def_id) => self.definitions.def_path(def_id).to_string_no_crate(),
                     None => format!("{:?}", node),
                 };
@@ -335,9 +333,7 @@ impl<'a, 'hir> Visitor<'hir> for NodeCollector<'a, 'hir> {
         debug!("visit_item: {:?}", i);
         debug_assert_eq!(
             i.hir_id.owner,
-            self.definitions
-                .opt_local_def_id(self.definitions.hir_id_to_node_id(i.hir_id))
-                .unwrap()
+            self.definitions.opt_hir_id_to_local_def_id(i.hir_id).unwrap()
         );
         self.with_dep_node_owner(i.hir_id.owner, i, |this, hash| {
             this.insert_with_hash(i.span, i.hir_id, Node::Item(i), hash);
@@ -369,9 +365,7 @@ impl<'a, 'hir> Visitor<'hir> for NodeCollector<'a, 'hir> {
     fn visit_trait_item(&mut self, ti: &'hir TraitItem<'hir>) {
         debug_assert_eq!(
             ti.hir_id.owner,
-            self.definitions
-                .opt_local_def_id(self.definitions.hir_id_to_node_id(ti.hir_id))
-                .unwrap()
+            self.definitions.opt_hir_id_to_local_def_id(ti.hir_id).unwrap()
         );
         self.with_dep_node_owner(ti.hir_id.owner, ti, |this, hash| {
             this.insert_with_hash(ti.span, ti.hir_id, Node::TraitItem(ti), hash);
@@ -385,9 +379,7 @@ impl<'a, 'hir> Visitor<'hir> for NodeCollector<'a, 'hir> {
     fn visit_impl_item(&mut self, ii: &'hir ImplItem<'hir>) {
         debug_assert_eq!(
             ii.hir_id.owner,
-            self.definitions
-                .opt_local_def_id(self.definitions.hir_id_to_node_id(ii.hir_id))
-                .unwrap()
+            self.definitions.opt_hir_id_to_local_def_id(ii.hir_id).unwrap()
         );
         self.with_dep_node_owner(ii.hir_id.owner, ii, |this, hash| {
             this.insert_with_hash(ii.span, ii.hir_id, Node::ImplItem(ii), hash);

src/librustc_middle/ty/context.rs

@@ -1101,9 +1101,9 @@ impl<'tcx> TyCtxt<'tcx> {
         };
 
         let mut trait_map: FxHashMap<_, FxHashMap<_, _>> = FxHashMap::default();
-        for (hir_id, v) in resolutions.trait_map.into_iter() {
+        for (hir_id, v) in krate.trait_map.iter() {
             let map = trait_map.entry(hir_id.owner).or_default();
-            map.insert(hir_id.local_id, StableVec::new(v));
+            map.insert(hir_id.local_id, StableVec::new(v.to_vec()));
         }
 
         GlobalCtxt {

src/librustc_middle/ty/mod.rs

@@ -121,7 +121,6 @@ pub struct ResolverOutputs {
     pub definitions: rustc_hir::definitions::Definitions,
     pub cstore: Box<CrateStoreDyn>,
     pub extern_crate_map: FxHashMap<LocalDefId, CrateNum>,
-    pub trait_map: FxHashMap<hir::HirId, Vec<hir::TraitCandidate>>,
     pub maybe_unused_trait_imports: FxHashSet<LocalDefId>,
     pub maybe_unused_extern_crates: Vec<(LocalDefId, Span)>,
     pub export_map: ExportMap<LocalDefId>,

src/librustc_resolve/lib.rs

@@ -1109,6 +1109,10 @@ impl rustc_ast_lowering::Resolver for Resolver<'_> {
     fn next_node_id(&mut self) -> NodeId {
         self.next_node_id()
     }
+
+    fn trait_map(&self) -> &NodeMap<Vec<TraitCandidate>> {
+        &self.trait_map
+    }
 }
 
 impl<'a> Resolver<'a> {
@@ -1284,11 +1288,6 @@ impl<'a> Resolver<'a> {
         let definitions = self.definitions;
         let extern_crate_map = self.extern_crate_map;
         let export_map = self.export_map;
-        let trait_map = self
-            .trait_map
-            .into_iter()
-            .map(|(k, v)| (definitions.node_id_to_hir_id(k), v))
-            .collect();
         let maybe_unused_trait_imports = self.maybe_unused_trait_imports;
         let maybe_unused_extern_crates = self.maybe_unused_extern_crates;
         let glob_map = self.glob_map;
@@ -1297,7 +1296,6 @@ impl<'a> Resolver<'a> {
             cstore: Box::new(self.crate_loader.into_cstore()),
             extern_crate_map,
             export_map,
-            trait_map,
             glob_map,
             maybe_unused_trait_imports,
             maybe_unused_extern_crates,
@@ -1315,11 +1313,6 @@ impl<'a> Resolver<'a> {
             cstore: Box::new(self.cstore().clone()),
             extern_crate_map: self.extern_crate_map.clone(),
             export_map: self.export_map.clone(),
-            trait_map: self
-                .trait_map
-                .iter()
-                .map(|(&k, v)| (self.definitions.node_id_to_hir_id(k), v.clone()))
-                .collect(),
             glob_map: self.glob_map.clone(),
             maybe_unused_trait_imports: self.maybe_unused_trait_imports.clone(),
             maybe_unused_extern_crates: self.maybe_unused_extern_crates.clone(),