Skip to content

Commit

Permalink
Auto merge of rust-lang#126409 - pacak:incr-uplorry, r=michaelwoerister
Browse files Browse the repository at this point in the history
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 rust-lang#107226
Fixes rust-lang#125367
  • Loading branch information
bors committed Jun 20, 2024
2 parents 1208edd + 12f8d12 commit 1d96de2
Show file tree
Hide file tree
Showing 6 changed files with 66 additions and 43 deletions.
17 changes: 6 additions & 11 deletions compiler/rustc_hir/src/definitions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
12 changes: 2 additions & 10 deletions compiler/rustc_middle/src/dep_graph/dep_node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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()
Expand Down
8 changes: 4 additions & 4 deletions compiler/rustc_middle/src/query/on_disk_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
14 changes: 3 additions & 11 deletions compiler/rustc_middle/src/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
}

Expand Down
18 changes: 11 additions & 7 deletions compiler/rustc_query_system/src/dep_graph/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
40 changes: 40 additions & 0 deletions tests/incremental/unrecoverable_query.rs
Original file line number Diff line number Diff line change
@@ -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();
}

0 comments on commit 1d96de2

Please sign in to comment.