Auto merge of #67668 - matthewjasper:or-patterns, r=pnkfelix

Implement MIR lowering for or-patterns

This is the last thing needed to get meaningful run-pass tests for or-patterns. There probably need to be more tests before stabilizing this, but the most important cases should have been covered.

Note: we can generate exponentially large MIR CFGs when using or-patterns containing bindings, type ascriptions, or that are for a match arm with a guard. `src/test/mir-opt/exponential-or.rs` shows the best case for what we currently do.

cc #54883
closes #60350
closes #67514

cc @Centril
r? @pnkfelix

Author: bors

Diff for: src/librustc_feature/active.rs

@@ -556,7 +556,6 @@ pub const INCOMPLETE_FEATURES: &[Symbol] = &[
     sym::impl_trait_in_bindings,
     sym::generic_associated_types,
     sym::const_generics,
-    sym::or_patterns,
     sym::let_chains,
     sym::raw_dylib,
     sym::const_trait_impl,

Diff for: src/librustc_mir_build/build/block.rs

@@ -149,7 +149,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                             &pattern,
                             UserTypeProjections::none(),
                             &mut |this, _, _, _, node, span, _, _| {
-                                this.storage_live_binding(block, node, span, OutsideGuard);
+                                this.storage_live_binding(block, node, span, OutsideGuard, true);
                                 this.schedule_drop_for_binding(node, span, OutsideGuard);
                             },
                         )

Diff for: src/librustc_mir_build/build/matches/mod.rs

@@ -16,6 +16,7 @@ use crate::build::matches::{Ascription, Binding, Candidate, MatchPair};
 use crate::build::Builder;
 use crate::hair::{self, *};
 use rustc::mir::interpret::truncate;
+use rustc::mir::Place;
 use rustc::ty;
 use rustc::ty::layout::{Integer, IntegerExt, Size};
 use rustc_attr::{SignedInt, UnsignedInt};
@@ -24,10 +25,33 @@ use rustc_hir::RangeEnd;
 use std::mem;
 
 impl<'a, 'tcx> Builder<'a, 'tcx> {
-    crate fn simplify_candidate<'pat>(&mut self, candidate: &mut Candidate<'pat, 'tcx>) {
+    /// Simplify a candidate so that all match pairs require a test.
+    ///
+    /// This method will also split a candidate where the only match-pair is an
+    /// or-pattern into multiple candidates. This is so that
+    ///
+    /// match x {
+    ///     0 | 1 => { ... },
+    ///     2 | 3 => { ... },
+    /// }
+    ///
+    /// only generates a single switch. If this happens this method returns
+    /// `true`.
+    pub(super) fn simplify_candidate<'pat>(
+        &mut self,
+        candidate: &mut Candidate<'pat, 'tcx>,
+    ) -> bool {
         // repeatedly simplify match pairs until fixed point is reached
         loop {
             let match_pairs = mem::take(&mut candidate.match_pairs);
+
+            if let [MatchPair { pattern: Pat { kind: box PatKind::Or { pats }, .. }, place }] =
+                *match_pairs
+            {
+                candidate.subcandidates = self.create_or_subcandidates(candidate, place, pats);
+                return true;
+            }
+
             let mut changed = false;
             for match_pair in match_pairs {
                 match self.simplify_match_pair(match_pair, candidate) {
@@ -40,11 +64,35 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                 }
             }
             if !changed {
-                return; // if we were not able to simplify any, done.
+                // Move or-patterns to the end, because they can result in us
+                // creating additional candidates, so we want to test them as
+                // late as possible.
+                candidate
+                    .match_pairs
+                    .sort_by_key(|pair| matches!(*pair.pattern.kind, PatKind::Or { .. }));
+                return false; // if we were not able to simplify any, done.
             }
         }
     }
 
+    /// Given `candidate` that has a single or-pattern for its match-pairs,
+    /// creates a fresh candidate for each of its input subpatterns passed via
+    /// `pats`.
+    fn create_or_subcandidates<'pat>(
+        &mut self,
+        candidate: &Candidate<'pat, 'tcx>,
+        place: Place<'tcx>,
+        pats: &'pat [Pat<'tcx>],
+    ) -> Vec<Candidate<'pat, 'tcx>> {
+        pats.iter()
+            .map(|pat| {
+                let mut candidate = Candidate::new(place, pat, candidate.has_guard);
+                self.simplify_candidate(&mut candidate);
+                candidate
+            })
+            .collect()
+    }
+
     /// Tries to simplify `match_pair`, returning `Ok(())` if
     /// successful. If successful, new match pairs and bindings will
     /// have been pushed into the candidate. If no simplification is

