Rollup merge of #150422 - Zalathar:slice-len, r=Nadrieril

mir_build: Add a `SliceLenOp` enum for use by slice-length cases/tests

The main change in this PR introduces a `SliceLenOp` enum, to be used by the enum variants for slice-length tests in match lowering.

Slice-length tests must distinguish between `==` tests, and `>=` tests. The existing code represents this distinction with either a boolean (in `TestableCase`) or a `mir::BinOp` (in `TestKind`). Using a dedicated enum makes it easier to see what the two possible values represent.

This PR also includes a few relevant cleanups that would otherwise conflict.

r? Nadrieril

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> {
@@ -212,66 +214,71 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                 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::Len { 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) => {
-                        // on true, min_len = len = $actual_length,
-                        // on false, len != $actual_length
+                match (test_len.cmp(&pat_len), pat_op) {
+                    (Ordering::Equal, SliceLenOp::Equal) => {
+                        // 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, true) => {
-                        // This can match both if $actual_len = test_len >= pat_len,
-                        // and if $actual_len > test_len. We can't advance.
+                    (Ordering::Equal | Ordering::Greater, SliceLenOp::GreaterOrEqual) => {
+                        // 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, false) => {
-                        // test_len != pat_len, so if $actual_len = test_len, then
-                        // $actual_len != pat_len.
+                    (Ordering::Greater, SliceLenOp::Equal) => {
+                        // 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)
                     }
                 }
             }
             (
-                &TestKind::Len { 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) => {
-                        // $actual_len >= test_len = pat_len,
-                        // so we can match.
+                match (test_len.cmp(&pat_len), pat_op) {
+                    (Ordering::Equal, SliceLenOp::GreaterOrEqual) => {
+                        // 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, false) => {
-                        // test_len <= pat_len. If $actual_len < test_len,
-                        // then it is also < pat_len, so the test passing is
-                        // necessary (but insufficient).
+                    (Ordering::Less, _) | (Ordering::Equal, SliceLenOp::Equal) => {
+                        // 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, false) => {
-                        // test_len > pat_len. If $actual_len >= test_len > pat_len,
-                        // then we know we won't have a match.
+                    (Ordering::Greater, SliceLenOp::Equal) => {
+                        // 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, true) => {
-                        // test_len < pat_len, and is therefore less
-                        // strict. This can still go both ways.
+                    (Ordering::Greater, SliceLenOp::GreaterOrEqual) => {
+                        // 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
                     }
@@ -338,7 +345,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                 TestKind::Switch { .. }
                 | TestKind::SwitchInt { .. }
                 | TestKind::If
-                | TestKind::Len { .. }
+                | TestKind::SliceLen { .. }
                 | TestKind::Range { .. }
                 | TestKind::Eq { .. }
                 | TestKind::Deref { .. },

compiler/rustc_mir_build/src/builder/matches/match_pair.rs

@@ -8,7 +8,7 @@ use rustc_middle::ty::{self, Ty, TypeVisitableExt};
 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 @@ impl<'tcx> MatchPairTree<'tcx> {
                 );
 
                 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
+                        },
                     })
                 }
             }

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<PatRange<'tcx>>),
-    Slice { len: u64, variable_length: bool },
+    Slice { len: u64, op: SliceLenOp },
     Deref { temp: Place<'tcx>, mutability: Mutability },
     Never,
     Or { pats: Box<[FlatPat<'tcx>]> },
@@ -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.
@@ -1368,7 +1382,7 @@ enum TestKind<'tcx> {
     Range(Arc<PatRange<'tcx>>),
 
     /// Test that the length of the slice is `== len` or `>= len`.
-    Len { len: u64, op: BinOp },
+    SliceLen { len: u64, op: SliceLenOp },
 
     /// Call `Deref::deref[_mut]` on the value.
     Deref {
@@ -1381,14 +1395,15 @@ 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>,
+/// 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.

compiler/rustc_mir_build/src/builder/matches/test.rs

@@ -20,7 +20,7 @@ use tracing::{debug, instrument};
 
 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 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                 TestKind::Range(Arc::clone(range))
             }
 
-            TestableCase::Slice { len, variable_length } => {
-                let op = if variable_length { BinOp::Ge } else { BinOp::Eq };
-                TestKind::Len { len, op }
-            }
+            TestableCase::Slice { len, op } => TestKind::SliceLen { len, op },
 
             TestableCase::Deref { temp, mutability } => TestKind::Deref { temp, mutability },
 
@@ -312,7 +309,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                 }
             }
 
-            TestKind::Len { len, op } => {
+            TestKind::SliceLen { len, op } => {
                 let usize_ty = self.tcx.types.usize;
                 let actual = self.temp(usize_ty, test.span);
 
@@ -332,7 +329,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                     success_block,
                     fail_block,
                     source_info,
-                    op,
+                    match op {
+                        SliceLenOp::Equal => BinOp::Eq,
+                        SliceLenOp::GreaterOrEqual => BinOp::Ge,
+                    },
                     Operand::Move(actual),
                     Operand::Move(expected),
                 );