Rollup merge of #156963 - aerooneqq:defs-refactoring, r=petrochenkov

definitions: remove `DefPathTable`, use `LocalDefId` instead of `DefIndex`

This PR removes `DefPathTable` and uses `LocalDefId` instead of `DefIndex` where possible.

r? @petrochenkov

Author: JonathanBrouwer

compiler/rustc_ast_lowering/src/lib.rs

@@ -542,7 +542,7 @@ pub fn lower_to_hir(tcx: TyCtxt<'_>, (): ()) -> mid_hir::Crate<'_> {
     let ast_index = index_crate(&resolver, &krate);
     let mut owners = IndexVec::from_fn_n(
         |_| hir::MaybeOwner::Phantom,
-        tcx.definitions_untracked().def_index_count(),
+        tcx.definitions_untracked().num_definitions(),
     );
 
     let mut lowerer = item::ItemLowerer {

compiler/rustc_hir/src/definitions.rs

@@ -20,84 +20,6 @@ pub use crate::def_id::DefPathHash;
 use crate::def_id::{CRATE_DEF_INDEX, CrateNum, DefIndex, LOCAL_CRATE, LocalDefId, StableCrateId};
 use crate::def_path_hash_map::DefPathHashMap;
 
-/// The `DefPathTable` maps `DefIndex`es to `DefKey`s and vice versa.
-/// Internally the `DefPathTable` holds a tree of `DefKey`s, where each `DefKey`
-/// stores the `DefIndex` of its parent.
-/// There is one `DefPathTable` for each crate.
-#[derive(Debug)]
-pub struct DefPathTable {
-    stable_crate_id: StableCrateId,
-    index_to_key: IndexVec<DefIndex, DefKey>,
-    // We do only store the local hash, as all the definitions are from the current crate.
-    def_path_hashes: IndexVec<DefIndex, Hash64>,
-    def_path_hash_to_index: DefPathHashMap,
-}
-
-impl DefPathTable {
-    fn new(stable_crate_id: StableCrateId) -> DefPathTable {
-        DefPathTable {
-            stable_crate_id,
-            index_to_key: Default::default(),
-            def_path_hashes: Default::default(),
-            def_path_hash_to_index: Default::default(),
-        }
-    }
-
-    fn allocate(&mut self, key: DefKey, def_path_hash: DefPathHash) -> DefIndex {
-        // Assert that all DefPathHashes correctly contain the local crate's StableCrateId.
-        debug_assert_eq!(self.stable_crate_id, def_path_hash.stable_crate_id());
-        let local_hash = def_path_hash.local_hash();
-
-        let index = self.index_to_key.push(key);
-        debug!("DefPathTable::insert() - {key:?} <-> {index:?}");
-
-        self.def_path_hashes.push(local_hash);
-        debug_assert!(self.def_path_hashes.len() == self.index_to_key.len());
-
-        // Check for hash collisions of DefPathHashes. These should be
-        // exceedingly rare.
-        if let Some(existing) = self.def_path_hash_to_index.insert(&local_hash, &index) {
-            let def_path1 = DefPath::make(LOCAL_CRATE, existing, |idx| self.def_key(idx));
-            let def_path2 = DefPath::make(LOCAL_CRATE, index, |idx| self.def_key(idx));
-
-            // Continuing with colliding DefPathHashes can lead to correctness
-            // issues. We must abort compilation.
-            //
-            // The likelihood of such a collision is very small, so actually
-            // running into one could be indicative of a poor hash function
-            // being used.
-            //
-            // See the documentation for DefPathHash for more information.
-            panic!(
-                "found DefPathHash collision between {def_path1:#?} and {def_path2:#?}. \
-                    Compilation cannot continue."
-            );
-        }
-
-        index
-    }
-
-    #[inline(always)]
-    pub fn def_key(&self, index: DefIndex) -> DefKey {
-        self.index_to_key[index]
-    }
-
-    #[instrument(level = "trace", skip(self), ret)]
-    #[inline(always)]
-    pub fn def_path_hash(&self, index: DefIndex) -> DefPathHash {
-        let hash = self.def_path_hashes[index];
-        DefPathHash::new(self.stable_crate_id, hash)
-    }
-
-    pub fn enumerated_keys_and_path_hashes(
-        &self,
-    ) -> impl Iterator<Item = (DefIndex, &DefKey, DefPathHash)> + ExactSizeIterator {
-        self.index_to_key
-            .iter_enumerated()
-            .map(move |(index, key)| (index, key, self.def_path_hash(index)))
-    }
-}
-
 #[derive(Debug, Default, Clone)]
 pub struct PerParentDisambiguatorState {
     #[cfg(debug_assertions)]
@@ -123,12 +45,13 @@ impl LocalDefIdMap<PerParentDisambiguatorState> {
     }
 }
 
-/// The definition table containing node definitions.
-/// It holds the `DefPathTable` for `LocalDefId`s/`DefPath`s.
-/// It also stores mappings to convert `LocalDefId`s to/from `HirId`s.
 #[derive(Debug)]
 pub struct Definitions {
-    table: DefPathTable,
+    stable_crate_id: StableCrateId,
+    def_id_to_key: IndexVec<LocalDefId, DefKey>,
+    // We do only store the local hash, as all the definitions are from the current crate.
+    def_path_hashes: IndexVec<LocalDefId, Hash64>,
+    def_path_hash_to_index: DefPathHashMap,
 }
 
 /// A unique identifier that we can use to lookup a definition
@@ -167,7 +90,7 @@ impl DefKey {
         // Construct the new DefPathHash, making sure that the `crate_id`
         // portion of the hash is properly copied from the parent. This way the
         // `crate_id` part will be recursively propagated from the root to all
-        // DefPathHashes in this DefPathTable.
+        // DefPathHashes in this Definitions.
         DefPathHash::new(parent.stable_crate_id(), local_hash)
     }
 
@@ -324,30 +247,23 @@ pub enum DefPathData {
 }
 
 impl Definitions {
-    pub fn def_path_table(&self) -> &DefPathTable {
-        &self.table
-    }
-
-    /// Gets the number of definitions.
-    pub fn def_index_count(&self) -> usize {
-        self.table.index_to_key.len()
-    }
-
-    #[inline]
+    #[inline(always)]
     pub fn def_key(&self, id: LocalDefId) -> DefKey {
-        self.table.def_key(id.local_def_index)
+        self.def_id_to_key[id]
     }
 
+    #[instrument(level = "trace", skip(self), ret)]
     #[inline(always)]
     pub fn def_path_hash(&self, id: LocalDefId) -> DefPathHash {
-        self.table.def_path_hash(id.local_def_index)
+        DefPathHash::new(self.stable_crate_id, self.def_path_hashes[id])
     }
 
     /// 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).
+    #[inline]
     pub fn def_path(&self, id: LocalDefId) -> DefPath {
         DefPath::make(LOCAL_CRATE, id.local_def_index, |index| {
             self.def_key(LocalDefId { local_def_index: index })
@@ -375,12 +291,62 @@ impl Definitions {
         let def_path_hash =
             DefPathHash::new(stable_crate_id, Hash64::new(stable_crate_id.as_u64()));
 
+        let mut defs = Definitions {
+            stable_crate_id,
+            def_path_hashes: Default::default(),
+            def_id_to_key: Default::default(),
+            def_path_hash_to_index: Default::default(),
+        };
+
         // Create the root definition.
-        let mut table = DefPathTable::new(stable_crate_id);
-        let root = LocalDefId { local_def_index: table.allocate(key, def_path_hash) };
+        let root = defs.allocate(key, def_path_hash);
         assert_eq!(root.local_def_index, CRATE_DEF_INDEX);
 
-        Definitions { table }
+        defs
+    }
+
+    fn allocate(&mut self, key: DefKey, def_path_hash: DefPathHash) -> LocalDefId {
+        // Assert that all DefPathHashes correctly contain the local crate's StableCrateId.
+        debug_assert_eq!(self.stable_crate_id, def_path_hash.stable_crate_id());
+        let local_hash = def_path_hash.local_hash();
+
+        let def_id = self.def_id_to_key.push(key);
+        debug!("def_id_to_key.push() - {key:?} <-> {:?}", def_id.local_def_index);
+
+        self.def_path_hashes.push(local_hash);
+        debug_assert!(self.def_path_hashes.len() == self.def_id_to_key.len());
+
+        // Check for hash collisions of DefPathHashes. These should be
+        // exceedingly rare.
+        if let Some(existing) =
+            self.def_path_hash_to_index.insert(&local_hash, &def_id.local_def_index)
+        {
+            let def_path1 = self.def_path(LocalDefId { local_def_index: existing });
+            let def_path2 = self.def_path(def_id);
+
+            // Continuing with colliding DefPathHashes can lead to correctness
+            // issues. We must abort compilation.
+            //
+            // The likelihood of such a collision is very small, so actually
+            // running into one could be indicative of a poor hash function
+            // being used.
+            //
+            // See the documentation for DefPathHash for more information.
+            panic!(
+                "found DefPathHash collision between {def_path1:#?} and {def_path2:#?}. \
+                    Compilation cannot continue."
+            );
+        }
+
+        def_id
+    }
+
+    pub fn enumerated_keys_and_path_hashes(
+        &self,
+    ) -> impl Iterator<Item = (DefIndex, &DefKey, DefPathHash)> + ExactSizeIterator {
+        self.def_id_to_key
+            .iter_enumerated()
+            .map(move |(def_id, key)| (def_id.local_def_index, key, self.def_path_hash(def_id)))
     }
 
     /// Creates a definition with a parent definition.
@@ -423,13 +389,13 @@ impl Definitions {
             disambiguated_data: DisambiguatedDefPathData { data, disambiguator },
         };
 
-        let parent_hash = self.table.def_path_hash(parent.local_def_index);
+        let parent_hash = self.def_path_hash(parent);
         let def_path_hash = key.compute_stable_hash(parent_hash);
 
         debug!("create_def: after disambiguation, key = {:?}", key);
 
         // Create the definition.
-        LocalDefId { local_def_index: self.table.allocate(key, def_path_hash) }
+        self.allocate(key, def_path_hash)
     }
 
     #[inline(always)]
@@ -439,19 +405,18 @@ impl Definitions {
     /// if the `DefPathHash` is from a previous compilation session and
     /// the def-path does not exist anymore.
     pub fn local_def_path_hash_to_def_id(&self, hash: DefPathHash) -> Option<LocalDefId> {
-        debug_assert!(hash.stable_crate_id() == self.table.stable_crate_id);
-        self.table
-            .def_path_hash_to_index
+        debug_assert!(hash.stable_crate_id() == self.stable_crate_id);
+        self.def_path_hash_to_index
             .get(&hash.local_hash())
             .map(|local_def_index| LocalDefId { local_def_index })
     }
 
     pub fn def_path_hash_to_def_index_map(&self) -> &DefPathHashMap {
-        &self.table.def_path_hash_to_index
+        &self.def_path_hash_to_index
     }
 
     pub fn num_definitions(&self) -> usize {
-        self.table.def_path_hashes.len()
+        self.def_path_hashes.len()
     }
 }
 

compiler/rustc_metadata/src/rmeta/encoder.rs

@@ -12,7 +12,7 @@ use rustc_data_structures::temp_dir::MaybeTempDir;
 use rustc_data_structures::thousands::usize_with_underscores;
 use rustc_hir as hir;
 use rustc_hir::attrs::{AttributeKind, EncodeCrossCrate};
-use rustc_hir::def_id::{CRATE_DEF_ID, CRATE_DEF_INDEX, LOCAL_CRATE, LocalDefId, LocalDefIdSet};
+use rustc_hir::def_id::{CRATE_DEF_ID, LOCAL_CRATE, LocalDefId, LocalDefIdSet};
 use rustc_hir::definitions::DefPathData;
 use rustc_hir::find_attr;
 use rustc_hir_pretty::id_to_string;
@@ -510,18 +510,20 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
     }
 
     fn encode_def_path_table(&mut self) {
-        let table = self.tcx.def_path_table();
+        let defs = self.tcx.definitions();
         if self.is_proc_macro {
-            for def_index in std::iter::once(CRATE_DEF_INDEX)
-                .chain(self.tcx.resolutions(()).proc_macros.iter().map(|p| p.local_def_index))
+            for def_id in std::iter::once(CRATE_DEF_ID)
+                .chain(self.tcx.resolutions(()).proc_macros.iter().copied())
             {
-                let def_key = self.lazy(table.def_key(def_index));
-                let def_path_hash = table.def_path_hash(def_index);
-                self.tables.def_keys.set_some(def_index, def_key);
-                self.tables.def_path_hashes.set(def_index, def_path_hash.local_hash().as_u64());
+                let def_key = self.lazy(defs.def_key(def_id));
+                let def_path_hash = defs.def_path_hash(def_id);
+                self.tables.def_keys.set_some(def_id.local_def_index, def_key);
+                self.tables
+                    .def_path_hashes
+                    .set(def_id.local_def_index, def_path_hash.local_hash().as_u64());
             }
         } else {
-            for (def_index, def_key, def_path_hash) in table.enumerated_keys_and_path_hashes() {
+            for (def_index, def_key, def_path_hash) in defs.enumerated_keys_and_path_hashes() {
                 let def_key = self.lazy(def_key);
                 self.tables.def_keys.set_some(def_index, def_key);
                 self.tables.def_path_hashes.set(def_index, def_path_hash.local_hash().as_u64());

compiler/rustc_middle/src/ty/context.rs

@@ -1349,13 +1349,13 @@ impl<'tcx> TyCtxt<'tcx> {
         }
     }
 
-    pub fn def_path_table(self) -> &'tcx rustc_hir::definitions::DefPathTable {
+    pub fn definitions(self) -> &'tcx rustc_hir::definitions::Definitions {
         // Depend on the `analysis` query to ensure compilation if finished.
         self.ensure_ok().analysis(());
 
         // Freeze definitions once we start iterating on them, to prevent adding new ones
         // while iterating. If some query needs to add definitions, it should be `ensure`d above.
-        self.untracked.definitions.freeze().def_path_table()
+        self.untracked.definitions.freeze()
     }
 
     pub fn def_path_hash_to_def_index_map(