From 2d5b31af806d725db5294be883204e8c8c22ab5c Mon Sep 17 00:00:00 2001 From: Zalathar Date: Fri, 26 Dec 2025 16:31:29 +1100 Subject: [PATCH 1/4] Hoist `Test` struct above `TestKind` enum, and adjust comments This makes it easier to see that `Test` is just a span plus a test kind, and `TestKind` is where all the interesting details are. --- .../src/builder/matches/mod.rs | 26 +++++++++++-------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/compiler/rustc_mir_build/src/builder/matches/mod.rs b/compiler/rustc_mir_build/src/builder/matches/mod.rs index 1da983462887..424ff515ca5f 100644 --- a/compiler/rustc_mir_build/src/builder/matches/mod.rs +++ b/compiler/rustc_mir_build/src/builder/matches/mod.rs @@ -1332,7 +1332,21 @@ pub(crate) struct MatchPairTree<'tcx> { pattern_span: Span, } -/// See [`Test`] for more. +/// A runtime test to perform to determine which candidates match a scrutinee place. +/// +/// The kind of test to perform is indicated by [`TestKind`]. +#[derive(Debug)] +pub(crate) struct Test<'tcx> { + span: Span, + kind: TestKind<'tcx>, +} + +/// The kind of runtime test to perform to determine which candidates match a +/// scrutinee place. This is the main component of [`Test`]. +/// +/// Some of these variants don't contain the constant value(s) being tested +/// against, because those values are stored in the corresponding bucketed +/// candidates instead. #[derive(Clone, Debug, PartialEq)] enum TestKind<'tcx> { /// Test what enum variant a value is. @@ -1381,16 +1395,6 @@ enum TestKind<'tcx> { Never, } -/// A test to perform to determine which [`Candidate`] matches a value. -/// -/// [`Test`] is just the test to perform; it does not include the value -/// to be tested. -#[derive(Debug)] -pub(crate) struct Test<'tcx> { - span: Span, - kind: TestKind<'tcx>, -} - /// The branch to be taken after a test. #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] enum TestBranch<'tcx> { From 197b5efb036cf328aad63ca170fa55572aea8cbc Mon Sep 17 00:00:00 2001 From: Zalathar Date: Sat, 27 Dec 2025 15:02:07 +1100 Subject: [PATCH 2/4] Rename `TestKind::Len` to `SliceLen` --- compiler/rustc_mir_build/src/builder/matches/buckets.rs | 6 +++--- compiler/rustc_mir_build/src/builder/matches/mod.rs | 2 +- compiler/rustc_mir_build/src/builder/matches/test.rs | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_mir_build/src/builder/matches/buckets.rs b/compiler/rustc_mir_build/src/builder/matches/buckets.rs index f8af50ee52fe..9a227486a4f1 100644 --- a/compiler/rustc_mir_build/src/builder/matches/buckets.rs +++ b/compiler/rustc_mir_build/src/builder/matches/buckets.rs @@ -213,7 +213,7 @@ fn choose_bucket_for_candidate( } ( - &TestKind::Len { len: test_len, op: BinOp::Eq }, + &TestKind::SliceLen { len: test_len, op: BinOp::Eq }, &TestableCase::Slice { len, variable_length }, ) => { match (test_len.cmp(&len), variable_length) { @@ -245,7 +245,7 @@ fn choose_bucket_for_candidate( } } ( - &TestKind::Len { len: test_len, op: BinOp::Ge }, + &TestKind::SliceLen { len: test_len, op: BinOp::Ge }, &TestableCase::Slice { len, variable_length }, ) => { // the test is `$actual_len >= test_len` @@ -338,7 +338,7 @@ fn choose_bucket_for_candidate( TestKind::Switch { .. } | TestKind::SwitchInt { .. } | TestKind::If - | TestKind::Len { .. } + | TestKind::SliceLen { .. } | TestKind::Range { .. } | TestKind::Eq { .. } | TestKind::Deref { .. }, diff --git a/compiler/rustc_mir_build/src/builder/matches/mod.rs b/compiler/rustc_mir_build/src/builder/matches/mod.rs index 424ff515ca5f..492a01aa27f1 100644 --- a/compiler/rustc_mir_build/src/builder/matches/mod.rs +++ b/compiler/rustc_mir_build/src/builder/matches/mod.rs @@ -1382,7 +1382,7 @@ enum TestKind<'tcx> { Range(Arc>), /// Test that the length of the slice is `== len` or `>= len`. - Len { len: u64, op: BinOp }, + SliceLen { len: u64, op: BinOp }, /// Call `Deref::deref[_mut]` on the value. Deref { diff --git a/compiler/rustc_mir_build/src/builder/matches/test.rs b/compiler/rustc_mir_build/src/builder/matches/test.rs index 4a2823bf5d6b..853a7fafc728 100644 --- a/compiler/rustc_mir_build/src/builder/matches/test.rs +++ b/compiler/rustc_mir_build/src/builder/matches/test.rs @@ -52,7 +52,7 @@ pub(super) fn pick_test_for_match_pair( TestableCase::Slice { len, variable_length } => { let op = if variable_length { BinOp::Ge } else { BinOp::Eq }; - TestKind::Len { len, op } + TestKind::SliceLen { len, op } } TestableCase::Deref { temp, mutability } => TestKind::Deref { temp, mutability }, @@ -312,7 +312,7 @@ pub(super) fn perform_test( } } - TestKind::Len { len, op } => { + TestKind::SliceLen { len, op } => { let usize_ty = self.tcx.types.usize; let actual = self.temp(usize_ty, test.span); From c62772a7a6e42a69ebcdd2670bdde4d75c4a325b Mon Sep 17 00:00:00 2001 From: Zalathar Date: Fri, 26 Dec 2025 16:36:23 +1100 Subject: [PATCH 3/4] Add a `SliceLenOp` enum for use by slice-length cases/tests --- .../src/builder/matches/buckets.rs | 32 ++++++++++--------- .../src/builder/matches/match_pair.rs | 13 ++++++-- .../src/builder/matches/mod.rs | 15 +++++++-- .../src/builder/matches/test.rs | 12 +++---- 4 files changed, 47 insertions(+), 25 deletions(-) diff --git a/compiler/rustc_mir_build/src/builder/matches/buckets.rs b/compiler/rustc_mir_build/src/builder/matches/buckets.rs index 9a227486a4f1..4201f8f78c16 100644 --- a/compiler/rustc_mir_build/src/builder/matches/buckets.rs +++ b/compiler/rustc_mir_build/src/builder/matches/buckets.rs @@ -1,12 +1,14 @@ use std::cmp::Ordering; use rustc_data_structures::fx::FxIndexMap; -use rustc_middle::mir::{BinOp, Place}; +use rustc_middle::mir::Place; use rustc_middle::span_bug; use tracing::debug; use crate::builder::Builder; -use crate::builder::matches::{Candidate, PatConstKind, Test, TestBranch, TestKind, TestableCase}; +use crate::builder::matches::{ + Candidate, PatConstKind, SliceLenOp, Test, TestBranch, TestKind, TestableCase, +}; /// Output of [`Builder::partition_candidates_into_buckets`]. pub(crate) struct PartitionedCandidates<'tcx, 'b, 'c> { @@ -213,11 +215,11 @@ fn choose_bucket_for_candidate( } ( - &TestKind::SliceLen { len: test_len, op: BinOp::Eq }, - &TestableCase::Slice { len, variable_length }, + &TestKind::SliceLen { len: test_len, op: SliceLenOp::Equal }, + &TestableCase::Slice { len: pat_len, op: pat_op }, ) => { - match (test_len.cmp(&len), variable_length) { - (Ordering::Equal, false) => { + match (test_len.cmp(&pat_len), pat_op) { + (Ordering::Equal, SliceLenOp::Equal) => { // on true, min_len = len = $actual_length, // on false, len != $actual_length fully_matched = true; @@ -230,13 +232,13 @@ fn choose_bucket_for_candidate( fully_matched = false; Some(TestBranch::Failure) } - (Ordering::Equal | Ordering::Greater, true) => { + (Ordering::Equal | Ordering::Greater, SliceLenOp::GreaterOrEqual) => { // This can match both if $actual_len = test_len >= pat_len, // and if $actual_len > test_len. We can't advance. fully_matched = false; None } - (Ordering::Greater, false) => { + (Ordering::Greater, SliceLenOp::Equal) => { // test_len != pat_len, so if $actual_len = test_len, then // $actual_len != pat_len. fully_matched = false; @@ -245,31 +247,31 @@ fn choose_bucket_for_candidate( } } ( - &TestKind::SliceLen { len: test_len, op: BinOp::Ge }, - &TestableCase::Slice { len, variable_length }, + &TestKind::SliceLen { len: test_len, op: SliceLenOp::GreaterOrEqual }, + &TestableCase::Slice { len: pat_len, op: pat_op }, ) => { // the test is `$actual_len >= test_len` - match (test_len.cmp(&len), variable_length) { - (Ordering::Equal, true) => { + match (test_len.cmp(&pat_len), pat_op) { + (Ordering::Equal, SliceLenOp::GreaterOrEqual) => { // $actual_len >= test_len = pat_len, // so we can match. fully_matched = true; Some(TestBranch::Success) } - (Ordering::Less, _) | (Ordering::Equal, false) => { + (Ordering::Less, _) | (Ordering::Equal, SliceLenOp::Equal) => { // test_len <= pat_len. If $actual_len < test_len, // then it is also < pat_len, so the test passing is // necessary (but insufficient). fully_matched = false; Some(TestBranch::Success) } - (Ordering::Greater, false) => { + (Ordering::Greater, SliceLenOp::Equal) => { // test_len > pat_len. If $actual_len >= test_len > pat_len, // then we know we won't have a match. fully_matched = false; Some(TestBranch::Failure) } - (Ordering::Greater, true) => { + (Ordering::Greater, SliceLenOp::GreaterOrEqual) => { // test_len < pat_len, and is therefore less // strict. This can still go both ways. fully_matched = false; diff --git a/compiler/rustc_mir_build/src/builder/matches/match_pair.rs b/compiler/rustc_mir_build/src/builder/matches/match_pair.rs index 798110c8b090..386ca61a6124 100644 --- a/compiler/rustc_mir_build/src/builder/matches/match_pair.rs +++ b/compiler/rustc_mir_build/src/builder/matches/match_pair.rs @@ -8,7 +8,7 @@ use crate::builder::Builder; use crate::builder::expr::as_place::{PlaceBase, PlaceBuilder}; use crate::builder::matches::{ - FlatPat, MatchPairTree, PatConstKind, PatternExtraData, TestableCase, + FlatPat, MatchPairTree, PatConstKind, PatternExtraData, SliceLenOp, TestableCase, }; impl<'a, 'tcx> Builder<'a, 'tcx> { @@ -277,11 +277,20 @@ pub(super) fn for_pattern( ); if prefix.is_empty() && slice.is_some() && suffix.is_empty() { + // This pattern is shaped like `[..]`. It can match a slice + // of any length, so no length test is needed. None } else { + // Any other shape of slice pattern requires a length test. + // Slice patterns with a `..` subpattern require a minimum + // length; those without `..` require an exact length. Some(TestableCase::Slice { len: u64::try_from(prefix.len() + suffix.len()).unwrap(), - variable_length: slice.is_some(), + op: if slice.is_some() { + SliceLenOp::GreaterOrEqual + } else { + SliceLenOp::Equal + }, }) } } diff --git a/compiler/rustc_mir_build/src/builder/matches/mod.rs b/compiler/rustc_mir_build/src/builder/matches/mod.rs index 492a01aa27f1..421065a89411 100644 --- a/compiler/rustc_mir_build/src/builder/matches/mod.rs +++ b/compiler/rustc_mir_build/src/builder/matches/mod.rs @@ -1264,7 +1264,7 @@ enum TestableCase<'tcx> { Variant { adt_def: ty::AdtDef<'tcx>, variant_index: VariantIdx }, Constant { value: ty::Value<'tcx>, kind: PatConstKind }, Range(Arc>), - Slice { len: u64, variable_length: bool }, + Slice { len: u64, op: SliceLenOp }, Deref { temp: Place<'tcx>, mutability: Mutability }, Never, Or { pats: Box<[FlatPat<'tcx>]> }, @@ -1382,7 +1382,7 @@ enum TestKind<'tcx> { Range(Arc>), /// Test that the length of the slice is `== len` or `>= len`. - SliceLen { len: u64, op: BinOp }, + SliceLen { len: u64, op: SliceLenOp }, /// Call `Deref::deref[_mut]` on the value. Deref { @@ -1395,6 +1395,17 @@ enum TestKind<'tcx> { Never, } +/// Indicates the kind of slice-length constraint imposed by a slice pattern, +/// or its corresponding test. +#[derive(Debug, Clone, Copy, PartialEq)] +enum SliceLenOp { + /// The slice pattern can only match a slice with exactly `len` elements. + Equal, + /// The slice pattern can match a slice with `len` or more elements + /// (i.e. it contains a `..` subpattern in the middle). + GreaterOrEqual, +} + /// The branch to be taken after a test. #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] enum TestBranch<'tcx> { diff --git a/compiler/rustc_mir_build/src/builder/matches/test.rs b/compiler/rustc_mir_build/src/builder/matches/test.rs index 853a7fafc728..cac4f7b7ab4f 100644 --- a/compiler/rustc_mir_build/src/builder/matches/test.rs +++ b/compiler/rustc_mir_build/src/builder/matches/test.rs @@ -20,7 +20,7 @@ use crate::builder::Builder; use crate::builder::matches::{ - MatchPairTree, PatConstKind, Test, TestBranch, TestKind, TestableCase, + MatchPairTree, PatConstKind, SliceLenOp, Test, TestBranch, TestKind, TestableCase, }; impl<'a, 'tcx> Builder<'a, 'tcx> { @@ -50,10 +50,7 @@ pub(super) fn pick_test_for_match_pair( TestKind::Range(Arc::clone(range)) } - TestableCase::Slice { len, variable_length } => { - let op = if variable_length { BinOp::Ge } else { BinOp::Eq }; - TestKind::SliceLen { len, op } - } + TestableCase::Slice { len, op } => TestKind::SliceLen { len, op }, TestableCase::Deref { temp, mutability } => TestKind::Deref { temp, mutability }, @@ -332,7 +329,10 @@ pub(super) fn perform_test( success_block, fail_block, source_info, - op, + match op { + SliceLenOp::Equal => BinOp::Eq, + SliceLenOp::GreaterOrEqual => BinOp::Ge, + }, Operand::Move(actual), Operand::Move(expected), ); From cfcb4169b35f6733882c3785551c75e7b4732f3d Mon Sep 17 00:00:00 2001 From: Zalathar Date: Sat, 27 Dec 2025 15:08:42 +1100 Subject: [PATCH 4/4] Overhaul comments for bucketing slice-length cases/tests --- .../src/builder/matches/buckets.rs | 43 +++++++++++-------- 1 file changed, 24 insertions(+), 19 deletions(-) diff --git a/compiler/rustc_mir_build/src/builder/matches/buckets.rs b/compiler/rustc_mir_build/src/builder/matches/buckets.rs index 4201f8f78c16..fce35aa9ef30 100644 --- a/compiler/rustc_mir_build/src/builder/matches/buckets.rs +++ b/compiler/rustc_mir_build/src/builder/matches/buckets.rs @@ -214,33 +214,40 @@ fn choose_bucket_for_candidate( Some(if value { TestBranch::Success } else { TestBranch::Failure }) } + // Determine how the proposed slice-length test interacts with the + // slice pattern we're currently looking at. + // + // Keep in mind the invariant that a case is not allowed to succeed + // on multiple arms of the same test. For example, even though the + // test `len == 4` logically implies `len >= 4` on its success arm, + // the case `len >= 4` could also succeed on the test's failure arm, + // so it can't be included in the success bucket or failure bucket. ( &TestKind::SliceLen { len: test_len, op: SliceLenOp::Equal }, &TestableCase::Slice { len: pat_len, op: pat_op }, ) => { match (test_len.cmp(&pat_len), pat_op) { (Ordering::Equal, SliceLenOp::Equal) => { - // on true, min_len = len = $actual_length, - // on false, len != $actual_length + // E.g. test is `len == 4` and pattern is `len == 4`. + // Pattern is fully matched on the success arm. fully_matched = true; Some(TestBranch::Success) } (Ordering::Less, _) => { - // test_len < pat_len. If $actual_len = test_len, - // then $actual_len < pat_len and we don't have - // enough elements. + // E.g. test is `len == 4` and pattern is `len == 5` or `len >= 5`. + // Pattern can only succeed on the failure arm, but isn't fully matched. fully_matched = false; Some(TestBranch::Failure) } (Ordering::Equal | Ordering::Greater, SliceLenOp::GreaterOrEqual) => { - // This can match both if $actual_len = test_len >= pat_len, - // and if $actual_len > test_len. We can't advance. + // E.g. test is `len == 4` and pattern is `len >= 4` or `len >= 3`. + // Pattern could succeed on both arms, so it can't be bucketed. fully_matched = false; None } (Ordering::Greater, SliceLenOp::Equal) => { - // test_len != pat_len, so if $actual_len = test_len, then - // $actual_len != pat_len. + // E.g. test is `len == 4` and pattern is `len == 3`. + // Pattern can only succeed on the failure arm, but isn't fully matched. fully_matched = false; Some(TestBranch::Failure) } @@ -250,30 +257,28 @@ fn choose_bucket_for_candidate( &TestKind::SliceLen { len: test_len, op: SliceLenOp::GreaterOrEqual }, &TestableCase::Slice { len: pat_len, op: pat_op }, ) => { - // the test is `$actual_len >= test_len` match (test_len.cmp(&pat_len), pat_op) { (Ordering::Equal, SliceLenOp::GreaterOrEqual) => { - // $actual_len >= test_len = pat_len, - // so we can match. + // E.g. test is `len >= 4` and pattern is `len >= 4`. + // Pattern is fully matched on the success arm. fully_matched = true; Some(TestBranch::Success) } (Ordering::Less, _) | (Ordering::Equal, SliceLenOp::Equal) => { - // test_len <= pat_len. If $actual_len < test_len, - // then it is also < pat_len, so the test passing is - // necessary (but insufficient). + // E.g. test is `len >= 4` and pattern is `len == 5` or `len >= 5` or `len == 4`. + // Pattern can only succeed on the success arm, but isn't fully matched. fully_matched = false; Some(TestBranch::Success) } (Ordering::Greater, SliceLenOp::Equal) => { - // test_len > pat_len. If $actual_len >= test_len > pat_len, - // then we know we won't have a match. + // E.g. test is `len >= 4` and pattern is `len == 3`. + // Pattern can only succeed on the failure arm, but isn't fully matched. fully_matched = false; Some(TestBranch::Failure) } (Ordering::Greater, SliceLenOp::GreaterOrEqual) => { - // test_len < pat_len, and is therefore less - // strict. This can still go both ways. + // E.g. test is `len >= 4` and pattern is `len >= 3`. + // Pattern could succeed on both arms, so it can't be bucketed. fully_matched = false; None }