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

implement the skeleton of the updated trait solver #105661

Merged
merged 2 commits into from
Dec 23, 2022

Conversation

lcnr
Copy link
Contributor

@lcnr lcnr commented Dec 13, 2022

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

This is mostly following the architecture discussed in the types team meetup.

After discussing the desired changes for the trait solver, we encountered cyclic dependencies between them. Most notably between changing evaluate to be canonical and returning inference constraints. We cannot canonicalize evaluate without returning inference constraints due to coinductive cycles. However, caching inference constraints also relies on canonicalization. Implementing both of these changes at once in-place is not feasible.

This somewhat closely mirrors the current evaluate implementation with the following notable differences:

  • it moves project into the core solver, allowing us to correctly deal with coinductive projections (will be required for implied bounds, perfect derive)
  • it changes trait solver overflow to be non-fatal (required to backcompat breakage from changes to the iteration order of nested goals, deferred projection equality, generally very useful)
  • it returns inference constraints and canonicalizes inputs and outputs (required for a lot things, most notably merging fulfill and evaluate, and deferred projection equality)
  • it is implemented to work with lazy normalization

A lot of things aren't yet implemented, but the remaining FIXMEs should all be fairly self-contained and parallelizable. If the architecture looks correct and is what we want here, I would like to quickly merge this and then split the work.

r? @compiler-errors / @rust-lang/types :3

@rustbot rustbot added A-meta Area: Issues & PRs about the rust-lang/rust repository itself 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. labels Dec 13, 2022
@lcnr lcnr force-pushed the evaluate-new branch 2 times, most recently from afa7c1e to 491f0fb Compare December 13, 2022 15:03
@bors

This comment was marked as outdated.

@lcnr lcnr force-pushed the evaluate-new branch 3 times, most recently from 0d7b5b5 to 84c9bb3 Compare December 14, 2022 16:41
@rust-log-analyzer

This comment was marked as outdated.

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.

Cool!

Other team members or initiative group members can and should leave reviews inline after this lands if they have the opportunty to read through this PR, but landing this now is very low risk because it's not hooked up to anything yet 😸

@compiler-errors
Copy link
Member

@bors r+

not compelled to put rollup=never b/c it's a big PR but doesn't really touch existing files, nor should we need to bisect here really.

@bors
Copy link
Contributor

bors commented Dec 20, 2022

📌 Commit 89fb51a2a29cbb2a3fd5a8e8e4fc327f3ab7fc44 has been approved by compiler-errors

It is now in the queue for this repository.

@bors 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 Dec 20, 2022
@bors
Copy link
Contributor

bors commented Dec 20, 2022

⌛ Testing commit 89fb51a2a29cbb2a3fd5a8e8e4fc327f3ab7fc44 with merge c88a482905f2889192a86e9a8a31368d9c819547...

@bors
Copy link
Contributor

bors commented Dec 20, 2022

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Dec 20, 2022
@rust-log-analyzer

This comment has been minimized.

@lcnr
Copy link
Contributor Author

lcnr commented Dec 20, 2022

@bors r=compiler-errors

@bors
Copy link
Contributor

bors commented Dec 20, 2022

📌 Commit 750bf36 has been approved by compiler-errors

It is now in the queue for this repository.

@bors bors removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 20, 2022
@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Dec 20, 2022
fee1-dead added a commit to fee1-dead-contrib/rust that referenced this pull request Dec 21, 2022
implement the skeleton of the updated trait solver

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

This is mostly following the architecture discussed in the types team meetup.

After discussing the desired changes for the trait solver, we encountered cyclic dependencies between them. Most notably between changing evaluate to be canonical and returning inference constraints. We cannot canonicalize evaluate without returning inference constraints due to coinductive cycles. However, caching inference constraints also relies on canonicalization. Implementing both of these changes at once in-place is not feasible.

This somewhat closely mirrors the current `evaluate` implementation with the following notable differences:
- it moves `project` into the core solver, allowing us to correctly deal with coinductive projections (will be required for implied bounds, perfect derive)
- it changes trait solver overflow to be non-fatal (required to backcompat breakage from changes to the iteration order of nested goals, deferred projection equality, generally very useful)
- it returns inference constraints and canonicalizes inputs and outputs (required for a lot things, most notably merging fulfill and evaluate, and deferred projection equality)
- it is implemented to work with lazy normalization

A lot of things aren't yet implemented, but the remaining FIXMEs should all be fairly self-contained and parallelizable. If the architecture looks correct and is what we want here, I would like to quickly merge this and then split the work.

r? `@compiler-errors` / `@rust-lang/types` :3
fee1-dead added a commit to fee1-dead-contrib/rust that referenced this pull request Dec 21, 2022
implement the skeleton of the updated trait solver

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

This is mostly following the architecture discussed in the types team meetup.

After discussing the desired changes for the trait solver, we encountered cyclic dependencies between them. Most notably between changing evaluate to be canonical and returning inference constraints. We cannot canonicalize evaluate without returning inference constraints due to coinductive cycles. However, caching inference constraints also relies on canonicalization. Implementing both of these changes at once in-place is not feasible.

This somewhat closely mirrors the current `evaluate` implementation with the following notable differences:
- it moves `project` into the core solver, allowing us to correctly deal with coinductive projections (will be required for implied bounds, perfect derive)
- it changes trait solver overflow to be non-fatal (required to backcompat breakage from changes to the iteration order of nested goals, deferred projection equality, generally very useful)
- it returns inference constraints and canonicalizes inputs and outputs (required for a lot things, most notably merging fulfill and evaluate, and deferred projection equality)
- it is implemented to work with lazy normalization

A lot of things aren't yet implemented, but the remaining FIXMEs should all be fairly self-contained and parallelizable. If the architecture looks correct and is what we want here, I would like to quickly merge this and then split the work.

r? ``@compiler-errors`` / ``@rust-lang/types`` :3
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 23, 2022
Rollup of 6 pull requests

Successful merges:

 - rust-lang#105661 (implement the skeleton of the updated trait solver)
 - rust-lang#105853 (Make the pre-push script work on directories with spaces)
 - rust-lang#106043 (Move tests)
 - rust-lang#106048 (Run `tidy` in its own job in PR CI)
 - rust-lang#106055 (Check arg expressions properly on error in `confirm_builtin_call`)
 - rust-lang#106067 (A few metadata nits)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit fd5af8c into rust-lang:master Dec 23, 2022
@rustbot rustbot added this to the 1.68.0 milestone Dec 23, 2022
@lcnr lcnr deleted the evaluate-new branch January 2, 2023 11:50
@BoxyUwU BoxyUwU added the WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver) label Jan 11, 2023
@lcnr lcnr mentioned this pull request Sep 4, 2024
14 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-meta Area: Issues & PRs about the rust-lang/rust repository itself S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. 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)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants