Exhaustive match of number treated as non-exhaustive

[ebiggers]

The following function does not compile because rustc determines that the patterns in the match statement are non-exhaustive. However, the pattern matches every possible value of the integer type, and the compiler should be able to detect this.

fn wont_compile(x : u8) { 
    match (x) {
        0x00 .. 0xff => { }
    }
}

[jkleint]

I assume this is the same issue:

fn main() {
    println!("{}", match 1i {
        n if n > 0  => "greater",
        n if n < 0  => "less",
        n if n == 0 => "equal",
    });
}
// error: non-exhaustive patterns: `_` not covered [E0004]

[thecoshman]

Whilst I agree that your example is exhaustive, it's not a good example.

match 1i {
    n if n > 0  => "greater",
    n if n < 0  => "less",
    _ => "equal",
}

This is effectively the same, except it compiles. Keep in mind match checks the patterns in order. You wouldn't write:

if (n > 0) { "greater" } 
else if (n < 0) { "less" }
else if (n == 0) { "equal" }

you would just drop the 'if (n == 0)' from that last clause.

[drewm1980]

Doing the test for n==0 is less efficient, but IMHO more readable. I would probably use your pattern but leave n==0 commented out.

[steveklabnik]

The compiler is never going to be able to fully determine these kinds of things, as they would involve evaluating certain things at compile time. For example:

0x00 ... (0xff + 1 - 1) => { }

This arm would be equivalent, but would report as non-exhaustive, too.

As such, I'm giving this a close. I don't think we can improve this.

[chris-morgan]

@steveklabnik I disagree with your closing of this issue; could you please open it again? The issue is entirely genuine, is blocking correct code (though admittedly there is an easy workaround: change the last pattern to an _) and is a nuisance.

Moreover, your fabricated example is invalid, because you can’t do arithmetic in patterns.

@jkleint Your example is a completely different issue and is not solvable. n > 0 || n < 0 || n == 0 is not necessarily true, though it is declared by the programmer to be true if Eq and Ord are implemented for the type.

[steveklabnik]

@chris-morgan I'm not understanding how this issue is solvable. To grok the range, we need to evaluate code at compile time, it can't work.

[chris-morgan]

@steveklabnik it’s a pattern, so the numbers are literals or constants. No additional evaluation required. The compiler needs to know them in order to compile the code anyway.

[jkleint]

@chris-morgan aren't Eq and Ord implemented for integers? My example was an integer. :)

[carloslbello]

I've written a POC script that compares a set of ranges with 1 or more exhaustive ranges for the respective type and outputs any ranges that are not covered: https://gist.github.com/aarzee/4395cb34eef31ea4e5c4

I'm interested in implementing this logic in the compiler but looking at check_match.rs I have absolutely no idea how I would do this. Also, this code has opportunities to take advantage of more types in the future, as currently floats aren't supported by it, and specialization and native inclusive ranges have not yet been added to the language.

[jonas-schievink]

Implementing this would cause the following to fail to compile since the last arm is unreachable:

match 0u8 {
    0...255 => {}
    _ => {}
}

Should a warning be emitted first? Is an RFC needed? Should this be ignored without warning, like it is now? Make it a deny-by-default Lint to prevent breaking dependencies? So many questions!

[chris-morgan]

To be sure, implementing this will cause certain code that compiles now to cease to compile, but it was bad code to begin with and a bug in the compiler is being fixed. And it is very unlikely to actually affect anyone in practice, because I doubt that anyone has actually written such code. Thus it fits easily inside the stability-retention guidelines. I do not believe any RFC is required, nor any ability to disable the new behaviour.

[jonas-schievink]

I doubt that anyone has actually written such code

Not this exact code, sure. But maybe an opcode dispatch switch that handles all possible byte values? Or one that handles all undefined opcodes with an arm like 0xab ... 0xff => panic!("invalid"), and an additional _ => unreachable!() (because this is currently required).
Maybe some people match on the high bit of a byte like this: 0 ... 127 => "high bit clear", 128 ... 255 => "high bit set", _ => unreachable!()
All of these would stop compiling if this is implemented, so I think the impact should at least be evaluated.

[chris-morgan]

Hmm, good point. I myself have just changed the last branch into _, preferring not to put an extra and superfluous unreachable!() into the code, but I can see that someone could easily have gone the other way.

[jonas-schievink]

I wrote a bit of code that at least seems to work semi-correctly for a few test cases: https://play.rust-lang.org/?gist=f3d4e2e9559d84319432&version=stable

It's O(n²) in the number of literal matches (or ranges) for each variable matched against, but I haven't actually measured if there's any impact (there are never too many literal or range patterns for a single variable - 256 seems still useful for matching all u8 values, but I doubt there's much code with more than that).

Feel free to break it play with it using the examples at the bottom!

[petrochenkov]

For reference, @quantheory wrote an RFC about this - rust-lang/rfcs#880, it was postponed. He wrote some implementation too https://github.com/quantheory/int_range_check.

[comex]

FWIW, GCC can do this: https://goo.gl/4f1i6H (no warning, but a warning appears if you change 255 to something else). Clang can't tell the difference.

[steveklabnik]

I'm going to close this in favor of an RFC, as this would be a pretty big change. @quantheory's might be a good starting point!