Incorrect "unreachable pattern" warning.

[steffahn]

Not much to comment about. The supposedly “unreachable” pattern is obviously reached.

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

(Playground)

Output:

42

Errors:

   Compiling playground v0.0.1 (/playground)
warning: unreachable pattern
 --> src/main.rs:4:17
  |
4 |         (a,_) | (_,a) if a > 10 => {println!("{}", a)}
  |                 ^^^^^
  |
  = note: `#[warn(unreachable_patterns)]` on by default

    Finished release [optimized] target(s) in 0.78s
     Running `target/release/playground`

This issue has been assigned to @AminArria via this comment.

[Centril]

Looks like this was injected in 1.41.0, ostensibly in the exhaustiveness checking refactorings.
To find out, let's cc @rustbot ping cleanup

cc @varkor @Nadrieril

[rustbot]

Hey Cleanup Crew ICE-breakers! This bug has been identified as a good
"Cleanup ICE-breaking candidate". In case it's useful, here are some
instructions for tackling these sorts of bugs. Maybe take a look?
Thanks! <3

cc @AminArria @chrissimpkins @contrun @DutchGhost @elshize @ethanboxx @h-michael @HallerPatrick @hdhoang @hellow554 @imtsuki @jakevossen5 @kanru @KarlK90 @LeSeulArtichaut @MAdrianMattocks @matheus-consoli @mental32 @nmccarty @Noah-Kennedy @pard68 @PeytonT @pierreN @Redblueflame @RobbieClarken @RobertoSnap @robjtede @SarthakSingh31 @senden9 @shekohex @sinato @spastorino @turboladen @woshilapin @yerke

[Centril]

The relevant code is in check_match::check_arms:

fn check_arms<'p, 'tcx>(
    cx: &mut MatchCheckCtxt<'p, 'tcx>,
    arms: &[(&'p super::Pat<'tcx>, HirId, bool)],
) -> Matrix<'p, 'tcx> {
    let mut seen = Matrix::empty();
    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) {
            NotUseful => { ... } // Elided, this branch should not be taken in this case.
            Useful(unreachable_subpatterns) => {
                for pat in unreachable_subpatterns {
                    unreachable_pattern(cx.tcx, pat.span, id, None);
                }
            }
            UsefulWithWitness(_) => bug!(),
        }
        if !has_guard {
            seen.push(v);
            if catchall.is_none() && pat_is_catchall(pat) {
                catchall = Some(pat.span);
            }
        }
    }
    seen
}

From the point of exhaustiveness checking this algorithm is correct, but not from a linting POV in the presence of guards. Previously, before the or-patterns reform, we merely interpreted each | as a separate arm, so this was correctly handled from a linting perspective in the NotUseful => { ... } arm. Now however, the linting will happen in the Useful(unreachable_subpatterns) => { ... } arm, as according to is_useful(...), unaware of whether the arm has_guard, the pattern itself has an unreachable alternative.

To fix this, presumably we have to propagate has_guard into is_useful, while being careful not to declare anything exhaustive which isn't for the soundness sensitive bits.

[steffahn]

In case this is still relevant.. I went trying out cargo-bisect-rustc and it gave me:

regressed nightly: nightly-2019-12-04
regressed commit: f577b0e