`evaluate_goal` avoid unnecessary step #142774

lcnr · 2025-06-20T07:54:18Z

This does not mess with the debug logging for the trait solver and is a very nice cleanup for #142735. E.g. for

#[derive(Clone)]
struct Wrapper<T>(T);
#[derive(Clone)]
struct Nested; // using a separate type to avoid the fast paths
fn is_clone<T: Clone>() {}
fn main() {
    is_clone::<Wrapper<Nested>>();
}

We get the following proof tree with RUSTC_LOG=rustc_type_ir::search_graph=debug,rustc_next_trait_solver=debug

 rustc_next_trait_solver::solve::eval_ctxt::evaluate_root_goal goal=Goal { param_env: ParamEnv { caller_bounds: [] }, predicate: Binder { value: TraitPredicate(<Wrapper<Nested> as std::clone::Clone>, polarity:Positive), bound_vars: [] } }, generate_proof_tree=No, span=src/main.rs:7:5: 7:34 (#0), stalled_on=None
   rustc_type_ir::search_graph::evaluate_goal input=CanonicalQueryInput { canonical: Canonical { value: QueryInput { goal: Goal { param_env: ParamEnv { caller_bounds: [] }, predicate: Binder { value: TraitPredicate(<Wrapper<Nested> as std::clone::Clone>, polarity:Positive), bound_vars: [] } }, predefined_opaques_in_body: PredefinedOpaques(PredefinedOpaquesData { opaque_types: [] }) }, max_universe: U0, variables: [] }, typing_mode: Analysis { defining_opaque_types_and_generators: [] } }, step_kind_from_parent=Unknown
     rustc_next_trait_solver::solve::eval_ctxt::probe::enter source=Impl(DefId(0:10 ~ main[21d2]::{impl#0}))
       rustc_next_trait_solver::solve::eval_ctxt::add_goal source=ImplWhereBound, goal=Goal { param_env: ParamEnv { caller_bounds: [] }, predicate: Binder { value: TraitPredicate(<_ as std::marker::Sized>, polarity:Positive), bound_vars: [] } }
       rustc_next_trait_solver::solve::eval_ctxt::add_goal source=ImplWhereBound, goal=Goal { param_env: ParamEnv { caller_bounds: [] }, predicate: Binder { value: TraitPredicate(<_ as std::clone::Clone>, polarity:Positive), bound_vars: [] } }
       rustc_type_ir::search_graph::evaluate_goal input=CanonicalQueryInput { canonical: Canonical { value: QueryInput { goal: Goal { param_env: ParamEnv { caller_bounds: [] }, predicate: Binder { value: TraitPredicate(<Nested as std::clone::Clone>, polarity:Positive), bound_vars: [] } }, predefined_opaques_in_body: PredefinedOpaques(PredefinedOpaquesData { opaque_types: [] }) }, max_universe: U0, variables: [] }, typing_mode: Analysis { defining_opaque_types_and_generators: [] } }, step_kind_from_parent=Unknown
         0ms DEBUG rustc_type_ir::search_graph global cache hit, required_depth=0
         0ms DEBUG rustc_type_ir::search_graph return=Ok(Canonical { value: Response { certainty: Yes, var_values: CanonicalVarValues { var_values: [] }, external_constraints: ExternalConstraints(ExternalConstraintsData { region_constraints: [], opaque_types: [], normalization_nested_goals: NestedNormalizationGoals([]) }) }, max_universe: U0, variables: [] })
     rustc_next_trait_solver::solve::eval_ctxt::probe::enter source=BuiltinImpl(Misc)
     rustc_next_trait_solver::solve::trait_goals::merge_trait_candidates candidates=[Candidate { source: Impl(DefId(0:10 ~ main[21d2]::{impl#0})), result: Canonical { value: Response { certainty: Yes, var_values: CanonicalVarValues { var_values: [] }, external_constraints: ExternalConstraints(ExternalConstraintsData { region_constraints: [], opaque_types: [], normalization_nested_goals: NestedNormalizationGoals([]) }) }, max_universe: U0, variables: [] } }]
       0ms DEBUG rustc_next_trait_solver::solve::trait_goals return=Ok((Canonical { value: Response { certainty: Yes, var_values: CanonicalVarValues { var_values: [] }, external_constraints: ExternalConstraints(ExternalConstraintsData { region_constraints: [], opaque_types: [], normalization_nested_goals: NestedNormalizationGoals([]) }) }, max_universe: U0, variables: [] }, Some(Misc)))
     0ms DEBUG rustc_type_ir::search_graph insert global cache, evaluation_result=EvaluationResult { encountered_overflow: false, required_depth: 1, heads: CycleHeads { heads: {} }, nested_goals: NestedGoals { nested_goals: {} }, result: Ok(Canonical { value: Response { certainty: Yes, var_values: CanonicalVarValues { var_values: [] }, external_constraints: ExternalConstraints(ExternalConstraintsData { region_constraints: [], opaque_types: [], normalization_nested_goals: NestedNormalizationGoals([]) }) }, max_universe: U0, variables: [] }) }
     0ms DEBUG rustc_type_ir::search_graph return=Ok(Canonical { value: Response { certainty: Yes, var_values: CanonicalVarValues { var_values: [] }, external_constraints: ExternalConstraints(ExternalConstraintsData { region_constraints: [], opaque_types: [], normalization_nested_goals: NestedNormalizationGoals([]) }) }, max_universe: U0, variables: [] })

rustbot · 2025-06-20T07:54:22Z

r? @oli-obk

rustbot has assigned @oli-obk.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

rustbot · 2025-06-20T08:11:00Z

Some changes occurred to the core trait solver

cc @rust-lang/initiative-trait-system-refactor

oli-obk · 2025-06-20T08:20:52Z

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

        );
+        canonical_goal_evaluation.query_result(canonical_response);
+        goal_evaluation.canonical_goal_evaluation(canonical_goal_evaluation);


hmm, now that I understand the various new_foo/foo APIs on ProofTreeBuilder I have questions 😆

could these be one function taking a closure and thus being more of a transaction? The current use cases seem to say so, but I'm guessing there will be more uses and you want to keep a more general API?

some doc comments would have helped (at least linking functions together that belong together)

some functions take &mut self but do not need it

canonical_goal_evalution mutating self seems surprising. I would have expected something with apply or finish in the name

could these be one function taking a closure and thus being more of a transaction? The current use cases seem to say so, but I'm guessing there will be more uses and you want to keep a more general API?

this concrete case yes, ignoring the issue of this potentially being a perf hit 🤔 I think it should be fine... I personally feel slightly iffy about having proof tree stuff be "in control of the control flow", but expect that using closures hare is easier to read

some functions take &mut self but do not need it

unsurprising :3

canonical_goal_evalution mutating self seems surprising. I would have expected something with apply or finish in the name

I agree, ideally we just have WipGoalEvaluation be the Root and not have a separate root state.

Mind pushing these cleanups until after #142735/to not do them in this PR? I am changing the control flow inside of the search graph and don't fully know how this will impact stuff.

Absolutely. I just wanted to understand things xD

oli-obk · 2025-06-20T08:22:20Z

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

+        let response = match canonical_result {
+            Err(e) => return Err(e),
            Ok(response) => response,
        };


Suggested change

let response = match canonical_result {

Err(e) => return Err(e),

Ok(response) => response,

};

let response = canonical_result?

considered it, i actually kinda feel like explicitly matching is better than using ? on a local 🤔 but fine to apply this if u think it's significantly better :3

I do think it is, but I understand verbosity as a signal that something unusual is going on can be useful. I just see this getting changed as a drive by in the future without a comment, and if you add a comment to the one liner you have as much signal as the expanded version

oli-obk

r=me once base PR lands

lcnr · 2025-06-23T09:50:54Z

@bors2 try @rust-timer queue

rust-timer · 2025-06-23T09:50:57Z

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

rust-bors · 2025-06-23T09:50:58Z

⌛ Trying commit e8e32c3 with merge 9101c8e…

To cancel the try build, run the command @bors2 try cancel.

`evaluate_goal` avoid unnecessary step based on #142617. This does not mess with the debug logging for the trait solver and is a very nice cleanup for #142735. E.g. for ```rust #[derive(Clone)] struct Wrapper<T>(T); #[derive(Clone)] struct Nested; // using a separate type to avoid the fast paths fn is_clone<T: Clone>() {} fn main() { is_clone::<Wrapper<Nested>>(); } ``` We get the following proof tree with `RUSTC_LOG=rustc_type_ir::search_graph=debug,rustc_next_trait_solver=debug` ``` rustc_next_trait_solver::solve::eval_ctxt::evaluate_root_goal goal=Goal { param_env: ParamEnv { caller_bounds: [] }, predicate: Binder { value: TraitPredicate(<Wrapper<Nested> as std::clone::Clone>, polarity:Positive), bound_vars: [] } }, generate_proof_tree=No, span=src/main.rs:7:5: 7:34 (#0), stalled_on=None rustc_type_ir::search_graph::evaluate_goal input=CanonicalQueryInput { canonical: Canonical { value: QueryInput { goal: Goal { param_env: ParamEnv { caller_bounds: [] }, predicate: Binder { value: TraitPredicate(<Wrapper<Nested> as std::clone::Clone>, polarity:Positive), bound_vars: [] } }, predefined_opaques_in_body: PredefinedOpaques(PredefinedOpaquesData { opaque_types: [] }) }, max_universe: U0, variables: [] }, typing_mode: Analysis { defining_opaque_types_and_generators: [] } }, step_kind_from_parent=Unknown rustc_next_trait_solver::solve::eval_ctxt::probe::enter source=Impl(DefId(0:10 ~ main[21d2]::{impl#0})) rustc_next_trait_solver::solve::eval_ctxt::add_goal source=ImplWhereBound, goal=Goal { param_env: ParamEnv { caller_bounds: [] }, predicate: Binder { value: TraitPredicate(<_ as std::marker::Sized>, polarity:Positive), bound_vars: [] } } rustc_next_trait_solver::solve::eval_ctxt::add_goal source=ImplWhereBound, goal=Goal { param_env: ParamEnv { caller_bounds: [] }, predicate: Binder { value: TraitPredicate(<_ as std::clone::Clone>, polarity:Positive), bound_vars: [] } } rustc_type_ir::search_graph::evaluate_goal input=CanonicalQueryInput { canonical: Canonical { value: QueryInput { goal: Goal { param_env: ParamEnv { caller_bounds: [] }, predicate: Binder { value: TraitPredicate(<Nested as std::clone::Clone>, polarity:Positive), bound_vars: [] } }, predefined_opaques_in_body: PredefinedOpaques(PredefinedOpaquesData { opaque_types: [] }) }, max_universe: U0, variables: [] }, typing_mode: Analysis { defining_opaque_types_and_generators: [] } }, step_kind_from_parent=Unknown 0ms DEBUG rustc_type_ir::search_graph global cache hit, required_depth=0 0ms DEBUG rustc_type_ir::search_graph return=Ok(Canonical { value: Response { certainty: Yes, var_values: CanonicalVarValues { var_values: [] }, external_constraints: ExternalConstraints(ExternalConstraintsData { region_constraints: [], opaque_types: [], normalization_nested_goals: NestedNormalizationGoals([]) }) }, max_universe: U0, variables: [] }) rustc_next_trait_solver::solve::eval_ctxt::probe::enter source=BuiltinImpl(Misc) rustc_next_trait_solver::solve::trait_goals::merge_trait_candidates candidates=[Candidate { source: Impl(DefId(0:10 ~ main[21d2]::{impl#0})), result: Canonical { value: Response { certainty: Yes, var_values: CanonicalVarValues { var_values: [] }, external_constraints: ExternalConstraints(ExternalConstraintsData { region_constraints: [], opaque_types: [], normalization_nested_goals: NestedNormalizationGoals([]) }) }, max_universe: U0, variables: [] } }] 0ms DEBUG rustc_next_trait_solver::solve::trait_goals return=Ok((Canonical { value: Response { certainty: Yes, var_values: CanonicalVarValues { var_values: [] }, external_constraints: ExternalConstraints(ExternalConstraintsData { region_constraints: [], opaque_types: [], normalization_nested_goals: NestedNormalizationGoals([]) }) }, max_universe: U0, variables: [] }, Some(Misc))) 0ms DEBUG rustc_type_ir::search_graph insert global cache, evaluation_result=EvaluationResult { encountered_overflow: false, required_depth: 1, heads: CycleHeads { heads: {} }, nested_goals: NestedGoals { nested_goals: {} }, result: Ok(Canonical { value: Response { certainty: Yes, var_values: CanonicalVarValues { var_values: [] }, external_constraints: ExternalConstraints(ExternalConstraintsData { region_constraints: [], opaque_types: [], normalization_nested_goals: NestedNormalizationGoals([]) }) }, max_universe: U0, variables: [] }) } 0ms DEBUG rustc_type_ir::search_graph return=Ok(Canonical { value: Response { certainty: Yes, var_values: CanonicalVarValues { var_values: [] }, external_constraints: ExternalConstraints(ExternalConstraintsData { region_constraints: [], opaque_types: [], normalization_nested_goals: NestedNormalizationGoals([]) }) }, max_universe: U0, variables: [] }) ```

rust-bors · 2025-06-23T12:19:35Z

☀️ Try build successful (CI)
Build commit: 9101c8e (9101c8e8c9b6dedb01a16139d396b84ae6b90a97, parent: 22be76b7e259f27bf3e55eb931f354cd8b69d55f)

rust-timer · 2025-06-23T12:19:37Z

Queued 9101c8e with parent 22be76b, future comparison URL.
There are currently 3 preceding artifacts in the queue.
It will probably take at least ~5.4 hours until the benchmark run finishes.

rustbot assigned oli-obk Jun 20, 2025

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver) labels Jun 20, 2025

This comment has been minimized.

Sign in to view

lcnr force-pushed the search_graph-2 branch from 9904d31 to 3f8cf47 Compare June 20, 2025 08:10

lcnr force-pushed the search_graph-2 branch from 3f8cf47 to 571a1f7 Compare June 20, 2025 08:13

oli-obk reviewed Jun 20, 2025

View reviewed changes

oli-obk approved these changes Jun 20, 2025

View reviewed changes

This comment has been minimized.

Sign in to view

lcnr added 2 commits June 23, 2025 11:50

evaluate_goal: avoid unnecessary step

5132e6c

inspect: merge [Canonical]GoalEvaluation

e8e32c3

lcnr force-pushed the search_graph-2 branch from 571a1f7 to e8e32c3 Compare June 23, 2025 09:50

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jun 23, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

`evaluate_goal` avoid unnecessary step #142774

`evaluate_goal` avoid unnecessary step #142774

lcnr commented Jun 20, 2025 •

edited

Loading

Uh oh!

rustbot commented Jun 20, 2025

Uh oh!

This comment has been minimized.

rustbot commented Jun 20, 2025

Uh oh!

oli-obk Jun 20, 2025

Uh oh!

lcnr Jun 20, 2025

Uh oh!

oli-obk Jun 20, 2025

Uh oh!

oli-obk Jun 20, 2025

Uh oh!

lcnr Jun 20, 2025 •

edited

Loading

Uh oh!

oli-obk Jun 20, 2025

Uh oh!

oli-obk left a comment

Uh oh!

This comment has been minimized.

lcnr commented Jun 23, 2025

Uh oh!

rust-timer commented Jun 23, 2025

Uh oh!

rust-bors bot commented Jun 23, 2025

Uh oh!

rust-bors bot commented Jun 23, 2025

Uh oh!

rust-timer commented Jun 23, 2025

Uh oh!

Uh oh!

evaluate_goal avoid unnecessary step #142774

Are you sure you want to change the base?

evaluate_goal avoid unnecessary step #142774

Conversation

lcnr commented Jun 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Jun 20, 2025

Uh oh!

This comment has been minimized.

rustbot commented Jun 20, 2025

Uh oh!

oli-obk Jun 20, 2025

Choose a reason for hiding this comment

Uh oh!

lcnr Jun 20, 2025

Choose a reason for hiding this comment

Uh oh!

oli-obk Jun 20, 2025

Choose a reason for hiding this comment

Uh oh!

oli-obk Jun 20, 2025

Choose a reason for hiding this comment

Uh oh!

lcnr Jun 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

oli-obk Jun 20, 2025

Choose a reason for hiding this comment

Uh oh!

oli-obk left a comment

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

lcnr commented Jun 23, 2025

Uh oh!

rust-timer commented Jun 23, 2025

Uh oh!

rust-bors bot commented Jun 23, 2025

Uh oh!

rust-bors bot commented Jun 23, 2025

Uh oh!

rust-timer commented Jun 23, 2025

Uh oh!

Uh oh!

`evaluate_goal` avoid unnecessary step #142774

`evaluate_goal` avoid unnecessary step #142774

lcnr commented Jun 20, 2025 •

edited

Loading

lcnr Jun 20, 2025 •

edited

Loading