Incorrect non-exaustive match statement (where the user may think that all case are covered)

[robinmoussu]

Given the following code:

match (order(lhs), order(rhs)) {
    (lhs_, rhs_) if lhs_  < rhs_ => std::cmp::Ordering::Less,
    (lhs_, rhs_) if lhs_  > rhs_ => std::cmp::Ordering::Greater,
    (lhs_, rhs_) if lhs_ == rhs_ => lhs.cmp(rhs),
}

The current output is:

  2 src/main.rs|26 col 15 error   4| non-exhaustive patterns: `(_, _)` not covered                                                                                                                                                                            
  3 ||    |
  4 || 26 |         match (order(lhs), order(rhs)) {
  5 ||    |               ^^^^^^^^^^^^^^^^^^^^^^^^ pattern `(_, _)` not covered
  6 ||    |
  7 ||    = help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms
  8 ||    = note: the matched value is of type `(i32, i32)`

For more information about this error, try `rustc --explain E0004`.

At first, I thought it was a compiler bug (an understandable one since exhaustiveness checking is hard), but then I realized that an buggy (or adversarial) implementation of Ord could lead to lhs being neither less than, greater than nor equal to rhs. So the compiler is right, just confusing.

Ideally the output should look like (line 8 has been edited):

  2 src/main.rs|26 col 15 error   4| non-exhaustive patterns: `(_, _)` not covered                                                                                                                                                                            
  3 ||    |
  4 || 26 |         match (order(lhs), order(rhs)) {
  5 ||    |               ^^^^^^^^^^^^^^^^^^^^^^^^ pattern `(_, _)` not covered
  6 ||    |
  7 ||    = help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms
  8 ||    = help: a bad implementation of comparison operators of the matched type could lead to all cases not being handled
  9 ||    = 
 10 ||    = note: the matched value is of type `(i32, i32)`

For more information about this error, try `rustc --explain E0004`.

And the error E0004 could contain or more detailed explanation of why such code isn’t covering all cases.

It addition, but I’m less sure of this, a suggestion to be made to either change the last arm to a wildcard (_ => lhs.cmp(rhs) or to add an extra arm _ => unreachable!() with an explanation of why the user should choose one or the other.

Of course in this specific case (i32, i32) we could trust the implementation, but I’m not sure either that it would be a good idea to special-case them (since it would be even more confusing for a user type that is not special cased).

[PatchMixolydic]

This doesn't necessarily require an adversarial implementation of the comparison operators; (f32::NAN, f32::NAN) will not match any of these arms. This code panics (playground):

fn main() {
    match (f32::NAN, f32::NAN) {
        (x, y) if x > y => {},
        (x, y) if x < y => {},
        (x, y) if x == y => {},
        _ => unreachable!("oh no"),
    }
}

This is a correct implementation of the comparison operators since f32 implements PartialOrd, but not Ord, thus certain values of f32 are allowed to not have a defined order. Ord provides stronger guarantees, but as you've already mentioned, an adversarial implementation can easily break them.

@rustbot modify labels: +A-exhaustiveness-checking +D-papercut +D-terse

[workingjubilee]

I do not believe if guards factor into exhaustiveness checking at all, do they? Aside from them not being considered equivalent to the bare pattern. I believe the help could be improved, but it should probably go in an entirely different direction and explain that as an if-guard is actually "arbitrary code that returns a bool", it is not considered logically related.

[BGR360]

I do not believe if guards factor into exhaustiveness checking at all, do they?

Looks like nope (playground):

fn foo(i: i32) {
    match i {
        i if i == 0 => {},
        i if i != 0 => {},
    }
}

fn main() {}

error[E0004]: non-exhaustive patterns: `_` not covered
 --> src/main.rs:2:11
  |
2 |     match i {
  |           ^ pattern `_` not covered
  |
  = help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms
  = note: the matched value is of type `i32`

[Nadrieril]

Guards indeed don't and likely will never count into exhaustiveness checking. I think that's a common mistake though? It would be nice if we could detect this.

A potential good start would be: if every arm in a match has a guard it's almost surely a mistake, so we can remind the user that guards don't count. If someone want to have a go, you likely want to add a check somewhere in this function

[ericmarkmartin]

@rustbot claim

[Nadrieril]

A note was added in #113019, thank you @ericmarkmartin! I think that's the most we can do about this issue