diff --git a/compiler/rustc_middle/src/ty/query/mod.rs b/compiler/rustc_middle/src/ty/query/mod.rs index 187f86a52f4..b269dd09b72 100644 --- a/compiler/rustc_middle/src/ty/query/mod.rs +++ b/compiler/rustc_middle/src/ty/query/mod.rs @@ -220,7 +220,7 @@ pub(crate) fn try_load_from_on_disk_cache<'tcx>(tcx: TyCtxt<'tcx>, dep_node: &De .map(|c| c.is_green()) .unwrap_or(false)); - let key = as DepNodeParams>>::recover(tcx, dep_node).unwrap(); + let key = as DepNodeParams>>::recover(tcx, dep_node).unwrap_or_else(|| panic!("Failed to recover key for {:?} with hash {}", dep_node, dep_node.hash)); if queries::$name::cache_on_disk(tcx, &key, None) { let _ = tcx.$name(key); } diff --git a/compiler/rustc_query_system/src/dep_graph/dep_node.rs b/compiler/rustc_query_system/src/dep_graph/dep_node.rs index 3d9e739cd28..09e5dc857a7 100644 --- a/compiler/rustc_query_system/src/dep_graph/dep_node.rs +++ b/compiler/rustc_query_system/src/dep_graph/dep_node.rs @@ -53,6 +53,19 @@ use std::hash::Hash; #[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, Encodable, Decodable)] pub struct DepNode { pub kind: K, + // Important - whenever a `DepNode` is constructed, we need to make + // sure to register a `DefPathHash -> DefId` mapping if needed. + // This is currently done in two places: + // + // * When a `DepNode::construct` is called, `arg.to_fingerprint()` + // is responsible for calling `OnDiskCache::store_foreign_def_id_hash` + // if needed + // * When a `DepNode` is loaded from the `PreviousDepGraph`, + // then `PreviousDepGraph::index_to_node` is responsible for calling + // `tcx.register_reused_dep_path_hash` + // + // FIXME: Enforce this by preventing manual construction of `DefNode` + // (e.g. add a `_priv: ()` field) pub hash: PackedFingerprint, } diff --git a/compiler/rustc_query_system/src/dep_graph/graph.rs b/compiler/rustc_query_system/src/dep_graph/graph.rs index ac37b296b53..8bde552e2d4 100644 --- a/compiler/rustc_query_system/src/dep_graph/graph.rs +++ b/compiler/rustc_query_system/src/dep_graph/graph.rs @@ -7,7 +7,6 @@ use rustc_data_structures::sync::{AtomicU32, AtomicU64, Lock, Lrc, Ordering}; use rustc_data_structures::unlikely; use rustc_errors::Diagnostic; use rustc_index::vec::{Idx, IndexVec}; -use rustc_span::def_id::DefPathHash; use parking_lot::{Condvar, Mutex}; use smallvec::{smallvec, SmallVec}; @@ -555,7 +554,7 @@ impl DepGraph { // We never try to mark eval_always nodes as green debug_assert!(!dep_node.kind.is_eval_always()); - debug_assert_eq!(data.previous.index_to_node(prev_dep_node_index), *dep_node); + data.previous.debug_assert_eq(prev_dep_node_index, *dep_node); let prev_deps = data.previous.edge_targets_from(prev_dep_node_index); @@ -573,7 +572,7 @@ impl DepGraph { "try_mark_previous_green({:?}) --- found dependency {:?} to \ be immediately green", dep_node, - data.previous.index_to_node(dep_dep_node_index) + data.previous.debug_dep_node(dep_dep_node_index), ); current_deps.push(node_index); } @@ -586,12 +585,12 @@ impl DepGraph { "try_mark_previous_green({:?}) - END - dependency {:?} was \ immediately red", dep_node, - data.previous.index_to_node(dep_dep_node_index) + data.previous.debug_dep_node(dep_dep_node_index) ); return None; } None => { - let dep_dep_node = &data.previous.index_to_node(dep_dep_node_index); + let dep_dep_node = &data.previous.index_to_node(dep_dep_node_index, tcx); // We don't know the state of this dependency. If it isn't // an eval_always node, let's try to mark it green recursively. @@ -700,18 +699,6 @@ impl DepGraph { data.current.intern_node(*dep_node, current_deps, fingerprint) }; - // We have just loaded a deserialized `DepNode` from the previous - // compilation session into the current one. If this was a foreign `DefId`, - // then we stored additional information in the incr comp cache when we - // initially created its fingerprint (see `DepNodeParams::to_fingerprint`) - // We won't be calling `to_fingerprint` again for this `DepNode` (we no longer - // have the original value), so we need to copy over this additional information - // from the old incremental cache into the new cache that we serialize - // and the end of this compilation session. - if dep_node.kind.can_reconstruct_query_key() { - tcx.register_reused_dep_path_hash(DefPathHash(dep_node.hash.into())); - } - // ... emitting any stored diagnostic ... // FIXME: Store the fact that a node has diagnostics in a bit in the dep graph somewhere @@ -814,7 +801,7 @@ impl DepGraph { for prev_index in data.colors.values.indices() { match data.colors.get(prev_index) { Some(DepNodeColor::Green(_)) => { - let dep_node = data.previous.index_to_node(prev_index); + let dep_node = data.previous.index_to_node(prev_index, tcx); tcx.try_load_from_on_disk_cache(&dep_node); } None | Some(DepNodeColor::Red) => { diff --git a/compiler/rustc_query_system/src/dep_graph/prev.rs b/compiler/rustc_query_system/src/dep_graph/prev.rs index 29357ce9449..9298b652da2 100644 --- a/compiler/rustc_query_system/src/dep_graph/prev.rs +++ b/compiler/rustc_query_system/src/dep_graph/prev.rs @@ -1,7 +1,9 @@ use super::serialized::{SerializedDepGraph, SerializedDepNodeIndex}; use super::{DepKind, DepNode}; +use crate::dep_graph::DepContext; use rustc_data_structures::fingerprint::Fingerprint; use rustc_data_structures::fx::FxHashMap; +use rustc_span::def_id::DefPathHash; #[derive(Debug, Encodable, Decodable)] pub struct PreviousDepGraph { @@ -31,7 +33,44 @@ impl PreviousDepGraph { } #[inline] - pub fn index_to_node(&self, dep_node_index: SerializedDepNodeIndex) -> DepNode { + pub fn index_to_node>( + &self, + dep_node_index: SerializedDepNodeIndex, + tcx: CTX, + ) -> DepNode { + let dep_node = self.data.nodes[dep_node_index]; + // We have just loaded a deserialized `DepNode` from the previous + // compilation session into the current one. If this was a foreign `DefId`, + // then we stored additional information in the incr comp cache when we + // initially created its fingerprint (see `DepNodeParams::to_fingerprint`) + // We won't be calling `to_fingerprint` again for this `DepNode` (we no longer + // have the original value), so we need to copy over this additional information + // from the old incremental cache into the new cache that we serialize + // and the end of this compilation session. + if dep_node.kind.can_reconstruct_query_key() { + tcx.register_reused_dep_path_hash(DefPathHash(dep_node.hash.into())); + } + dep_node + } + + /// When debug assertions are enabled, asserts that the dep node at `dep_node_index` is equal to `dep_node`. + /// This method should be preferred over manually calling `index_to_node`. + /// Calls to `index_to_node` may affect global state, so gating a call + /// to `index_to_node` on debug assertions could cause behavior changes when debug assertions + /// are enabled. + #[inline] + pub fn debug_assert_eq(&self, dep_node_index: SerializedDepNodeIndex, dep_node: DepNode) { + debug_assert_eq!(self.data.nodes[dep_node_index], dep_node); + } + + /// Obtains a debug-printable version of the `DepNode`. + /// See `debug_assert_eq` for why this should be preferred over manually + /// calling `dep_node_index` + pub fn debug_dep_node(&self, dep_node_index: SerializedDepNodeIndex) -> impl std::fmt::Debug { + // We're returning the `DepNode` without calling `register_reused_dep_path_hash`, + // but `impl Debug` return type means that it can only be used for debug printing. + // So, there's no risk of calls trying to create new dep nodes that have this + // node as a dependency self.data.nodes[dep_node_index] } diff --git a/src/test/incremental/auxiliary/issue-79661.rs b/src/test/incremental/auxiliary/issue-79661.rs new file mode 100644 index 00000000000..cd32a52ebfd --- /dev/null +++ b/src/test/incremental/auxiliary/issue-79661.rs @@ -0,0 +1,6 @@ +#![feature(rustc_attrs)] + +#[cfg_attr(any(rpass2, rpass3), doc = "Some comment")] +pub struct Foo; + +pub struct Wrapper(Foo); diff --git a/src/test/incremental/issue-79661-missing-def-path-hash.rs b/src/test/incremental/issue-79661-missing-def-path-hash.rs new file mode 100644 index 00000000000..f86fb33fbf6 --- /dev/null +++ b/src/test/incremental/issue-79661-missing-def-path-hash.rs @@ -0,0 +1,14 @@ +// aux-build:issue-79661.rs +// revisions: rpass1 rpass2 rpass3 + +// Regression test for issue #79661 +// We were failing to copy over a DefPathHash->DefId mapping +// from the old incremental cache to the new incremental cache +// when we ended up forcing a query. As a result, a subsequent +// unchanged incremental run would crash due to the missing mapping + +extern crate issue_79661; +use issue_79661::Wrapper; + +pub struct Outer(Wrapper); +fn main() {}