Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit 59ad2ae

Browse files
authoredAug 14, 2024
Rollup merge of #128828 - lcnr:search-graph-11, r=compiler-errors
`-Znext-solver` caching This PR has two major changes while also fixing multiple issues found via fuzzing. The main optimization is the ability to not discard provisional cache entries when popping the highest cycle head the entry depends on. This fixes the hang in Fuchsia with `-Znext-solver=coherence`. It also bails if the result of a fixpoint iteration is ambiguous, even without reaching a fixpoint. This is necessary to avoid exponential blowup if a coinductive cycle results in ambiguity, e.g. due to unknowable candidates in coherence. Updating stack entries pretty much exclusively happens lazily now, so `fn check_invariants` ended up being mostly useless and I've removed it. See https://gist.github.com/lcnr/8de338fdb2685581e17727bbfab0622a for the invariants we would be able to assert with it. For a general overview, see the in-process update of the relevant rustc-dev-guide chapter: https://hackmd.io/1ALkSjKlSCyQG-dVb_PUHw r? ```@compiler-errors```
2 parents 196d256 + 0aa17a4 commit 59ad2ae

File tree

13 files changed

+892
-631
lines changed

13 files changed

+892
-631
lines changed
 

‎compiler/rustc_middle/src/arena.rs

-4
Original file line numberDiff line numberDiff line change
@@ -61,10 +61,6 @@ macro_rules! arena_types {
6161
[] dtorck_constraint: rustc_middle::traits::query::DropckConstraint<'tcx>,
6262
[] candidate_step: rustc_middle::traits::query::CandidateStep<'tcx>,
6363
[] autoderef_bad_ty: rustc_middle::traits::query::MethodAutoderefBadTy<'tcx>,
64-
[] canonical_goal_evaluation:
65-
rustc_type_ir::solve::inspect::CanonicalGoalEvaluationStep<
66-
rustc_middle::ty::TyCtxt<'tcx>
67-
>,
6864
[] query_region_constraints: rustc_middle::infer::canonical::QueryRegionConstraints<'tcx>,
6965
[] type_op_subtype:
7066
rustc_middle::infer::canonical::Canonical<'tcx,

‎compiler/rustc_middle/src/ty/context.rs

-9
Original file line numberDiff line numberDiff line change
@@ -107,8 +107,6 @@ impl<'tcx> Interner for TyCtxt<'tcx> {
107107
self.mk_predefined_opaques_in_body(data)
108108
}
109109
type DefiningOpaqueTypes = &'tcx ty::List<LocalDefId>;
110-
type CanonicalGoalEvaluationStepRef =
111-
&'tcx solve::inspect::CanonicalGoalEvaluationStep<TyCtxt<'tcx>>;
112110
type CanonicalVars = CanonicalVarInfos<'tcx>;
113111
fn mk_canonical_var_infos(self, infos: &[ty::CanonicalVarInfo<Self>]) -> Self::CanonicalVars {
114112
self.mk_canonical_var_infos(infos)
@@ -277,13 +275,6 @@ impl<'tcx> Interner for TyCtxt<'tcx> {
277275
self.debug_assert_args_compatible(def_id, args);
278276
}
279277

280-
fn intern_canonical_goal_evaluation_step(
281-
self,
282-
step: solve::inspect::CanonicalGoalEvaluationStep<TyCtxt<'tcx>>,
283-
) -> &'tcx solve::inspect::CanonicalGoalEvaluationStep<TyCtxt<'tcx>> {
284-
self.arena.alloc(step)
285-
}
286-
287278
fn mk_type_list_from_iter<I, T>(self, args: I) -> T::Output
288279
where
289280
I: Iterator<Item = T>,

‎compiler/rustc_next_trait_solver/src/solve/inspect/build.rs

+14-92
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,10 @@
55
//! see the comment on [ProofTreeBuilder].
66
77
use std::marker::PhantomData;
8-
use std::mem;
98

109
use derive_where::derive_where;
1110
use rustc_type_ir::inherent::*;
12-
use rustc_type_ir::{self as ty, search_graph, Interner};
11+
use rustc_type_ir::{self as ty, Interner};
1312

1413
use crate::delegate::SolverDelegate;
1514
use crate::solve::eval_ctxt::canonical;
@@ -94,31 +93,10 @@ impl<I: Interner> WipGoalEvaluation<I> {
9493
}
9594
}
9695

97-
#[derive_where(PartialEq, Eq; I: Interner)]
98-
pub(in crate::solve) enum WipCanonicalGoalEvaluationKind<I: Interner> {
99-
Overflow,
100-
CycleInStack,
101-
ProvisionalCacheHit,
102-
Interned { final_revision: I::CanonicalGoalEvaluationStepRef },
103-
}
104-
105-
impl<I: Interner> std::fmt::Debug for WipCanonicalGoalEvaluationKind<I> {
106-
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
107-
match self {
108-
Self::Overflow => write!(f, "Overflow"),
109-
Self::CycleInStack => write!(f, "CycleInStack"),
110-
Self::ProvisionalCacheHit => write!(f, "ProvisionalCacheHit"),
111-
Self::Interned { final_revision: _ } => {
112-
f.debug_struct("Interned").finish_non_exhaustive()
113-
}
114-
}
115-
}
116-
}
117-
11896
#[derive_where(PartialEq, Eq, Debug; I: Interner)]
11997
struct WipCanonicalGoalEvaluation<I: Interner> {
12098
goal: CanonicalInput<I>,
121-
kind: Option<WipCanonicalGoalEvaluationKind<I>>,
99+
encountered_overflow: bool,
122100
/// Only used for uncached goals. After we finished evaluating
123101
/// the goal, this is interned and moved into `kind`.
124102
final_revision: Option<WipCanonicalGoalEvaluationStep<I>>,
@@ -127,25 +105,17 @@ struct WipCanonicalGoalEvaluation<I: Interner> {
127105

128106
impl<I: Interner> WipCanonicalGoalEvaluation<I> {
129107
fn finalize(self) -> inspect::CanonicalGoalEvaluation<I> {
130-
// We've already interned the final revision in
131-
// `fn finalize_canonical_goal_evaluation`.
132-
assert!(self.final_revision.is_none());
133-
let kind = match self.kind.unwrap() {
134-
WipCanonicalGoalEvaluationKind::Overflow => {
108+
inspect::CanonicalGoalEvaluation {
109+
goal: self.goal,
110+
kind: if self.encountered_overflow {
111+
assert!(self.final_revision.is_none());
135112
inspect::CanonicalGoalEvaluationKind::Overflow
136-
}
137-
WipCanonicalGoalEvaluationKind::CycleInStack => {
138-
inspect::CanonicalGoalEvaluationKind::CycleInStack
139-
}
140-
WipCanonicalGoalEvaluationKind::ProvisionalCacheHit => {
141-
inspect::CanonicalGoalEvaluationKind::ProvisionalCacheHit
142-
}
143-
WipCanonicalGoalEvaluationKind::Interned { final_revision } => {
113+
} else {
114+
let final_revision = self.final_revision.unwrap().finalize();
144115
inspect::CanonicalGoalEvaluationKind::Evaluation { final_revision }
145-
}
146-
};
147-
148-
inspect::CanonicalGoalEvaluation { goal: self.goal, kind, result: self.result.unwrap() }
116+
},
117+
result: self.result.unwrap(),
118+
}
149119
}
150120
}
151121

@@ -308,7 +278,7 @@ impl<D: SolverDelegate<Interner = I>, I: Interner> ProofTreeBuilder<D> {
308278
) -> ProofTreeBuilder<D> {
309279
self.nested(|| WipCanonicalGoalEvaluation {
310280
goal,
311-
kind: None,
281+
encountered_overflow: false,
312282
final_revision: None,
313283
result: None,
314284
})
@@ -329,11 +299,11 @@ impl<D: SolverDelegate<Interner = I>, I: Interner> ProofTreeBuilder<D> {
329299
}
330300
}
331301

332-
pub fn canonical_goal_evaluation_kind(&mut self, kind: WipCanonicalGoalEvaluationKind<I>) {
302+
pub fn canonical_goal_evaluation_overflow(&mut self) {
333303
if let Some(this) = self.as_mut() {
334304
match this {
335305
DebugSolver::CanonicalGoalEvaluation(canonical_goal_evaluation) => {
336-
assert_eq!(canonical_goal_evaluation.kind.replace(kind), None);
306+
canonical_goal_evaluation.encountered_overflow = true;
337307
}
338308
_ => unreachable!(),
339309
};
@@ -547,51 +517,3 @@ impl<D: SolverDelegate<Interner = I>, I: Interner> ProofTreeBuilder<D> {
547517
}
548518
}
549519
}
550-
551-
impl<D, I> search_graph::ProofTreeBuilder<I> for ProofTreeBuilder<D>
552-
where
553-
D: SolverDelegate<Interner = I>,
554-
I: Interner,
555-
{
556-
fn try_apply_proof_tree(
557-
&mut self,
558-
proof_tree: Option<I::CanonicalGoalEvaluationStepRef>,
559-
) -> bool {
560-
if !self.is_noop() {
561-
if let Some(final_revision) = proof_tree {
562-
let kind = WipCanonicalGoalEvaluationKind::Interned { final_revision };
563-
self.canonical_goal_evaluation_kind(kind);
564-
true
565-
} else {
566-
false
567-
}
568-
} else {
569-
true
570-
}
571-
}
572-
573-
fn on_provisional_cache_hit(&mut self) {
574-
self.canonical_goal_evaluation_kind(WipCanonicalGoalEvaluationKind::ProvisionalCacheHit);
575-
}
576-
577-
fn on_cycle_in_stack(&mut self) {
578-
self.canonical_goal_evaluation_kind(WipCanonicalGoalEvaluationKind::CycleInStack);
579-
}
580-
581-
fn finalize_canonical_goal_evaluation(
582-
&mut self,
583-
tcx: I,
584-
) -> Option<I::CanonicalGoalEvaluationStepRef> {
585-
self.as_mut().map(|this| match this {
586-
DebugSolver::CanonicalGoalEvaluation(evaluation) => {
587-
let final_revision = mem::take(&mut evaluation.final_revision).unwrap();
588-
let final_revision =
589-
tcx.intern_canonical_goal_evaluation_step(final_revision.finalize());
590-
let kind = WipCanonicalGoalEvaluationKind::Interned { final_revision };
591-
assert_eq!(evaluation.kind.replace(kind), None);
592-
final_revision
593-
}
594-
_ => unreachable!(),
595-
})
596-
}
597-
}

‎compiler/rustc_next_trait_solver/src/solve/search_graph.rs

+44-22
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,13 @@
1+
use std::convert::Infallible;
12
use std::marker::PhantomData;
23

34
use rustc_type_ir::inherent::*;
4-
use rustc_type_ir::search_graph::{self, CycleKind, UsageKind};
5+
use rustc_type_ir::search_graph::{self, PathKind};
56
use rustc_type_ir::solve::{CanonicalInput, Certainty, QueryResult};
67
use rustc_type_ir::Interner;
78

8-
use super::inspect::{self, ProofTreeBuilder};
9-
use super::FIXPOINT_STEP_LIMIT;
9+
use super::inspect::ProofTreeBuilder;
10+
use super::{has_no_inference_or_external_constraints, FIXPOINT_STEP_LIMIT};
1011
use crate::delegate::SolverDelegate;
1112

1213
/// This type is never constructed. We only use it to implement `search_graph::Delegate`
@@ -22,43 +23,48 @@ where
2223
{
2324
type Cx = D::Interner;
2425

26+
const ENABLE_PROVISIONAL_CACHE: bool = true;
27+
type ValidationScope = Infallible;
28+
fn enter_validation_scope(
29+
_cx: Self::Cx,
30+
_input: CanonicalInput<I>,
31+
) -> Option<Self::ValidationScope> {
32+
None
33+
}
34+
2535
const FIXPOINT_STEP_LIMIT: usize = FIXPOINT_STEP_LIMIT;
2636

2737
type ProofTreeBuilder = ProofTreeBuilder<D>;
38+
fn inspect_is_noop(inspect: &mut Self::ProofTreeBuilder) -> bool {
39+
inspect.is_noop()
40+
}
2841

42+
const DIVIDE_AVAILABLE_DEPTH_ON_OVERFLOW: usize = 4;
2943
fn recursion_limit(cx: I) -> usize {
3044
cx.recursion_limit()
3145
}
3246

3347
fn initial_provisional_result(
3448
cx: I,
35-
kind: CycleKind,
49+
kind: PathKind,
3650
input: CanonicalInput<I>,
3751
) -> QueryResult<I> {
3852
match kind {
39-
CycleKind::Coinductive => response_no_constraints(cx, input, Certainty::Yes),
40-
CycleKind::Inductive => response_no_constraints(cx, input, Certainty::overflow(false)),
53+
PathKind::Coinductive => response_no_constraints(cx, input, Certainty::Yes),
54+
PathKind::Inductive => response_no_constraints(cx, input, Certainty::overflow(false)),
4155
}
4256
}
4357

44-
fn reached_fixpoint(
45-
cx: I,
46-
kind: UsageKind,
58+
fn is_initial_provisional_result(
59+
cx: Self::Cx,
60+
kind: PathKind,
4761
input: CanonicalInput<I>,
48-
provisional_result: Option<QueryResult<I>>,
4962
result: QueryResult<I>,
5063
) -> bool {
51-
if let Some(r) = provisional_result {
52-
r == result
53-
} else {
54-
match kind {
55-
UsageKind::Single(CycleKind::Coinductive) => {
56-
response_no_constraints(cx, input, Certainty::Yes) == result
57-
}
58-
UsageKind::Single(CycleKind::Inductive) => {
59-
response_no_constraints(cx, input, Certainty::overflow(false)) == result
60-
}
61-
UsageKind::Mixed => false,
64+
match kind {
65+
PathKind::Coinductive => response_no_constraints(cx, input, Certainty::Yes) == result,
66+
PathKind::Inductive => {
67+
response_no_constraints(cx, input, Certainty::overflow(false)) == result
6268
}
6369
}
6470
}
@@ -68,14 +74,30 @@ where
6874
inspect: &mut ProofTreeBuilder<D>,
6975
input: CanonicalInput<I>,
7076
) -> QueryResult<I> {
71-
inspect.canonical_goal_evaluation_kind(inspect::WipCanonicalGoalEvaluationKind::Overflow);
77+
inspect.canonical_goal_evaluation_overflow();
7278
response_no_constraints(cx, input, Certainty::overflow(true))
7379
}
7480

7581
fn on_fixpoint_overflow(cx: I, input: CanonicalInput<I>) -> QueryResult<I> {
7682
response_no_constraints(cx, input, Certainty::overflow(false))
7783
}
7884

85+
fn is_ambiguous_result(result: QueryResult<I>) -> bool {
86+
result.is_ok_and(|response| {
87+
has_no_inference_or_external_constraints(response)
88+
&& matches!(response.value.certainty, Certainty::Maybe(_))
89+
})
90+
}
91+
92+
fn propagate_ambiguity(
93+
cx: I,
94+
for_input: CanonicalInput<I>,
95+
from_result: QueryResult<I>,
96+
) -> QueryResult<I> {
97+
let certainty = from_result.unwrap().value.certainty;
98+
response_no_constraints(cx, for_input, certainty)
99+
}
100+
79101
fn step_is_coinductive(cx: I, input: CanonicalInput<I>) -> bool {
80102
input.value.goal.predicate.is_coinductive(cx)
81103
}

‎compiler/rustc_trait_selection/src/solve/inspect/analyse.rs

+3-7
Original file line numberDiff line numberDiff line change
@@ -334,13 +334,9 @@ impl<'a, 'tcx> InspectGoal<'a, 'tcx> {
334334

335335
pub fn candidates(&'a self) -> Vec<InspectCandidate<'a, 'tcx>> {
336336
let mut candidates = vec![];
337-
let last_eval_step = match self.evaluation_kind {
338-
inspect::CanonicalGoalEvaluationKind::Overflow
339-
| inspect::CanonicalGoalEvaluationKind::CycleInStack
340-
| inspect::CanonicalGoalEvaluationKind::ProvisionalCacheHit => {
341-
warn!("unexpected root evaluation: {:?}", self.evaluation_kind);
342-
return vec![];
343-
}
337+
let last_eval_step = match &self.evaluation_kind {
338+
// An annoying edge case in case the recursion limit is 0.
339+
inspect::CanonicalGoalEvaluationKind::Overflow => return vec![],
344340
inspect::CanonicalGoalEvaluationKind::Evaluation { final_revision } => final_revision,
345341
};
346342

‎compiler/rustc_type_ir/src/binder.rs

+8-16
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use derive_where::derive_where;
88
use rustc_macros::{HashStable_NoContext, TyDecodable, TyEncodable};
99
#[cfg(feature = "nightly")]
1010
use rustc_serialize::Decodable;
11-
use tracing::debug;
11+
use tracing::instrument;
1212

1313
use crate::data_structures::SsoHashSet;
1414
use crate::fold::{FallibleTypeFolder, TypeFoldable, TypeFolder, TypeSuperFoldable};
@@ -817,28 +817,20 @@ impl<'a, I: Interner> ArgFolder<'a, I> {
817817
/// As indicated in the diagram, here the same type `&'a i32` is instantiated once, but in the
818818
/// first case we do not increase the De Bruijn index and in the second case we do. The reason
819819
/// is that only in the second case have we passed through a fn binder.
820+
#[instrument(level = "trace", skip(self), fields(binders_passed = self.binders_passed), ret)]
820821
fn shift_vars_through_binders<T: TypeFoldable<I>>(&self, val: T) -> T {
821-
debug!(
822-
"shift_vars(val={:?}, binders_passed={:?}, has_escaping_bound_vars={:?})",
823-
val,
824-
self.binders_passed,
825-
val.has_escaping_bound_vars()
826-
);
827-
828822
if self.binders_passed == 0 || !val.has_escaping_bound_vars() {
829-
return val;
823+
val
824+
} else {
825+
ty::fold::shift_vars(self.cx, val, self.binders_passed)
830826
}
831-
832-
let result = ty::fold::shift_vars(TypeFolder::cx(self), val, self.binders_passed);
833-
debug!("shift_vars: shifted result = {:?}", result);
834-
835-
result
836827
}
837828

838829
fn shift_region_through_binders(&self, region: I::Region) -> I::Region {
839830
if self.binders_passed == 0 || !region.has_escaping_bound_vars() {
840-
return region;
831+
region
832+
} else {
833+
ty::fold::shift_region(self.cx, region, self.binders_passed)
841834
}
842-
ty::fold::shift_region(self.cx, region, self.binders_passed)
843835
}
844836
}

‎compiler/rustc_type_ir/src/fold.rs

+5-6
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@
4848
use std::mem;
4949

5050
use rustc_index::{Idx, IndexVec};
51-
use tracing::debug;
51+
use tracing::instrument;
5252

5353
use crate::data_structures::Lrc;
5454
use crate::inherent::*;
@@ -417,15 +417,14 @@ pub fn shift_region<I: Interner>(cx: I, region: I::Region, amount: u32) -> I::Re
417417
}
418418
}
419419

420+
#[instrument(level = "trace", skip(cx), ret)]
420421
pub fn shift_vars<I: Interner, T>(cx: I, value: T, amount: u32) -> T
421422
where
422423
T: TypeFoldable<I>,
423424
{
424-
debug!("shift_vars(value={:?}, amount={})", value, amount);
425-
426425
if amount == 0 || !value.has_escaping_bound_vars() {
427-
return value;
426+
value
427+
} else {
428+
value.fold_with(&mut Shifter::new(cx, amount))
428429
}
429-
430-
value.fold_with(&mut Shifter::new(cx, amount))
431430
}
There was a problem loading the remainder of the diff.

0 commit comments

Comments
 (0)
Failed to load comments.