add flag for enabling global cache usage for proof trees and printing proof trees on error #113296

BoxyUwU · 2023-07-03T17:20:46Z

This adds a few new things:

-Zdump-solver-proof-tree=always/never/on-error
- always/never were previosuly specifiable by whether the flag exists or not, th new flag is on_error which reruns obligations of fulfillment and selection errors with proof tree generation enabled and prints them out
-Zdump-solver-proof-tree-uses-cache
- allows forcing global cache to be used or unused for all generated proof trees, global cache is enabled by default for always so that it accurately represents what happend. This flag currently would affect misc uses of GenerateProofTree::Yes which will be added in the future for things like diagnostics logic and rustdoc's auto_trait file. We can fix this when we start using proof tree generation for those use cases if it's desirable.

I also changed the output to go straight to stdout instead of going through debug! so that -Zdump-solver-proof-tree can be adequately used on nightly not just a locally built toolchain.

The idea for on-error is that it should hopefully make it easier to quickly figure out "why doesnt this code compile"- you just pass in -Zdump-solver-proof-tree=on-error and you'll only get proof trees you care about.

r? @lcnr @compiler-errors

rustbot · 2023-07-03T17:20:53Z

Some changes occurred to the core trait solver

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

compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs

compiler/rustc_trait_selection/src/solve/eval_ctxt.rs

compiler/rustc_session/src/options.rs

compiler/rustc_trait_selection/src/solve/eval_ctxt.rs

compiler-errors · 2023-07-03T20:02:34Z

compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs

@@ -3499,3 +3513,16 @@ pub enum DefIdOrName {
    DefId(DefId),
    Name(&'static str),
 }
+
+pub fn dump_proof_tree<'tcx>(o: &Obligation<'tcx, ty::Predicate<'tcx>>, infcx: &InferCtxt<'tcx>) {


Noting that since this happens during error reporting, there may have been inference done in between when the goal was registered in the fulfillment context and now, but it's probably fine, since the proof tree will probably still explain why something failed pretty clearly.

oh that's actually kind of unfortunate

compiler/rustc_session/src/options.rs

compiler/rustc_trait_selection/src/solve/eval_ctxt.rs

compiler/rustc_session/src/options.rs

compiler/rustc_trait_selection/src/solve/eval_ctxt.rs

compiler/rustc_trait_selection/src/solve/inspect.rs

lcnr

r=me after review

compiler/rustc_session/src/options.rs

compiler/rustc_session/src/config.rs

lcnr · 2023-07-04T09:29:18Z

compiler/rustc_session/src/options.rs

+        "dump a proof tree for every goal evaluated by the new trait solver. If the flag is specified without any options after it
+        then it defaults to `always`. If the flag is not specified at all it defaults to `on-request`."),
+    dump_solver_proof_tree_use_cache: Option<bool> = (None, parse_opt_bool, [UNTRACKED],
+        "determines whether proof tree generation uses the global cache"),


Suggested change

"determines whether proof tree generation uses the global cache"),

"determines whether dumping proof trees should disable the global cache"),

this flag should not affect general proof tree generation, only the proof trees which should get dumped

that's.... not how the code works. the flag does cause us to use or not use the global cache during proof tree creation. It's not a step done when we dump the proof tree to edit all the "GLOBAL CACHE HIT" to actual proof trees. Implementing it that way sounds trickier than what was done in this PR

from the description:

This flag currently would affect misc uses of GenerateProofTree::Yes which will be added in the future for things like diagnostics logic and rustdoc's auto_trait file. We can fix this when we start using proof tree generation for those use cases if it's desirable.

When -Zdump-solver-proof-tree-use-cache=no/yes is set it acts on every call to evaluate_goal with GenerateProofTree::Yes and also to all proof trees generated by -Zdump-solver-proof-tree=always

my thought was that setting this to true does/will not prevent diagnostics and auto trait computation from disabling the cache, so this only affects trait solving when we construct a tree for "proof tree dumping"

ideally that's how it would work but right now it wouldn't if you implemented auto_trait/diagnostics by callign evalaute_goal with GenerateProofTree::Yes(UseGlobalCache::No).

