fix `#[loop_match]` on diverging loop #144783

folkertdev · 2025-08-01T13:35:47Z

tracking issue: #132306

this generated invalid MIR before. issue #143806 still has an issue where we assign state = state which is invalid in MIR. Fixing that problem is tricky, so I'd like to do that separately.

r? @bjorn3

tests/ui/loop-match/diverges.rs

bjorn3 · 2025-08-01T13:38:39Z

r? compiler

given that this was debugged by the both of us.

folkertdev · 2025-08-01T13:39:45Z

I guess but also we have all the context here, it's a one-liner, and has no consequences outside of loop-match code.

rustbot · 2025-08-01T14:15:40Z

Some changes occurred in match lowering

cc @Nadrieril

folkertdev · 2025-08-01T14:16:34Z

compiler/rustc_mir_build/src/builder/expr/into.rs

-                    let scrutinee_place_builder =
-                        unpack!(body_block = this.as_place_builder(body_block, scrutinee));
                    let scrutinee_span = this.thir.exprs[scrutinee].span;
+                    let scrutinee_place_builder = unpack!(
+                        body_block = this.lower_scrutinee(body_block, scrutinee, scrutinee_span)
+                    );
+


this change adds a place mention of the scrutinee that was missing before.

tests/ui/loop-match/diverges.rs

this generated invalid MIR before

folkertdev · 2025-08-10T12:34:36Z

I've added the MIR tests, but #143806 still has a separate problem where we basically desugar to a state = state assignment, which is invalid in MIR.

The reason is that we don't accurately replicate what a (labeled) block does. We kind of can't with the existing approach, because we don't have an ExprId for that block. I'm not sure how to fix that yet, but the fix here (essentially a one-liner) doesn't need to wait for that.

SparrowLii

LGTM

View changes since this review

SparrowLii · 2025-09-01T08:08:23Z

@bors r+

bors · 2025-09-01T08:08:26Z

📌 Commit 3e76b58 has been approved by SparrowLii

It is now in the queue for this repository.

bors · 2025-09-01T14:42:11Z

⌛ Testing commit 3e76b58 with merge 154037f...

bors · 2025-09-01T17:48:53Z

☀️ Test successful - checks-actions
Approved by: SparrowLii
Pushing 154037f to master...

github-actions · 2025-09-01T17:52:01Z

What is this?

This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.

Comparing c0bb3b9 (parent) -> 154037f (this PR)

Test differences

Show 6 test diffs

Stage 1

[mir-opt] tests/mir-opt/building/loop_match_diverges.rs: [missing] -> pass (J0)
[ui] tests/ui/loop-match/diverges.rs: [missing] -> pass (J0)

Stage 2

[mir-opt] tests/mir-opt/building/loop_match_diverges.rs: [missing] -> pass (J1)
[ui] tests/ui/loop-match/diverges.rs: [missing] -> pass (J2)

Additionally, 2 doctest diffs were found. These are ignored, as they are noisy.

Job group index

Test dashboard

Run

cargo run --manifest-path src/ci/citool/Cargo.toml -- \
    test-dashboard 154037ffb82714a8d6264a9153622637b170c706 --output-dir test-dashboard

And then open test-dashboard/index.html in your browser to see an overview of all executed tests.

Job duration changes

dist-x86_64-apple: 7912.1s -> 5796.5s (-26.7%)
dist-aarch64-linux: 8538.5s -> 6536.5s (-23.4%)
dist-aarch64-apple: 6892.3s -> 5914.5s (-14.2%)
x86_64-gnu-llvm-19-1: 3287.2s -> 3595.9s (9.4%)
x86_64-gnu-stable: 6868.2s -> 7490.7s (9.1%)
dist-i586-gnu-i586-i686-musl: 5431.2s -> 5882.8s (8.3%)
x86_64-msvc-1: 8425.8s -> 9010.1s (6.9%)
dist-s390x-linux: 4895.9s -> 5205.6s (6.3%)
x86_64-gnu-tools: 3584.1s -> 3358.1s (-6.3%)
aarch64-gnu-debug: 4127.7s -> 4353.4s (5.5%)

How to interpret the job duration changes?

Job durations can vary a lot, based on the actual runner instance
that executed the job, system noise, invalidated caches, etc. The table above is provided
mostly for t-infra members, for simpler debugging of potential CI slow-downs.

rust-timer · 2025-09-01T18:59:04Z

Finished benchmarking commit (154037f): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

This benchmark run did not return any relevant results for this metric.

Cycles

Results (secondary 3.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)	3.2%	[3.2%, 3.2%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 466.41s -> 467.56s (0.25%)
Artifact size: 388.46 MiB -> 388.51 MiB (0.01%)

folkertdev added the F-loop_match when you match up with someone and they really throw you for a loop label Aug 1, 2025

rustbot assigned bjorn3 Aug 1, 2025

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 1, 2025

bjorn3 reviewed Aug 1, 2025

View reviewed changes

tests/ui/loop-match/diverges.rs Outdated Show resolved Hide resolved

folkertdev force-pushed the loop-match-diverging-loop branch from 13d09ba to 36bf31d Compare August 1, 2025 13:38

rustbot assigned SparrowLii and unassigned bjorn3 Aug 1, 2025

folkertdev commented Aug 1, 2025

View reviewed changes

cjgillot reviewed Aug 3, 2025

View reviewed changes

tests/ui/loop-match/diverges.rs Show resolved Hide resolved

folkertdev added 2 commits August 4, 2025 19:23

fix #[loop_match] on diverging loop

9916ce3

this generated invalid MIR before

add place mention for #[loop_match] scrutinee

3e76b58

folkertdev force-pushed the loop-match-diverging-loop branch from f0c2097 to 3e76b58 Compare August 10, 2025 12:26

SparrowLii reviewed Sep 1, 2025

View reviewed changes

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 Sep 1, 2025

bors added the merged-by-bors This PR was explicitly merged by bors. label Sep 1, 2025

bors merged commit 154037f into rust-lang:master Sep 1, 2025
11 checks passed

rustbot added this to the 1.91.0 milestone Sep 1, 2025

github-actions bot mentioned this pull request Sep 5, 2025

Toolchain upgrade to nightly-2025-09-09 flux-rs/flux#1260

Merged

fix #[loop_match] on diverging loop #144783

fix #[loop_match] on diverging loop #144783

Uh oh!

Conversation

folkertdev commented Aug 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

bjorn3 commented Aug 1, 2025

Uh oh!

folkertdev commented Aug 1, 2025

Uh oh!

rustbot commented Aug 1, 2025

Uh oh!

folkertdev Aug 1, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

folkertdev commented Aug 10, 2025

Uh oh!

SparrowLii left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparrowLii commented Sep 1, 2025

Uh oh!

bors commented Sep 1, 2025

Uh oh!

bors commented Sep 1, 2025

Uh oh!

bors commented Sep 1, 2025

Uh oh!

Uh oh!

github-actions bot commented Sep 1, 2025

Test differences

Stage 1

Stage 2

Job duration changes

Uh oh!

rust-timer commented Sep 1, 2025

Overall result: no relevant changes - no action needed

Instruction count

Max RSS (memory usage)

Cycles

Binary size

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

fix `#[loop_match]` on diverging loop #144783

fix `#[loop_match]` on diverging loop #144783

folkertdev commented Aug 1, 2025 •

edited

Loading

SparrowLii left a comment •

edited by rustbot

Loading