Auto merge of rust-lang#121172 - Nadrieril:simplify-empty-selection, r=matthewjasper

match lowering: simplify empty candidate selection

In match lowering, `match_simplified_candidates` is tasked with removing candidates that are fully matched and linking them up properly. The code that does that was needlessly complicated; this PR simplifies it.

The overall change isn't big but I split it up into tiny commits to convince myself that I was correctly preserving behavior. The test changes are all due to the first commit. Let me know if you'd prefer me to split up the PR to make reviewing easier.

r? `@matthewjasper`

Author: bors

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

@@ -1235,53 +1235,43 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
         &mut self,
         span: Span,
         scrutinee_span: Span,
-        start_block: BasicBlock,
+        mut start_block: BasicBlock,
         otherwise_block: BasicBlock,
         candidates: &mut [&mut Candidate<'_, 'tcx>],
         fake_borrows: &mut Option<FxIndexSet<Place<'tcx>>>,
     ) {
-        // The candidates are sorted by priority. Check to see whether the
-        // higher priority candidates (and hence at the front of the slice)
-        // have satisfied all their match pairs.
-        let fully_matched = candidates.iter().take_while(|c| c.match_pairs.is_empty()).count();
-        debug!("match_candidates: {:?} candidates fully matched", fully_matched);
-        let (matched_candidates, unmatched_candidates) = candidates.split_at_mut(fully_matched);
-
-        let block = if !matched_candidates.is_empty() {
-            let otherwise_block =
-                self.select_matched_candidates(matched_candidates, start_block, fake_borrows);
-
-            if let Some(last_otherwise_block) = otherwise_block {
-                last_otherwise_block
-            } else {
-                // Any remaining candidates are unreachable.
-                if unmatched_candidates.is_empty() {
-                    return;
-                }
-                self.cfg.start_new_block()
+        match candidates {
+            [] => {
+                // If there are no candidates that still need testing, we're done. Since all matches are
+                // exhaustive, execution should never reach this point.
+                let source_info = self.source_info(span);
+                self.cfg.goto(start_block, source_info, otherwise_block);
+            }
+            [first, remaining @ ..] if first.match_pairs.is_empty() => {
+                // The first candidate has satisfied all its match pairs; we link it up and continue
+                // with the remaining candidates.
+                start_block = self.select_matched_candidate(first, start_block, fake_borrows);
+                self.match_simplified_candidates(
+                    span,
+                    scrutinee_span,
+                    start_block,
+                    otherwise_block,
+                    remaining,
+                    fake_borrows,
+                )
+            }
+            candidates => {
+                // The first candidate has some unsatisfied match pairs; we proceed to do more tests.
+                self.test_candidates_with_or(
+                    span,
+                    scrutinee_span,
+                    candidates,
+                    start_block,
+                    otherwise_block,
+                    fake_borrows,
+                );
             }
-        } else {
-            start_block
-        };
-
-        // If there are no candidates that still need testing, we're
-        // done. Since all matches are exhaustive, execution should
-        // never reach this point.
-        if unmatched_candidates.is_empty() {
-            let source_info = self.source_info(span);
-            self.cfg.goto(block, source_info, otherwise_block);
-            return;
         }
-
-        // Test for the remaining candidates.
-        self.test_candidates_with_or(
-            span,
-            scrutinee_span,
-            unmatched_candidates,
-            block,
-            otherwise_block,
-            fake_borrows,
-        );
     }
 
     /// Link up matched candidates.
@@ -1303,47 +1293,40 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
     /// * the [otherwise block] of the third pattern to a block with an
     ///   [`Unreachable` terminator](TerminatorKind::Unreachable).
     ///
-    /// In addition, we add fake edges from the otherwise blocks to the
+    /// In addition, we later add fake edges from the otherwise blocks to the
     /// pre-binding block of the next candidate in the original set of
     /// candidates.
     ///
     /// [pre-binding block]: Candidate::pre_binding_block
     /// [otherwise block]: Candidate::otherwise_block
-    fn select_matched_candidates(
+    fn select_matched_candidate(
         &mut self,
-        matched_candidates: &mut [&mut Candidate<'_, 'tcx>],
+        candidate: &mut Candidate<'_, 'tcx>,
         start_block: BasicBlock,
         fake_borrows: &mut Option<FxIndexSet<Place<'tcx>>>,
-    ) -> Option<BasicBlock> {
-        debug_assert!(
-            !matched_candidates.is_empty(),
-            "select_matched_candidates called with no candidates",
-        );
-        debug_assert!(
-            matched_candidates.iter().all(|c| c.subcandidates.is_empty()),
-            "subcandidates should be empty in select_matched_candidates",
-        );
+    ) -> BasicBlock {
+        assert!(candidate.otherwise_block.is_none());
+        assert!(candidate.pre_binding_block.is_none());
+        assert!(candidate.subcandidates.is_empty());
 
-        // Insert a borrows of prefixes of places that are bound and are
-        // behind a dereference projection.
-        //
-        // These borrows are taken to avoid situations like the following:
-        //
-        // match x[10] {
-        //     _ if { x = &[0]; false } => (),
-        //     y => (), // Out of bounds array access!
-        // }
-        //
-        // match *x {
-        //     // y is bound by reference in the guard and then by copy in the
-        //     // arm, so y is 2 in the arm!
-        //     y if { y == 1 && (x = &2) == () } => y,
-        //     _ => 3,
-        // }
         if let Some(fake_borrows) = fake_borrows {
-            for Binding { source, .. } in
-                matched_candidates.iter().flat_map(|candidate| &candidate.bindings)
-            {
+            // Insert a borrows of prefixes of places that are bound and are
+            // behind a dereference projection.
+            //
+            // These borrows are taken to avoid situations like the following:
+            //
+            // match x[10] {
+            //     _ if { x = &[0]; false } => (),
+            //     y => (), // Out of bounds array access!
+            // }
+            //
+            // match *x {
+            //     // y is bound by reference in the guard and then by copy in the
+            //     // arm, so y is 2 in the arm!
+            //     y if { y == 1 && (x = &2) == () } => y,
+            //     _ => 3,
+            // }
+            for Binding { source, .. } in &candidate.bindings {
                 if let Some(i) =
                     source.projection.iter().rposition(|elem| elem == ProjectionElem::Deref)
                 {
@@ -1357,38 +1340,14 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
             }
         }
 
-        let fully_matched_with_guard = matched_candidates
-            .iter()
-            .position(|c| !c.has_guard)
-            .unwrap_or(matched_candidates.len() - 1);
-
-        let (reachable_candidates, unreachable_candidates) =
-            matched_candidates.split_at_mut(fully_matched_with_guard + 1);
-
-        let mut next_prebinding = start_block;
-
-        for candidate in reachable_candidates.iter_mut() {
-            assert!(candidate.otherwise_block.is_none());
-            assert!(candidate.pre_binding_block.is_none());
-            candidate.pre_binding_block = Some(next_prebinding);
-            if candidate.has_guard {
-                // Create the otherwise block for this candidate, which is the
-                // pre-binding block for the next candidate.
-                next_prebinding = self.cfg.start_new_block();
-                candidate.otherwise_block = Some(next_prebinding);
-            }
-        }
-
-        debug!(
-            "match_candidates: add pre_binding_blocks for unreachable {:?}",
-            unreachable_candidates,
-        );
-        for candidate in unreachable_candidates {
-            assert!(candidate.pre_binding_block.is_none());
-            candidate.pre_binding_block = Some(self.cfg.start_new_block());
+        candidate.pre_binding_block = Some(start_block);
+        let otherwise_block = self.cfg.start_new_block();
+        if candidate.has_guard {
+            // Create the otherwise block for this candidate, which is the
+            // pre-binding block for the next candidate.
+            candidate.otherwise_block = Some(otherwise_block);
         }
-
-        reachable_candidates.last_mut().unwrap().otherwise_block
+        otherwise_block
     }
 
     /// Tests a candidate where there are only or-patterns left to test, or

tests/mir-opt/building/issue_101867.main.built.after.mir

@@ -27,13 +27,13 @@ fn main() -> () {
         StorageLive(_5);
         PlaceMention(_1);
         _6 = discriminant(_1);
-        switchInt(move _6) -> [1: bb5, otherwise: bb4];
+        switchInt(move _6) -> [1: bb6, otherwise: bb4];
     }
 
     bb1: {
         StorageLive(_3);
         StorageLive(_4);
-        _4 = begin_panic::<&str>(const "explicit panic") -> bb8;
+        _4 = begin_panic::<&str>(const "explicit panic") -> bb10;
     }
 
     bb2: {
@@ -48,27 +48,35 @@ fn main() -> () {
     }
 
     bb4: {
-        goto -> bb7;
+        goto -> bb9;
     }
 
     bb5: {
-        falseEdge -> [real: bb6, imaginary: bb4];
+        goto -> bb3;
     }
 
     bb6: {
+        falseEdge -> [real: bb8, imaginary: bb4];
+    }
+
+    bb7: {
+        goto -> bb4;
+    }
+
+    bb8: {
         _5 = ((_1 as Some).0: u8);
         _0 = const ();
         StorageDead(_5);
         StorageDead(_1);
         return;
     }
 
-    bb7: {
+    bb9: {
         StorageDead(_5);
         goto -> bb1;
     }
 
-    bb8 (cleanup): {
+    bb10 (cleanup): {
         resume;
     }
 }

tests/mir-opt/building/issue_49232.main.built.after.mir

@@ -17,15 +17,15 @@ fn main() -> () {
     }
 
     bb1: {
-        falseUnwind -> [real: bb2, unwind: bb12];
+        falseUnwind -> [real: bb2, unwind: bb14];
     }
 
     bb2: {
         StorageLive(_2);
         StorageLive(_3);
         _3 = const true;
         PlaceMention(_3);
-        switchInt(_3) -> [0: bb4, otherwise: bb5];
+        switchInt(_3) -> [0: bb4, otherwise: bb6];
     }
 
     bb3: {
@@ -34,51 +34,59 @@ fn main() -> () {
     }
 
     bb4: {
-        falseEdge -> [real: bb6, imaginary: bb5];
+        falseEdge -> [real: bb8, imaginary: bb6];
     }
 
     bb5: {
-        _0 = const ();
-        goto -> bb11;
+        goto -> bb3;
     }
 
     bb6: {
-        _2 = const 4_i32;
-        goto -> bb9;
+        _0 = const ();
+        goto -> bb13;
     }
 
     bb7: {
-        unreachable;
+        goto -> bb3;
     }
 
     bb8: {
-        goto -> bb9;
+        _2 = const 4_i32;
+        goto -> bb11;
     }
 
     bb9: {
+        unreachable;
+    }
+
+    bb10: {
+        goto -> bb11;
+    }
+
+    bb11: {
         FakeRead(ForLet(None), _2);
         StorageDead(_3);
         StorageLive(_5);
         StorageLive(_6);
         _6 = &_2;
-        _5 = std::mem::drop::<&i32>(move _6) -> [return: bb10, unwind: bb12];
+        _5 = std::mem::drop::<&i32>(move _6) -> [return: bb12, unwind: bb14];
     }
 
-    bb10: {
+    bb12: {
         StorageDead(_6);
         StorageDead(_5);
         _1 = const ();
         StorageDead(_2);
         goto -> bb1;
     }
 
-    bb11: {
+    bb13: {
         StorageDead(_3);
         StorageDead(_2);
         return;
     }
 
-    bb12 (cleanup): {
+    bb14 (cleanup): {
         resume;
     }
 }