Diff for: src/librustc_mir_build/build/matches/simplify.rs

@@ -24,7 +24,7 @@ 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 simplifiable pattern.
-    crate fn test<'pat>(&mut self, match_pair: &MatchPair<'pat, 'tcx>) -> Test<'tcx> {
+    pub(super) fn test<'pat>(&mut self, match_pair: &MatchPair<'pat, 'tcx>) -> Test<'tcx> {
         match *match_pair.pattern.kind {
             PatKind::Variant { ref adt_def, substs: _, variant_index: _, subpatterns: _ } => Test {
                 span: match_pair.pattern.span,
@@ -70,11 +70,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                 }
             }
 
-            PatKind::Or { .. } => self
-                .hir
-                .tcx()
-                .sess
-                .span_fatal(match_pair.pattern.span, "or-patterns are not fully implemented yet"),
+            PatKind::Or { .. } => bug!("or-patterns should have already been handled"),
 
             PatKind::AscribeUserType { .. }
             | PatKind::Array { .. }
@@ -85,7 +81,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
         }
     }
 
-    crate fn add_cases_to_switch<'pat>(
+    pub(super) fn add_cases_to_switch<'pat>(
         &mut self,
         test_place: &Place<'tcx>,
         candidate: &Candidate<'pat, 'tcx>,
@@ -129,7 +125,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
         }
     }
 
-    crate fn add_variants_to_switch<'pat>(
+    pub(super) fn add_variants_to_switch<'pat>(
         &mut self,
         test_place: &Place<'tcx>,
         candidate: &Candidate<'pat, 'tcx>,
@@ -156,10 +152,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
         }
     }
 
-    crate fn perform_test(
+    pub(super) fn perform_test(
         &mut self,
         block: BasicBlock,
-        place: &Place<'tcx>,
+        place: Place<'tcx>,
         test: &Test<'tcx>,
         make_target_blocks: impl FnOnce(&mut Self) -> Vec<BasicBlock>,
     ) {
@@ -209,7 +205,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                 );
                 let discr_ty = adt_def.repr.discr_type().to_ty(tcx);
                 let discr = self.temp(discr_ty, test.span);
-                self.cfg.push_assign(block, source_info, &discr, Rvalue::Discriminant(*place));
+                self.cfg.push_assign(block, source_info, &discr, Rvalue::Discriminant(place));
                 assert_eq!(values.len() + 1, targets.len());
                 self.cfg.terminate(
                     block,
@@ -233,20 +229,15 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                             0 => (second_bb, first_bb),
                             v => span_bug!(test.span, "expected boolean value but got {:?}", v),
                         };
-                        TerminatorKind::if_(
-                            self.hir.tcx(),
-                            Operand::Copy(*place),
-                            true_bb,
-                            false_bb,
-                        )
+                        TerminatorKind::if_(self.hir.tcx(), Operand::Copy(place), true_bb, false_bb)
                     } else {
                         bug!("`TestKind::SwitchInt` on `bool` should have two targets")
                     }
                 } else {
                     // The switch may be inexhaustive so we have a catch all block
                     debug_assert_eq!(options.len() + 1, target_blocks.len());
                     TerminatorKind::SwitchInt {
-                        discr: Operand::Copy(*place),
+                        discr: Operand::Copy(place),
                         switch_ty,
                         values: options.clone().into(),
                         targets: target_blocks,
@@ -271,7 +262,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                     if let [success, fail] = *make_target_blocks(self) {
                         assert_eq!(value.ty, ty);
                         let expect = self.literal_operand(test.span, value);
-                        let val = Operand::Copy(*place);
+                        let val = Operand::Copy(place);
                         self.compare(block, success, fail, source_info, BinOp::Eq, expect, val);
                     } else {
                         bug!("`TestKind::Eq` should have two target blocks");
@@ -286,7 +277,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                 // Test `val` by computing `lo <= val && val <= hi`, using primitive comparisons.
                 let lo = self.literal_operand(test.span, lo);
                 let hi = self.literal_operand(test.span, hi);
-                let val = Operand::Copy(*place);
+                let val = Operand::Copy(place);
 
                 if let [success, fail] = *target_blocks {
                     self.compare(
@@ -315,7 +306,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                 let actual = self.temp(usize_ty, test.span);
 
                 // actual = len(place)
-                self.cfg.push_assign(block, source_info, &actual, Rvalue::Len(*place));
+                self.cfg.push_assign(block, source_info, &actual, Rvalue::Len(place));
 
                 // expected = <N>
                 let expected = self.push_usize(block, source_info, len);
@@ -371,13 +362,13 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
         make_target_blocks: impl FnOnce(&mut Self) -> Vec<BasicBlock>,
         source_info: SourceInfo,
         value: &'tcx ty::Const<'tcx>,
-        place: &Place<'tcx>,
+        place: Place<'tcx>,
         mut ty: Ty<'tcx>,
     ) {
         use rustc::middle::lang_items::EqTraitLangItem;
 
         let mut expect = self.literal_operand(source_info.span, value);
-        let mut val = Operand::Copy(*place);
+        let mut val = Operand::Copy(place);
 
         // If we're using `b"..."` as a pattern, we need to insert an
         // unsizing coercion, as the byte string has the type `&[u8; N]`.
@@ -502,7 +493,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
     /// that it *doesn't* apply. For now, we return false, indicate that the
     /// test does not apply to this candidate, but it might be we can get
     /// tighter match code if we do something a bit different.
-    crate fn sort_candidate<'pat>(
+    pub(super) fn sort_candidate<'pat>(
         &mut self,
         test_place: &Place<'tcx>,
         test: &Test<'tcx>,
@@ -755,8 +746,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
         let downcast_place = tcx.mk_place_elem(match_pair.place, elem); // `(x as Variant)`
         let consequent_match_pairs = subpatterns.iter().map(|subpattern| {
             // e.g., `(x as Variant).0`
-            let place =
-                tcx.mk_place_field(downcast_place.clone(), subpattern.field, subpattern.pattern.ty);
+            let place = tcx.mk_place_field(downcast_place, subpattern.field, subpattern.pattern.ty);
             // e.g., `(x as Variant).0 @ P1`
             MatchPair::new(place, &subpattern.pattern)
         });

Diff for: src/librustc_mir_build/build/matches/test.rs

@@ -899,7 +899,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                             matches::ArmHasGuard(false),
                             Some((Some(&place), span)),
                         );
-                        unpack!(block = self.place_into_pattern(block, pattern, &place, false));
+                        unpack!(block = self.place_into_pattern(block, pattern, place, false));
                     }
                 }
                 self.source_scope = original_source_scope;

Diff for: src/librustc_mir_build/build/mod.rs

@@ -315,17 +315,6 @@ crate struct Arm<'tcx> {
     crate span: Span,
 }
 
-impl<'tcx> Arm<'tcx> {
-    // HACK(or_patterns; Centril | dlrobertson): Remove this and
-    // correctly handle each case in which this method is used.
-    crate fn top_pats_hack(&self) -> &[Pat<'tcx>] {
-        match &*self.pattern.kind {
-            PatKind::Or { pats } => pats,
-            _ => std::slice::from_ref(&self.pattern),
-        }
-    }
-}
-
 #[derive(Clone, Debug)]
 crate enum Guard<'tcx> {
     If(ExprRef<'tcx>),

Diff for: src/librustc_mir_build/hair/mod.rs

@@ -10,18 +10,18 @@ fn main() {
 //      ...
 //      _3 = std::option::Option::<bool>::Some(const true,);
 //      _4 = discriminant(_3);
-//      switchInt(move _4) -> [1isize: bb3, otherwise: bb2];
+//      switchInt(move _4) -> [1isize: bb2, otherwise: bb1];
 //  }
 //  bb1: {
-//      _2 = const 42i32;
+//      _2 = const 10i32;
 //      goto -> bb4;
 //  }
 //  bb2: {
-//      _2 = const 10i32;
-//      goto -> bb4;
+//      switchInt(((_3 as Some).0: bool)) -> [false: bb1, otherwise: bb3];
 //  }
 //  bb3: {
-//      switchInt(((_3 as Some).0: bool)) -> [false: bb2, otherwise: bb1];
+//      _2 = const 42i32;
+//      goto -> bb4;
 //  }
 //  bb4: {
 //      _1 = Add(move _2, const 0i32);
@@ -33,18 +33,18 @@ fn main() {
 //      ...
 //      _3 = const Scalar(0x01) : std::option::Option<bool>;
 //      _4 = const 1isize;
-//      switchInt(const 1isize) -> [1isize: bb3, otherwise: bb2];
+//      switchInt(const 1isize) -> [1isize: bb2, otherwise: bb1];
 //  }
 //  bb1: {
-//      _2 = const 42i32;
+//      _2 = const 10i32;
 //      goto -> bb4;
 //  }
 //  bb2: {
-//      _2 = const 10i32;
-//      goto -> bb4;
+//      switchInt(const true) -> [false: bb1, otherwise: bb3];
 //  }
 //  bb3: {
-//      switchInt(const true) -> [false: bb2, otherwise: bb1];
+//      _2 = const 42i32;
+//      goto -> bb4;
 //  }
 //  bb4: {
 //      _1 = Add(move _2, const 0i32);

Diff for: src/test/mir-opt/const_prop/discriminant.rs

@@ -0,0 +1,76 @@
+// Test that simple or-patterns don't get expanded to exponentially large CFGs
+
+// ignore-tidy-linelength
+
+#![feature(or_patterns)]
+
+fn match_tuple(x: (u32, bool, Option<i32>, u32)) -> u32 {
+    match x {
+        (y @ (1 | 4), true | false, Some(1 | 8) | None, z @ (6..=9 | 13..=16)) => y ^ z,
+        _ => 0,
+    }
+}
+
+fn main() {}
+
+// END RUST SOURCE
+
+// START rustc.match_tuple.SimplifyCfg-initial.after.mir
+// scope 1 {
+//     debug y => _7;
+//     debug z => _8;
+// }
+// bb0: {
+//     FakeRead(ForMatchedPlace, _1);
+//     switchInt((_1.0: u32)) -> [1u32: bb2, 4u32: bb2, otherwise: bb1];
+// }
+// bb1: {
+//     _0 = const 0u32;
+//     goto -> bb10;
+// }
+// bb2: {
+//     _2 = discriminant((_1.2: std::option::Option<i32>));
+//     switchInt(move _2) -> [0isize: bb4, 1isize: bb3, otherwise: bb1];
+// }
+// bb3: {
+//     switchInt((((_1.2: std::option::Option<i32>) as Some).0: i32)) -> [1i32: bb4, 8i32: bb4, otherwise: bb1];
+// }
+// bb4: {
+//     _5 = Le(const 6u32, (_1.3: u32));
+//     switchInt(move _5) -> [false: bb6, otherwise: bb5];
+// }
+// bb5: {
+//     _6 = Le((_1.3: u32), const 9u32);
+//     switchInt(move _6) -> [false: bb6, otherwise: bb8];
+// }
+// bb6: {
+//     _3 = Le(const 13u32, (_1.3: u32));
+//     switchInt(move _3) -> [false: bb1, otherwise: bb7];
+// }
+// bb7: {
+//     _4 = Le((_1.3: u32), const 16u32);
+//     switchInt(move _4) -> [false: bb1, otherwise: bb8];
+// }
+// bb8: {
+//     falseEdges -> [real: bb9, imaginary: bb1];
+// }
+// bb9: {
+//     StorageLive(_7);
+//     _7 = (_1.0: u32);
+//     StorageLive(_8);
+//     _8 = (_1.3: u32);
+//     StorageLive(_9);
+//     _9 = _7;
+//     StorageLive(_10);
+//     _10 = _8;
+//     _0 = BitXor(move _9, move _10);
+//     StorageDead(_10);
+//     StorageDead(_9);
+//     StorageDead(_8);
+//     StorageDead(_7);
+//     goto -> bb10;
+// }
+// bb10: {
+//     return;
+// }
+// END rustc.match_tuple.SimplifyCfg-initial.after.mir