Skip to content

mir_build: Clarify parts of if-condition lowering - #161861

Merged
rust-bors[bot] merged 3 commits into
rust-lang:mainfrom
Zalathar:if-then
Sep 1, 2026
Merged

mir_build: Clarify parts of if-condition lowering#161861
rust-bors[bot] merged 3 commits into
rust-lang:mainfrom
Zalathar:if-then

Conversation

@Zalathar

@Zalathar Zalathar commented Aug 27, 2026

Copy link
Copy Markdown
Member

I've been looking at this condition-lowering code a lot, and I noticed that some parts are hard to follow due to how functions and variables are named.

In particular, the function name then_else_break doesn't give much useful intuition for the fact that it lowers an if condition (or similar), and then proceeds if the condition was true or breaks if the condition was false.

Similarly, I was finding it hard to keep track of the two destinations selected after evaluating a condition, especially when dealing with complex subconditions like || or !, or looking at different callers. So this PR tries to establish a convention of always using (true_block, false_block) to refer to the condition's outcome paths, which can then be manipulated as appropriate by calling code.

There should be no observable change to compiler behaviour.

@Zalathar
Rename and combine then_else_break to lower_if_condition
a280dde 
The existing name doesn't give a good intuition for what's actually happening,
which is that we lower a (possibly complex) boolean condition and then proceed
if it's true or break if it's false.

This commit also directly exposes the arguments struct to callers, since it
makes the call sites more self-documenting.
@Zalathar
Describe if-condition arms as (true_block, false_block)
fde9cd0 
This convention is a little less intuitive for simple if-expressions, but is
easier to follow when dealing with complex nested conditions or with other
if-like constructs.
@Zalathar
Rename break_for_else to break_from_if_then_scope
422d1fd
@rustbot

rustbot commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

Some changes occurred in match lowering

cc @Nadrieril

@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 Aug 27, 2026
@rustbot

rustbot commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

r? @tiif

rustbot has assigned @tiif.
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

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: compiler, mir
  • compiler, mir expanded to 75 candidates
  • Random selection from 18 candidates

@Zalathar

Zalathar commented Aug 27, 2026

Copy link
Copy Markdown
Member Author

cc @dianne and @dingxiangfei2009 in case they have opinions on the code being touched here.

@Nadrieril

Nadrieril commented Aug 31, 2026

Copy link
Copy Markdown
Member

That's a huge improvement for me, I also found these bits hard to follow. I also appreciate the comments added.

r? me

@bors r+

@rust-bors

rust-bors Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

📌 Commit 422d1fd has been approved by Nadrieril

It is now in the queue for this repository.

@rust-bors rust-bors Bot added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Aug 31, 2026
@rust-bors rust-bors Bot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 31, 2026
jhpratt added a commit to jhpratt/rust that referenced this pull request Sep 1, 2026
@jhpratt
Rollup merge of rust-lang#161861 - Zalathar:if-then, r=Nadrieril
eece9a4 
mir_build: Clarify parts of if-condition lowering

I've been looking at this condition-lowering code a lot, and I noticed that some parts are hard to follow due to how functions and variables are named.

In particular, the function name `then_else_break` doesn't give much useful intuition for the fact that it lowers an `if` condition (or similar), and then proceeds if the condition was true or breaks if the condition was false.

Similarly, I was finding it hard to keep track of the two destinations selected after evaluating a condition, especially when dealing with complex subconditions like `||` or `!`, or looking at different callers. So this PR tries to establish a convention of always using `(true_block, false_block)` to refer to the condition's outcome paths, which can then be manipulated as appropriate by calling code.

There should be no observable change to compiler behaviour.
@jhpratt jhpratt mentioned this pull request Sep 1, 2026
Closed
jhpratt added a commit to jhpratt/rust that referenced this pull request Sep 1, 2026
@jhpratt
Rollup merge of rust-lang#161861 - Zalathar:if-then, r=Nadrieril
c62a074 
mir_build: Clarify parts of if-condition lowering

