Auto merge of #156963 - aerooneqq:defs-refactoring, r=<try>

definitions: remove `DefPathTable`, merge `index_to_key` and `def_path_hashes`

Author: bors

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,12 @@ 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,
+    // We do only store the local hash, as all the definitions are from the current crate.
+    table: IndexVec<DefIndex, (DefKey, Hash64)>,
+    def_path_hash_to_index: DefPathHashMap,
 }
 
 /// A unique identifier that we can use to lookup a definition
@@ -167,7 +89,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,23 +246,26 @@ pub enum DefPathData {
 }
 
 impl Definitions {
-    pub fn def_path_table(&self) -> &DefPathTable {
-        &self.table
+    #[inline(always)]
+    pub fn def_key(&self, id: LocalDefId) -> DefKey {
+        self.def_key_by_index(id.local_def_index)
     }
 
-    /// Gets the number of definitions.
-    pub fn def_index_count(&self) -> usize {
-        self.table.index_to_key.len()
+    #[inline(always)]
+    pub fn def_key_by_index(&self, index: DefIndex) -> DefKey {
+        self.table[index].0
     }
 
-    #[inline]
-    pub fn def_key(&self, id: LocalDefId) -> DefKey {
-        self.table.def_key(id.local_def_index)
+    #[instrument(level = "trace", skip(self), ret)]
+    #[inline(always)]
+    pub fn def_path_hash(&self, id: LocalDefId) -> DefPathHash {
+        self.def_path_hash_by_index(id.local_def_index)
     }
 
     #[inline(always)]
-    pub fn def_path_hash(&self, id: LocalDefId) -> DefPathHash {
-        self.table.def_path_hash(id.local_def_index)
+    pub fn def_path_hash_by_index(&self, index: DefIndex) -> DefPathHash {
+        let hash = self.table[index].1;
+        DefPathHash::new(self.stable_crate_id, hash)
     }
 
     /// Returns the path from the crate root to `index`. The root
@@ -349,9 +274,7 @@ impl Definitions {
     /// 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, id: LocalDefId) -> DefPath {
-        DefPath::make(LOCAL_CRATE, id.local_def_index, |index| {
-            self.def_key(LocalDefId { local_def_index: index })
-        })
+        DefPath::make(LOCAL_CRATE, id.local_def_index, |index| self.def_key_by_index(index))
     }
 
     /// Adds a root definition (no parent) and a few other reserved definitions.
@@ -376,11 +299,55 @@ impl Definitions {
             DefPathHash::new(stable_crate_id, Hash64::new(stable_crate_id.as_u64()));
 
         // 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 mut defs = Definitions {
+            stable_crate_id,
+            table: Default::default(),
+            def_path_hash_to_index: Default::default(),
+        };
+
+        let root = LocalDefId { local_def_index: 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) -> 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.table.push((key, local_hash));
+        debug!("Definitions::insert() - {key:?} <-> {index:?}");
+
+        // 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_by_index(idx));
+            let def_path2 = DefPath::make(LOCAL_CRATE, index, |idx| self.def_key_by_index(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
+    }
+
+    pub fn enumerated_keys_and_path_hashes(
+        &self,
+    ) -> impl Iterator<Item = (DefIndex, &DefKey, DefPathHash)> + ExactSizeIterator {
+        self.table
+            .iter_enumerated()
+            .map(move |(index, (key, _))| (index, key, self.def_path_hash_by_index(index)))
     }
 
     /// Creates a definition with a parent definition.
@@ -423,13 +390,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_by_index(parent.local_def_index);
         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) }
+        LocalDefId { local_def_index: self.allocate(key, def_path_hash) }
     }
 
     #[inline(always)]
@@ -439,19 +406,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.table.len()
     }
 }
 

compiler/rustc_metadata/src/rmeta/encoder.rs

@@ -510,18 +510,18 @@ 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))
             {
-                let def_key = self.lazy(table.def_key(def_index));
-                let def_path_hash = table.def_path_hash(def_index);
+                let def_key = self.lazy(defs.def_key_by_index(def_index));
+                let def_path_hash = defs.def_path_hash_by_index(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());
             }
         } 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

@@ -1343,13 +1343,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(