Skip to content

Commit

Permalink
Rollup merge of rust-lang#126553 - Nadrieril:expand-or-pat-into-above…
Browse files Browse the repository at this point in the history
…, r=matthewjasper

match lowering: expand or-candidates mixed with candidates above

This PR tweaks match lowering of or-patterns. Consider this:
```rust
match (x, y) {
    (1, true) => 1,
    (2, false) => 2,
    (1 | 2, true | false) => 3,
    (3 | 4, true | false) => 4,
    _ => 5,
}
```
One might hope that this can be compiled to a single `SwitchInt` on `x` followed by some boolean checks. Before this PR, we compile this to 3 `SwitchInt`s on `x`, because an arm that contains more than one or-pattern was compiled on its own. This PR groups branch `3` with the two branches above, getting us down to 2 `SwitchInt`s on `x`.

We can't in general expand or-patterns freely, because this interacts poorly with another optimization we do: or-pattern simplification. When an or-pattern doesn't involve bindings, we branch the success paths of all its alternatives to the same block. The drawback is that in a case like:
```rust
match (1, true) {
    (1 | 2, false) => unreachable!(),
    (2, _) => unreachable!(),
    _ => {}
}
```
if we used a single `SwitchInt`, by the time we test `false` we don't know whether we came from the `1` case or the `2` case, so we don't know where to go if `false` doesn't match.

Hence the limitation: we can process or-pattern alternatives alongside candidates that precede it, but not candidates that follow it. (Unless the or-pattern is the only remaining match pair of its candidate, in which case we can process it alongside whatever).

This PR allows the processing of or-pattern alternatives alongside candidates that precede it. One benefit is that we now process or-patterns in a single place in `mod.rs`.

r? `@matthewjasper`
  • Loading branch information
jieyouxu committed Jun 19, 2024
2 parents da963d6 + 7b764be commit b32d9ef
Show file tree
Hide file tree
Showing 11 changed files with 366 additions and 113 deletions.
217 changes: 120 additions & 97 deletions compiler/rustc_mir_build/src/build/matches/mod.rs

Large diffs are not rendered by default.

24 changes: 24 additions & 0 deletions tests/mir-opt/or_pattern.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// skip-filecheck

// EMIT_MIR or_pattern.shortcut_second_or.SimplifyCfg-initial.after.mir
fn shortcut_second_or() {
// Check that after matching `0`, failing to match `2 | 3` skips trying to match `(1, 2 | 3)`.
match ((0, 0), 0) {
(x @ (0, _) | x @ (_, 1), y @ 2 | y @ 3) => {}
_ => {}
}
}

// EMIT_MIR or_pattern.single_switchint.SimplifyCfg-initial.after.mir
fn single_switchint() {
// Check how many `SwitchInt`s we do. In theory a single one is necessary.
match (1, true) {
(1, true) => 1,
(2, false) => 2,
(1 | 2, true | false) => 3,
(3 | 4, true | false) => 4,
_ => 5,
};
}

fn main() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
// MIR for `shortcut_second_or` after SimplifyCfg-initial

