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

match lowering: test one or pattern at a time #121175

Merged
merged 2 commits into from
Feb 21, 2024

Conversation

Nadrieril
Copy link
Member

This is a bit more opinionated than the previous PRs. On the face of it this is less efficient and more complex than before, but I personally found the loop that digs into leaf_candidates on each iteration very confusing. Instead this does "generate code for this or-pattern" then "generate further code for each branch if needed" in two steps.

Incidentally this way we don't require or patterns to be sorted at the end. It's still an important optimization but I find it clearer to not rely on it for correctness.

r? @matthewjasper

@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. labels Feb 16, 2024
@Nadrieril
Copy link
Member Author

I wanna iron this out, I did something weird with the otherwise blocks.

@rustbot author

@rustbot rustbot 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 Feb 18, 2024
@Nadrieril
Copy link
Member Author

Fixed it

@rustbot ready

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

@bors r+

@bors
Copy link
Contributor

bors commented Feb 21, 2024

📌 Commit 806ee5f has been approved by matthewjasper

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 Feb 21, 2024
@Nadrieril Nadrieril force-pushed the simplify-or-selection branch 2 times, most recently from 95b968e to e712fb6 Compare February 21, 2024 10:28
@Nadrieril
Copy link
Member Author

Oops, raced you, I was rebasing on top of master. Will restart when CI is green, commits are identical

@bors r-

@bors bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Feb 21, 2024
@rust-log-analyzer

This comment was marked as resolved.

@Nadrieril
Copy link
Member Author

(identical except for a Clone derive that I had removed in the other PR for some reason)

@bors r=matthewjasper

@bors
Copy link
Contributor

bors commented Feb 21, 2024

📌 Commit c1514a6 has been approved by matthewjasper

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 21, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 21, 2024
Rollup of 8 pull requests

Successful merges:

 - rust-lang#121044 (Support async trait bounds in macros)
 - rust-lang#121175 (match lowering: test one or pattern at a time)
 - rust-lang#121340 (bootstrap: apply most of clippy's suggestions)
 - rust-lang#121347 (compiletest: support auxiliaries with auxiliaries)
 - rust-lang#121359 (miscellaneous type system improvements)
 - rust-lang#121366 (Remove `diagnostic_builder.rs`)
 - rust-lang#121379 (Remove an `unchecked_error_guaranteed` call.)
 - rust-lang#121396 (make it possible for outside crates to inspect a mir::ConstValue with the interpreter)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 4daa43a into rust-lang:master Feb 21, 2024
11 checks passed
@rustbot rustbot added this to the 1.78.0 milestone Feb 21, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Feb 21, 2024
Rollup merge of rust-lang#121175 - Nadrieril:simplify-or-selection, r=matthewjasper

match lowering: test one or pattern at a time

This is a bit more opinionated than the previous PRs. On the face of it this is less efficient and more complex than before, but I personally found the loop that digs into `leaf_candidates` on each iteration very confusing. Instead this does "generate code for this or-pattern" then "generate further code for each branch if needed" in two steps.

Incidentally this way we don't _require_ or patterns to be sorted at the end. It's still an important optimization but I find it clearer to not rely on it for correctness.

r? `@matthewjasper`
@Nadrieril Nadrieril deleted the simplify-or-selection branch February 21, 2024 18:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants