Introduce TestCase enum to replace most matching on PatKind

compiler/rustc_mir_build/src/build/matches/mod.rs

@@ -16,7 +16,7 @@ use rustc_data_structures::{
 };
 use rustc_index::bit_set::BitSet;
 use rustc_middle::middle::region;
-use rustc_middle::mir::*;
+use rustc_middle::mir::{self, *};
 use rustc_middle::thir::{self, *};
 use rustc_middle::ty::{self, CanonicalUserTypeAnnotation, Ty};
 use rustc_span::symbol::Symbol;
@@ -1052,18 +1052,31 @@ struct Ascription<'tcx> {
     variance: ty::Variance,
 }
 
+#[derive(Debug, Clone)]
+enum TestCase<'pat, 'tcx> {
+    Irrefutable,
+    Variant { adt_def: ty::AdtDef<'tcx>, variant_index: VariantIdx },
+    Constant { value: mir::Const<'tcx> },
+    Range(&'pat PatRange<'tcx>),
+    Slice { len: usize, variable_length: bool },
+    Or,
+}
+
 #[derive(Debug, Clone)]
 pub(crate) struct MatchPair<'pat, 'tcx> {
-    // This place...
+    /// This place...
     place: PlaceBuilder<'tcx>,
 
-    // ... must match this pattern.
+    /// ... must pass this test...
-    // Invariant: after creation and simplification in `Candidate::new()`, all match pairs must be
+    // Invariant: after creation and simplification in `Candidate::new()`, this must not be
-    // simplified, i.e. require a test.
+    // `Irrefutable`.
-    pattern: &'pat Pat<'tcx>,
+    test_case: TestCase<'pat, 'tcx>,
 
-    /// Precomputed sub-match pairs of `pattern`.
+    /// ... and these subpairs must match.
     subpairs: Vec<Self>,
+
+    /// The pattern this was created from.
+    pattern: &'pat Pat<'tcx>,
 }
 
 /// See [`Test`] for more.

compiler/rustc_mir_build/src/build/matches/simplify.rs

@@ -13,7 +13,7 @@
 //! testing a value against a constant.
 
 use crate::build::expr::as_place::PlaceBuilder;
-use crate::build::matches::{Ascription, Binding, Candidate, MatchPair};
+use crate::build::matches::{Ascription, Binding, Candidate, MatchPair, TestCase};
 use crate::build::Builder;
 use rustc_infer::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKind};
 use rustc_middle::thir::{self, *};
@@ -128,14 +128,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
         ascriptions: &mut Vec<Ascription<'tcx>>,
         match_pairs: &mut Vec<MatchPair<'pat, 'tcx>>,
     ) -> Result<(), MatchPair<'pat, 'tcx>> {
+        // Collect bindings and ascriptions.
         match match_pair.pattern.kind {
-            PatKind::Leaf { .. }
-            | PatKind::Deref { .. }
-            | PatKind::Array { .. }
-            | PatKind::Never
-            | PatKind::Wild
-            | PatKind::Error(_) => {}
-
             PatKind::AscribeUserType {
                 ascription: thir::Ascription { ref annotation, variance },
                 ..
@@ -203,47 +197,17 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                 }
             }
 
-            PatKind::Constant { .. } => {
+            _ => {}
-                // FIXME normalize patterns when possible
-                return Err(match_pair);
-            }
-
-            PatKind::Range(ref range) => {
-                if range.is_full_range(self.tcx) != Some(true) {
-                    return Err(match_pair);
-                }
-            }
-
-            PatKind::Slice { ref prefix, ref slice, ref suffix } => {
-                if !(prefix.is_empty() && slice.is_some() && suffix.is_empty()) {
-                    self.simplify_match_pairs(&mut match_pair.subpairs, bindings, ascriptions);
-                    return Err(match_pair);
-                }
-            }
-
-            PatKind::Variant { adt_def, args, variant_index, subpatterns: _ } => {
-                let irrefutable = adt_def.variants().iter_enumerated().all(|(i, v)| {
-                    i == variant_index || {
-                        (self.tcx.features().exhaustive_patterns
-                            || self.tcx.features().min_exhaustive_patterns)
-                            && !v
-                                .inhabited_predicate(self.tcx, adt_def)
-                                .instantiate(self.tcx, args)
-                                .apply_ignore_module(self.tcx, self.param_env)
-                    }
-                }) && (adt_def.did().is_local()
-                    || !adt_def.is_variant_list_non_exhaustive());
-                if !irrefutable {
-                    self.simplify_match_pairs(&mut match_pair.subpairs, bindings, ascriptions);
-                    return Err(match_pair);
-                }
-            }
-
-            PatKind::Or { .. } => return Err(match_pair),
         }
 