I've been looking at this condition-lowering code a lot, and I noticed that some parts are hard to follow due to how functions and variables are named.

In particular, the function name `then_else_break` doesn't give much useful intuition for the fact that it lowers an `if` condition (or similar), and then proceeds if the condition was true or breaks if the condition was false.

Similarly, I was finding it hard to keep track of the two destinations selected after evaluating a condition, especially when dealing with complex subconditions like `||` or `!`, or looking at different callers. So this PR tries to establish a convention of always using `(true_block, false_block)` to refer to the condition's outcome paths, which can then be manipulated as appropriate by calling code.

There should be no observable change to compiler behaviour.
@jhpratt jhpratt mentioned this pull request Sep 1, 2026
Closed
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Sep 1, 2026
@JonathanBrouwer
Rollup merge of rust-lang#161861 - Zalathar:if-then, r=Nadrieril
77754cc 
mir_build: Clarify parts of if-condition lowering

I've been looking at this condition-lowering code a lot, and I noticed that some parts are hard to follow due to how functions and variables are named.

In particular, the function name `then_else_break` doesn't give much useful intuition for the fact that it lowers an `if` condition (or similar), and then proceeds if the condition was true or breaks if the condition was false.

Similarly, I was finding it hard to keep track of the two destinations selected after evaluating a condition, especially when dealing with complex subconditions like `||` or `!`, or looking at different callers. So this PR tries to establish a convention of always using `(true_block, false_block)` to refer to the condition's outcome paths, which can then be manipulated as appropriate by calling code.

There should be no observable change to compiler behaviour.
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Sep 1, 2026
@JonathanBrouwer
Rollup merge of rust-lang#161861 - Zalathar:if-then, r=Nadrieril
44a3dd1 
mir_build: Clarify parts of if-condition lowering

I've been looking at this condition-lowering code a lot, and I noticed that some parts are hard to follow due to how functions and variables are named.

In particular, the function name `then_else_break` doesn't give much useful intuition for the fact that it lowers an `if` condition (or similar), and then proceeds if the condition was true or breaks if the condition was false.

Similarly, I was finding it hard to keep track of the two destinations selected after evaluating a condition, especially when dealing with complex subconditions like `||` or `!`, or looking at different callers. So this PR tries to establish a convention of always using `(true_block, false_block)` to refer to the condition's outcome paths, which can then be manipulated as appropriate by calling code.

There should be no observable change to compiler behaviour.
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Sep 1, 2026
@JonathanBrouwer
Rollup merge of rust-lang#161861 - Zalathar:if-then, r=Nadrieril
fad242c 
mir_build: Clarify parts of if-condition lowering

I've been looking at this condition-lowering code a lot, and I noticed that some parts are hard to follow due to how functions and variables are named.

In particular, the function name `then_else_break` doesn't give much useful intuition for the fact that it lowers an `if` condition (or similar), and then proceeds if the condition was true or breaks if the condition was false.

Similarly, I was finding it hard to keep track of the two destinations selected after evaluating a condition, especially when dealing with complex subconditions like `||` or `!`, or looking at different callers. So this PR tries to establish a convention of always using `(true_block, false_block)` to refer to the condition's outcome paths, which can then be manipulated as appropriate by calling code.

There should be no observable change to compiler behaviour.
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Sep 1, 2026
@JonathanBrouwer
Rollup merge of rust-lang#161861 - Zalathar:if-then, r=Nadrieril
a1aa55e 
mir_build: Clarify parts of if-condition lowering

I've been looking at this condition-lowering code a lot, and I noticed that some parts are hard to follow due to how functions and variables are named.

In particular, the function name `then_else_break` doesn't give much useful intuition for the fact that it lowers an `if` condition (or similar), and then proceeds if the condition was true or breaks if the condition was false.

Similarly, I was finding it hard to keep track of the two destinations selected after evaluating a condition, especially when dealing with complex subconditions like `||` or `!`, or looking at different callers. So this PR tries to establish a convention of always using `(true_block, false_block)` to refer to the condition's outcome paths, which can then be manipulated as appropriate by calling code.

There should be no observable change to compiler behaviour.
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Sep 1, 2026
@JonathanBrouwer
Rollup merge of rust-lang#161861 - Zalathar:if-then, r=Nadrieril
58abd71 
mir_build: Clarify parts of if-condition lowering

I've been looking at this condition-lowering code a lot, and I noticed that some parts are hard to follow due to how functions and variables are named.

In particular, the function name `then_else_break` doesn't give much useful intuition for the fact that it lowers an `if` condition (or similar), and then proceeds if the condition was true or breaks if the condition was false.

Similarly, I was finding it hard to keep track of the two destinations selected after evaluating a condition, especially when dealing with complex subconditions like `||` or `!`, or looking at different callers. So this PR tries to establish a convention of always using `(true_block, false_block)` to refer to the condition's outcome paths, which can then be manipulated as appropriate by calling code.

There should be no observable change to compiler behaviour.
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Sep 1, 2026
@JonathanBrouwer
Rollup merge of rust-lang#161861 - Zalathar:if-then, r=Nadrieril
81b14bb 
mir_build: Clarify parts of if-condition lowering

I've been looking at this condition-lowering code a lot, and I noticed that some parts are hard to follow due to how functions and variables are named.

In particular, the function name `then_else_break` doesn't give much useful intuition for the fact that it lowers an `if` condition (or similar), and then proceeds if the condition was true or breaks if the condition was false.

Similarly, I was finding it hard to keep track of the two destinations selected after evaluating a condition, especially when dealing with complex subconditions like `||` or `!`, or looking at different callers. So this PR tries to establish a convention of always using `(true_block, false_block)` to refer to the condition's outcome paths, which can then be manipulated as appropriate by calling code.

There should be no observable change to compiler behaviour.
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Sep 1, 2026
@JonathanBrouwer
Rollup merge of rust-lang#161861 - Zalathar:if-then, r=Nadrieril
6fe8d16 
mir_build: Clarify parts of if-condition lowering

I've been looking at this condition-lowering code a lot, and I noticed that some parts are hard to follow due to how functions and variables are named.

In particular, the function name `then_else_break` doesn't give much useful intuition for the fact that it lowers an `if` condition (or similar), and then proceeds if the condition was true or breaks if the condition was false.

Similarly, I was finding it hard to keep track of the two destinations selected after evaluating a condition, especially when dealing with complex subconditions like `||` or `!`, or looking at different callers. So this PR tries to establish a convention of always using `(true_block, false_block)` to refer to the condition's outcome paths, which can then be manipulated as appropriate by calling code.

There should be no observable change to compiler behaviour.
@JonathanBrouwer JonathanBrouwer mentioned this pull request Sep 1, 2026
Merged
rust-bors Bot pushed a commit that referenced this pull request Sep 1, 2026
@bors
Auto merge of #162117 - JonathanBrouwer:rollup-vMnIfR5, r=JonathanBro…
a433023 
…uwer

Rollup of 16 pull requests

Successful merges:

 - #137720 (support `#[target_feature(enable = ...)]` on `#[naked]` functions)
 - #160534 (stabilize smart pointer map functions)
 - #160551 (mir_build: Clearly distinguish or/refutable/irrefutable patterns during match lowering)
 - #160989 (Make sin, cos, exp, exp2, log, log2, log10 generic)
 - #161861 (mir_build: Clarify parts of if-condition lowering)
 - #161929 (explicitly track inherent const generic args kind)
 - #162040 (bootstrap: stage0 to cbae9b4)
 - #162063 (Switch dist-aarch64-linux to EC2 and update dist-x86_64-linux)
 - #161353 (Add test for parallel compiler reproducible build)
 - #161937 (A series of Polonius Alpha refactors)
 - #162051 (`rustc_feature` cleanups)
 - #162055 (remove `_{style}` recovery for diagnostic structs)
 - #162075 (Move track_caller on closures gating to attribute parsing)
 - #162079 (std: implement `File::fsync` for Hermit)
 - #162097 (Deduplicate `InstrumentFnAttr`)
 - #162115 (fix typo in feature documentation)
