Skip to content

match lowering: consistently merge simple or-patterns #123067

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 2 commits into from
Mar 26, 2024

Conversation

Nadrieril
Copy link
Member

There are two places where we expand or-patterns in match lowering: the main one is test_candidates_with_or, and there's one in match_candidates that's an optimization for the simple case where the whole pattern is just one or-pattern.

To reduce duplication, we merge or-pattern alternatives into a single block when possible, but we only to that in test_candidates_with_or. This PR fixes this oversight and merges them in match_candidates too.

This is a part of splitting up #122046 into smaller bits.

We have to make sure we set it everywhere that we set `subcandidates`.
@rustbot
Copy link
Collaborator

rustbot commented Mar 25, 2024

r? @pnkfelix

rustbot has assigned @pnkfelix.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@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 Mar 25, 2024
@Nadrieril
Copy link
Member Author

r? @oli-obk

@rustbot rustbot assigned oli-obk and unassigned pnkfelix Mar 25, 2024
@oli-obk
Copy link
Contributor

oli-obk commented Mar 26, 2024

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Mar 26, 2024

📌 Commit d1d9aa3 has been approved by oli-obk

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 Mar 26, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 26, 2024
…iaskrgr

Rollup of 10 pull requests

Successful merges:

 - rust-lang#122766 (store segment and module in `UnresolvedImportError`)
 - rust-lang#122996 (simplify_branches: add comment)
 - rust-lang#123047 (triagebot: Add notification of 2024 issues)
 - rust-lang#123066 (CFI: (actually) check that methods are object-safe before projecting their receivers to `dyn Trait` in CFI)
 - rust-lang#123067 (match lowering: consistently merge simple or-patterns)
 - rust-lang#123069 (Revert `cargo update` changes and bump `download-artifact` to v4)
 - rust-lang#123070 (Add my former address to .mailmap)
 - rust-lang#123086 (Fix doc link to BufWriter in std::fs::File documentation)
 - rust-lang#123090 (Remove `CacheSelector` trait now that we can use GATs)
 - rust-lang#123091 (Delegation: fix ICE on wrong `self` resolution)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 94e8c6c into rust-lang:master Mar 26, 2024
@rustbot rustbot added this to the 1.79.0 milestone Mar 26, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Mar 26, 2024
Rollup merge of rust-lang#123067 - Nadrieril:always-simplify-or, r=oli-obk

match lowering: consistently merge simple or-patterns

There are two places where we expand or-patterns in match lowering: the main one is `test_candidates_with_or`, and there's one in `match_candidates` that's an optimization for the simple case where the whole pattern is just one or-pattern.

To reduce duplication, we merge or-pattern alternatives into a single block when possible, but we only to that in `test_candidates_with_or`. This PR fixes this oversight and merges them in `match_candidates` too.

This is a part of splitting up rust-lang#122046 into smaller bits.
@Nadrieril Nadrieril deleted the always-simplify-or branch March 26, 2024 22:05
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 1, 2024
…hewjasper

match lowering: handle or-patterns one layer at a time

`create_or_subcandidates` and `merge_trivial_subcandidates` both call themselves recursively to handle nested or-patterns, which is hard to follow. In this PR I avoid the need for that; we now process a single "layer" of or-patterns at a time.

