user0/rust
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Rollup merge of #148649 - lcnr:rarwwww, r=BoxyUwU

Browse source 
don't completely reset `HeadUsages`

This is really subtle ☠️ I've actually went and added testing for `search_graph.ignore_candidate_head_usages` to https://github.com/lcnr/search_graph_fuzz now. I should have done that when I originally implemented but didn't quite know how to do so back then.

The search graph is far too subtle to think through it manually. I've added the affected proof tree to https://github.com/rust-lang/trait-system-refactor-initiative/blob/main/notes/next-solver/search-graph/general.md#keeping-provisional-cache-entries-on-rerun. It's

- A
  - B
    - C (depends on B and gets dropped when rerunning)
      - D (does not depend on B so we keep it around when rerunning)
        - C (irrevant candidate)
        - A
      - B
    - D
      - C (irrevant candidate)
        - D
      - A
    - rerun
    - C (use provisional cache entry which doesn't depend on B)
    - D (use provisional cache entry which doesn't depend on B)

Fixes the ICE in https://github.com/rust-lang/trait-system-refactor-initiative/issues/246#issuecomment-3497832902. I think this issue is brittle enough that adding that as a test isn't really useful. Any small change to the search graph will prevent it from testing this. We do test this fix via the fuzzer.

r? `````@BoxyUwU`````
This commit is contained in:
Stuart Cook 2025-11-09 13:22:33 +11:00 • committed by GitHub
commit ec4e6e6559
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
1 changed files with 14 additions and 3 deletions
Download patch file Download diff file Expand all files Collapse all files

17
compiler/rustc_type_ir/src/search_graph/mod.rs
View file

@ -23,7 +23,7 @@ use derive_where::derive_where;
#[cfg(feature = "nightly")]
use rustc_macros::{Decodable_NoContext, Encodable_NoContext, HashStable_NoContext};
use rustc_type_ir::data_structures::HashMap;
use tracing::{debug, instrument};
use tracing::{debug, instrument, trace};
mod stack;
use stack::{Stack, StackDepth, StackEntry};
@ -916,6 +916,7 @@ impl<D: Delegate<Cx = X>, X: Cx> SearchGraph<D> {
/// heads from the stack. This may not necessarily mean that we've actually
/// reached a fixpoint for that cycle head, which impacts the way we rebase
/// provisional cache entries.
#[derive_where(Debug; X: Cx)]
enum RebaseReason<X: Cx> {
NoCycleUsages,
Ambiguity(X::AmbiguityInfo),
@ -950,6 +951,7 @@ impl<D: Delegate<Cx = X>, X: Cx> SearchGraph<D, X> {
/// cache entries to also be ambiguous. This causes some undesirable ambiguity for nested
/// goals whose result doesn't actually depend on this cycle head, but that's acceptable
/// to me.
#[instrument(level = "trace", skip(self, cx))]
fn rebase_provisional_cache_entries(
&mut self,
cx: X,
@ -969,6 +971,7 @@ impl<D: Delegate<Cx = X>, X: Cx> SearchGraph<D, X> {
let popped_head = if heads.highest_cycle_head_index() == popped_head_index {
heads.remove_highest_cycle_head()
} else {
debug_assert!(heads.highest_cycle_head_index() < popped_head_index);
return true;
};
@ -1057,6 +1060,8 @@ impl<D: Delegate<Cx = X>, X: Cx> SearchGraph<D, X> {
new_highest_head_index,
));
trace!(?input, ?entry, "rebased provisional cache entry");
true
});
!entries.is_empty()
@ -1379,7 +1384,8 @@ impl<D: Delegate<Cx = X>, X: Cx> SearchGraph<D, X> {
}
// Clear all provisional cache entries which depend on a previous provisional
// result of this goal and rerun.
// result of this goal and rerun. This does not remove goals which accessed this
// goal without depending on its result.
self.clear_dependent_provisional_results_for_rerun();
debug!(?result, "fixpoint changed provisional results");
@ -1399,7 +1405,12 @@ impl<D: Delegate<Cx = X>, X: Cx> SearchGraph<D, X> {
// similar to the previous iterations when reevaluating, it's better
// for caching if the reevaluation also starts out with `false`.
encountered_overflow: false,
usages: None,
// We keep provisional cache entries around if they used this goal
// without depending on its result.
//
// We still need to drop or rebase these cache entries once we've
// finished evaluating this goal.
usages: Some(HeadUsages::default()),
candidate_usages: None,
});
}