Return the otherwise_block instead of passing it as argument

This saves a few blocks and matches the common `unpack!` paradigm.

Author: Nadrieril

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

@@ -1250,11 +1250,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
         candidates: &mut [&mut Candidate<'pat, 'tcx>],
         refutable: bool,
     ) -> BasicBlock {
+        // This will generate code to test scrutinee_place and branch to the appropriate arm block.
         // See the doc comment on `match_candidates` for why we have an otherwise block.
-        let otherwise_block = self.cfg.start_new_block();
-
-        // This will generate code to test scrutinee_place and branch to the appropriate arm block
-        self.match_candidates(match_start_span, scrutinee_span, block, otherwise_block, candidates);
+        let otherwise_block =
+            self.match_candidates(match_start_span, scrutinee_span, block, candidates);
 
         // Link each leaf candidate to the `false_edge_start_block` of the next one.
         let mut previous_candidate: Option<&mut Candidate<'_, '_>> = None;
@@ -1305,27 +1304,24 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
         otherwise_block
     }
 
-    /// The main match algorithm. It begins with a set of candidates
-    /// `candidates` and has the job of generating code to determine
-    /// which of these candidates, if any, is the correct one. The
+    /// The main match algorithm. It begins with a set of candidates `candidates` and has the job of
+    /// generating code that branches to an appropriate block if the scrutinee matches one of these
+    /// candidates. The
     /// candidates are sorted such that the first item in the list
     /// has the highest priority. When a candidate is found to match
     /// the value, we will set and generate a branch to the appropriate
     /// pre-binding block.
     ///
-    /// If we find that *NONE* of the candidates apply, we branch to `otherwise_block`.
+    /// If none of the candidates apply, we continue to the returned `otherwise_block`.
     ///
     /// It might be surprising that the input can be non-exhaustive.
-    /// Indeed, initially, it is not, because all matches are
+    /// Indeed, for matches, initially, it is not, because all matches are
     /// exhaustive in Rust. But during processing we sometimes divide
     /// up the list of candidates and recurse with a non-exhaustive
     /// list. This is how our lowering approach (called "backtracking
     /// automaton" in the literature) works.
     /// See [`Builder::test_candidates`] for more details.
     ///
-    /// If `fake_borrows` is `Some`, then places which need fake borrows
-    /// will be added to it.
-    ///
     /// For an example of how we use `otherwise_block`, consider:
     /// ```
     /// # fn foo((x, y): (bool, bool)) -> u32 {
@@ -1350,7 +1346,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
     /// }
     /// if y {
     ///     if x {
-    ///         // This is actually unreachable because the `(true, true)` case was handled above.
+    ///         // This is actually unreachable because the `(true, true)` case was handled above,
+    ///         // but we don't know that from within the lowering algorithm.
     ///         // continue
     ///     } else {
     ///         return 3
@@ -1359,33 +1356,26 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
     ///     return 2
     /// }
     /// // this is the final `otherwise_block`, which is unreachable because the match was exhaustive.
-    /// unreachable!()
+    /// unreachable_unchecked()
     /// # }
     /// ```
     ///
     /// Every `continue` is an instance of branching to some `otherwise_block` somewhere deep within
     /// the algorithm. For more details on why we lower like this, see [`Builder::test_candidates`].
     ///
     /// Note how we test `x` twice. This is the tradeoff of backtracking automata: we prefer smaller
-    /// code size at the expense of non-optimal code paths.
+    /// code size so we accept non-optimal code paths.
     #[instrument(skip(self), level = "debug")]
     fn match_candidates(
         &mut self,
         span: Span,
         scrutinee_span: Span,
         start_block: BasicBlock,
-        otherwise_block: BasicBlock,
         candidates: &mut [&mut Candidate<'_, 'tcx>],
-    ) {
+    ) -> BasicBlock {
         ensure_sufficient_stack(|| {
-            self.match_candidates_with_enough_stack(
-                span,
-                scrutinee_span,
-                start_block,
-                otherwise_block,
-                candidates,
-            )
-        });
+            self.match_candidates_with_enough_stack(span, scrutinee_span, start_block, candidates)
+        })
     }
 
     /// Construct the decision tree for `candidates`. Don't call this, call `match_candidates`
@@ -1395,9 +1385,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
         span: Span,
         scrutinee_span: Span,
         start_block: BasicBlock,
-        otherwise_block: BasicBlock,
         candidates: &mut [&mut Candidate<'_, 'tcx>],
