mir_build: Add an extra intermediate step in MIR building for patterns by Zalathar · Pull Request #155144 · rust-lang/rust

Zalathar · 2026-04-11T08:37:00Z

This is an attempt to partly decouple the data structures created by match_pair.rs from the data structures that are ultimately consumed by the main part of match lowering.

In some ways this is a reversal from #137875. That PR succeeded in removing the TestCase::Irrefutable variant, by taking a pre-existing “simplification” step and fusing it directly into MatchPairTree::for_pattern. Unfortunately, in doing so it also reinforced a very high degree of coupling between the transformations performed in match_pair, and the data structures used by later steps.

My hope is that these changes will make it easier for follow-up work to further separate decision-making from MIR building when lowering patterns.

r? Nadrieril

rustbot · 2026-04-11T08:37:06Z

Nadrieril is not on the review rotation at the moment.
They may take a while to respond.

Zalathar · 2026-04-11T10:29:49Z

@bors try @rust-timer queue

mir_build: Add an extra intermediate step in MIR building for patterns

rust-bors · 2026-04-11T12:37:16Z

☀️ Try build successful (CI)
Build commit: 76367e4 (76367e4db9c5ebe2cd39a24f067d772b200f4158, parent: c29effdf79bdea928ea366a2a820ed15c1de5f43)

rust-timer · 2026-04-11T14:00:18Z

Finished benchmarking commit (76367e4): comparison URL.

Overall result: ❌✅ regressions and improvements - no action needed

Benchmarking means the PR may be perf-sensitive. It's automatically marked not fit for rolling up. Overriding is possible but disadvised: it risks changing compiler perf.

@bors rollup=never
@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.1%	[0.1%, 0.1%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.2%	[-0.2%, -0.2%]	1
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results (secondary 2.6%)

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)	4.8%	[0.9%, 6.8%]	3
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-3.9%	[-3.9%, -3.9%]	1
All ❌✅ (primary)	-	-	0

Cycles

Results (primary -9.8%, secondary -4.3%)

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)	-9.8%	[-20.6%, -3.1%]	13
Improvements ✅ (secondary)	-4.3%	[-8.0%, -2.5%]	4
All ❌✅ (primary)	-9.8%	[-20.6%, -3.1%]	13

Binary size

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

Bootstrap: 491.148s -> 509.241s (3.68%)
Artifact size: 394.19 MiB -> 394.20 MiB (0.00%)

Zalathar · 2026-04-18T12:06:25Z

Marking this as ready for consideration.

I'm not entirely confident about the name “InterPat”, and I'm aware that this change doesn't yet demonstrate any clear benefit on its own, but I think it's a worthwhile step towards hopefully being able to disentangle match-lowering a bit more.

rustbot · 2026-04-18T12:06:30Z

Some changes occurred in match lowering

cc @Nadrieril

Nadrieril

I like this, the separation of concerns is very clean. I'm on board, I only have a couple nits. Agreed that InterPat isn't an amazing name but it's used in a single file so who cares really.

View changes since this review

Nadrieril · 2026-04-24T14:23:35Z

+        // Recursively squash any subpatterns into refutable `MatchPairTree` forests.
+        let mut subpairs = vec![];
+        for subpat in inter_pat.subpats {
+            MatchPairTree::squash_inter_pat(subpat, &mut subpairs, extra_data);
+        }
+
+        if let Some(testable_case) = inter_pat.testable_case {


For clarity I'd prefer the rest of the function to be in the else branch of the "or pattern" case

Nadrieril · 2026-04-24T14:25:43Z

-                // In order to please the borrow checker, when lowering a pattern
-                // like `x @ subpat` we must establish any bindings in `subpat`
-                // before establishing the binding for `x`.


I think that comment get lost, I don't see it anymore

I found the original comment to be too long and verbose, so I rewrote it as this shorter comment in squash_inter_pat:

// If present, the binding must be pushed _after_ traversing subpatterns. // This is so that when lowering something like `x @ NonCopy { copy_field }`, // the binding to `copy_field` will occur before the binding for `x`. // See <https://github.com/rust-lang/rust/issues/69971> for more background.

Do you think that's fine, or is there more detail you'd like me to retain?

(Note that the ordering restriction is not yet relevant when building InterPat; it only becomes important when squashing InterPat into MatchPairTree.)

Nadrieril · 2026-04-24T14:30:18Z

        match_pairs: &mut Vec<Self>, // Newly-created nodes are added to this vector
        extra_data: &mut PatternExtraData<'tcx>, // Bindings/ascriptions are added here
    ) {
+        extra_data.ascriptions.extend(inter_pat.ascriptions);


Would you mind starting the function with a let InterPat { ascriptions, ....} = inter_pat with an exhaustive field list so we're sure not to forget a field?

Nadrieril · 2026-04-24T14:31:07Z

+            ascriptions: vec![],
+            is_never: inter_pat.is_never,
+        };
+        MatchPairTree::squash_inter_pat(inter_pat, &mut match_pairs, &mut extra_data);


