Skip to content

Commit

Permalink
Fix incorrect pattern warning "unreachable pattern"
Browse files Browse the repository at this point in the history
- Added is_under_guard parameter to _match::is_useful and
  only add to the matrix if false
- Added comments explaining behavior
  • Loading branch information
AminArria committed Mar 26, 2020
1 parent 3c1d9ad commit 5796e6e
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 8 deletions.
42 changes: 36 additions & 6 deletions src/librustc_mir_build/hair/pattern/_match.rs
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 {
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
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) {

// 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

0 comments on commit 5796e6e

Please sign in to comment.