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: simplify block creation #120978

Merged
merged 1 commit into from
Feb 13, 2024
Merged

Conversation

Nadrieril
Copy link
Member

@Nadrieril Nadrieril commented Feb 12, 2024

Match lowering was doing complicated things with block creation. As far as I can tell it was trying to avoid creating unneeded blocks, but of the three places that start out with otherwise = &mut None, two of them called otherwise.unwrap_or_else(|| self.cfg.start_new_block()) anyway. As far as I can tell the only place where this PR makes a difference is in lower_match_tree, which did indeed sometimes avoid creating the unreachable final block + FakeRead. Unless this is important I propose we do the naive thing instead.

I have not checked all the graph isomorphisms by hand, but at a glance the test diff looks sensible.

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 12, 2024
@matthewjasper
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Feb 13, 2024

📌 Commit faaf81b 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 13, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 13, 2024
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#114877 (unstable-book: add quick-edit link)
 - rust-lang#120548 (rustdoc: Fix handling of doc_auto_cfg feature for cfg attributes on glob reexport)
 - rust-lang#120549 (modify alias-relate to also normalize ambiguous opaques)
 - rust-lang#120959 (Remove good path delayed bugs)
 - rust-lang#120978 (match lowering: simplify block creation)
 - rust-lang#121019 (coverage: Simplify some parts of the coverage span refiner)
 - rust-lang#121021 (Extend intra-doc link chapter in the rustdoc book)
 - rust-lang#121031 (RustWrapper: adapt for coverage mapping API changes)

Failed merges:

 - rust-lang#121014 (Remove `force_print_diagnostic`)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit b785fdb into rust-lang:master Feb 13, 2024
11 checks passed
@rustbot rustbot added this to the 1.78.0 milestone Feb 13, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Feb 13, 2024
Rollup merge of rust-lang#120978 - Nadrieril:sane-blocks, r=matthewjasper

match lowering: simplify block creation

Match lowering was doing complicated things with block creation. As far as I can tell it was trying to avoid creating unneeded blocks, but of the three places that start out with `otherwise = &mut None`, two of them called `otherwise.unwrap_or_else(|| self.cfg.start_new_block())` anyway. As far as I can tell the only place where this PR makes a difference is in `lower_match_tree`, which did indeed sometimes avoid creating the unreachable final block + FakeRead. Unless this is important I propose we do the naive thing instead.

I have not checked all the graph isomorphisms by hand, but at a glance the test diff looks sensible.

r? `@matthewjasper`
@Nadrieril Nadrieril deleted the sane-blocks branch February 13, 2024 20:00
Zalathar added a commit to Zalathar/rust that referenced this pull request Apr 17, 2024
This comment was historically inside a block guarded by
`if let Some(otherwise_block) = otherwise`.

When rust-lang#120978 made the otherwise block non-optional, it also flattened that
region of code. Doing so left this comment awkwardly stranded aboven an
unrelated line of code, without its original context.

We can restore that context by moving it above the declaration of `otherwise`.
Zalathar added a commit to Zalathar/rust that referenced this pull request Apr 17, 2024
This comment was historically inside a block guarded by
`if let Some(otherwise_block) = otherwise`.

When rust-lang#120978 made the otherwise block non-optional, it also flattened that
region of code. Doing so left this comment awkwardly stranded above an
unrelated line of code, without its original context.

We can restore that context by moving it above the declaration of `otherwise`.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Apr 18, 2024
Move confusing comment about otherwise blocks in `lower_match_tree`

This comment was historically inside a block guarded by `if let Some(otherwise_block) = otherwise`.

When rust-lang#120978 made the “otherwise block” non-optional, it also flattened that region of code. Doing so left this comment awkwardly stranded above an unrelated line of code, without its original context.

We can restore that context by moving it above the declaration of `otherwise`.

r? `@Nadrieril`
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Apr 18, 2024
Move confusing comment about otherwise blocks in `lower_match_tree`

This comment was historically inside a block guarded by `if let Some(otherwise_block) = otherwise`.

When rust-lang#120978 made the “otherwise block” non-optional, it also flattened that region of code. Doing so left this comment awkwardly stranded above an unrelated line of code, without its original context.

We can restore that context by moving it above the declaration of `otherwise`.

r? ``@Nadrieril``
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Apr 18, 2024
Rollup merge of rust-lang#124064 - Zalathar:otherwise-block, r=Nadrieril

Move confusing comment about otherwise blocks in `lower_match_tree`

This comment was historically inside a block guarded by `if let Some(otherwise_block) = otherwise`.

When rust-lang#120978 made the “otherwise block” non-optional, it also flattened that region of code. Doing so left this comment awkwardly stranded above an unrelated line of code, without its original context.

We can restore that context by moving it above the declaration of `otherwise`.

r? ``@Nadrieril``
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.

4 participants