yes, but that would completely ignore this compile flag

compiler/rustc_trait_selection/src/solve/inspect.rs

lcnr · 2023-07-04T14:10:51Z

@bors r+ rollup

bors · 2023-07-04T14:10:54Z

📌 Commit 284b614 has been approved by lcnr

It is now in the queue for this repository.

…iaskrgr Rollup of 5 pull requests Successful merges: - rust-lang#113192 (`assemble_candidates_after_normalizing_self_ty` docs) - rust-lang#113251 (Use scoped-tls for SMIR to map between TyCtxt and SMIR datastructures) - rust-lang#113282 (Update platform-support.md to improve ARM target descriptions) - rust-lang#113296 (add flag for enabling global cache usage for proof trees and printing proof trees on error) - rust-lang#113324 (implement `ConstEvaluatable` goals in new solver) r? `@ghost` `@rustbot` modify labels: rollup

rustbot assigned lcnr Jul 3, 2023

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 Jul 3, 2023

BoxyUwU changed the title ~~add flag for disabling global cache for proof trees and printing proof trees on error~~ add flag for enabling global cache usage for proof trees and printing proof trees on error Jul 3, 2023

compiler-errors reviewed Jul 3, 2023

View reviewed changes

compiler/rustc_trait_selection/src/solve/eval_ctxt.rs Outdated Show resolved Hide resolved

BoxyUwU force-pushed the proof_trees_on_error branch 2 times, most recently from 90d171d to 96049a8 Compare July 3, 2023 19:56

add flag for disabling global cache and printing proof trees on error

040aa58

BoxyUwU force-pushed the proof_trees_on_error branch from 96049a8 to 040aa58 Compare July 3, 2023 20:00

compiler-errors reviewed Jul 3, 2023

View reviewed changes

reviews

2ad00f4

lcnr reviewed Jul 4, 2023

View reviewed changes

compiler/rustc_session/src/options.rs Outdated Show resolved Hide resolved

change flag name

adefeb8

lcnr reviewed Jul 4, 2023

View reviewed changes

compiler/rustc_trait_selection/src/solve/eval_ctxt.rs Outdated Show resolved Hide resolved

move logic for flags into separate function

276d628

lcnr reviewed Jul 4, 2023

View reviewed changes

compiler/rustc_trait_selection/src/solve/inspect.rs Outdated Show resolved Hide resolved

lcnr reviewed Jul 4, 2023

View reviewed changes

reviews

284b614

BoxyUwU force-pushed the proof_trees_on_error branch from 5bed20b to 284b614 Compare July 4, 2023 13:56

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 4, 2023

matthiaskrgr mentioned this pull request Jul 4, 2023

Rollup of 5 pull requests #113330

Merged

bors merged commit 494e67c into rust-lang:master Jul 5, 2023

rustbot added this to the 1.72.0 milestone Jul 5, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add flag for enabling global cache usage for proof trees and printing proof trees on error #113296

add flag for enabling global cache usage for proof trees and printing proof trees on error #113296

BoxyUwU commented Jul 3, 2023 •

edited

Loading

rustbot commented Jul 3, 2023

compiler-errors Jul 3, 2023

BoxyUwU Jul 4, 2023

lcnr left a comment

lcnr Jul 4, 2023

BoxyUwU Jul 4, 2023

BoxyUwU Jul 4, 2023

lcnr Jul 4, 2023

BoxyUwU Jul 4, 2023

lcnr Jul 4, 2023

lcnr commented Jul 4, 2023

bors commented Jul 4, 2023

	"determines whether proof tree generation uses the global cache"),
	"determines whether dumping proof trees should disable the global cache"),

add flag for enabling global cache usage for proof trees and printing proof trees on error #113296

add flag for enabling global cache usage for proof trees and printing proof trees on error #113296

Conversation

BoxyUwU commented Jul 3, 2023 • edited Loading

rustbot commented Jul 3, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lcnr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lcnr commented Jul 4, 2023

bors commented Jul 4, 2023

BoxyUwU commented Jul 3, 2023 •

edited

Loading