Skip to content
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

Fix incorrect pattern warning "unreachable pattern" #70413

Merged
merged 2 commits into from
Mar 27, 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
42 changes: 36 additions & 6 deletions src/librustc_mir_build/hair/pattern/_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1619,12 +1619,17 @@ impl<'tcx> fmt::Debug for MissingConstructors<'tcx> {
/// relation to preceding patterns, it is not reachable) and exhaustiveness
/// checking (if a wildcard pattern is useful in relation to a matrix, the
/// matrix isn't exhaustive).
///
/// `is_under_guard` is used to inform if the pattern has a guard. If it
/// has one it must not be inserted into the matrix. This shouldn't be
/// relied on for soundness.
crate fn is_useful<'p, 'tcx>(
cx: &mut MatchCheckCtxt<'p, 'tcx>,
matrix: &Matrix<'p, 'tcx>,
v: &PatStack<'p, 'tcx>,
witness_preference: WitnessPreference,
hir_id: HirId,
is_under_guard: bool,
is_top_level: bool,
) -> Usefulness<'tcx, 'p> {
let &Matrix(ref rows) = matrix;
Expand Down Expand Up @@ -1653,7 +1658,7 @@ crate fn is_useful<'p, 'tcx>(
let mut unreachable_pats = Vec::new();
let mut any_is_useful = false;
for v in vs {
let res = is_useful(cx, &matrix, &v, witness_preference, hir_id, false);
let res = is_useful(cx, &matrix, &v, witness_preference, hir_id, is_under_guard, false);
match res {
Useful(pats) => {
any_is_useful = true;
Expand All @@ -1664,7 +1669,10 @@ crate fn is_useful<'p, 'tcx>(
bug!("Encountered or-pat in `v` during exhaustiveness checking")
}
}
matrix.push(v);
// If pattern has a guard don't add it to the matrix
if !is_under_guard {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add a comment here as well re. the purpose of this check.

Copy link
Contributor Author

@AminArria AminArria Mar 26, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added in 4b1aa9fa

matrix.push(v);
}
}
return if any_is_useful { Useful(unreachable_pats) } else { NotUseful };
}
Expand Down Expand Up @@ -1712,7 +1720,18 @@ crate fn is_useful<'p, 'tcx>(
Some(hir_id),
)
.into_iter()
.map(|c| is_useful_specialized(cx, matrix, v, c, pcx.ty, witness_preference, hir_id))
.map(|c| {
is_useful_specialized(
cx,
matrix,
v,
c,
pcx.ty,
witness_preference,
hir_id,
is_under_guard,
)
})
.find(|result| result.is_useful())
.unwrap_or(NotUseful)
} else {
Expand Down Expand Up @@ -1746,14 +1765,24 @@ crate fn is_useful<'p, 'tcx>(
split_grouped_constructors(cx.tcx, cx.param_env, pcx, all_ctors, matrix, DUMMY_SP, None)
.into_iter()
.map(|c| {
is_useful_specialized(cx, matrix, v, c, pcx.ty, witness_preference, hir_id)
is_useful_specialized(
cx,
matrix,
v,
c,
pcx.ty,
witness_preference,
hir_id,
is_under_guard,
)
})
.find(|result| result.is_useful())
.unwrap_or(NotUseful)
} else {
let matrix = matrix.specialize_wildcard();
let v = v.to_tail();
let usefulness = is_useful(cx, &matrix, &v, witness_preference, hir_id, false);
let usefulness =
is_useful(cx, &matrix, &v, witness_preference, hir_id, is_under_guard, false);

// In this case, there's at least one "free"
// constructor that is only matched against by
Expand Down Expand Up @@ -1810,14 +1839,15 @@ fn is_useful_specialized<'p, 'tcx>(
lty: Ty<'tcx>,
witness_preference: WitnessPreference,
hir_id: HirId,
is_under_guard: bool,
) -> Usefulness<'tcx, 'p> {
debug!("is_useful_specialized({:#?}, {:#?}, {:?})", v, ctor, lty);

let ctor_wild_subpatterns =
cx.pattern_arena.alloc_from_iter(ctor.wildcard_subpatterns(cx, lty));
let matrix = matrix.specialize_constructor(cx, &ctor, ctor_wild_subpatterns);
v.specialize_constructor(cx, &ctor, ctor_wild_subpatterns)
.map(|v| is_useful(cx, &matrix, &v, witness_preference, hir_id, false))
.map(|v| is_useful(cx, &matrix, &v, witness_preference, hir_id, is_under_guard, false))
.map(|u| u.apply_constructor(cx, &ctor, lty))
.unwrap_or(NotUseful)
}
Expand Down
7 changes: 5 additions & 2 deletions src/librustc_mir_build/hair/pattern/check_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,7 @@ fn check_arms<'p, 'tcx>(
let mut catchall = None;
for (arm_index, (pat, id, has_guard)) in arms.iter().copied().enumerate() {
let v = PatStack::from_pattern(pat);
match is_useful(cx, &seen, &v, LeaveOutWitness, id, true) {
match is_useful(cx, &seen, &v, LeaveOutWitness, id, has_guard, true) {
NotUseful => {
match source {
hir::MatchSource::IfDesugar { .. } | hir::MatchSource::WhileDesugar => bug!(),
Expand Down Expand Up @@ -410,7 +410,10 @@ fn check_not_useful<'p, 'tcx>(
) -> Result<(), Vec<super::Pat<'tcx>>> {
let wild_pattern = cx.pattern_arena.alloc(super::Pat::wildcard_from_ty(ty));
let v = PatStack::from_pattern(wild_pattern);
match is_useful(cx, matrix, &v, ConstructWitness, hir_id, true) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you leave a note re. the false in this case?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added in 4b1aa9fa


// false is given for `is_under_guard` argument due to the wildcard
// pattern not having a guard
match is_useful(cx, matrix, &v, ConstructWitness, hir_id, false, true) {
NotUseful => Ok(()), // This is good, wildcard pattern isn't reachable.
UsefulWithWitness(pats) => Err(if pats.is_empty() {
bug!("Exhaustiveness check returned no witnesses")
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// check-pass

#![deny(unreachable_patterns)]

#![feature(or_patterns)]
fn main() {
match (3,42) {
(a,_) | (_,a) if a > 10 => {println!("{}", a)}
_ => ()
}

match Some((3,42)) {
Some((a, _)) | Some((_, a)) if a > 10 => {println!("{}", a)}
_ => ()

}

match Some((3,42)) {
Some((a, _) | (_, a)) if a > 10 => {println!("{}", a)}
_ => ()
}
}