Skip to content

Commit 00c51bd

Browse files
authored
Rollup merge of #125510 - lcnr:change-proof-trees-to-be-shallow, r=compiler-errors
remove proof tree formatting, make em shallow Debugging via tracing `RUSTC_LOG=rustc_trait_selection::solve=debug` is now imo slightly more readable then the actual proof tree formatter. Removing everything that's not needed for the `analyse` visitor allows us to remove a bunch of code. I personally believe that we should continue to use tracing over proof trees for debugging: - it eagerly prints, allowing us to debug ICEs - the proof tree builder ends up going out of sync with the actual runtime behavior, which is confusing - using shallow proof trees is a lot more performant as we frequently do not recurse into all nested goals when using an analyse visitor - this allows us to clean up the implementation and remove some code r? ```@compiler-errors```
2 parents 4d13c96 + ebd9f35 commit 00c51bd

File tree

16 files changed

+135
-505
lines changed

16 files changed

+135
-505
lines changed

compiler/rustc_interface/src/tests.rs

+1-4
Original file line numberDiff line numberDiff line change
@@ -799,10 +799,7 @@ fn test_unstable_options_tracking_hash() {
799799
tracked!(mir_opt_level, Some(4));
800800
tracked!(move_size_limit, Some(4096));
801801
tracked!(mutable_noalias, false);
802-
tracked!(
803-
next_solver,
804-
Some(NextSolverConfig { coherence: true, globally: false, dump_tree: Default::default() })
805-
);
802+
tracked!(next_solver, Some(NextSolverConfig { coherence: true, globally: false }));
806803
tracked!(no_generate_arange_section, true);
807804
tracked!(no_jump_tables, true);
808805
tracked!(no_link, true);

compiler/rustc_middle/src/arena.rs

+4-1
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,10 @@ 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: rustc_next_trait_solver::solve::inspect::GoalEvaluationStep<rustc_middle::ty::TyCtxt<'tcx>>,
64+
[] canonical_goal_evaluation:
65+
rustc_next_trait_solver::solve::inspect::CanonicalGoalEvaluationStep<
66+
rustc_middle::ty::TyCtxt<'tcx>
67+
>,
6568
[] query_region_constraints: rustc_middle::infer::canonical::QueryRegionConstraints<'tcx>,
6669
[] type_op_subtype:
6770
rustc_middle::infer::canonical::Canonical<'tcx,

compiler/rustc_middle/src/traits/solve/cache.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ pub struct EvaluationCache<'tcx> {
1717
#[derive(Debug, PartialEq, Eq)]
1818
pub struct CacheData<'tcx> {
1919
pub result: QueryResult<'tcx>,
20-
pub proof_tree: Option<&'tcx [inspect::GoalEvaluationStep<TyCtxt<'tcx>>]>,
20+
pub proof_tree: Option<&'tcx inspect::CanonicalGoalEvaluationStep<TyCtxt<'tcx>>>,
2121
pub additional_depth: usize,
2222
pub encountered_overflow: bool,
2323
}
@@ -28,7 +28,7 @@ impl<'tcx> EvaluationCache<'tcx> {
2828
&self,
2929
tcx: TyCtxt<'tcx>,
3030
key: CanonicalInput<'tcx>,
31-
proof_tree: Option<&'tcx [inspect::GoalEvaluationStep<TyCtxt<'tcx>>]>,
31+
proof_tree: Option<&'tcx inspect::CanonicalGoalEvaluationStep<TyCtxt<'tcx>>>,
3232
additional_depth: usize,
3333
encountered_overflow: bool,
3434
cycle_participants: FxHashSet<CanonicalInput<'tcx>>,
@@ -107,7 +107,7 @@ struct Success<'tcx> {
107107
#[derive(Clone, Copy)]
108108
pub struct QueryData<'tcx> {
109109
pub result: QueryResult<'tcx>,
110-
pub proof_tree: Option<&'tcx [inspect::GoalEvaluationStep<TyCtxt<'tcx>>]>,
110+
pub proof_tree: Option<&'tcx inspect::CanonicalGoalEvaluationStep<TyCtxt<'tcx>>>,
111111
}
112112

113113
/// The cache entry for a goal `CanonicalInput`.

compiler/rustc_middle/src/ty/context.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,8 @@ impl<'tcx> Interner for TyCtxt<'tcx> {
103103
type PredefinedOpaques = solve::PredefinedOpaques<'tcx>;
104104
type DefiningOpaqueTypes = &'tcx ty::List<LocalDefId>;
105105
type ExternalConstraints = ExternalConstraints<'tcx>;
106-
type GoalEvaluationSteps = &'tcx [solve::inspect::GoalEvaluationStep<TyCtxt<'tcx>>];
106+
type CanonicalGoalEvaluationStepRef =
107+
&'tcx solve::inspect::CanonicalGoalEvaluationStep<TyCtxt<'tcx>>;
107108

108109
type Ty = Ty<'tcx>;
109110
type Tys = &'tcx List<Ty<'tcx>>;

compiler/rustc_session/src/config.rs

-10
Original file line numberDiff line numberDiff line change
@@ -796,16 +796,6 @@ pub struct NextSolverConfig {
796796
/// Whether the new trait solver should be enabled everywhere.
797797
/// This is only `true` if `coherence` is also enabled.
798798
pub globally: bool,
799-
/// Whether to dump proof trees after computing a proof tree.
800-
pub dump_tree: DumpSolverProofTree,
801-
}
802-
803-
#[derive(Default, Debug, Copy, Clone, Hash, PartialEq, Eq)]
804-
pub enum DumpSolverProofTree {
805-
Always,
806-
OnError,
807-
#[default]
808-
Never,
809799
}
810800

811801
#[derive(Clone)]

compiler/rustc_session/src/options.rs

+4-22
Original file line numberDiff line numberDiff line change
@@ -399,7 +399,8 @@ mod desc {
399399
pub const parse_instrument_xray: &str = "either a boolean (`yes`, `no`, `on`, `off`, etc), or a comma separated list of settings: `always` or `never` (mutually exclusive), `ignore-loops`, `instruction-threshold=N`, `skip-entry`, `skip-exit`";
400400
pub const parse_unpretty: &str = "`string` or `string=string`";
401401
pub const parse_treat_err_as_bug: &str = "either no value or a non-negative number";
402-
pub const parse_next_solver_config: &str = "a comma separated list of solver configurations: `globally` (default), `coherence`, `dump-tree`, `dump-tree-on-error";
402+
pub const parse_next_solver_config: &str =
403+
"a comma separated list of solver configurations: `globally` (default), and `coherence`";
403404
pub const parse_lto: &str =
404405
"either a boolean (`yes`, `no`, `on`, `off`, etc), `thin`, `fat`, or omitted";
405406
pub const parse_linker_plugin_lto: &str =
@@ -1058,39 +1059,20 @@ mod parse {
10581059
if let Some(config) = v {
10591060
let mut coherence = false;
10601061
let mut globally = true;
1061-
let mut dump_tree = None;
10621062
for c in config.split(',') {
10631063
match c {
10641064
"globally" => globally = true,
10651065
"coherence" => {
10661066
globally = false;
10671067
coherence = true;
10681068
}
1069-
"dump-tree" => {
1070-
if dump_tree.replace(DumpSolverProofTree::Always).is_some() {
1071-
return false;
1072-
}
1073-
}
1074-
"dump-tree-on-error" => {
1075-
if dump_tree.replace(DumpSolverProofTree::OnError).is_some() {
1076-
return false;
1077-
}
1078-
}
10791069
_ => return false,
10801070
}
10811071
}
10821072

1083-
*slot = Some(NextSolverConfig {
1084-
coherence: coherence || globally,
1085-
globally,
1086-
dump_tree: dump_tree.unwrap_or_default(),
1087-
});
1073+
*slot = Some(NextSolverConfig { coherence: coherence || globally, globally });
10881074
} else {
1089-
*slot = Some(NextSolverConfig {
1090-
coherence: true,
1091-
globally: true,
1092-
dump_tree: Default::default(),
1093-
});
1075+
*slot = Some(NextSolverConfig { coherence: true, globally: true });
10941076
}
10951077

10961078
true

compiler/rustc_trait_selection/src/solve/eval_ctxt/mod.rs

+5-22
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,3 @@
1-
use std::io::Write;
2-
use std::ops::ControlFlow;
3-
41
use rustc_data_structures::stack::ensure_sufficient_stack;
52
use rustc_hir::def_id::DefId;
63
use rustc_infer::infer::at::ToTrace;
@@ -20,10 +17,10 @@ use rustc_middle::ty::{
2017
self, InferCtxtLike, OpaqueTypeKey, Ty, TyCtxt, TypeFoldable, TypeSuperVisitable,
2118
TypeVisitable, TypeVisitableExt, TypeVisitor,
2219
};
23-
use rustc_session::config::DumpSolverProofTree;
2420
use rustc_span::DUMMY_SP;
2521
use rustc_type_ir::{self as ir, CanonicalVarValues, Interner};
2622
use rustc_type_ir_macros::{Lift_Generic, TypeFoldable_Generic, TypeVisitable_Generic};
23+
use std::ops::ControlFlow;
2724

2825
use crate::traits::coherence;
2926
use crate::traits::vtable::{count_own_vtable_entries, prepare_vtable_segments, VtblSegment};
@@ -135,8 +132,7 @@ impl<I: Interner> NestedGoals<I> {
135132
#[derive(PartialEq, Eq, Debug, Hash, HashStable, Clone, Copy)]
136133
pub enum GenerateProofTree {
137134
Yes,
138-
IfEnabled,
139-
Never,
135+
No,
140136
}
141137

142138
#[extension(pub trait InferCtxtEvalExt<'tcx>)]
@@ -182,7 +178,7 @@ impl<'a, 'tcx> EvalCtxt<'a, InferCtxt<'tcx>> {
182178
infcx,
183179
search_graph: &mut search_graph,
184180
nested_goals: NestedGoals::new(),
185-
inspect: ProofTreeBuilder::new_maybe_root(infcx.tcx, generate_proof_tree),
181+
inspect: ProofTreeBuilder::new_maybe_root(generate_proof_tree),
186182

187183
// Only relevant when canonicalizing the response,
188184
// which we don't do within this evaluation context.
@@ -197,23 +193,14 @@ impl<'a, 'tcx> EvalCtxt<'a, InferCtxt<'tcx>> {
197193
};
198194
let result = f(&mut ecx);
199195

200-
let tree = ecx.inspect.finalize();
201-
if let (Some(tree), DumpSolverProofTree::Always) = (
202-
&tree,
203-
infcx.tcx.sess.opts.unstable_opts.next_solver.map(|c| c.dump_tree).unwrap_or_default(),
204-
) {
205-
let mut lock = std::io::stdout().lock();
206-
let _ = lock.write_fmt(format_args!("{tree:?}\n"));
207-
let _ = lock.flush();
208-
}
209-
196+
let proof_tree = ecx.inspect.finalize();
210197
assert!(
211198
ecx.nested_goals.is_empty(),
212199
"root `EvalCtxt` should not have any goals added to it"
213200
);
214201

215202
assert!(search_graph.is_empty());
216-
(result, tree)
203+
(result, proof_tree)
217204
}
218205

219206
/// Creates a nested evaluation context that shares the same search graph as the
@@ -483,7 +470,6 @@ impl<'a, 'tcx> EvalCtxt<'a, InferCtxt<'tcx>> {
483470
// the certainty of all the goals.
484471
#[instrument(level = "trace", skip(self))]
485472
pub(super) fn try_evaluate_added_goals(&mut self) -> Result<Certainty, NoSolution> {
486-
self.inspect.start_evaluate_added_goals();
487473
let mut response = Ok(Certainty::overflow(false));
488474
for _ in 0..FIXPOINT_STEP_LIMIT {
489475
// FIXME: This match is a bit ugly, it might be nice to change the inspect
@@ -501,8 +487,6 @@ impl<'a, 'tcx> EvalCtxt<'a, InferCtxt<'tcx>> {
501487
}
502488
}
503489

504-
self.inspect.evaluate_added_goals_result(response);
505-
506490
if response.is_err() {
507491
self.tainted = Err(NoSolution);
508492
}
@@ -515,7 +499,6 @@ impl<'a, 'tcx> EvalCtxt<'a, InferCtxt<'tcx>> {
515499
/// Goals for the next step get directly added to the nested goals of the `EvalCtxt`.
516500
fn evaluate_added_goals_step(&mut self) -> Result<Option<Certainty>, NoSolution> {
517501
let tcx = self.tcx();
518-
self.inspect.start_evaluate_added_goals_step();
519502
let mut goals = core::mem::take(&mut self.nested_goals);
520503

521504
// If this loop did not result in any progress, what's our final certainty.

compiler/rustc_trait_selection/src/solve/fulfill.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ impl<'tcx> ObligationStorage<'tcx> {
7979
// change.
8080
self.overflowed.extend(self.pending.extract_if(|o| {
8181
let goal = o.clone().into();
82-
let result = infcx.evaluate_root_goal(goal, GenerateProofTree::Never).0;
82+
let result = infcx.evaluate_root_goal(goal, GenerateProofTree::No).0;
8383
match result {
8484
Ok((has_changed, _)) => has_changed,
8585
_ => false,
@@ -159,7 +159,7 @@ impl<'tcx> TraitEngine<'tcx> for FulfillmentCtxt<'tcx> {
159159
let mut has_changed = false;
160160
for obligation in self.obligations.unstalled_for_select() {
161161
let goal = obligation.clone().into();
162-
let result = infcx.evaluate_root_goal(goal, GenerateProofTree::IfEnabled).0;
162+
let result = infcx.evaluate_root_goal(goal, GenerateProofTree::No).0;
163163
self.inspect_evaluated_obligation(infcx, &obligation, &result);
164164
let (changed, certainty) = match result {
165165
Ok(result) => result,
@@ -246,7 +246,7 @@ fn fulfillment_error_for_stalled<'tcx>(
246246
root_obligation: PredicateObligation<'tcx>,
247247
) -> FulfillmentError<'tcx> {
248248
let (code, refine_obligation) = infcx.probe(|_| {
249-
match infcx.evaluate_root_goal(root_obligation.clone().into(), GenerateProofTree::Never).0 {
249+
match infcx.evaluate_root_goal(root_obligation.clone().into(), GenerateProofTree::No).0 {
250250
Ok((_, Certainty::Maybe(MaybeCause::Ambiguity))) => {
251251
(FulfillmentErrorCode::Ambiguity { overflow: None }, true)
252252
}

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

+2-11
Original file line numberDiff line numberDiff line change
@@ -317,7 +317,6 @@ impl<'a, 'tcx> InspectGoal<'a, 'tcx> {
317317
inspect::ProbeStep::RecordImplArgs { impl_args: i } => {
318318
assert_eq!(impl_args.replace(i), None);
319319
}
320-
inspect::ProbeStep::EvaluateGoals(_) => (),
321320
}
322321
}
323322

@@ -359,13 +358,7 @@ impl<'a, 'tcx> InspectGoal<'a, 'tcx> {
359358
warn!("unexpected root evaluation: {:?}", self.evaluation_kind);
360359
return vec![];
361360
}
362-
inspect::CanonicalGoalEvaluationKind::Evaluation { revisions } => {
363-
if let Some(last) = revisions.last() {
364-
last
365-
} else {
366-
return vec![];
367-
}
368-
}
361+
inspect::CanonicalGoalEvaluationKind::Evaluation { final_revision } => final_revision,
369362
};
370363

371364
let mut nested_goals = vec![];
@@ -392,9 +385,7 @@ impl<'a, 'tcx> InspectGoal<'a, 'tcx> {
392385
normalizes_to_term_hack: Option<NormalizesToTermHack<'tcx>>,
393386
source: GoalSource,
394387
) -> Self {
395-
let inspect::GoalEvaluation { uncanonicalized_goal, kind, evaluation } = root;
396-
let inspect::GoalEvaluationKind::Root { orig_values } = kind else { unreachable!() };
397-
388+
let inspect::GoalEvaluation { uncanonicalized_goal, orig_values, evaluation } = root;
398389
let result = evaluation.result.and_then(|ok| {
399390
if let Some(term_hack) = normalizes_to_term_hack {
400391
infcx

0 commit comments

Comments
 (0)