rust-bors Bot pushed a commit that referenced this pull request Sep 1, 2026
@bors
Unrolled build for #161861 in rollup 162117
9b66008 
Rollup merge of #161861 - Zalathar:if-then, r=Nadrieril

mir_build: Clarify parts of if-condition lowering

I've been looking at this condition-lowering code a lot, and I noticed that some parts are hard to follow due to how functions and variables are named.

In particular, the function name `then_else_break` doesn't give much useful intuition for the fact that it lowers an `if` condition (or similar), and then proceeds if the condition was true or breaks if the condition was false.

Similarly, I was finding it hard to keep track of the two destinations selected after evaluating a condition, especially when dealing with complex subconditions like `||` or `!`, or looking at different callers. So this PR tries to establish a convention of always using `(true_block, false_block)` to refer to the condition's outcome paths, which can then be manipulated as appropriate by calling code.

There should be no observable change to compiler behaviour.
@rust-bors
rust-bors Bot merged commit 997ffdb into rust-lang:main Sep 1, 2026
13 checks passed
@rustbot rustbot added this to the 1.100.0 milestone Sep 1, 2026
@rust-timer

rust-timer commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

Finished benchmarking commit (9b66008): comparison URL.

Overall result: ✅ improvements - no action needed

Benchmarking means the PR may be perf-sensitive. Consider adding rollup=never if this change is not fit for rolling up.

@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

mean range count
Regressions ❌
(primary)		 - - 0
Regressions ❌
(secondary)		 - - 0
Improvements ✅
(primary)		 - - 0
Improvements ✅
(secondary)		 -0.3% [-0.3%, -0.3%] 1
All ❌✅ (primary) - - 0

Max RSS (memory usage)

Results (secondary -2.2%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)		 - - 0
Regressions ❌
(secondary)		 - - 0
Improvements ✅
(primary)		 - - 0
Improvements ✅
(secondary)		 -2.2% [-2.2%, -2.2%] 1
All ❌✅ (primary) - - 0

Cycles

This perf run didn't have relevant results for this metric.

Binary size

This perf run didn't have relevant results for this metric.

Bootstrap: missing data
Artifact size: 400.59 MiB -> 400.52 MiB (-0.02%)

@Zalathar
Zalathar deleted the if-then branch September 1, 2026 23:34
pull Bot pushed a commit to LeeeeeeM/miri that referenced this pull request Sep 2, 2026
@bors
Auto merge of #162117 - JonathanBrouwer:rollup-vMnIfR5, r=JonathanBro…
25a3c06 
…uwer

Rollup of 16 pull requests

Successful merges:

 - rust-lang/rust#137720 (support `#[target_feature(enable = ...)]` on `#[naked]` functions)
 - rust-lang/rust#160534 (stabilize smart pointer map functions)
 - rust-lang/rust#160551 (mir_build: Clearly distinguish or/refutable/irrefutable patterns during match lowering)
 - rust-lang/rust#160989 (Make sin, cos, exp, exp2, log, log2, log10 generic)
 - rust-lang/rust#161861 (mir_build: Clarify parts of if-condition lowering)
 - rust-lang/rust#161929 (explicitly track inherent const generic args kind)
 - rust-lang/rust#162040 (bootstrap: stage0 to cbae9b4cae2b108f6a3d18cfe6075714bb739463)
 - rust-lang/rust#162063 (Switch dist-aarch64-linux to EC2 and update dist-x86_64-linux)
 - rust-lang/rust#161353 (Add test for parallel compiler reproducible build)
 - rust-lang/rust#161937 (A series of Polonius Alpha refactors)
 - rust-lang/rust#162051 (`rustc_feature` cleanups)
 - rust-lang/rust#162055 (remove `_{style}` recovery for diagnostic structs)
 - rust-lang/rust#162075 (Move track_caller on closures gating to attribute parsing)
 - rust-lang/rust#162079 (std: implement `File::fsync` for Hermit)
 - rust-lang/rust#162097 (Deduplicate `InstrumentFnAttr`)
 - rust-lang/rust#162115 (fix typo in feature documentation)