-        // Simplifiable pattern; we replace it with its subpairs.
+        if let TestCase::Irrefutable = match_pair.test_case {
-        match_pairs.append(&mut match_pair.subpairs);
+            // Simplifiable pattern; we replace it with its subpairs.
-        Ok(())
+            match_pairs.append(&mut match_pair.subpairs);
+            Ok(())
+        } else {
+            // Unsimplifiable pattern; we recursively simplify its subpairs.
+            self.simplify_match_pairs(&mut match_pair.subpairs, bindings, ascriptions);
+            Err(match_pair)
+        }
     }
 }

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

@@ -6,7 +6,7 @@
 // the candidates based on the result.
 
 use crate::build::expr::as_place::PlaceBuilder;
-use crate::build::matches::{Candidate, MatchPair, Test, TestKind};
+use crate::build::matches::{Candidate, MatchPair, Test, TestCase, TestKind};
 use crate::build::Builder;
 use rustc_data_structures::fx::FxIndexMap;
 use rustc_hir::{LangItem, RangeEnd};
@@ -29,12 +29,12 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
     ///
     /// It is a bug to call this with a not-fully-simplified pattern.
     pub(super) fn test<'pat>(&mut self, match_pair: &MatchPair<'pat, 'tcx>) -> Test<'tcx> {
-        let kind = match match_pair.pattern.kind {
+        let kind = match match_pair.test_case {
-            PatKind::Variant { adt_def, args: _, variant_index: _, subpatterns: _ } => {
+            TestCase::Variant { adt_def, variant_index: _ } => {
                 TestKind::Switch { adt_def, variants: BitSet::new_empty(adt_def.variants().len()) }
             }
 
-            PatKind::Constant { .. } if is_switch_ty(match_pair.pattern.ty) => {
+            TestCase::Constant { .. } if is_switch_ty(match_pair.pattern.ty) => {
                 // For integers, we use a `SwitchInt` match, which allows
                 // us to handle more cases.
                 TestKind::SwitchInt {
@@ -46,30 +46,25 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                 }
             }
 
-            PatKind::Constant { value } => TestKind::Eq { value, ty: match_pair.pattern.ty },
+            TestCase::Constant { value } => TestKind::Eq { value, ty: match_pair.pattern.ty },
 
-            PatKind::Range(ref range) => {
+            TestCase::Range(range) => {
                 assert_eq!(range.ty, match_pair.pattern.ty);
-                TestKind::Range(range.clone())
+                TestKind::Range(Box::new(range.clone()))
             }
 
-            PatKind::Slice { ref prefix, ref slice, ref suffix } => {
+            TestCase::Slice { len, variable_length } => {
-                let len = prefix.len() + suffix.len();
+                let op = if variable_length { BinOp::Ge } else { BinOp::Eq };
-                let op = if slice.is_some() { BinOp::Ge } else { BinOp::Eq };
                 TestKind::Len { len: len as u64, op }
             }
 
-            PatKind::Or { .. } => bug!("or-patterns should have already been handled"),
+            TestCase::Or { .. } => bug!("or-patterns should have already been handled"),
 
-            PatKind::AscribeUserType { .. }
+            TestCase::Irrefutable => span_bug!(
-            | PatKind::InlineConstant { .. }
+                match_pair.pattern.span,
-            | PatKind::Array { .. }
+                "simplifiable pattern found: {:?}",
-            | PatKind::Wild
+                match_pair.pattern
-            | PatKind::Binding { .. }
+            ),
-            | PatKind::Never
-            | PatKind::Leaf { .. }
-            | PatKind::Deref { .. }
-            | PatKind::Error(_) => self.error_simplifiable(match_pair),
         };
 
         Test { span: match_pair.pattern.span, kind }
@@ -86,32 +81,20 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
             return false;
         };
 
-        match match_pair.pattern.kind {
+        match match_pair.test_case {
-            PatKind::Constant { value } => {
+            TestCase::Constant { value } => {
                 options.entry(value).or_insert_with(|| value.eval_bits(self.tcx, self.param_env));
                 true
             }
-            PatKind::Variant { .. } => {
+            TestCase::Variant { .. } => {
                 panic!("you should have called add_variants_to_switch instead!");
             }
-            PatKind::Range(ref range) => {
+            TestCase::Range(ref range) => {
                 // Check that none of the switch values are in the range.
                 self.values_not_contained_in_range(&*range, options).unwrap_or(false)
             }
-            PatKind::Slice { .. }
+            // don't know how to add these patterns to a switch
-            | PatKind::Array { .. }
+            _ => false,
-            | PatKind::Wild
-            | PatKind::Never
-            | PatKind::Or { .. }
-            | PatKind::Binding { .. }
-            | PatKind::AscribeUserType { .. }
-            | PatKind::InlineConstant { .. }
-            | PatKind::Leaf { .. }
-            | PatKind::Deref { .. }
-            | PatKind::Error(_) => {
-                // don't know how to add these patterns to a switch
-                false
-            }
         }
     }
 
@@ -126,17 +109,15 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
             return false;
         };
 
-        match match_pair.pattern.kind {
+        match match_pair.test_case {
-            PatKind::Variant { adt_def: _, variant_index, .. } => {
+            TestCase::Variant { variant_index, .. } => {
                 // We have a pattern testing for variant `variant_index`
                 // set the corresponding index to true
                 variants.insert(variant_index);
                 true
             }
-            _ => {
+            // don't know how to add these patterns to a switch
-                // don't know how to add these patterns to a switch
+            _ => false,
-                false
-            }
         }
     }
 
@@ -583,12 +564,12 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
             candidate.match_pairs.iter().enumerate().find(|&(_, mp)| mp.place == *test_place)?;
 
         let fully_matched;
-        let ret = match (&test.kind, &match_pair.pattern.kind) {
+        let ret = match (&test.kind, &match_pair.test_case) {
             // If we are performing a variant switch, then this
             // informs variant patterns, but nothing else.
             (
                 &TestKind::Switch { adt_def: tested_adt_def, .. },
-                &PatKind::Variant { adt_def, variant_index, .. },
+                &TestCase::Variant { adt_def, variant_index },
             ) => {
                 assert_eq!(adt_def, tested_adt_def);
                 fully_matched = true;
@@ -604,14 +585,14 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
             //
             // FIXME(#29623) we could use PatKind::Range to rule
             // things out here, in some cases.
-            (TestKind::SwitchInt { switch_ty: _, options }, PatKind::Constant { value })
+            (TestKind::SwitchInt { switch_ty: _, options }, TestCase::Constant { value })
                 if is_switch_ty(match_pair.pattern.ty) =>
             {
                 fully_matched = true;
                 let index = options.get_index_of(value).unwrap();
                 Some(index)
             }
-            (TestKind::SwitchInt { switch_ty: _, options }, PatKind::Range(range)) => {
+            (TestKind::SwitchInt { switch_ty: _, options }, TestCase::Range(range)) => {
                 fully_matched = false;
                 let not_contained =
                     self.values_not_contained_in_range(&*range, options).unwrap_or(false);
@@ -629,11 +610,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
 
             (
                 &TestKind::Len { len: test_len, op: BinOp::Eq },
-                PatKind::Slice { prefix, slice, suffix },
+                &TestCase::Slice { len, variable_length },
             ) => {
-                let pat_len = (prefix.len() + suffix.len()) as u64;
+                match (test_len.cmp(&(len as u64)), variable_length) {
-                match (test_len.cmp(&pat_len), slice) {
+                    (Ordering::Equal, false) => {
-                    (Ordering::Equal, &None) => {
                         // on true, min_len = len = $actual_length,
                         // on false, len != $actual_length
                         fully_matched = true;
@@ -646,13 +626,13 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                         fully_matched = false;
                         Some(1)
                     }
-                    (Ordering::Equal | Ordering::Greater, &Some(_)) => {
+                    (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.
                         fully_matched = false;
                         None
                     }
-                    (Ordering::Greater, &None) => {
+                    (Ordering::Greater, false) => {
                         // test_len != pat_len, so if $actual_len = test_len, then
                         // $actual_len != pat_len.
                         fully_matched = false;
@@ -662,31 +642,30 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
             }
             (
                 &TestKind::Len { len: test_len, op: BinOp::Ge },
-                PatKind::Slice { prefix, slice, suffix },
+                &TestCase::Slice { len, variable_length },
             ) => {
                 // the test is `$actual_len >= test_len`
-                let pat_len = (prefix.len() + suffix.len()) as u64;
+                match (test_len.cmp(&(len as u64)), variable_length) {
-                match (test_len.cmp(&pat_len), slice) {
+                    (Ordering::Equal, true) => {
-                    (Ordering::Equal, &Some(_)) => {
                         // $actual_len >= test_len = pat_len,
                         // so we can match.
                         fully_matched = true;
                         Some(0)
                     }
-                    (Ordering::Less, _) | (Ordering::Equal, &None) => {
+                    (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).
                         fully_matched = false;
                         Some(0)
                     }
-                    (Ordering::Greater, &None) => {
+                    (Ordering::Greater, false) => {
                         // test_len > pat_len. If $actual_len >= test_len > pat_len,
                         // then we know we won't have a match.
                         fully_matched = false;
                         Some(1)
                     }
-                    (Ordering::Greater, &Some(_)) => {
+                    (Ordering::Greater, true) => {
                         // test_len < pat_len, and is therefore less
                         // strict. This can still go both ways.
                         fully_matched = false;
@@ -695,8 +674,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                 }
             }
 
-            (TestKind::Range(test), PatKind::Range(pat)) => {
+            (TestKind::Range(test), &TestCase::Range(pat)) => {
-                if test == pat {
+                if test.as_ref() == pat {
                     fully_matched = true;
                     Some(0)
                 } else {
@@ -706,7 +685,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                     if !test.overlaps(pat, self.tcx, self.param_env)? { Some(1) } else { None }
                 }
             }
-            (TestKind::Range(range), &PatKind::Constant { value }) => {
+            (TestKind::Range(range), &TestCase::Constant { value }) => {
                 fully_matched = false;
                 if !range.contains(value, self.tcx, self.param_env)? {
                     // `value` is not contained in the testing range,
@@ -729,7 +708,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                 // However, at this point we can still encounter or-patterns that were extracted
                 // from previous calls to `sort_candidate`, so we need to manually address that
                 // case to avoid panicking in `self.test()`.
-                if let PatKind::Or { .. } = &match_pair.pattern.kind {
+                if let TestCase::Or { .. } = &match_pair.test_case {
                     return None;
                 }
 
@@ -752,18 +731,12 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
             let match_pair = candidate.match_pairs.remove(match_pair_index);
             candidate.match_pairs.extend(match_pair.subpairs);
             // Move or-patterns to the end.
-            candidate
+            candidate.match_pairs.sort_by_key(|pair| matches!(pair.test_case, TestCase::Or { .. }));
-                .match_pairs
-                .sort_by_key(|pair| matches!(pair.pattern.kind, PatKind::Or { .. }));
         }
 
         ret
     }
 
-    fn error_simplifiable<'pat>(&mut self, match_pair: &MatchPair<'pat, 'tcx>) -> ! {
-        span_bug!(match_pair.pattern.span, "simplifiable pattern found: {:?}", match_pair.pattern)
-    }
-
     fn values_not_contained_in_range(
         &self,
         range: &PatRange<'tcx>,

compiler/rustc_mir_build/src/build/matches/util.rs

@@ -1,6 +1,5 @@
-use crate::build::expr::as_place::PlaceBase;
+use crate::build::expr::as_place::{PlaceBase, PlaceBuilder};
-use crate::build::expr::as_place::PlaceBuilder;
+use crate::build::matches::{MatchPair, TestCase};
-use crate::build::matches::MatchPair;
 use crate::build::Builder;
 use rustc_middle::mir::*;
 use rustc_middle::thir::*;
@@ -118,16 +117,23 @@ impl<'pat, 'tcx> MatchPair<'pat, 'tcx> {
         }
 
         let mut subpairs = Vec::new();
-        match pattern.kind {
+        let test_case = match pattern.kind {
-            PatKind::Constant { .. }
+            PatKind::Never | PatKind::Wild | PatKind::Error(_) => TestCase::Irrefutable,
-            | PatKind::Range(_)
+            PatKind::Or { .. } => TestCase::Or,
-            | PatKind::Or { .. }
+
-            | PatKind::Never
+            PatKind::Range(ref range) => {
-            | PatKind::Wild
+                if range.is_full_range(cx.tcx) == Some(true) {
-            | PatKind::Error(_) => {}
+                    TestCase::Irrefutable
+                } else {
+                    TestCase::Range(range)
+                }
+            }
+
+            PatKind::Constant { value } => TestCase::Constant { value },
 
             PatKind::AscribeUserType { ref subpattern, .. } => {
                 subpairs.push(MatchPair::new(place.clone(), subpattern, cx));
+                TestCase::Irrefutable
             }
 
             PatKind::Binding { ref subpattern, .. } => {
@@ -135,32 +141,65 @@ impl<'pat, 'tcx> MatchPair<'pat, 'tcx> {
                     // this is the `x @ P` case; have to keep matching against `P` now
                     subpairs.push(MatchPair::new(place.clone(), subpattern, cx));
                 }
+                TestCase::Irrefutable
             }
 
             PatKind::InlineConstant { subpattern: ref pattern, .. } => {
                 subpairs.push(MatchPair::new(place.clone(), pattern, cx));
+                TestCase::Irrefutable
             }
 
-            PatKind::Slice { ref prefix, ref slice, ref suffix }
+            PatKind::Array { ref prefix, ref slice, ref suffix } => {
-            | PatKind::Array { ref prefix, ref slice, ref suffix } => {
                 cx.prefix_slice_suffix(&mut subpairs, &place, prefix, slice, suffix);
+                TestCase::Irrefutable
+            }
+            PatKind::Slice { ref prefix, ref slice, ref suffix } => {
+                cx.prefix_slice_suffix(&mut subpairs, &place, prefix, slice, suffix);
+
+                if prefix.is_empty() && slice.is_some() && suffix.is_empty() {
+                    TestCase::Irrefutable
+                } else {
+                    TestCase::Slice {
+                        len: prefix.len() + suffix.len(),
+                        variable_length: slice.is_some(),
+                    }
+                }
             }
 
-            PatKind::Variant { adt_def, variant_index, ref subpatterns, .. } => {
+            PatKind::Variant { adt_def, variant_index, args, ref subpatterns } => {
                 let downcast_place = place.clone().downcast(adt_def, variant_index); // `(x as Variant)`
                 subpairs = cx.field_match_pairs(downcast_place, subpatterns);
+
+                let irrefutable = adt_def.variants().iter_enumerated().all(|(i, v)| {
+                    i == variant_index || {
+                        (cx.tcx.features().exhaustive_patterns
+                            || cx.tcx.features().min_exhaustive_patterns)
+                            && !v
+                                .inhabited_predicate(cx.tcx, adt_def)
+                                .instantiate(cx.tcx, args)
+                                .apply_ignore_module(cx.tcx, cx.param_env)
+                    }
+                }) && (adt_def.did().is_local()
+                    || !adt_def.is_variant_list_non_exhaustive());
+                if irrefutable {
+                    TestCase::Irrefutable
+                } else {
+                    TestCase::Variant { adt_def, variant_index }
+                }
             }
 
             PatKind::Leaf { ref subpatterns } => {
                 subpairs = cx.field_match_pairs(place.clone(), subpatterns);
+                TestCase::Irrefutable
             }
 
             PatKind::Deref { ref subpattern } => {
                 let place_builder = place.clone().deref();
                 subpairs.push(MatchPair::new(place_builder, subpattern, cx));
+                TestCase::Irrefutable
             }
-        }
+        };
 
-        MatchPair { place, pattern, subpairs }
+        MatchPair { place, test_case, subpairs, pattern }
     }
 }