Skip to content

Commit 90160f1

Browse files
authored
Rollup merge of rust-lang#148649 - lcnr:rarwwww, r=BoxyUwU
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 rust-lang/trait-system-refactor-initiative#246 (comment). 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`
2 parents af8636b + c07f11a commit 90160f1

File tree

1 file changed

+14
-3
lines changed
  • compiler/rustc_type_ir/src/search_graph

1 file changed

+14
-3
lines changed

compiler/rustc_type_ir/src/search_graph/mod.rs

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ use derive_where::derive_where;
2323
#[cfg(feature = "nightly")]
2424
use rustc_macros::{Decodable_NoContext, Encodable_NoContext, HashStable_NoContext};
2525
use rustc_type_ir::data_structures::HashMap;
26-
use tracing::{debug, instrument};
26+
use tracing::{debug, instrument, trace};
2727

2828
mod stack;
2929
use stack::{Stack, StackDepth, StackEntry};
@@ -916,6 +916,7 @@ impl<D: Delegate<Cx = X>, X: Cx> SearchGraph<D> {
916916
/// heads from the stack. This may not necessarily mean that we've actually
917917
/// reached a fixpoint for that cycle head, which impacts the way we rebase
918918
/// provisional cache entries.
919+
#[derive_where(Debug; X: Cx)]
919920
enum RebaseReason<X: Cx> {
920921
NoCycleUsages,
921922
Ambiguity(X::AmbiguityInfo),
@@ -950,6 +951,7 @@ impl<D: Delegate<Cx = X>, X: Cx> SearchGraph<D, X> {
950951
/// cache entries to also be ambiguous. This causes some undesirable ambiguity for nested
951952
/// goals whose result doesn't actually depend on this cycle head, but that's acceptable
952953
/// to me.
954+
#[instrument(level = "trace", skip(self, cx))]
953955
fn rebase_provisional_cache_entries(
954956
&mut self,
955957
cx: X,
@@ -969,6 +971,7 @@ impl<D: Delegate<Cx = X>, X: Cx> SearchGraph<D, X> {
969971
let popped_head = if heads.highest_cycle_head_index() == popped_head_index {
970972
heads.remove_highest_cycle_head()
971973
} else {
974+
debug_assert!(heads.highest_cycle_head_index() < popped_head_index);
972975
return true;
973976
};
974977

@@ -1057,6 +1060,8 @@ impl<D: Delegate<Cx = X>, X: Cx> SearchGraph<D, X> {
10571060
new_highest_head_index,
10581061
));
10591062

1063+
trace!(?input, ?entry, "rebased provisional cache entry");
1064+
10601065
true
10611066
});
10621067
!entries.is_empty()
@@ -1379,7 +1384,8 @@ impl<D: Delegate<Cx = X>, X: Cx> SearchGraph<D, X> {
13791384
}
13801385

13811386
// Clear all provisional cache entries which depend on a previous provisional
1382-
// result of this goal and rerun.
1387+
// result of this goal and rerun. This does not remove goals which accessed this
1388+
// goal without depending on its result.
13831389
self.clear_dependent_provisional_results_for_rerun();
13841390

13851391
debug!(?result, "fixpoint changed provisional results");
@@ -1399,7 +1405,12 @@ impl<D: Delegate<Cx = X>, X: Cx> SearchGraph<D, X> {
13991405
// similar to the previous iterations when reevaluating, it's better
14001406
// for caching if the reevaluation also starts out with `false`.
14011407
encountered_overflow: false,
1402-
usages: None,
1408+
// We keep provisional cache entries around if they used this goal
1409+
// without depending on its result.
1410+
//
1411+
// We still need to drop or rebase these cache entries once we've
1412+
// finished evaluating this goal.
1413+
usages: Some(HeadUsages::default()),
14031414
candidate_usages: None,
14041415
});
14051416
}

0 commit comments

Comments
 (0)