Skip to content

mir_build: Split MatchPairTree into testable and or-pattern variants #158551

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.

Sign up for GitHub

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

Closed
Zalathar wants to merge 2 commits into from
Closed

mir_build: Split MatchPairTree into testable and or-pattern variants #158551

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
558d78d
Split `MatchPairTree` into testable and or-pattern variants
Zalathar Jun 28, 2026
0b32bdd
Make `TestableMatchPairTree::place` non-optional
Zalathar Jun 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 10 additions & 10 deletions compiler/rustc_mir_build/src/builder/matches/buckets.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,9 +133,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
// away.)
let (match_pair_index, match_pair) = candidate
.match_pairs
.testable_match_pairs
.iter()
.enumerate()
.find(|&(_, mp)| mp.place == Some(test_place))?;
.find(|&(_, mp)| mp.place == test_place)?;

// If true, the match pair is completely entailed by its corresponding test
// branch, so it can be removed. If false, the match pair is _compatible_
Expand Down Expand Up @@ -172,11 +173,12 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
matches!(range.contains(value, self.tcx), None | Some(true))
})
};
let is_conflicting_candidate = |candidate: &&mut Candidate<'tcx>| {
candidate.match_pairs.iter().any(|mp| {
mp.place == Some(test_place) && is_covering_range(&mp.testable_case)
})
};
let is_conflicting_candidate =
|candidate: &&mut Candidate<'tcx>| {
candidate.match_pairs.testable_match_pairs.iter().any(|mp| {
mp.place == test_place && is_covering_range(&mp.testable_case)
})
};
if prior_candidates
.get(&TestBranch::Failure)
.is_some_and(|candidates| candidates.iter().any(is_conflicting_candidate))
Expand Down Expand Up @@ -363,10 +365,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {

if fully_matched {
// Replace the match pair by its sub-pairs.
let match_pair = candidate.match_pairs.remove(match_pair_index);
candidate.match_pairs.extend(match_pair.subpairs);
// Move or-patterns to the end.
candidate.sort_match_pairs();
let match_pair = candidate.match_pairs.testable_match_pairs.remove(match_pair_index);
candidate.match_pairs.push_all(match_pair.subpairs);
}

ret
Expand Down
22 changes: 12 additions & 10 deletions compiler/rustc_mir_build/src/builder/matches/match_pair.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ use rustc_span::Span;
use crate::builder::Builder;
use crate::builder::expr::as_place::{PlaceBase, PlaceBuilder};
use crate::builder::matches::{
FlatPat, MatchPairTree, PatConstKind, PatternExtraData, SliceLenOp, TestableCase,
FlatPat, MatchPairTree, OrMatchPairTree, PatConstKind, PatternExtraData, SliceLenOp,
TestableCase, TestableMatchPairTree,
};

/// For an array or slice pattern's subpatterns (prefix/slice/suffix), returns a list
Expand Down Expand Up @@ -142,12 +143,8 @@ fn squash_inter_pat<'tcx>(
extra_data.bindings.push(super::SubpatternBindings::FromOrPattern);
}

match_pairs.push(MatchPairTree {
// Or-patterns never need a place during MIR building.
place: None,
testable_case: TestableCase::Or { pats: or_subpats },
subpairs: vec![],
pattern_span,
match_pairs.push(MatchPairTree::Or {
or_match_pair: OrMatchPairTree { or_subpats, pattern_span },
});
} else {
// We're dealing with a node that isn't an or-pattern.
Expand All @@ -165,9 +162,14 @@ fn squash_inter_pat<'tcx>(
// If this match is inside a closure, it's essential that the place
// we're testing was actually captured! Be sure to keep `ExprUseVisitor`
// in sync with the refutability checks in this module.
assert!(place.is_some());
assert!(!matches!(testable_case, TestableCase::Or { .. }));
match_pairs.push(MatchPairTree { place, testable_case, subpairs, pattern_span });
match_pairs.push(MatchPairTree::Testable {
testable_match_pair: TestableMatchPairTree {
place: place.expect("non-or nodes always have a place"),
testable_case,
subpairs,
pattern_span,
},
})
} else {
// This pattern is irrefutable, so it doesn't need its own match-pair node.
// Just push its refutable subpatterns instead, if any.
Expand Down
141 changes: 76 additions & 65 deletions compiler/rustc_mir_build/src/builder/matches/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1028,10 +1028,7 @@ struct Candidate<'tcx> {
/// - After a candidate's subcandidates have been lowered, a copy of any remaining
/// or-patterns is added to each leaf subcandidate
/// (see [`Builder::test_remaining_match_pairs_after_or`]).
///
/// Invariants:
/// - All or-patterns ([`TestableCase::Or`]) have been sorted to the end.
match_pairs: Vec<MatchPairTree<'tcx>>,
match_pairs: MatchPairsQueue<'tcx>,

/// ...and if this is non-empty, one of these subcandidates also has to match...
///
Expand Down Expand Up @@ -1100,31 +1097,16 @@ impl<'tcx> Candidate<'tcx> {

/// Incorporates an already-simplified [`FlatPat`] into a new candidate.
fn from_flat_pat(flat_pat: FlatPat<'tcx>, has_guard: bool) -> Self {
let mut this = Candidate {
match_pairs: flat_pat.match_pairs,
Candidate {
match_pairs: MatchPairsQueue::new(flat_pat.match_pairs),
extra_data: flat_pat.extra_data,
has_guard,
subcandidates: Vec::new(),
or_span: None,
otherwise_block: None,
pre_binding_block: None,
false_edge_start_block: None,
};
this.sort_match_pairs();
this
}

/// Restores the invariant that or-patterns must be sorted to the end.
fn sort_match_pairs(&mut self) {
self.match_pairs.sort_by_key(|pair| matches!(pair.testable_case, TestableCase::Or { .. }));
}

/// Returns whether the first match pair of this candidate is an or-pattern.
fn starts_with_or_pattern(&self) -> bool {
matches!(
&*self.match_pairs,
[MatchPairTree { testable_case: TestableCase::Or { .. }, .. }, ..]
)
}
}

/// Visit the leaf candidates (those with no subcandidates) contained in
Expand All @@ -1151,6 +1133,45 @@ impl<'tcx> Candidate<'tcx> {
}
}

#[derive(Debug)]
struct MatchPairsQueue<'tcx> {
/// Match pairs that can be "tested" directly, because they are not or-patterns.
testable_match_pairs: Vec<TestableMatchPairTree<'tcx>>,
/// Or-patterns, which must be expanded before their subpatterns can participate in tests.
/// These should only be processed after `testable_match_pairs` is empty
/// (see [`Self::starts_with_or_pattern`]).
or_match_pairs: Vec<OrMatchPairTree<'tcx>>,
}
Comment on lines +1136 to +1144

@Nadrieril Nadrieril Jun 29, 2026 •
edited by rustbot
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately I don't think that's the right approach: we will need to support having or-patterns not sorted to the end in order to fix #158387. Also, it's not a "must": sorting or-patterns at the end is purely an optimization (well, except the union case), so forgetting the invariant isn't a huge issue.

View changes since the review

@Zalathar Zalathar Jun 30, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm yeah, I can appreciate not wanting to further entrench the or-pattern reordering behaviour.


impl<'tcx> MatchPairsQueue<'tcx> {
fn new(match_pairs: Vec<MatchPairTree<'tcx>>) -> Self {
let mut this = MatchPairsQueue { testable_match_pairs: vec![], or_match_pairs: vec![] };
this.push_all(match_pairs);
this
}

fn push_all(&mut self, match_pairs: impl IntoIterator<Item = MatchPairTree<'tcx>>) {
for match_pair in match_pairs {
match match_pair {
MatchPairTree::Testable { testable_match_pair } => {
self.testable_match_pairs.push(testable_match_pair)
}
MatchPairTree::Or { or_match_pair } => self.or_match_pairs.push(or_match_pair),
}
}
}

fn is_empty(&self) -> bool {
let MatchPairsQueue { testable_match_pairs, or_match_pairs } = self;
testable_match_pairs.is_empty() && or_match_pairs.is_empty()
}

fn starts_with_or_pattern(&self) -> bool {
let MatchPairsQueue { testable_match_pairs, or_match_pairs } = self;
testable_match_pairs.is_empty() && !or_match_pairs.is_empty()
}
}

/// A depth-first traversal of the `Candidate` and all of its recursive
/// subcandidates.
///
Expand Down Expand Up @@ -1211,10 +1232,6 @@ struct Ascription<'tcx> {
/// Created by [`MatchPairTree`], and then inspected primarily by:
/// - [`Builder::pick_test_for_match_pair`] (to choose a test)
/// - [`Builder::choose_bucket_for_candidate`] (to see how the test interacts with a match pair)
///
/// Note that or-patterns are not tested directly like the other variants.
/// Instead they participate in or-pattern expansion, where they are transformed into
/// subcandidates. See [`Builder::expand_and_match_or_candidates`].
#[derive(Debug, Clone)]
enum TestableCase<'tcx> {
Variant { adt_def: ty::AdtDef<'tcx>, variant_index: VariantIdx },
Expand All @@ -1223,7 +1240,6 @@ enum TestableCase<'tcx> {
Slice { len: u64, op: SliceLenOp },
Deref { temp: Place<'tcx>, mutability: Mutability },
Never,
Or { pats: Box<[FlatPat<'tcx>]> },
}

impl<'tcx> TestableCase<'tcx> {
Expand Down Expand Up @@ -1261,28 +1277,30 @@ enum PatConstKind {
/// Each node also has a list of subpairs (possibly empty) that must also match,
/// and some additional information from the THIR pattern it represents.
#[derive(Debug, Clone)]
pub(crate) struct MatchPairTree<'tcx> {
/// This place...
///
/// ---
/// This can be `None` if it referred to a non-captured place in a closure.
///
/// Invariant: Can only be `None` when `testable_case` is `Or`.
/// Therefore this must be `Some(_)` after or-pattern expansion.
place: Option<Place<'tcx>>,
enum MatchPairTree<'tcx> {
Testable { testable_match_pair: TestableMatchPairTree<'tcx> },
Or { or_match_pair: OrMatchPairTree<'tcx> },
}

/// ... must pass this test...
#[derive(Debug, Clone)]
struct TestableMatchPairTree<'tcx> {
/// Place to be tested.
place: Place<'tcx>,
/// Test to perform, and the desired outcome.
testable_case: TestableCase<'tcx>,

/// ... and these subpairs must match.
///
/// ---
/// Subpairs typically represent tests that can only be performed after their
/// parent has succeeded. For example, the pattern `Some(3)` might have an
/// outer match pair that tests for the variant `Some`, and then a subpair
/// that tests its field for the value `3`.
subpairs: Vec<Self>,
subpairs: Vec<MatchPairTree<'tcx>>,
/// Span field of the THIR pattern this node was created from.
pattern_span: Span,
}

#[derive(Debug, Clone)]
struct OrMatchPairTree<'tcx> {
/// Subpatterns that are the alternatives of this or-pattern.
or_subpats: Box<[FlatPat<'tcx>]>,
/// Span field of the THIR pattern this node was created from.
pattern_span: Span,
}
Expand Down Expand Up @@ -1779,7 +1797,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
let remainder_start = self.select_matched_candidate(first, start_block);
remainder_start.and(remaining)
}
candidates if candidates.iter().any(|candidate| candidate.starts_with_or_pattern()) => {
candidates if candidates.iter().any(|c| c.match_pairs.starts_with_or_pattern()) => {
// If any candidate starts with an or-pattern, we want to expand or-patterns
// before we do any more tests.
//
Expand Down Expand Up @@ -1882,7 +1900,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
.position(|candidate| {
// If a candidate starts with an or-pattern and has more match pairs,
// we can expand it, but we must stop expanding _after_ it.
candidate.match_pairs.len() > 1 && candidate.starts_with_or_pattern()
candidate.match_pairs.starts_with_or_pattern()
&& candidate.match_pairs.or_match_pairs.len() > 1
})
.map(|pos| pos + 1) // Stop _after_ the found candidate
.unwrap_or(candidates.len()); // Otherwise, include all candidates
Expand All @@ -1893,8 +1912,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
// or-patterns are expanded in their parent's relative position.
let mut expanded_candidates = Vec::new();
for candidate in candidates_to_expand.iter_mut() {
if candidate.starts_with_or_pattern() {
let or_match_pair = candidate.match_pairs.remove(0);
if candidate.match_pairs.starts_with_or_pattern() {
let or_match_pair = candidate.match_pairs.or_match_pairs.remove(0);
// Expand the or-pattern into subcandidates.
self.create_or_subcandidates(candidate, or_match_pair);
// Collect the newly created subcandidates.
Expand Down Expand Up @@ -1948,12 +1967,12 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
fn create_or_subcandidates(
&mut self,
candidate: &mut Candidate<'tcx>,
match_pair: MatchPairTree<'tcx>,
or_match_pair: OrMatchPairTree<'tcx>,
) {
let TestableCase::Or { pats } = match_pair.testable_case else { bug!() };
debug!("expanding or-pattern: candidate={:#?}\npats={:#?}", candidate, pats);
candidate.or_span = Some(match_pair.pattern_span);
candidate.subcandidates = pats
debug!("expanding or-pattern: candidate={candidate:?}, or_match_pair={or_match_pair:?}");
candidate.or_span = Some(or_match_pair.pattern_span);
candidate.subcandidates = or_match_pair
.or_subpats
.into_iter()
.map(|flat_pat| Candidate::from_flat_pat(flat_pat, candidate.has_guard))
.collect();
Expand Down Expand Up @@ -2101,7 +2120,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
scrutinee_span: Span,
candidate: &mut Candidate<'tcx>,
) {
if candidate.match_pairs.is_empty() {
assert!(candidate.match_pairs.testable_match_pairs.is_empty());
if candidate.match_pairs.or_match_pairs.is_empty() {
return;
}

Expand All @@ -2112,14 +2132,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
last_otherwise = leaf_candidate.otherwise_block;
});

let remaining_match_pairs = mem::take(&mut candidate.match_pairs);
// We're testing match pairs that remained after an `Or`, so the remaining
// pairs should all be `Or` too, due to the sorting invariant.
debug_assert!(
remaining_match_pairs
.iter()
.all(|match_pair| matches!(match_pair.testable_case, TestableCase::Or { .. }))
);
let remaining_or_match_pairs = mem::take(&mut candidate.match_pairs.or_match_pairs);

// Visit each leaf candidate within this subtree, add a copy of the remaining
// match pairs to it, and then recursively lower the rest of the match tree
Expand All @@ -2129,7 +2142,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
// and removed, so `extend` and assignment are equivalent,
// but extending can also recycle any existing vector capacity.
assert!(leaf_candidate.match_pairs.is_empty());
leaf_candidate.match_pairs.extend(remaining_match_pairs.iter().cloned());
leaf_candidate.match_pairs.or_match_pairs.extend_from_slice(&remaining_or_match_pairs);

let or_start = leaf_candidate.pre_binding_block.unwrap();
let otherwise =
Expand Down Expand Up @@ -2167,13 +2180,11 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
/// [`Range`]: TestKind::Range
fn pick_test(&mut self, candidates: &[&mut Candidate<'tcx>]) -> (Place<'tcx>, Test<'tcx>) {
// Extract the match-pair from the highest priority candidate
let match_pair = &candidates[0].match_pairs[0];
let match_pair = &candidates[0].match_pairs.testable_match_pairs[0];
let test = self.pick_test_for_match_pair(match_pair);
// Unwrap is ok after simplification.
let match_place = match_pair.place.unwrap();
debug!(?test, ?match_pair);

(match_place, test)
(match_pair.place, test)
}

/// This is the most subtle part of the match lowering algorithm. At this point, there are
Expand Down
11 changes: 2 additions & 9 deletions compiler/rustc_mir_build/src/builder/matches/test.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ use std::sync::Arc;

use rustc_data_structures::fx::FxIndexMap;
use rustc_hir::{LangItem, RangeEnd};
use rustc_middle::bug;
use rustc_middle::mir::*;
use rustc_middle::ty::util::IntTypeExt;
use rustc_middle::ty::{self, GenericArg, Ty, TyCtxt};
Expand All @@ -19,16 +18,14 @@ use tracing::{debug, instrument};

use crate::builder::Builder;
use crate::builder::matches::{
MatchPairTree, PatConstKind, SliceLenOp, Test, TestBranch, TestKind, TestableCase,
PatConstKind, SliceLenOp, Test, TestBranch, TestKind, TestableCase, TestableMatchPairTree,
};

impl<'a, 'tcx> Builder<'a, 'tcx> {
/// Identifies what test is needed to decide if `match_pair` is applicable.
///
/// It is a bug to call this with a not-fully-simplified pattern.
pub(super) fn pick_test_for_match_pair(
&mut self,
match_pair: &MatchPairTree<'tcx>,
match_pair: &TestableMatchPairTree<'tcx>,
) -> Test<'tcx> {
let kind = match match_pair.testable_case {
TestableCase::Variant { adt_def, variant_index: _ } => TestKind::Switch { adt_def },
Expand All @@ -51,10 +48,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
TestableCase::Deref { temp, mutability } => TestKind::Deref { temp, mutability },

TestableCase::Never => TestKind::Never,

// Or-patterns are not tested directly; instead they are expanded into subcandidates,
// which are then distinguished by testing whatever non-or patterns they contain.
TestableCase::Or { .. } => bug!("or-patterns should have already been handled"),
};

Test { span: match_pair.pattern_span, kind }
Expand Down
Loading
Loading