Auto merge of #79915 - Aaron1011:fix/fix-reuse-def-path-hash, r=petrochenkov

Use `def_path_hash_to_def_id` when re-using a `RawDefId`

Fixes #79890

Previously, we just copied a `RawDefId` from the 'old' map to the 'new'
map. However, the `RawDefId` for a given `DefPathHash` may be different
in the current compilation session. Using `def_path_hash_to_def_id`
ensures that the `RawDefId` we use is valid in the current session.

Author: bors

compiler/rustc_middle/src/dep_graph/mod.rs

@@ -93,7 +93,7 @@ impl<'tcx> DepContext for TyCtxt<'tcx> {
 
     fn register_reused_dep_path_hash(&self, hash: DefPathHash) {
         if let Some(cache) = self.queries.on_disk_cache.as_ref() {
-            cache.register_reused_dep_path_hash(hash)
+            cache.register_reused_dep_path_hash(*self, hash)
         }
     }
 

compiler/rustc_middle/src/ty/query/on_disk_cache.rs

@@ -454,6 +454,7 @@ impl<'sess> OnDiskCache<'sess> {
     fn try_remap_cnum(&self, tcx: TyCtxt<'_>, cnum: u32) -> Option<CrateNum> {
         let cnum_map =
             self.cnum_map.get_or_init(|| Self::compute_cnum_map(tcx, &self.prev_cnums[..]));
+        debug!("try_remap_cnum({}): cnum_map={:?}", cnum, cnum_map);
 
         cnum_map[CrateNum::from_u32(cnum)]
     }
@@ -466,9 +467,22 @@ impl<'sess> OnDiskCache<'sess> {
             .insert(hash, RawDefId { krate: def_id.krate.as_u32(), index: def_id.index.as_u32() });
     }
 
-    pub fn register_reused_dep_path_hash(&self, hash: DefPathHash) {
-        if let Some(old_id) = self.foreign_def_path_hashes.get(&hash) {
-            self.latest_foreign_def_path_hashes.lock().insert(hash, *old_id);
+    /// If the given `hash` still exists in the current compilation,
+    /// calls `store_foreign_def_id` with its current `DefId`.
+    ///
+    /// Normally, `store_foreign_def_id_hash` can be called directly by
+    /// the dependency graph when we construct a `DepNode`. However,
+    /// when we re-use a deserialized `DepNode` from the previous compilation
+    /// session, we only have the `DefPathHash` available. This method is used
+    /// to that any `DepNode` that we re-use has a `DefPathHash` -> `RawId` written
+    /// out for usage in the next compilation session.
+    pub fn register_reused_dep_path_hash(&self, tcx: TyCtxt<'tcx>, hash: DefPathHash) {
+        // We can't simply copy the `RawDefId` from `foreign_def_path_hashes` to
+        // `latest_foreign_def_path_hashes`, since the `RawDefId` might have
+        // changed in the current compilation session (e.g. we've added/removed crates,
+        // or added/removed definitions before/after the target definition).
+        if let Some(def_id) = self.def_path_hash_to_def_id(tcx, hash) {
+            self.store_foreign_def_id_hash(def_id, hash);
         }
     }
 
@@ -592,6 +606,7 @@ impl<'sess> OnDiskCache<'sess> {
         match cache.entry(hash) {
             Entry::Occupied(e) => *e.get(),
             Entry::Vacant(e) => {
+                debug!("def_path_hash_to_def_id({:?})", hash);
                 // Check if the `DefPathHash` corresponds to a definition in the current
                 // crate
                 if let Some(def_id) = self.local_def_path_hash_to_def_id.get(&hash).cloned() {
@@ -605,9 +620,11 @@ impl<'sess> OnDiskCache<'sess> {
                 // current compilation session, the crate is guaranteed to be the same
                 // (otherwise, we would compute a different `DefPathHash`).
                 let raw_def_id = self.get_raw_def_id(&hash)?;
+                debug!("def_path_hash_to_def_id({:?}): raw_def_id = {:?}", hash, raw_def_id);
                 // If the owning crate no longer exists, the corresponding definition definitely
                 // no longer exists.
                 let krate = self.try_remap_cnum(tcx, raw_def_id.krate)?;
+                debug!("def_path_hash_to_def_id({:?}): krate = {:?}", hash, krate);
                 // If our `DefPathHash` corresponded to a definition in the local crate,
                 // we should have either found it in `local_def_path_hash_to_def_id`, or
                 // never attempted to load it in the first place. Any query result or `DepNode`
@@ -621,6 +638,7 @@ impl<'sess> OnDiskCache<'sess> {
                 // Try to find a definition in the current session, using the previous `DefIndex`
                 // as an initial guess.
                 let opt_def_id = tcx.cstore.def_path_hash_to_def_id(krate, raw_def_id.index, hash);
+                debug!("def_path_to_def_id({:?}): opt_def_id = {:?}", hash, opt_def_id);
                 e.insert(opt_def_id);
                 opt_def_id
             }

compiler/rustc_query_system/src/dep_graph/graph.rs

@@ -596,9 +596,9 @@ impl<K: DepKind> DepGraph<K> {
                     // an eval_always node, let's try to mark it green recursively.
                     if !dep_dep_node.kind.is_eval_always() {
                         debug!(
-                            "try_mark_previous_green({:?}) --- state of dependency {:?} \
+                            "try_mark_previous_green({:?}) --- state of dependency {:?} ({}) \
                                  is unknown, trying to mark it green",
-                            dep_node, dep_dep_node
+                            dep_node, dep_dep_node, dep_dep_node.hash,
                         );
 
                         let node_index = self.try_mark_previous_green(

src/test/incremental/auxiliary/issue-79890.rs

@@ -0,0 +1 @@
+pub trait MyTrait {}

src/test/incremental/issue-79890-imported-crates-changed.rs

@@ -0,0 +1,7 @@
+// aux-build:issue-79890.rs
+// revisions:rpass1 rpass2 rpass3
+// compile-flags:--extern issue_79890 --test
+// edition:2018
+
+// Tests that we don't ICE when the set of imported crates changes
+#[cfg(rpass2)] use issue_79890::MyTrait;