Auto merge of rust-lang#119031 - Nadrieril:two-phase-match-lowering, …

…r=<try> [Experiment] Play with match lowering Match lowering to MIR has the reputation of being the most intricate piece of the compiler, and after banging my head on it for a bit I sure hope there isn't anything more intricate somewhere else. It's good quality code but I hope to unentangle it. This PR is me wrestling with it and asking `rustc-timer` for its opinion. r? `@ghost`
rust-lang-ci · Mar 2, 2024 · c703bf4 · c703bf4
2 parents 5119208 + 6433f90
commit c703bf4
Show file tree

Hide file tree

Showing 16 changed files with 373 additions and 389 deletions.
diff --git a/compiler/rustc_mir_build/src/build/matches/mod.rs b/compiler/rustc_mir_build/src/build/matches/mod.rs
diff --git a/compiler/rustc_mir_build/src/build/matches/test.rs b/compiler/rustc_mir_build/src/build/matches/test.rs
diff --git a/compiler/rustc_mir_build/src/build/matches/util.rs b/compiler/rustc_mir_build/src/build/matches/util.rs
@@ -122,9 +122,12 @@ impl<'pat, 'tcx> MatchPair<'pat, 'tcx> {
         let mut subpairs = Vec::new();
         let test_case = match pattern.kind {
             PatKind::Never | PatKind::Wild | PatKind::Error(_) => default_irrefutable(),
-            PatKind::Or { ref pats } => TestCase::Or {
-                pats: pats.iter().map(|pat| FlatPat::new(place.clone(), pat, cx)).collect(),
-            },
+            PatKind::Or { ref pats } => {
+                let pats: Box<[_]> =
+                    pats.iter().map(|pat| FlatPat::new(place.clone(), pat, cx)).collect();
+                let simple = pats.iter().all(|fpat| fpat.simple);
+                TestCase::Or { pats, simple }
+            }
 
             PatKind::Range(ref range) => {
                 if range.is_full_range(cx.tcx) == Some(true) {
@@ -260,6 +263,12 @@ impl<'pat, 'tcx> MatchPair<'pat, 'tcx> {
 
         MatchPair { place, test_case, subpairs, pattern }
     }
+
+    /// Whether this recursively contains no bindings or ascriptions.
+    pub(super) fn is_simple(&self) -> bool {
+        !matches!(self.test_case, TestCase::Or { simple: false, .. })
+            && self.subpairs.iter().all(|p| p.is_simple())
+    }
 }
 
 pub(super) struct FakeBorrowCollector<'a, 'b, 'tcx> {

diff --git a/tests/mir-opt/building/match_false_edges.full_tested_match.built.after.mir b/tests/mir-opt/building/match_false_edges.full_tested_match.built.after.mir
@@ -28,7 +28,7 @@ fn full_tested_match() -> () {
         _2 = Option::<i32>::Some(const 42_i32);
         PlaceMention(_2);
         _3 = discriminant(_2);
-        switchInt(move _3) -> [0: bb2, 1: bb4, otherwise: bb1];
+        switchInt(move _3) -> [0: bb5, 1: bb2, otherwise: bb1];
     }
 
     bb1: {
@@ -37,20 +37,20 @@ fn full_tested_match() -> () {
     }
 
     bb2: {
-        _1 = (const 3_i32, const 3_i32);
-        goto -> bb13;
+        falseEdge -> [real: bb7, imaginary: bb3];
     }
 
     bb3: {
-        goto -> bb1;
+        falseEdge -> [real: bb12, imaginary: bb5];
     }
 
     bb4: {
-        falseEdge -> [real: bb7, imaginary: bb5];
+        goto -> bb1;
     }
 
     bb5: {
-        falseEdge -> [real: bb12, imaginary: bb2];
+        _1 = (const 3_i32, const 3_i32);
+        goto -> bb13;
     }
 
     bb6: {
@@ -91,7 +91,7 @@ fn full_tested_match() -> () {
     bb11: {
         StorageDead(_7);
         StorageDead(_6);
-        goto -> bb5;
+        goto -> bb3;
     }
 
     bb12: {

diff --git a/tests/mir-opt/building/match_false_edges.full_tested_match2.built.after.mir b/tests/mir-opt/building/match_false_edges.full_tested_match2.built.after.mir
@@ -28,7 +28,7 @@ fn full_tested_match2() -> () {
         _2 = Option::<i32>::Some(const 42_i32);
         PlaceMention(_2);
         _3 = discriminant(_2);
-        switchInt(move _3) -> [0: bb2, 1: bb4, otherwise: bb1];
+        switchInt(move _3) -> [0: bb5, 1: bb2, otherwise: bb1];
     }
 
     bb1: {
@@ -37,18 +37,10 @@ fn full_tested_match2() -> () {
     }
 
     bb2: {
-        falseEdge -> [real: bb12, imaginary: bb5];
+        falseEdge -> [real: bb7, imaginary: bb5];
     }
 
     bb3: {
-        goto -> bb1;
-    }
-
-    bb4: {
-        falseEdge -> [real: bb7, imaginary: bb2];
-    }
-
-    bb5: {
         StorageLive(_9);
         _9 = ((_2 as Some).0: i32);
         StorageLive(_10);
@@ -59,6 +51,14 @@ fn full_tested_match2() -> () {
         goto -> bb13;
     }
 
+    bb4: {
+        goto -> bb1;
+    }
+
+    bb5: {
+        falseEdge -> [real: bb12, imaginary: bb3];
+    }
+
     bb6: {
         goto -> bb1;
     }
@@ -97,7 +97,7 @@ fn full_tested_match2() -> () {
     bb11: {
         StorageDead(_7);
         StorageDead(_6);
-        falseEdge -> [real: bb5, imaginary: bb2];
+        falseEdge -> [real: bb3, imaginary: bb5];
     }
 
     bb12: {

diff --git a/tests/mir-opt/early_otherwise_branch.opt2.EarlyOtherwiseBranch.diff b/tests/mir-opt/early_otherwise_branch.opt2.EarlyOtherwiseBranch.diff
@@ -30,7 +30,7 @@
           StorageDead(_5);
           StorageDead(_4);
           _8 = discriminant((_3.0: std::option::Option<u32>));
--         switchInt(move _8) -> [0: bb2, 1: bb3, otherwise: bb1];
+-         switchInt(move _8) -> [0: bb3, 1: bb2, otherwise: bb1];
 +         StorageLive(_11);
 +         _11 = discriminant((_3.1: std::option::Option<u32>));
 +         StorageLive(_12);
@@ -48,12 +48,12 @@
 
       bb2: {
 -         _6 = discriminant((_3.1: std::option::Option<u32>));
--         switchInt(move _6) -> [0: bb5, otherwise: bb1];
+-         switchInt(move _6) -> [1: bb4, otherwise: bb1];
 -     }
 - 
 -     bb3: {
 -         _7 = discriminant((_3.1: std::option::Option<u32>));
--         switchInt(move _7) -> [1: bb4, otherwise: bb1];
+-         switchInt(move _7) -> [0: bb5, otherwise: bb1];
 -     }
 - 
 -     bb4: {

diff --git a/tests/mir-opt/early_otherwise_branch_noopt.noopt1.EarlyOtherwiseBranch.diff b/tests/mir-opt/early_otherwise_branch_noopt.noopt1.EarlyOtherwiseBranch.diff
@@ -36,7 +36,7 @@
           StorageDead(_5);
           StorageDead(_4);
           _8 = discriminant((_3.0: std::option::Option<u32>));
-          switchInt(move _8) -> [0: bb2, 1: bb4, otherwise: bb1];
+          switchInt(move _8) -> [0: bb3, 1: bb2, otherwise: bb1];
       }
 
       bb1: {
@@ -45,17 +45,17 @@
 
       bb2: {
           _6 = discriminant((_3.1: std::option::Option<u32>));
-          switchInt(move _6) -> [0: bb3, 1: bb7, otherwise: bb1];
+          switchInt(move _6) -> [0: bb6, 1: bb5, otherwise: bb1];
       }
 
       bb3: {
-          _0 = const 3_u32;
-          goto -> bb8;
+          _7 = discriminant((_3.1: std::option::Option<u32>));
+          switchInt(move _7) -> [0: bb4, 1: bb7, otherwise: bb1];
       }
 
       bb4: {
-          _7 = discriminant((_3.1: std::option::Option<u32>));
-          switchInt(move _7) -> [0: bb6, 1: bb5, otherwise: bb1];
+          _0 = const 3_u32;
+          goto -> bb8;
       }
 
       bb5: {

diff --git a/..._scopes.complicated_match.panic-abort.SimplifyCfg-initial.after-ElaborateDrops.after.diff b/..._scopes.complicated_match.panic-abort.SimplifyCfg-initial.after-ElaborateDrops.after.diff
@@ -42,11 +42,15 @@
       }
 
       bb2: {
--         switchInt((_2.0: bool)) -> [0: bb3, otherwise: bb4];
+-         switchInt((_2.0: bool)) -> [0: bb4, otherwise: bb3];
 +         switchInt((_2.0: bool)) -> [0: bb3, otherwise: bb17];
       }
 
       bb3: {
+-         falseEdge -> [real: bb20, imaginary: bb4];
+-     }
+- 
+-     bb4: {
           StorageLive(_15);
           _15 = (_2.1: bool);
           StorageLive(_16);
@@ -55,19 +59,16 @@
 +         goto -> bb16;
       }
 
-      bb4: {
--         falseEdge -> [real: bb20, imaginary: bb3];
--     }
-- 
 -     bb5: {
--         falseEdge -> [real: bb13, imaginary: bb4];
+-         falseEdge -> [real: bb13, imaginary: bb3];
 -     }
 - 
 -     bb6: {
 -         falseEdge -> [real: bb8, imaginary: bb5];
 -     }
 - 
 -     bb7: {
++     bb4: {
           _0 = const 1_i32;
 -         drop(_7) -> [return: bb18, unwind: bb25];
 +         drop(_7) -> [return: bb15, unwind: bb22];
@@ -183,7 +184,7 @@
           StorageDead(_12);
           StorageDead(_8);
           StorageDead(_6);
--         falseEdge -> [real: bb2, imaginary: bb4];
+-         falseEdge -> [real: bb2, imaginary: bb3];
 +         goto -> bb2;
       }
 

diff --git a/...scopes.complicated_match.panic-unwind.SimplifyCfg-initial.after-ElaborateDrops.after.diff b/...scopes.complicated_match.panic-unwind.SimplifyCfg-initial.after-ElaborateDrops.after.diff
@@ -42,11 +42,15 @@
       }
 
       bb2: {
--         switchInt((_2.0: bool)) -> [0: bb3, otherwise: bb4];
+-         switchInt((_2.0: bool)) -> [0: bb4, otherwise: bb3];
 +         switchInt((_2.0: bool)) -> [0: bb3, otherwise: bb17];
       }
 
       bb3: {
+-         falseEdge -> [real: bb20, imaginary: bb4];
+-     }
+- 
+-     bb4: {
           StorageLive(_15);
           _15 = (_2.1: bool);
           StorageLive(_16);
@@ -55,19 +59,16 @@
 +         goto -> bb16;
       }
 
-      bb4: {
--         falseEdge -> [real: bb20, imaginary: bb3];
--     }
-- 
 -     bb5: {
--         falseEdge -> [real: bb13, imaginary: bb4];
+-         falseEdge -> [real: bb13, imaginary: bb3];
 -     }
 - 
 -     bb6: {
 -         falseEdge -> [real: bb8, imaginary: bb5];
 -     }
 - 
 -     bb7: {
++     bb4: {
           _0 = const 1_i32;
 -         drop(_7) -> [return: bb18, unwind: bb25];
 +         drop(_7) -> [return: bb15, unwind: bb22];
@@ -183,7 +184,7 @@
           StorageDead(_12);
           StorageDead(_8);
           StorageDead(_6);
--         falseEdge -> [real: bb2, imaginary: bb4];
+-         falseEdge -> [real: bb2, imaginary: bb3];
 +         goto -> bb2;
       }
 

diff --git a/tests/ui/or-patterns/bindings-runpass-2.rs b/tests/ui/or-patterns/bindings-runpass-2.rs
@@ -26,5 +26,6 @@ fn main() {
     assert_eq!(or_at(Err(7)), 207);
     assert_eq!(or_at(Err(8)), 8);
     assert_eq!(or_at(Err(20)), 220);
+    assert_eq!(or_at(Err(34)), 134);
     assert_eq!(or_at(Err(50)), 500);
 }
diff --git a/tests/ui/or-patterns/inner-or-pat.or3.stderr b/tests/ui/or-patterns/inner-or-pat.or3.stderr
@@ -1,5 +1,5 @@
 error[E0308]: mismatched types
-  --> $DIR/inner-or-pat.rs:38:54
+  --> $DIR/inner-or-pat.rs:36:54
    |
 LL |     match x {
    |           - this expression has type `&str`

diff --git a/tests/ui/or-patterns/inner-or-pat.or4.stderr b/tests/ui/or-patterns/inner-or-pat.or4.stderr
@@ -1,5 +1,5 @@
 error[E0408]: variable `x` is not bound in all patterns
-  --> $DIR/inner-or-pat.rs:53:37
+  --> $DIR/inner-or-pat.rs:51:37
    |
 LL |         (x @ "red" | (x @ "blue" |  "red")) => {
    |                       -             ^^^^^ pattern doesn't bind `x`

diff --git a/tests/ui/or-patterns/inner-or-pat.rs b/tests/ui/or-patterns/inner-or-pat.rs
@@ -1,7 +1,5 @@
-//@ revisions: or1 or2 or3 or4 or5
+//@ revisions: or1 or3 or4
 //@ [or1] run-pass
-//@ [or2] run-pass
-//@ [or5] run-pass
 
 #![allow(unreachable_patterns)]
 #![allow(unused_variables)]

diff --git a/tests/ui/or-patterns/issue-70413-no-unreachable-pat-and-guard.rs b/tests/ui/or-patterns/issue-70413-no-unreachable-pat-and-guard.rs
@@ -1,21 +1,20 @@
-//@ check-pass
+//@ run-pass
 
 #![deny(unreachable_patterns)]
 
 fn main() {
-    match (3,42) {
-        (a,_) | (_,a) if a > 10 => {println!("{}", a)}
-        _ => ()
+    match (3, 42) {
+        (a, _) | (_, a) if a > 10 => {}
+        _ => unreachable!(),
     }
 
-    match Some((3,42)) {
-        Some((a, _)) | Some((_, a)) if a > 10 => {println!("{}", a)}
-        _ => ()
-
+    match Some((3, 42)) {
+        Some((a, _)) | Some((_, a)) if a > 10 => {}
+        _ => unreachable!(),
     }
 
-    match Some((3,42)) {
-        Some((a, _) | (_, a)) if a > 10 => {println!("{}", a)}
-        _ => ()
+    match Some((3, 42)) {
+        Some((a, _) | (_, a)) if a > 10 => {}
+        _ => unreachable!(),
     }
 }
diff --git a/tests/ui/or-patterns/search-via-bindings.rs b/tests/ui/or-patterns/search-via-bindings.rs
@@ -42,6 +42,23 @@ fn search_old_style(target: (bool, bool, bool)) -> u32 {
     }
 }
 
+// Check that a dummy or-pattern also leads to running the guard multiple times.
+fn search_with_dummy(target: (bool, bool)) -> u32 {
+    let x = ((false, true), (false, true), ());
+    let mut guard_count = 0;
+    match x {
+        ((a, _) | (_, a), (b, _) | (_, b), _ | _)
+            if {
+                guard_count += 1;
+                (a, b) == target
+            } =>
+        {
+            guard_count
+        }
+        _ => unreachable!(),
+    }
+}
+
 fn main() {
     assert_eq!(search((false, false, false)), 1);
     assert_eq!(search((false, false, true)), 2);
@@ -60,4 +77,9 @@ fn main() {
     assert_eq!(search_old_style((true, false, true)), 6);
     assert_eq!(search_old_style((true, true, false)), 7);
     assert_eq!(search_old_style((true, true, true)), 8);
+
+    assert_eq!(search_with_dummy((false, false)), 1);
+    assert_eq!(search_with_dummy((false, true)), 3);
+    assert_eq!(search_with_dummy((true, false)), 5);
+    assert_eq!(search_with_dummy((true, true)), 7);
 }
diff --git a/tests/ui/pattern/usefulness/integer-ranges/regression-switchint-sorting-with-ranges.rs b/tests/ui/pattern/usefulness/integer-ranges/regression-switchint-sorting-with-ranges.rs
@@ -0,0 +1,14 @@
+//@ run-pass
+//
+// Regression test for match lowering to MIR: when gathering candidates, by the time we get to the
+// range we know the range will only match on the failure case of the switchint. Hence we mustn't
+// add the `1` to the switchint or the range would be incorrectly sorted.
+#![allow(unreachable_patterns)]
+fn main() {
+    match 1 {
+        10 => unreachable!(),
+        0..=5 => {}
+        1 => unreachable!(),
+        _ => unreachable!(),
+    }
+}