Skip to content
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

Closed
wants to merge 7 commits into from
Closed

Conversation

lcnr
Copy link
Contributor

@lcnr lcnr commented Jul 23, 2024

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

lcnr added 7 commits July 23, 2024 14:21
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.
@rustbot 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 labels Jul 23, 2024
@rustbot
Copy link
Collaborator

rustbot commented Jul 23, 2024

Some changes occurred to the core trait solver

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

@bors
Copy link
Contributor

bors commented Jul 26, 2024

☔ The latest upstream changes (presumably #127042) made this pull request unmergeable. Please resolve the merge conflicts.

Copy link
Member

@compiler-errors compiler-errors left a 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:
Copy link
Member

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

@compiler-errors compiler-errors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 5, 2024
@compiler-errors
Copy link
Member

Closing in favor of #128828

@lcnr lcnr deleted the search-graph-10 branch August 13, 2024 14:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. 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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants