Auto merge of #126409 - pacak:incr-uplorry, r=michaelwoerister

Trying to address an incremental compilation issues

This pull request contains two independent changes, one makes it so when `try_force_from_dep_node` fails to recover a query - it marks the node as "red" instead of "green" and the second one makes Debug impl for `DepNode` less panicky if it encounters something from the previous compilation that doesn't map to anything in the current one.

I'm not 100% confident that this is the correct approach, but so far I managed to find a bunch of comments suggesting that some things are allowed to fail in a certain way and changes I made are allowing for those things to fail this way and it fixes all the small reproducers I managed to find.

Compilation panic this pull request avoids is caused by an automatically generated code on an associated type and it is not happening if something else marks it as outdated first (or close like that, but scenario is quite obscure).

Fixes #107226
Fixes #125367

Author: bors

compiler/rustc_hir/src/definitions.rs

@@ -377,22 +377,17 @@ impl Definitions {
     }
 
     #[inline(always)]
-    pub fn local_def_path_hash_to_def_id(
-        &self,
-        hash: DefPathHash,
-        err_msg: &dyn std::fmt::Debug,
-    ) -> LocalDefId {
+    /// Returns `None` if the `DefPathHash` does not correspond to a `LocalDefId`
+    /// in the current compilation session. This can legitimately happen if the
+    /// `DefPathHash` is from a `DefId` in an upstream crate or, during incr. comp.,
+    /// 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);
-        #[cold]
-        #[inline(never)]
-        fn err(err_msg: &dyn std::fmt::Debug) -> ! {
-            panic!("{err_msg:?}")
-        }
         self.table
             .def_path_hash_to_index
             .get(&hash.local_hash())
             .map(|local_def_index| LocalDefId { local_def_index })
-            .unwrap_or_else(|| err(err_msg))
     }
 
     pub fn def_path_hash_to_def_index_map(&self) -> &DefPathHashMap {

compiler/rustc_middle/src/dep_graph/dep_node.rs

@@ -194,10 +194,7 @@ impl DepNodeExt for DepNode {
     /// has been removed.
     fn extract_def_id(&self, tcx: TyCtxt<'_>) -> Option<DefId> {
         if tcx.fingerprint_style(self.kind) == FingerprintStyle::DefPathHash {
-            Some(tcx.def_path_hash_to_def_id(
-                DefPathHash(self.hash.into()),
-                &("Failed to extract DefId", self.kind, self.hash),
-            ))
+            tcx.def_path_hash_to_def_id(DefPathHash(self.hash.into()))
         } else {
             None
         }
@@ -390,12 +387,7 @@ impl<'tcx> DepNodeParams<TyCtxt<'tcx>> for HirId {
         if tcx.fingerprint_style(dep_node.kind) == FingerprintStyle::HirId {
             let (local_hash, local_id) = Fingerprint::from(dep_node.hash).split();
             let def_path_hash = DefPathHash::new(tcx.stable_crate_id(LOCAL_CRATE), local_hash);
-            let def_id = tcx
-                .def_path_hash_to_def_id(
-                    def_path_hash,
-                    &("Failed to extract HirId", dep_node.kind, dep_node.hash),
-                )
-                .expect_local();
+            let def_id = tcx.def_path_hash_to_def_id(def_path_hash)?.expect_local();
             let local_id = local_id
                 .as_u64()
                 .try_into()

compiler/rustc_middle/src/query/on_disk_cache.rs

@@ -733,10 +733,10 @@ impl<'a, 'tcx> SpanDecoder for CacheDecoder<'a, 'tcx> {
         // If we get to this point, then all of the query inputs were green,
         // which means that the definition with this hash is guaranteed to
         // still exist in the current compilation session.
-        self.tcx.def_path_hash_to_def_id(
-            def_path_hash,
-            &("Failed to convert DefPathHash", def_path_hash),
-        )
+        match self.tcx.def_path_hash_to_def_id(def_path_hash) {
+            Some(r) => r,
+            None => panic!("Failed to convert DefPathHash {def_path_hash:?}"),
+        }
     }
 
     fn decode_attr_id(&mut self) -> rustc_span::AttrId {

compiler/rustc_middle/src/ty/context.rs

@@ -1677,25 +1677,17 @@ impl<'tcx> TyCtxt<'tcx> {
     /// Converts a `DefPathHash` to its corresponding `DefId` in the current compilation
     /// session, if it still exists. This is used during incremental compilation to
     /// turn a deserialized `DefPathHash` into its current `DefId`.
-    pub fn def_path_hash_to_def_id(
-        self,
-        hash: DefPathHash,
-        err_msg: &dyn std::fmt::Debug,
-    ) -> DefId {
+    pub fn def_path_hash_to_def_id(self, hash: DefPathHash) -> Option<DefId> {
         debug!("def_path_hash_to_def_id({:?})", hash);
 
         let stable_crate_id = hash.stable_crate_id();
 
         // If this is a DefPathHash from the local crate, we can look up the
         // DefId in the tcx's `Definitions`.
         if stable_crate_id == self.stable_crate_id(LOCAL_CRATE) {
-            self.untracked
-                .definitions
-                .read()
-                .local_def_path_hash_to_def_id(hash, err_msg)
-                .to_def_id()
+            Some(self.untracked.definitions.read().local_def_path_hash_to_def_id(hash)?.to_def_id())
         } else {
-            self.def_path_hash_to_def_id_extern(hash, stable_crate_id)
+            Some(self.def_path_hash_to_def_id_extern(hash, stable_crate_id))
         }
     }
 

compiler/rustc_query_system/src/dep_graph/mod.rs

@@ -51,20 +51,24 @@ pub trait DepContext: Copy {
     }
 
     /// Try to force a dep node to execute and see if it's green.
+    ///
+    /// Returns true if the query has actually been forced. It is valid that a query
+    /// fails to be forced, e.g. when the query key cannot be reconstructed from the
+    /// dep-node or when the query kind outright does not support it.
     #[inline]
     #[instrument(skip(self, frame), level = "debug")]
     fn try_force_from_dep_node(self, dep_node: DepNode, frame: Option<&MarkFrame<'_>>) -> bool {
         let cb = self.dep_kind_info(dep_node.kind);
         if let Some(f) = cb.force_from_dep_node {
-            if let Err(value) = panic::catch_unwind(panic::AssertUnwindSafe(|| {
-                f(self, dep_node);
-            })) {
-                if !value.is::<rustc_errors::FatalErrorMarker>() {
-                    print_markframe_trace(self.dep_graph(), frame);
+            match panic::catch_unwind(panic::AssertUnwindSafe(|| f(self, dep_node))) {
+                Err(value) => {
+                    if !value.is::<rustc_errors::FatalErrorMarker>() {
+                        print_markframe_trace(self.dep_graph(), frame);
+                    }
+                    panic::resume_unwind(value)
                 }
-                panic::resume_unwind(value)
+                Ok(query_has_been_forced) => query_has_been_forced,
             }
-            true
         } else {
             false
         }

tests/incremental/unrecoverable_query.rs

@@ -0,0 +1,40 @@
+// If it is impossible to find query arguments just from the hash
+// compiler should treat the node as red
+
+// In this test prior to fixing compiler was having problems figuring out
+// drop impl for T inside of m
+
+//@ revisions:cfail1 cfail2
+//@ compile-flags: --crate-type=lib
+//@ build-pass
+
+pub trait P {
+    type A;
+}
+
+struct S;
+
+impl P for S {
+    type A = C;
+}
+
+struct T<D: P>(D::A, Z<D>);
+
+struct Z<D: P>(D::A, String);
+
+impl<D: P> T<D> {
+    pub fn i() -> Self {
+        loop {}
+    }
+}
+
+enum C {
+    #[cfg(cfail1)]
+    Up(()),
+    #[cfg(cfail2)]
+    Lorry(()),
+}
+
+pub fn m() {
+    T::<S>::i();
+}