From 4030b7394845602eebb42f40c0e847be803af58f Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Thu, 2 Jul 2020 21:03:59 +0100 Subject: [PATCH 1/2] Use `Span`s to identify unreachable subpatterns in or-patterns --- src/librustc_mir_build/hair/pattern/_match.rs | 14 +++++++------- src/librustc_mir_build/hair/pattern/check_match.rs | 4 ++-- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/librustc_mir_build/hair/pattern/_match.rs b/src/librustc_mir_build/hair/pattern/_match.rs index 18b92bf29bf..b5a655e4218 100644 --- a/src/librustc_mir_build/hair/pattern/_match.rs +++ b/src/librustc_mir_build/hair/pattern/_match.rs @@ -1246,15 +1246,15 @@ impl<'p, 'tcx> Fields<'p, 'tcx> { } #[derive(Clone, Debug)] -crate enum Usefulness<'tcx, 'p> { +crate enum Usefulness<'tcx> { /// Carries a list of unreachable subpatterns. Used only in the presence of or-patterns. - Useful(Vec<&'p Pat<'tcx>>), + Useful(Vec), /// Carries a list of witnesses of non-exhaustiveness. UsefulWithWitness(Vec>), NotUseful, } -impl<'tcx, 'p> Usefulness<'tcx, 'p> { +impl<'tcx> Usefulness<'tcx> { fn new_useful(preference: WitnessPreference) -> Self { match preference { ConstructWitness => UsefulWithWitness(vec![Witness(vec![])]), @@ -1269,7 +1269,7 @@ impl<'tcx, 'p> Usefulness<'tcx, 'p> { } } - fn apply_constructor( + fn apply_constructor<'p>( self, cx: &MatchCheckCtxt<'p, 'tcx>, ctor: &Constructor<'tcx>, @@ -1828,7 +1828,7 @@ crate fn is_useful<'p, 'tcx>( hir_id: HirId, is_under_guard: bool, is_top_level: bool, -) -> Usefulness<'tcx, 'p> { +) -> Usefulness<'tcx> { let &Matrix(ref rows) = matrix; debug!("is_useful({:#?}, {:#?})", matrix, v); @@ -1861,7 +1861,7 @@ crate fn is_useful<'p, 'tcx>( any_is_useful = true; unreachable_pats.extend(pats); } - NotUseful => unreachable_pats.push(v.head()), + NotUseful => unreachable_pats.push(v.head().span), UsefulWithWitness(_) => { bug!("Encountered or-pat in `v` during exhaustiveness checking") } @@ -2014,7 +2014,7 @@ fn is_useful_specialized<'p, 'tcx>( witness_preference: WitnessPreference, hir_id: HirId, is_under_guard: bool, -) -> Usefulness<'tcx, 'p> { +) -> Usefulness<'tcx> { debug!("is_useful_specialized({:#?}, {:#?}, {:?})", v, ctor, ty); // We cache the result of `Fields::wildcards` because it is used a lot. diff --git a/src/librustc_mir_build/hair/pattern/check_match.rs b/src/librustc_mir_build/hair/pattern/check_match.rs index 6fc447a87f5..6b8110fde69 100644 --- a/src/librustc_mir_build/hair/pattern/check_match.rs +++ b/src/librustc_mir_build/hair/pattern/check_match.rs @@ -392,8 +392,8 @@ fn check_arms<'p, 'tcx>( } } Useful(unreachable_subpatterns) => { - for pat in unreachable_subpatterns { - unreachable_pattern(cx.tcx, pat.span, id, None); + for span in unreachable_subpatterns { + unreachable_pattern(cx.tcx, span, id, None); } } UsefulWithWitness(_) => bug!(), From 3cb31b6699558737b1a4650537f0facdc8cb7852 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Thu, 2 Jul 2020 21:49:58 +0100 Subject: [PATCH 2/2] Fix #71977 --- src/librustc_mir_build/hair/pattern/_match.rs | 36 ++++++++++++++--- .../exhaustiveness-unreachable-pattern.rs | 28 +++++++++++++ .../exhaustiveness-unreachable-pattern.stderr | 40 ++++++++++++------- .../ui/or-patterns/search-via-bindings.rs | 1 - 4 files changed, 85 insertions(+), 20 deletions(-) diff --git a/src/librustc_mir_build/hair/pattern/_match.rs b/src/librustc_mir_build/hair/pattern/_match.rs index b5a655e4218..372cb783f50 100644 --- a/src/librustc_mir_build/hair/pattern/_match.rs +++ b/src/librustc_mir_build/hair/pattern/_match.rs @@ -276,6 +276,7 @@ use self::Usefulness::*; use self::WitnessPreference::*; use rustc_data_structures::captures::Captures; +use rustc_data_structures::fx::FxHashSet; use rustc_index::vec::Idx; use super::{compare_const_vals, PatternFoldable, PatternFolder}; @@ -1852,16 +1853,35 @@ crate fn is_useful<'p, 'tcx>( // We need to push the already-seen patterns into the matrix in order to detect redundant // branches like `Some(_) | Some(0)`. We also keep track of the unreachable subpatterns. let mut matrix = matrix.clone(); - let mut unreachable_pats = Vec::new(); + // `Vec` of all the unreachable branches of the current or-pattern. + let mut unreachable_branches = Vec::new(); + // Subpatterns that are unreachable from all branches. E.g. in the following case, the last + // `true` is unreachable only from one branch, so it is overall reachable. + // ``` + // match (true, true) { + // (true, true) => {} + // (false | true, false | true) => {} + // } + // ``` + let mut unreachable_subpats = FxHashSet::default(); + // Whether any branch at all is useful. let mut any_is_useful = false; + for v in vs { let res = is_useful(cx, &matrix, &v, witness_preference, hir_id, is_under_guard, false); match res { Useful(pats) => { - any_is_useful = true; - unreachable_pats.extend(pats); + if !any_is_useful { + any_is_useful = true; + // Initialize with the first set of unreachable subpatterns encountered. + unreachable_subpats = pats.into_iter().collect(); + } else { + // Keep the patterns unreachable from both this and previous branches. + unreachable_subpats = + pats.into_iter().filter(|p| unreachable_subpats.contains(p)).collect(); + } } - NotUseful => unreachable_pats.push(v.head().span), + NotUseful => unreachable_branches.push(v.head().span), UsefulWithWitness(_) => { bug!("Encountered or-pat in `v` during exhaustiveness checking") } @@ -1871,7 +1891,13 @@ crate fn is_useful<'p, 'tcx>( matrix.push(v); } } - return if any_is_useful { Useful(unreachable_pats) } else { NotUseful }; + if any_is_useful { + // Collect all the unreachable patterns. + unreachable_branches.extend(unreachable_subpats); + return Useful(unreachable_branches); + } else { + return NotUseful; + } } // FIXME(Nadrieril): Hack to work around type normalization issues (see #72476). diff --git a/src/test/ui/or-patterns/exhaustiveness-unreachable-pattern.rs b/src/test/ui/or-patterns/exhaustiveness-unreachable-pattern.rs index 44bae282d88..a1147cb5cfc 100644 --- a/src/test/ui/or-patterns/exhaustiveness-unreachable-pattern.rs +++ b/src/test/ui/or-patterns/exhaustiveness-unreachable-pattern.rs @@ -29,6 +29,9 @@ fn main() { (1, 4 | 5) => {} //~ ERROR unreachable pattern _ => {} } + match (true, true) { + (false | true, false | true) => (), + } match (Some(0u8),) { (None | Some(1 | 2),) => {} (Some(1),) => {} //~ ERROR unreachable pattern @@ -67,4 +70,29 @@ fn main() { | 1) => {} _ => {} } + + // A subpattern that is only unreachable in one branch is overall reachable. + match (true, true) { + (true, true) => {} + (false | true, false | true) => {} + } + match (true, true) { + (true, false) => {} + (false, true) => {} + (false | true, false | true) => {} + } + // A subpattern that is unreachable in all branches is overall unreachable. + match (true, true) { + (false, true) => {} + (true, true) => {} + (false | true, false + | true) => {} //~ ERROR unreachable + } + match (true, true) { + (true, false) => {} + (true, true) => {} + (false + | true, //~ ERROR unreachable + false | true) => {} + } } diff --git a/src/test/ui/or-patterns/exhaustiveness-unreachable-pattern.stderr b/src/test/ui/or-patterns/exhaustiveness-unreachable-pattern.stderr index bef6f8270bc..d92b545a869 100644 --- a/src/test/ui/or-patterns/exhaustiveness-unreachable-pattern.stderr +++ b/src/test/ui/or-patterns/exhaustiveness-unreachable-pattern.stderr @@ -53,52 +53,64 @@ LL | (1, 4 | 5) => {} | ^^^^^^^^^^ error: unreachable pattern - --> $DIR/exhaustiveness-unreachable-pattern.rs:34:9 + --> $DIR/exhaustiveness-unreachable-pattern.rs:37:9 | LL | (Some(1),) => {} | ^^^^^^^^^^ error: unreachable pattern - --> $DIR/exhaustiveness-unreachable-pattern.rs:35:9 + --> $DIR/exhaustiveness-unreachable-pattern.rs:38:9 | LL | (None,) => {} | ^^^^^^^ error: unreachable pattern - --> $DIR/exhaustiveness-unreachable-pattern.rs:40:9 + --> $DIR/exhaustiveness-unreachable-pattern.rs:43:9 | LL | ((1..=4,),) => {} | ^^^^^^^^^^^ error: unreachable pattern - --> $DIR/exhaustiveness-unreachable-pattern.rs:45:14 + --> $DIR/exhaustiveness-unreachable-pattern.rs:48:14 | LL | (1 | 1,) => {} | ^ error: unreachable pattern - --> $DIR/exhaustiveness-unreachable-pattern.rs:52:15 - | -LL | | 0] => {} - | ^ - -error: unreachable pattern - --> $DIR/exhaustiveness-unreachable-pattern.rs:50:15 + --> $DIR/exhaustiveness-unreachable-pattern.rs:53:15 | LL | | 0 | ^ error: unreachable pattern - --> $DIR/exhaustiveness-unreachable-pattern.rs:60:10 + --> $DIR/exhaustiveness-unreachable-pattern.rs:55:15 + | +LL | | 0] => {} + | ^ + +error: unreachable pattern + --> $DIR/exhaustiveness-unreachable-pattern.rs:63:10 | LL | [1 | ^ error: unreachable pattern - --> $DIR/exhaustiveness-unreachable-pattern.rs:66:14 + --> $DIR/exhaustiveness-unreachable-pattern.rs:69:14 | LL | Some(0 | ^ -error: aborting due to 16 previous errors +error: unreachable pattern + --> $DIR/exhaustiveness-unreachable-pattern.rs:89:15 + | +LL | | true) => {} + | ^^^^ + +error: unreachable pattern + --> $DIR/exhaustiveness-unreachable-pattern.rs:95:15 + | +LL | | true, + | ^^^^ + +error: aborting due to 18 previous errors diff --git a/src/test/ui/or-patterns/search-via-bindings.rs b/src/test/ui/or-patterns/search-via-bindings.rs index eb127b881cd..067e617373a 100644 --- a/src/test/ui/or-patterns/search-via-bindings.rs +++ b/src/test/ui/or-patterns/search-via-bindings.rs @@ -3,7 +3,6 @@ // run-pass #![feature(or_patterns)] -#![allow(unreachable_patterns)] // FIXME(or-patterns) this shouldn't trigger fn search(target: (bool, bool, bool)) -> u32 { let x = ((false, true), (false, true), (false, true));