-    ) {
+    ) -> BasicBlock {
         if let [first, ..] = candidates {
             if first.false_edge_start_block.is_none() {
                 first.false_edge_start_block = Some(start_block);
@@ -1408,9 +1397,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
         let rest = match candidates {
             [] => {
                 // If there are no candidates that still need testing, we're done.
-                let source_info = self.source_info(span);
-                self.cfg.goto(start_block, source_info, otherwise_block);
-                return;
+                return start_block;
             }
             [first, remaining @ ..] if first.match_pairs.is_empty() => {
                 // The first candidate has satisfied all its match pairs; we link it up and continue
@@ -1431,13 +1418,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
 
         // Process any candidates that remain.
         let BlockAnd(start_block, remaining_candidates) = rest;
-        self.match_candidates(
-            span,
-            scrutinee_span,
-            start_block,
-            otherwise_block,
-            remaining_candidates,
-        );
+        self.match_candidates(span, scrutinee_span, start_block, remaining_candidates)
     }
 
     /// Link up matched candidates.
@@ -1542,14 +1523,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
         }
 
         // Process the expanded candidates.
-        let remainder_start = self.cfg.start_new_block();
-        // There might be new or-patterns obtained from expanding the old ones, so we call
-        // `match_candidates` again.
-        self.match_candidates(
+        let remainder_start = self.match_candidates(
             span,
             scrutinee_span,
             start_block,
-            remainder_start,
             expanded_candidates.as_mut_slice(),
         );
 
@@ -1648,6 +1625,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
         self.merge_trivial_subcandidates(candidate);
 
         if !candidate.match_pairs.is_empty() {
+            let or_span = candidate.or_span.unwrap_or(candidate.extra_data.span);
+            let source_info = self.source_info(or_span);
             // If more match pairs remain, test them after each subcandidate.
             // We could add them to the or-candidates before the call to `test_or_pattern` but this
             // would make it impossible to detect simplifiable or-patterns. That would guarantee
@@ -1661,6 +1640,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                 assert!(leaf_candidate.match_pairs.is_empty());
                 leaf_candidate.match_pairs.extend(remaining_match_pairs.iter().cloned());
                 let or_start = leaf_candidate.pre_binding_block.unwrap();
+                let otherwise =
+                    self.match_candidates(span, scrutinee_span, or_start, &mut [leaf_candidate]);
                 // In a case like `(P | Q, R | S)`, if `P` succeeds and `R | S` fails, we know `(Q,
                 // R | S)` will fail too. If there is no guard, we skip testing of `Q` by branching
                 // directly to `last_otherwise`. If there is a guard,
@@ -1671,13 +1652,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                 } else {
                     last_otherwise.unwrap()
                 };
-                self.match_candidates(
-                    span,
-                    scrutinee_span,
-                    or_start,
-                    or_otherwise,
-                    &mut [leaf_candidate],
-                );
+                self.cfg.goto(otherwise, source_info, or_otherwise);
             });
         }
     }
@@ -1956,15 +1931,12 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
         let target_blocks: FxIndexMap<_, _> = target_candidates
             .into_iter()
             .map(|(branch, mut candidates)| {
-                let candidate_start = self.cfg.start_new_block();
-                self.match_candidates(
-                    span,
-                    scrutinee_span,
-                    candidate_start,
-                    remainder_start,
-                    &mut *candidates,
-                );
-                (branch, candidate_start)
+                let branch_start = self.cfg.start_new_block();
+                let branch_otherwise =
+                    self.match_candidates(span, scrutinee_span, branch_start, &mut *candidates);
+                let source_info = self.source_info(span);
+                self.cfg.goto(branch_otherwise, source_info, remainder_start);
+                (branch, branch_start)
             })
             .collect();
 

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: bb4, otherwise: bb3];
     }
 
     bb1: {
         StorageLive(_3);
         StorageLive(_4);
-        _4 = begin_panic::<&str>(const "explicit panic") -> bb9;
+        _4 = begin_panic::<&str>(const "explicit panic") -> bb8;
     }
 
     bb2: {
@@ -43,35 +43,31 @@ fn main() -> () {
     }
 
     bb3: {
-        goto -> bb8;
+        goto -> bb7;
     }
 
     bb4: {
-        goto -> bb3;
+        falseEdge -> [real: bb6, imaginary: bb3];
     }
 
     bb5: {
-        falseEdge -> [real: bb7, imaginary: bb3];
+        goto -> bb3;
     }
 
     bb6: {
-        goto -> bb4;
-    }
-
-    bb7: {
         _5 = ((_1 as Some).0: u8);
         _0 = const ();
         StorageDead(_5);
         StorageDead(_1);
         return;
     }
 
-    bb8: {
+    bb7: {
         StorageDead(_5);
         goto -> bb1;
     }
 
-    bb9 (cleanup): {
+    bb8 (cleanup): {
         resume;
     }
 }

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

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