By calling back into `match_candidates`, we only need to expand one layer at a time. Conversely, since we always try to simplify a layer that we just expanded (thanks to rust-lang#123067), we only have to merge one layer at a time.

r? `@matthewjasper`
github-actions bot pushed a commit to rust-lang/miri that referenced this pull request Apr 3, 2024
match lowering: handle or-patterns one layer at a time

`create_or_subcandidates` and `merge_trivial_subcandidates` both call themselves recursively to handle nested or-patterns, which is hard to follow. In this PR I avoid the need for that; we now process a single "layer" of or-patterns at a time.

By calling back into `match_candidates`, we only need to expand one layer at a time. Conversely, since we always try to simplify a layer that we just expanded (thanks to rust-lang/rust#123067), we only have to merge one layer at a time.

r? `@matthewjasper`
lnicola pushed a commit to lnicola/rust-analyzer that referenced this pull request Apr 7, 2024
match lowering: handle or-patterns one layer at a time

`create_or_subcandidates` and `merge_trivial_subcandidates` both call themselves recursively to handle nested or-patterns, which is hard to follow. In this PR I avoid the need for that; we now process a single "layer" of or-patterns at a time.

By calling back into `match_candidates`, we only need to expand one layer at a time. Conversely, since we always try to simplify a layer that we just expanded (thanks to rust-lang/rust#123067), we only have to merge one layer at a time.

r? `@matthewjasper`
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this pull request Apr 27, 2024
match lowering: handle or-patterns one layer at a time

`create_or_subcandidates` and `merge_trivial_subcandidates` both call themselves recursively to handle nested or-patterns, which is hard to follow. In this PR I avoid the need for that; we now process a single "layer" of or-patterns at a time.

By calling back into `match_candidates`, we only need to expand one layer at a time. Conversely, since we always try to simplify a layer that we just expanded (thanks to rust-lang/rust#123067), we only have to merge one layer at a time.

r? `@matthewjasper`
Shunpoco added a commit to Shunpoco/rust that referenced this pull request Dec 30, 2024
From rust-lang#123067, this test should ensure that there is no duplicated instruction block against or-patterns.

Signed-off-by: Shunpoco <tkngsnsk313320@gmail.com>
Shunpoco added a commit to Shunpoco/rust that referenced this pull request Jan 1, 2025
From rust-lang#123067, this test should ensure that there is no duplicated basic block against or-patterns.

Signed-off-by: Shunpoco <tkngsnsk313320@gmail.com>
Shunpoco added a commit to Shunpoco/rust that referenced this pull request Jan 4, 2025
From rust-lang#123067, this test should ensure that there is no duplicated basic block against or-patterns.

Signed-off-by: Shunpoco <tkngsnsk313320@gmail.com>
jieyouxu added a commit to jieyouxu/rust that referenced this pull request Feb 28, 2025
…DianQK

Add FileCheck annotations to mir-opt/issues

This resolves a part of rust-lang#116971 .

The directory `tests/mir-opt/issues` has only one test issue_75439.rs which should add FileCheck annotations.

Originally it was introduced in rust-lang#75580 to confirm that there were duplicated basic blocks against or-patterns, but in rust-lang#123067 the duplication was resolved. So FileCheck should ensure that there is no such duplication any more.
jieyouxu added a commit to jieyouxu/rust that referenced this pull request Feb 28, 2025
…DianQK

Add FileCheck annotations to mir-opt/issues

This resolves a part of rust-lang#116971 .

The directory `tests/mir-opt/issues` has only one test issue_75439.rs which should add FileCheck annotations.

Originally it was introduced in rust-lang#75580 to confirm that there were duplicated basic blocks against or-patterns, but in rust-lang#123067 the duplication was resolved. So FileCheck should ensure that there is no such duplication any more.
jieyouxu added a commit to jieyouxu/rust that referenced this pull request Feb 28, 2025
…DianQK

Add FileCheck annotations to mir-opt/issues

This resolves a part of rust-lang#116971 .

The directory `tests/mir-opt/issues` has only one test issue_75439.rs which should add FileCheck annotations.

Originally it was introduced in rust-lang#75580 to confirm that there were duplicated basic blocks against or-patterns, but in rust-lang#123067 the duplication was resolved. So FileCheck should ensure that there is no such duplication any more.
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Mar 1, 2025
Rollup merge of rust-lang#134943 - Shunpoco:116971-mir-opt-issues, r=DianQK

Add FileCheck annotations to mir-opt/issues

This resolves a part of rust-lang#116971 .

The directory `tests/mir-opt/issues` has only one test issue_75439.rs which should add FileCheck annotations.

Originally it was introduced in rust-lang#75580 to confirm that there were duplicated basic blocks against or-patterns, but in rust-lang#123067 the duplication was resolved. So FileCheck should ensure that there is no such duplication any more.
# for free to join this conversation on GitHub. Already have an account? # 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