Expand or-candidates mixed with candidates above

Nadrieril · Nadrieril · commit 7b764be9f1dd · 2024-06-16T18:39:50.000+02:00
We can't mix them with candidates below them, but we can mix them with
candidates above.
diff --git a/compiler/rustc_mir_build/src/build/matches/mod.rs b/compiler/rustc_mir_build/src/build/matches/mod.rs
@@ -1364,61 +1364,105 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
         otherwise_block: BasicBlock,
         candidates: &mut [&mut Candidate<'pat, 'tcx>],
     ) {
-        let expand_or_pats = candidates.iter().any(|candidate| {
-            matches!(&*candidate.match_pairs, [MatchPair { test_case: TestCase::Or { .. }, .. }])
-        });
+        // We process or-patterns here. If any candidate starts with an or-pattern, we have to
+        // expand the or-pattern before we can proceed further.
+        //
+        // We can't expand them freely however. The rule is: if the candidate has an or-pattern as
+        // its only remaining match pair, we can expand it freely. If it has other match pairs, we
+        // can expand it but we can't process more candidates after it.
+        //
+        // If we didn't stop, the `otherwise` cases could get mixed up. E.g. in the following,
+        // or-pattern simplification (in `merge_trivial_subcandidates`) makes it so the `1` and `2`
+        // cases branch to a same block (which then tests `false`). If we took `(2, _)` in the same
+        // set of candidates, when we reach the block that tests `false` we don't know whether we
+        // came from `1` or `2`, hence we can't know where to branch on failure.
+        // ```ignore(illustrative)
+        // match (1, true) {
+        //     (1 | 2, false) => {},
+        //     (2, _) => {},
+        //     _ => {}
+        // }
+        // ```
+        //
+        // We therefore split the `candidates` slice in two, expand or-patterns in the first half,
+        // and process both halves separately.
+        let mut expand_until = 0;
+        for (i, candidate) in candidates.iter().enumerate() {
+            if matches!(
+                &*candidate.match_pairs,
+                [MatchPair { test_case: TestCase::Or { .. }, .. }, ..]
+            ) {
+                expand_until = i + 1;
+                if candidate.match_pairs.len() > 1 {
+                    break;
+                }
+            }
+        }
+        let (candidates_to_expand, remaining_candidates) = candidates.split_at_mut(expand_until);
 
         ensure_sufficient_stack(|| {
-            if expand_or_pats {
-                // Split a candidate in which 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.
-                let mut new_candidates = Vec::new();
-                for candidate in candidates.iter_mut() {
-                    if let [MatchPair { test_case: TestCase::Or { .. }, .. }] =
+            if candidates_to_expand.is_empty() {
+                // No candidates start with an or-pattern, we can continue.
+                self.match_expanded_candidates(
+                    span,
+                    scrutinee_span,
+                    start_block,
+                    otherwise_block,
+                    remaining_candidates,
+                );
+            } else {
+                // Expand one level of or-patterns for each candidate in `candidates_to_expand`.
+                let mut expanded_candidates = Vec::new();
+                for candidate in candidates_to_expand.iter_mut() {
+                    if let [MatchPair { test_case: TestCase::Or { .. }, .. }, ..] =
                         &*candidate.match_pairs
                     {
-                        let match_pair = candidate.match_pairs.pop().unwrap();
-                        self.create_or_subcandidates(candidate, match_pair);
+                        let or_match_pair = candidate.match_pairs.remove(0);
+                        // Expand the or-pattern into subcandidates.
+                        self.create_or_subcandidates(candidate, or_match_pair);
+                        // Collect the newly created subcandidates.
                         for subcandidate in candidate.subcandidates.iter_mut() {
-                            new_candidates.push(subcandidate);
+                            expanded_candidates.push(subcandidate);
                         }
                     } else {
-                        new_candidates.push(candidate);
+                        expanded_candidates.push(candidate);
                     }
                 }
+
+                // 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(
                     span,
                     scrutinee_span,
                     start_block,
-                    otherwise_block,
-                    new_candidates.as_mut_slice(),
+                    remainder_start,
+                    expanded_candidates.as_mut_slice(),
                 );
 
-                for candidate in candidates {
+                // Simplify subcandidates and process any leftover match pairs.
+                for candidate in candidates_to_expand {
                     if !candidate.subcandidates.is_empty() {
-                        self.merge_trivial_subcandidates(candidate);
+                        self.finalize_or_candidate(span, scrutinee_span, candidate);
                     }
                 }
-            } else {
-                self.match_simplified_candidates(
+
+                // Process the remaining candidates.
+                self.match_candidates(
                     span,
                     scrutinee_span,
-                    start_block,
+                    remainder_start,
                     otherwise_block,
-                    candidates,
+                    remaining_candidates,
                 );
             }
         });
     }
 
-    fn match_simplified_candidates(
+    /// Construct the decision tree for `candidates`. Caller must ensure that no candidate in
+    /// `candidates` starts with an or-pattern.
+    fn match_expanded_candidates(
         &mut self,
         span: Span,
         scrutinee_span: Span,
@@ -1443,7 +1487,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                 // 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);
-                self.match_simplified_candidates(
+                self.match_expanded_candidates(
                     span,
                     scrutinee_span,
                     start_block,
@@ -1453,7 +1497,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
             }
             candidates => {
                 // The first candidate has some unsatisfied match pairs; we proceed to do more tests.
-                self.test_candidates_with_or(
+                self.test_candidates(
                     span,
                     scrutinee_span,
                     candidates,
@@ -1506,8 +1550,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
         otherwise_block
     }
 
-    /// Tests a candidate where there are only or-patterns left to test, or
-    /// forwards to [Builder::test_candidates].
+    /// Simplify subcandidates and process any leftover match pairs. The candidate should have been
+    /// expanded with `create_or_subcandidates`.
     ///
     /// Given a pattern `(P | Q, R | S)` we (in principle) generate a CFG like
     /// so:
@@ -1559,45 +1603,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
     ///      |
     ///     ...
     /// ```
-    fn test_candidates_with_or(
-        &mut self,
-        span: Span,
-        scrutinee_span: Span,
-        candidates: &mut [&mut Candidate<'_, 'tcx>],
-        start_block: BasicBlock,
-        otherwise_block: BasicBlock,
-    ) {
-        let (first_candidate, remaining_candidates) = candidates.split_first_mut().unwrap();
-        assert!(first_candidate.subcandidates.is_empty());
-        if !matches!(first_candidate.match_pairs[0].test_case, TestCase::Or { .. }) {
-            self.test_candidates(span, scrutinee_span, candidates, start_block, otherwise_block);
-            return;
-        }
-
-        let first_match_pair = first_candidate.match_pairs.remove(0);
-        let remainder_start = self.cfg.start_new_block();
-        // Test the alternatives of this or-pattern.
-        self.test_or_pattern(
-            span,
-            scrutinee_span,
-            first_candidate,
-            start_block,
-            remainder_start,
-            first_match_pair,
-        );
-
-        // Test the remaining candidates.
-        self.match_candidates(
-            span,
-            scrutinee_span,
-            remainder_start,
-            otherwise_block,
-            remaining_candidates,
-        );
-    }
-
-    /// Simplify subcandidates and process any leftover match pairs. The candidate should have been
-    /// expanded with `create_or_subcandidates`.
     fn finalize_or_candidate(
         &mut self,
         span: Span,
@@ -1634,40 +1639,17 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                 } else {
                     last_otherwise.unwrap()
                 };
-                self.test_candidates_with_or(
+                self.match_candidates(
                     span,
                     scrutinee_span,
-                    &mut [leaf_candidate],
                     or_start,
                     or_otherwise,
+                    &mut [leaf_candidate],
                 );
             });
         }
     }
 
-    #[instrument(skip(self, start_block, otherwise_block, candidate, match_pair), level = "debug")]
-    fn test_or_pattern<'pat>(
-        &mut self,
-        span: Span,
-        scrutinee_span: Span,
-        candidate: &mut Candidate<'pat, 'tcx>,
-        start_block: BasicBlock,
-        otherwise_block: BasicBlock,
-        match_pair: MatchPair<'pat, 'tcx>,
-    ) {
-        let or_span = match_pair.pattern.span;
-        self.create_or_subcandidates(candidate, match_pair);
-        let mut or_candidate_refs: Vec<_> = candidate.subcandidates.iter_mut().collect();
-        self.match_candidates(
-            or_span,
-            or_span,
-            start_block,
-            otherwise_block,
-            &mut or_candidate_refs,
-        );
-        self.finalize_or_candidate(span, scrutinee_span, candidate);
-    }
-
     /// Given a match-pair that corresponds to an or-pattern, expand each subpattern into a new
     /// subcandidate. Any candidate that has been expanded that way should be passed to
     /// `finalize_or_candidate` after its subcandidates have been processed.
diff --git a/tests/mir-opt/or_pattern.single_switchint.SimplifyCfg-initial.after.mir b/tests/mir-opt/or_pattern.single_switchint.SimplifyCfg-initial.after.mir
@@ -10,67 +10,63 @@ fn single_switchint() -> () {
         StorageLive(_2);
         _2 = (const 1_i32, const true);
         PlaceMention(_2);
-        switchInt((_2.0: i32)) -> [1: bb6, 2: bb8, otherwise: bb1];
+        switchInt((_2.0: i32)) -> [1: bb2, 2: bb4, otherwise: bb1];
     }
 
     bb1: {
-        switchInt((_2.0: i32)) -> [1: bb3, 2: bb3, otherwise: bb2];
+        switchInt((_2.0: i32)) -> [3: bb8, 4: bb8, otherwise: bb7];
     }
 
     bb2: {
-        switchInt((_2.0: i32)) -> [3: bb5, 4: bb5, otherwise: bb4];
+        switchInt((_2.1: bool)) -> [0: bb6, otherwise: bb3];
     }
 
     bb3: {
-        falseEdge -> [real: bb12, imaginary: bb2];
+        falseEdge -> [real: bb9, imaginary: bb4];
     }
 
     bb4: {
-        _1 = const 5_i32;
-        goto -> bb14;
+        switchInt((_2.1: bool)) -> [0: bb5, otherwise: bb6];
     }
 
     bb5: {
-        falseEdge -> [real: bb13, imaginary: bb4];
+        falseEdge -> [real: bb10, imaginary: bb6];
     }
 
     bb6: {
-        switchInt((_2.1: bool)) -> [0: bb1, otherwise: bb7];
+        falseEdge -> [real: bb11, imaginary: bb1];
     }
 
     bb7: {
-        falseEdge -> [real: bb10, imaginary: bb8];
+        _1 = const 5_i32;
+        goto -> bb13;
     }
 
     bb8: {
-        switchInt((_2.1: bool)) -> [0: bb9, otherwise: bb1];
+        falseEdge -> [real: bb12, imaginary: bb7];
     }
 
     bb9: {
-        falseEdge -> [real: bb11, imaginary: bb1];
-    }
-
-    bb10: {
         _1 = const 1_i32;
-        goto -> bb14;
+        goto -> bb13;
     }
 
-    bb11: {
+    bb10: {
         _1 = const 2_i32;
-        goto -> bb14;
+        goto -> bb13;
     }
 
-    bb12: {
+    bb11: {
         _1 = const 3_i32;
-        goto -> bb14;
+        goto -> bb13;
     }
 
-    bb13: {
+    bb12: {
         _1 = const 4_i32;
-        goto -> bb14;
+        goto -> bb13;
     }
 
-    bb14: {
+    bb13: {
         StorageDead(_2);
         StorageDead(_1);
         _0 = const ();
