From 459dae94a1c5ea658e78dbe99d88108d9cacc51d Mon Sep 17 00:00:00 2001 From: mark Date: Sat, 7 Nov 2020 15:22:52 -0600 Subject: [PATCH 1/3] fix #72680 by explicitly checking for or-pattern before test --- .../rustc_mir_build/src/build/matches/test.rs | 10 +++ src/test/ui/match/issue-72680.rs | 65 +++++++++++++++++++ 2 files changed, 75 insertions(+) create mode 100644 src/test/ui/match/issue-72680.rs diff --git a/compiler/rustc_mir_build/src/build/matches/test.rs b/compiler/rustc_mir_build/src/build/matches/test.rs index 7bea8220ada74..e4bc4c0743243 100644 --- a/compiler/rustc_mir_build/src/build/matches/test.rs +++ b/compiler/rustc_mir_build/src/build/matches/test.rs @@ -671,6 +671,16 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { (&TestKind::Range { .. }, _) => None, (&TestKind::Eq { .. } | &TestKind::Len { .. }, _) => { + // We do call `test()` below to see what kind of test `match_pair` would require. + // If it is the same test as `test`, then we can just use `test`. + // + // However, `test()` assumes that there won't be any or-patterns, so we need to + // specially handle that here and return `None` (since the `test` clearly doesn't + // apply to an or-pattern). + if let PatKind::Or { .. } = &*match_pair.pattern.kind { + return None; + } + // These are all binary tests. // // FIXME(#29623) we can be more clever here diff --git a/src/test/ui/match/issue-72680.rs b/src/test/ui/match/issue-72680.rs new file mode 100644 index 0000000000000..3e8febb63add3 --- /dev/null +++ b/src/test/ui/match/issue-72680.rs @@ -0,0 +1,65 @@ +// run-pass + +#![feature(or_patterns)] + +fn main() { + assert_eq!(f("", 0), true); + assert_eq!(f("a", 1), true); + assert_eq!(f("b", 1), true); + + assert_eq!(f("", 1), false); + assert_eq!(f("a", 0), false); + assert_eq!(f("b", 0), false); + + assert_eq!(f("asdf", 032), false); + + //// + + assert_eq!(g(true, true, true), false); + assert_eq!(g(false, true, true), false); + assert_eq!(g(true, false, true), false); + assert_eq!(g(false, false, true), false); + assert_eq!(g(true, true, false), false); + + assert_eq!(g(false, true, false), true); + assert_eq!(g(true, false, false), true); + assert_eq!(g(false, false, false), true); + + //// + + assert_eq!(h(true, true, true), false); + assert_eq!(h(false, true, true), false); + assert_eq!(h(true, false, true), false); + assert_eq!(h(false, false, true), false); + assert_eq!(h(true, true, false), false); + + assert_eq!(h(false, true, false), true); + assert_eq!(h(true, false, false), true); + assert_eq!(h(false, false, false), true); +} + +fn f(s: &str, num: usize) -> bool { + match (s, num) { + ("", 0) | ("a" | "b", 1) => true, + + _ => false, + } +} + +fn g(x: bool, y: bool, z: bool) -> bool { + match (x, y, x, z) { + (true | false, false, true, false) => true, + (false, true | false, true | false, false) => true, + (true | false, true | false, true | false, true) => false, + (true, true | false, true | false, false) => false, + } +} + +fn h(x: bool, y: bool, z: bool) -> bool { + match (x, (y, (x, (z,)))) { + (true | false, (false, (true, (false,)))) => true, + (false, (true | false, (true | false, (false,)))) => true, + (true | false, (true | false, (true | false, (true,)))) => false, + (true, (true | false, (true | false, (false,)))) => false, + } +} From 43e4783ce3d833af82d76f6be12f3f4863b0638c Mon Sep 17 00:00:00 2001 From: mark Date: Mon, 9 Nov 2020 12:19:34 -0600 Subject: [PATCH 2/3] address reviewer comments --- .../rustc_mir_build/src/build/matches/test.rs | 11 +++-- src/test/ui/match/issue-72680.rs | 46 +++++++++---------- 2 files changed, 29 insertions(+), 28 deletions(-) diff --git a/compiler/rustc_mir_build/src/build/matches/test.rs b/compiler/rustc_mir_build/src/build/matches/test.rs index e4bc4c0743243..07173f41cd6db 100644 --- a/compiler/rustc_mir_build/src/build/matches/test.rs +++ b/compiler/rustc_mir_build/src/build/matches/test.rs @@ -671,12 +671,13 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { (&TestKind::Range { .. }, _) => None, (&TestKind::Eq { .. } | &TestKind::Len { .. }, _) => { - // We do call `test()` below to see what kind of test `match_pair` would require. - // If it is the same test as `test`, then we can just use `test`. + // The call to `self.test(&match_pair)` below is not actually used to generate any + // MIR. Instead, we just want to compare with `test` (the parameter of the method) + // to see if it is the same. // - // However, `test()` assumes that there won't be any or-patterns, so we need to - // specially handle that here and return `None` (since the `test` clearly doesn't - // apply to an or-pattern). + // However, at this point we can still encounter or-patterns that were extracted + // from previous calls to `sort_candidate`, so we need to manually address that + // case to avoid panicking in `self.test()`. if let PatKind::Or { .. } = &*match_pair.pattern.kind { return None; } diff --git a/src/test/ui/match/issue-72680.rs b/src/test/ui/match/issue-72680.rs index 3e8febb63add3..e6a723a6609f8 100644 --- a/src/test/ui/match/issue-72680.rs +++ b/src/test/ui/match/issue-72680.rs @@ -3,39 +3,39 @@ #![feature(or_patterns)] fn main() { - assert_eq!(f("", 0), true); - assert_eq!(f("a", 1), true); - assert_eq!(f("b", 1), true); + assert!(f("", 0)); + assert!(f("a", 1)); + assert!(f("b", 1)); - assert_eq!(f("", 1), false); - assert_eq!(f("a", 0), false); - assert_eq!(f("b", 0), false); + assert!(!f("", 1)); + assert!(!f("a", 0)); + assert!(!f("b", 0)); - assert_eq!(f("asdf", 032), false); + assert!(!f("asdf", 032)); //// - assert_eq!(g(true, true, true), false); - assert_eq!(g(false, true, true), false); - assert_eq!(g(true, false, true), false); - assert_eq!(g(false, false, true), false); - assert_eq!(g(true, true, false), false); + assert!(!g(true, true, true)); + assert!(!g(false, true, true)); + assert!(!g(true, false, true)); + assert!(!g(false, false, true)); + assert!(!g(true, true, false)); - assert_eq!(g(false, true, false), true); - assert_eq!(g(true, false, false), true); - assert_eq!(g(false, false, false), true); + assert!(g(false, true, false)); + assert!(g(true, false, false)); + assert!(g(false, false, false)); //// - assert_eq!(h(true, true, true), false); - assert_eq!(h(false, true, true), false); - assert_eq!(h(true, false, true), false); - assert_eq!(h(false, false, true), false); - assert_eq!(h(true, true, false), false); + assert!(!h(true, true, true)); + assert!(!h(false, true, true)); + assert!(!h(true, false, true)); + assert!(!h(false, false, true)); + assert!(!h(true, true, false)); - assert_eq!(h(false, true, false), true); - assert_eq!(h(true, false, false), true); - assert_eq!(h(false, false, false), true); + assert!(h(false, true, false)); + assert!(h(true, false, false)); + assert!(h(false, false, false)); } fn f(s: &str, num: usize) -> bool { From b825ae7d28294de48297cb2e426f68c2331b3fdc Mon Sep 17 00:00:00 2001 From: Who? Me?! Date: Sat, 14 Nov 2020 07:20:25 -0600 Subject: [PATCH 3/3] Style nit Co-authored-by: matthewjasper <20113453+matthewjasper@users.noreply.github.com> --- src/test/ui/match/issue-72680.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/ui/match/issue-72680.rs b/src/test/ui/match/issue-72680.rs index e6a723a6609f8..5b933edc8205e 100644 --- a/src/test/ui/match/issue-72680.rs +++ b/src/test/ui/match/issue-72680.rs @@ -11,7 +11,7 @@ fn main() { assert!(!f("a", 0)); assert!(!f("b", 0)); - assert!(!f("asdf", 032)); + assert!(!f("asdf", 32)); ////