Skip to content

Use Spans to identify unreachable subpatterns in or-patterns #73973

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jul 6, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 37 additions & 11 deletions src/librustc_mir_build/hair/pattern/_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -1246,15 +1247,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<Span>),
/// Carries a list of witnesses of non-exhaustiveness.
UsefulWithWitness(Vec<Witness<'tcx>>),
NotUseful,
}

impl<'tcx, 'p> Usefulness<'tcx, 'p> {
impl<'tcx> Usefulness<'tcx> {
fn new_useful(preference: WitnessPreference) -> Self {
match preference {
ConstructWitness => UsefulWithWitness(vec![Witness(vec![])]),
Expand All @@ -1269,7 +1270,7 @@ impl<'tcx, 'p> Usefulness<'tcx, 'p> {
}
}

fn apply_constructor(
fn apply_constructor<'p>(
self,
cx: &MatchCheckCtxt<'p, 'tcx>,
ctor: &Constructor<'tcx>,
Expand Down Expand Up @@ -1828,7 +1829,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);

Expand All @@ -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()),
NotUseful => unreachable_branches.push(v.head().span),
UsefulWithWitness(_) => {
bug!("Encountered or-pat in `v` during exhaustiveness checking")
}
Expand All @@ -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).
Expand Down Expand Up @@ -2014,7 +2040,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.
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_mir_build/hair/pattern/check_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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!(),
Expand Down
28 changes: 28 additions & 0 deletions src/test/ui/or-patterns/exhaustiveness-unreachable-pattern.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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) => {}
}
}
34 changes: 23 additions & 11 deletions src/test/ui/or-patterns/exhaustiveness-unreachable-pattern.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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
--> $DIR/exhaustiveness-unreachable-pattern.rs:53:15
|
LL | | 0] => {}
LL | | 0
| ^

error: unreachable pattern
--> $DIR/exhaustiveness-unreachable-pattern.rs:50:15
--> $DIR/exhaustiveness-unreachable-pattern.rs:55:15
|
LL | | 0
LL | | 0] => {}
| ^

error: unreachable pattern
--> $DIR/exhaustiveness-unreachable-pattern.rs:60:10
--> $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

1 change: 0 additions & 1 deletion src/test/ui/or-patterns/search-via-bindings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down