asukaminato0721 pushed a commit to asukaminato0721/rust-analyzer that referenced this pull request Sep 2, 2026
@bors
Auto merge of #162117 - JonathanBrouwer:rollup-vMnIfR5, r=JonathanBro…
a62eeeb 
…uwer

Rollup of 16 pull requests

Successful merges:

 - rust-lang/rust#137720 (support `#[target_feature(enable = ...)]` on `#[naked]` functions)
 - rust-lang/rust#160534 (stabilize smart pointer map functions)
 - rust-lang/rust#160551 (mir_build: Clearly distinguish or/refutable/irrefutable patterns during match lowering)
 - rust-lang/rust#160989 (Make sin, cos, exp, exp2, log, log2, log10 generic)
 - rust-lang/rust#161861 (mir_build: Clarify parts of if-condition lowering)
 - rust-lang/rust#161929 (explicitly track inherent const generic args kind)
 - rust-lang/rust#162040 (bootstrap: stage0 to cbae9b4cae2b108f6a3d18cfe6075714bb739463)
 - rust-lang/rust#162063 (Switch dist-aarch64-linux to EC2 and update dist-x86_64-linux)
 - rust-lang/rust#161353 (Add test for parallel compiler reproducible build)
 - rust-lang/rust#161937 (A series of Polonius Alpha refactors)
 - rust-lang/rust#162051 (`rustc_feature` cleanups)
 - rust-lang/rust#162055 (remove `_{style}` recovery for diagnostic structs)
 - rust-lang/rust#162075 (Move track_caller on closures gating to attribute parsing)
 - rust-lang/rust#162079 (std: implement `File::fsync` for Hermit)
 - rust-lang/rust#162097 (Deduplicate `InstrumentFnAttr`)
 - rust-lang/rust#162115 (fix typo in feature documentation)
github-actions Bot pushed a commit to rust-lang/rustc-dev-guide that referenced this pull request Sep 4, 2026
@bors
Auto merge of #162117 - JonathanBrouwer:rollup-vMnIfR5, r=JonathanBro…
5d0d292 
…uwer

Rollup of 16 pull requests

Successful merges:

 - rust-lang/rust#137720 (support `#[target_feature(enable = ...)]` on `#[naked]` functions)
 - rust-lang/rust#160534 (stabilize smart pointer map functions)
 - rust-lang/rust#160551 (mir_build: Clearly distinguish or/refutable/irrefutable patterns during match lowering)
 - rust-lang/rust#160989 (Make sin, cos, exp, exp2, log, log2, log10 generic)
 - rust-lang/rust#161861 (mir_build: Clarify parts of if-condition lowering)
 - rust-lang/rust#161929 (explicitly track inherent const generic args kind)
 - rust-lang/rust#162040 (bootstrap: stage0 to cbae9b4cae2b108f6a3d18cfe6075714bb739463)
 - rust-lang/rust#162063 (Switch dist-aarch64-linux to EC2 and update dist-x86_64-linux)
 - rust-lang/rust#161353 (Add test for parallel compiler reproducible build)
 - rust-lang/rust#161937 (A series of Polonius Alpha refactors)
 - rust-lang/rust#162051 (`rustc_feature` cleanups)
 - rust-lang/rust#162055 (remove `_{style}` recovery for diagnostic structs)
 - rust-lang/rust#162075 (Move track_caller on closures gating to attribute parsing)
 - rust-lang/rust#162079 (std: implement `File::fsync` for Hermit)
 - rust-lang/rust#162097 (Deduplicate `InstrumentFnAttr`)
 - rust-lang/rust#162115 (fix typo in feature documentation)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@Nadrieril Nadrieril

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

Milestone

1.100.0

Development

Successfully merging this pull request may close these issues.

5 participants

@Zalathar @rustbot @Nadrieril @rust-timer @tiif