fn shortcut_second_or() -> () {
let mut _0: ();
let mut _1: ((i32, i32), i32);
let mut _2: (i32, i32);
let _3: (i32, i32);
let _4: i32;
scope 1 {
debug x => _3;
debug y => _4;
}

bb0: {
StorageLive(_1);
StorageLive(_2);
_2 = (const 0_i32, const 0_i32);
_1 = (move _2, const 0_i32);
StorageDead(_2);
PlaceMention(_1);
switchInt(((_1.0: (i32, i32)).0: i32)) -> [0: bb4, otherwise: bb2];
}

bb1: {
_0 = const ();
goto -> bb14;
}

bb2: {
switchInt(((_1.0: (i32, i32)).1: i32)) -> [1: bb3, otherwise: bb1];
}

bb3: {
switchInt((_1.1: i32)) -> [2: bb7, 3: bb8, otherwise: bb1];
}

bb4: {
switchInt((_1.1: i32)) -> [2: bb5, 3: bb6, otherwise: bb1];
}

bb5: {
falseEdge -> [real: bb10, imaginary: bb6];
}

bb6: {
falseEdge -> [real: bb11, imaginary: bb2];
}

bb7: {
falseEdge -> [real: bb12, imaginary: bb8];
}

bb8: {
falseEdge -> [real: bb13, imaginary: bb1];
}

bb9: {
_0 = const ();
StorageDead(_4);
StorageDead(_3);
goto -> bb14;
}

bb10: {
StorageLive(_3);
_3 = (_1.0: (i32, i32));
StorageLive(_4);
_4 = (_1.1: i32);
goto -> bb9;
}

bb11: {
StorageLive(_3);
_3 = (_1.0: (i32, i32));
StorageLive(_4);
_4 = (_1.1: i32);
goto -> bb9;
}

bb12: {
StorageLive(_3);
_3 = (_1.0: (i32, i32));
StorageLive(_4);
_4 = (_1.1: i32);
goto -> bb9;
}

bb13: {
StorageLive(_3);
_3 = (_1.0: (i32, i32));
StorageLive(_4);
_4 = (_1.1: i32);
goto -> bb9;
}

bb14: {
StorageDead(_1);
return;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
// MIR for `single_switchint` after SimplifyCfg-initial

fn single_switchint() -> () {
let mut _0: ();
let _1: i32;
let mut _2: (i32, bool);

bb0: {
StorageLive(_1);
StorageLive(_2);
_2 = (const 1_i32, const true);
PlaceMention(_2);
switchInt((_2.0: i32)) -> [1: bb2, 2: bb4, otherwise: bb1];
}

bb1: {
switchInt((_2.0: i32)) -> [3: bb8, 4: bb8, otherwise: bb7];
}

bb2: {
switchInt((_2.1: bool)) -> [0: bb6, otherwise: bb3];
}

bb3: {
falseEdge -> [real: bb9, imaginary: bb4];
}

bb4: {
switchInt((_2.1: bool)) -> [0: bb5, otherwise: bb6];
}

bb5: {
falseEdge -> [real: bb10, imaginary: bb6];
}

bb6: {
falseEdge -> [real: bb11, imaginary: bb1];
}

bb7: {
_1 = const 5_i32;
goto -> bb13;
}

bb8: {
falseEdge -> [real: bb12, imaginary: bb7];
}

bb9: {
_1 = const 1_i32;
goto -> bb13;
}

bb10: {
_1 = const 2_i32;
goto -> bb13;
}

bb11: {
_1 = const 3_i32;
goto -> bb13;
}

bb12: {
_1 = const 4_i32;
goto -> bb13;
}

bb13: {
StorageDead(_2);
StorageDead(_1);
_0 = const ();
return;
}
}
1 change: 1 addition & 0 deletions tests/ui/or-patterns/bindings-runpass-2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
2 changes: 1 addition & 1 deletion tests/ui/or-patterns/inner-or-pat.or3.stderr
Original file line number Diff line number Diff line change
@@ -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`
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/or-patterns/inner-or-pat.or4.stderr
Original file line number Diff line number Diff line change
@@ -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`
Expand Down
4 changes: 1 addition & 3 deletions tests/ui/or-patterns/inner-or-pat.rs
Original file line number Diff line number Diff line change
@@ -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)]
Expand Down
21 changes: 10 additions & 11 deletions tests/ui/or-patterns/issue-70413-no-unreachable-pat-and-guard.rs
Original file line number Diff line number Diff line change
@@ -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!(),
}
}
22 changes: 22 additions & 0 deletions tests/ui/or-patterns/search-via-bindings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
}
11 changes: 11 additions & 0 deletions tests/ui/or-patterns/simplification_subtleties.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
//@ run-pass

#[allow(unreachable_patterns)]
fn main() {
// Test that we don't naively sort the two `2`s together and confuse the failure paths.
match (1, true) {
(1 | 2, false | false) => unreachable!(),
(2, _) => unreachable!(),
_ => {}
}
}

0 comments on commit b32d9ef

Please sign in to comment.