-
Notifications
You must be signed in to change notification settings - Fork 12.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor search graph even more! #128115
refactor search graph even more! #128115
Conversation
doing so requires overwriting global cache entries and generally adds significant complexity to the solver. This is also only ever done for root goals, so it feels easier to wrap the `evaluate_canonical_goal` in an ordinary query if necessary.
this allows us to only sometimes disable the global cache.
this makes it easier to maintain and modify going forward. There may be a small performance cost as we now need to access the provisional cache *and* walk through the stack to detect cycles. However, the provisional cache should be mostly empty and the stack should only have a few elements so the performance impact is likely minimal. Given the complexity of the search graph maintainability trumps linear performance improvements.
Some changes occurred to the core trait solver cc @rust-lang/initiative-trait-system-refactor |
☔ The latest upstream changes (presumably #127042) made this pull request unmergeable. Please resolve the merge conflicts. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
r=me. sorry, I thought I approved this.
@@ -61,10 +61,6 @@ macro_rules! arena_types { | |||
[] dtorck_constraint: rustc_middle::traits::query::DropckConstraint<'tcx>, | |||
[] candidate_step: rustc_middle::traits::query::CandidateStep<'tcx>, | |||
[] autoderef_bad_ty: rustc_middle::traits::query::MethodAutoderefBadTy<'tcx>, | |||
[] canonical_goal_evaluation: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
such cleanup
(un)surprised to see how much additional complexity caching proof trees was adding lol
Closing in favor of #128828 |
commits should be reviewed individually. this PR does yet again not contain any behavior changes. continues the work from #127627. the fuzzer implementation can be found in https://github.com/lcnr/search_graph_fuzz, even if that repository is not documented yet.
r? @compiler-errors