unreachable pattern lint should report some (or all) of the earlier match arms that subsume the unreachable one

[pnkfelix]

Code

enum Sym { A(i32, i32), B, C }

fn foo(s: Sym) -> &'static str {
    match s {
        Sym::B    => "B",
        Sym::A(_, 0) => "Am,0",
        Sym::A(0, _) => "A0,n",
        /* imagine a *lot* of other match arms inserted in here */
        Sym::C       => "C",
        Sym::A(0, 0) => "A0,0",
        Sym::A(_, _) => "Am,n",
    }
}

fn main() {
    foo(Sym::A(0, 0));
    foo(Sym::A(1, 1));
    foo(Sym::B);
    foo(Sym::C);
}

Current output

warning: unreachable pattern
  --> src/main.rs:10:9
   |
10 |         Sym::A(0, 0) => "A0,0",
   |         ^^^^^^^^^^^^
   |
   = note: `#[warn(unreachable_patterns)]` on by default

Desired output

warning: unreachable pattern
  --> src/main.rs:10:9
   |
10 |         Sym::A(0, 0) => "A0,0",
   |         ^^^^^^^^^^^^
   |
   = note: `#[warn(unreachable_patterns)]` on by default
   |
   = note: the first pattern that makes this pattern unreachable is:
  --> src/main.rs:6:9
   |
 6 |         Sym::A(_, 0) => "Am,0",
   |         ^^^^^^^^^^^^

Rationale and extra context

I was recently adding a new enum variant that had a ton of other enum variants. I had to update an associated match expression to include it. Then, at some later point, I forgot that I had updated that match expression, redundantly added new match arms for the same variant, and got the above diangostic (which was set to be denied in my project and so it was a hard error).

I spent a little while puzzling over how the pattern could possibly be unreachable before I realized my mistake, in part because the list was so long that I had inserted the new variants at different points in the (unsorted) series of match arms. It was relatively trivial for me to diagnose in my own case, but I think my example above helps illustrate hypothetical scenarios where the compiler could do this mechanical work for us.

(Another options might be to report all the arms that would each independently subsume the unreachable one, as I have two such arms in my example above. But I'm not sure it would be worth the effort of computing that set ... its probably good enough to give the user the first counter-example and let them iteratively invoke the compiler if that's the usage pattern they want to follow.)

Other cases

No response

Rust Version

playground nightly version: 1.81.0-nightly

(2024-07-12 c6727fc9b5c64cefa726)

Anything else?

No response

[pnkfelix]

In truth my original intent was to make an example that would motivate showing multiple arms that are collectively causing the unreachable arm to become unreachable. But I got the logic backward and gave an example that showed the opposite of that (i.e. multiple arms where each independently makes the problematic one unreachable).

Here's an example of what I mean by a collective set of arms making a later one unreachable (playground):

enum Sym { A(i32, i32), B, C }

fn foo(s: (Sym, bool)) -> &'static str {
    match s {
        (Sym::B, _)    => "B",
        (Sym::A(_, 0), true) => "Am,0",
        (Sym::A(0, _), false) => "A0,n",
        /* imagine a *lot* of other match arms inserted in here */
        (Sym::C, _)       => "C",
        (Sym::A(0, 0), _) => "A0,0",
        (Sym::A(_, _), _) => "Am,n",
    }
}

fn main() {
    foo((Sym::A(0, 0), true));
    foo((Sym::A(1, 1), false));
    foo((Sym::B, true));
    foo((Sym::C, false));
}

This is the scenario where I think the compiler can be most helpful in reporting multiple arms, because their cumulative effect is not trivial for a human to immediately see.

So I would like in this case to see output like:

warning: unreachable pattern
  --> src/main.rs:10:9
   |
10 |         (Sym::A(0, 0), _) => "A0,0",
   |         ^^^^^^^^^^^^^^^^^
   |
   = note: `#[warn(unreachable_patterns)]` on by default
   |
   = note: some patterns that collectively make this pattern unreachable are:
  --> src/main.rs:6:9
   |
 6 |         (Sym::A(_, 0), true) => "Am,0",
   |         ^^^^^^^^^^^^^^^^^^^^
 7 |         (Sym::A(0, _), false) => "A0,n",
   |         ^^^^^^^^^^^^^^^^^^^^^

[compiler-errors]

@Nadrieril could probably weigh in on the feasibility of recording this information (or possibly re-deriving it on error). Seems hard to do the former, from scanning and being vaguely familiar with it from reviewing Nadrieril's changes some months ago, but perhaps it's easier to redo on the error path.

[Nadrieril]

This sounds doable actually! We track intersection data today: we know for a given branch which branches above it match values in common. Unfortunately we only compute an underapproximation to prevent complexity blowups, but I'd guess it's good enough for diagnostics.

The report might look like "such and such branch intersect this one, they contribute to making it unreachable"

[estebank]

but I'd guess it's good enough for diagnostics.

Yeah, incomplete context is better than no context. All of the subtle cases I could think of are actually situations in which naïve output would be sufficient for the user to understand the issue.

[Nadrieril]

It's easy to also compute an over-approximation of the intersections actually. The following is a case where we can't compute the exact set of intersections. If we take the over-approximation, we get (here everything that matches (true, true, _) is considered to potentially intersect each other):

match ... {
	(true, _, _) => {}
	//^ note: this may match some of the same values
	(_, true, 0..10) => {}
	//^ note: this may match some of the same values
	(_, true, 10..) => {}
	//^ note: this may match some of the same values
	(_, true, 3) => {}
	//^ unreachable pattern
}

If we take the under-approximation we get an empty set of intersections. Not sure which is best.

EDIT: I got confused somehow, we do find an intersection with the second arm which is exactly what we'd like to report. I have to think some more.
EDIT2: see below

[Nadrieril]

After giving this more thought, it appears that the under-approximation is good enough that it still always contains a set of rows that cover the one we care about. Hence the lint is always able to list a set of arms that cause the unreachability.