nit: I have a preference for inlining squash_inter_pat into here, the current split took me a couple seconds to understand

squash_inter_pat is recursive (since it recursively squashes subpatterns), so I don't think it can be inlined.

This makes it easier to perform coordinated modifications to `FlatPat::new` and `MatchPairTree::from_pattern`, because the two functions are heavily coupled.

rustbot · 2026-04-25T00:40:45Z

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

Zalathar · 2026-04-25T00:41:15Z

(Rebased onto main to make things easier; no actual changes yet.)

Zalathar · 2026-04-25T01:02:33Z

+                // Or-patterns never need a place during MIR building.
+                place: None,


Note that currently we retain places for or-pattern nodes and then (I believe) never use them, so this is a deliberate change from current behaviour.

View changes since the review

Zalathar · 2026-04-25T04:23:37Z

Updated some comments (diff); still considering what to do about the style feedback.

This splits out a separate `InterPat` data structure from `MatchPairTree`/`FlatPat`/`Candidate`, which should hopefully make it easier to modify these earlier parts of match lowering without having to simultaneously adjust many use-sites in later steps.

rustbot assigned Nadrieril Apr 11, 2026

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Apr 11, 2026

Zalathar mentioned this pull request Apr 11, 2026

Move recursion out of MatchPairTree::for_pattern helpers #154943

Merged

This comment has been minimized.

Sign in to view

Zalathar force-pushed the inter-pat branch from 1d97342 to 05275f8 Compare April 11, 2026 10:26

This comment has been minimized.

Sign in to view

rust-bors Bot pushed a commit that referenced this pull request Apr 11, 2026

Auto merge of #155144 - Zalathar:inter-pat, r=<try>

76367e4

mir_build: Add an extra intermediate step in MIR building for patterns

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Apr 11, 2026

This comment has been minimized.

Sign in to view

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Apr 11, 2026

Zalathar force-pushed the inter-pat branch from 05275f8 to fb1ef5c Compare April 12, 2026 05:24

This comment was marked as resolved.

Sign in to view

Zalathar force-pushed the inter-pat branch 2 times, most recently from 0623749 to c9949ba Compare April 18, 2026 05:34

This comment has been minimized.

Sign in to view

Zalathar force-pushed the inter-pat branch 2 times, most recently from 9631275 to 7abdb0f Compare April 18, 2026 12:02

Zalathar marked this pull request as ready for review April 18, 2026 12:06

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 Apr 18, 2026

Nadrieril approved these changes Apr 24, 2026

View reviewed changes

Move FlatPat::new into match_pair.rs

f7038ea

This makes it easier to perform coordinated modifications to `FlatPat::new` and `MatchPairTree::from_pattern`, because the two functions are heavily coupled.

Zalathar force-pushed the inter-pat branch from 7abdb0f to b64ff41 Compare April 25, 2026 00:40

Zalathar commented Apr 25, 2026

View reviewed changes

Zalathar force-pushed the inter-pat branch from b64ff41 to 6640efd Compare April 25, 2026 04:21

Zalathar force-pushed the inter-pat branch from 6640efd to bce0c38 Compare April 25, 2026 04:49

Zalathar force-pushed the inter-pat branch from bce0c38 to 2165ec3 Compare April 25, 2026 04:52

		// Or-patterns never need a place during MIR building.
		place: None,

Uh oh!

Conversation

Zalathar commented Apr 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Apr 11, 2026

Uh oh!

This comment has been minimized.

Zalathar commented Apr 11, 2026

Uh oh!

This comment has been minimized.

This comment has been minimized.

rust-bors Bot commented Apr 11, 2026

Uh oh!

This comment has been minimized.

rust-timer commented Apr 11, 2026

Overall result: ❌✅ regressions and improvements - no action needed

Uh oh!

This comment was marked as resolved.

This comment has been minimized.

Zalathar commented Apr 18, 2026

Uh oh!

rustbot commented Apr 18, 2026

Uh oh!

Nadrieril 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!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rustbot commented Apr 25, 2026

Uh oh!

Zalathar commented Apr 25, 2026

Uh oh!

Zalathar Apr 25, 2026 • 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!

Zalathar commented Apr 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Zalathar commented Apr 11, 2026 •

edited

Loading

Nadrieril left a comment •

edited by rustbot

Loading

Zalathar Apr 25, 2026 •

edited by rustbot

Loading