New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exhaustive integer patterns tracking issue #50907

Closed
varkor opened this Issue May 19, 2018 · 17 comments

Comments

Projects
None yet
7 participants
@varkor
Member

varkor commented May 19, 2018

Tentative tracking issue for the exhaustive_integer_patterns feature, in which integer types may be exhaustively matched over their values. Original RFC thread: rust-lang/rfcs#1550.

#![feature(exhaustive_integer_patterns)]
#![feature(exclusive_range_pattern)]

fn matcher(x: u8) {
  match x { // ok -- every value has been accounted for
    0 .. 32 => { /* foo */ }
    32 => { /* bar */ }
    33 ..= 255 => { /* baz */ }
  }
}
@leonardo-m

This comment was marked as resolved.

leonardo-m commented May 19, 2018

Some common use cases, you don't need to support them all at once, but eventually Rust should handle them all:

#![feature(exclusive_range_pattern)]
#![feature(euclidean_division)]

fn foo1(x: u8) {
    match x {
        0 .. 120 => {},
        120 ..= 255 => {},
    }
}

fn foo2(x: u32) {
    match x % 3 {
        0 => {},
        1 => {},
        2 => {},
    }
}

fn foo3(x: i32) {
    match x.mod_euc(3) - 1 {
        -1 => {},
        0 => {},
        1 => {},
    }
}

fn main() {}
@varkor

This comment was marked as resolved.

Member

varkor commented May 20, 2018

@leonardo-m: I'm focusing on exhaustive matching over the entire type right now. Eventually, it'd be nice to be able to handle cases like modulo too, but that involves some significant changes if it's to work well and I'm not sure how plausible it really is (without a huge memory footprint).

@ishitatsuyuki

This comment was marked as resolved.

Member

ishitatsuyuki commented May 20, 2018

To range the result of arithmetic I think we have to implement it as something like type system contracts, which needs a full RFC.

@leonardo-m

This comment was marked as resolved.

leonardo-m commented May 20, 2018

The example with mod_euc could require some kind of contracts, but my second example with "x % 3" is within the compile-time capabilities of the DMD D language compiler (and it's faster than Rustc and it doesn't require lot of memory), some examples of what D Value Range Analysis does:

// temp.d(2,33): Error: cannot implicitly convert expression (x) of type const(uint) to ubyte
ubyte foo01(in uint x) { return x; }

ubyte foo02(in uint x) { return x % 300; } // Cannot implicitly convert
ubyte foo03(in uint x) { return x % 200; } // OK
ubyte foo04(in int x) { return x % 200; } // Cannot implicitly convert
ubyte foo05(in int x) { return x % 128 + 128; } // OK
byte foo06(in int x) { return x % 127; } // OK
byte foo07(in ubyte x) { return x - 128; } // OK
int foo08(in ubyte x) { return x - 128; } // OK
ubyte foo09(in uint x) { return x & 0b_1111; } // OK
ubyte foo10(in uint x) { return x / 10_000_000; } // Cannot implicitly convert
ubyte foo11(in uint x) { return x / 100_000_000; } // OK
ubyte foo12(in ubyte x, in ubyte y) { return x + y; } // Cannot implicitly convert
uint foo13(in ubyte x, in ubyte y) { return x + y; } // OK

void main() {}

I think Rust match{} could become equally smart.

I also think the same Value Range Analysis should allow code like this in Rust (note this uses into() instead of TryInto()):
fn foo07(x: u8) -> i8 { (x - 128).into() }

@ishitatsuyuki

This comment was marked as resolved.

Member

ishitatsuyuki commented May 20, 2018

I'd say that range analysis should be solely an optimization, as it's very inflexible.

And on the optimization side, LLVM should be already able to eliminate the dead path.

@leonardo-m

This comment was marked as resolved.

leonardo-m commented May 20, 2018

And on the optimization side, LLVM should be already able to eliminate the dead path.

That misses the main point of using into() instead of TryInto(): the first one is statically proved to be correct by the compiler, unlike the second. The optimization is not the important thing here.

@samlh

This comment was marked as resolved.

samlh commented May 20, 2018

@leonardo-m: I think supporting such a feature would be interesting, but concur it would need a separate RFC with more details.

@leonardo-m

This comment was marked as resolved.

leonardo-m commented May 20, 2018

I concur it's for other pull requests and another RFC.

bors added a commit that referenced this issue Aug 22, 2018

Auto merge of #50912 - varkor:exhaustive-integer-matching, r=arielb1
Exhaustive integer matching

This adds a new feature flag `exhaustive_integer_patterns` that enables exhaustive matching of integer types by their values. For example, the following is now accepted:
```rust
#![feature(exhaustive_integer_patterns)]
#![feature(exclusive_range_pattern)]

fn matcher(x: u8) {
  match x { // ok
    0 .. 32 => { /* foo */ }
    32 => { /* bar */ }
    33 ..= 255 => { /* baz */ }
  }
}
```
This matching is permitted on all integer (signed/unsigned and char) types. Sensible error messages are also provided. For example:
```rust
fn matcher(x: u8) {
  match x { //~ ERROR
    0 .. 32 => { /* foo */ }
  }
}
```
results in:
```
error[E0004]: non-exhaustive patterns: `32u8...255u8` not covered
 --> matches.rs:3:9
  |
6 |   match x {
  |         ^ pattern `32u8...255u8` not covered
```

This implements rust-lang/rfcs#1550 for #50907. While there hasn't been a full RFC for this feature, it was suggested that this might be a feature that obviously complements the existing exhaustiveness checks (e.g. for `bool`) and so a feature gate would be sufficient for now.
@varkor

This comment has been minimized.

Member

varkor commented Aug 22, 2018

This will be usable on the next Nightly under #![feature(exhaustive_integer_patterns)] now that #50912 merged!

@Centril

This comment has been minimized.

Contributor

Centril commented Sep 15, 2018

N.B: Before we stabilize this, I would like to see an RFC accepted that details and motivates (shouldn't be too hard) the changes.

@varkor

This comment has been minimized.

Member

varkor commented Sep 15, 2018

In #50912 (comment) it was proposed that this extension was filling in a gap in the existing exhaustiveness checks and underwent a T-lang FCP, so an RFC probably isn't necessary here.

@Centril

This comment has been minimized.

Contributor

Centril commented Sep 16, 2018

@varkor So, I had not read #50912 (comment). However, I do not view exhaustive integer matching as a bugfix or a trivial change. The feature was seen an addition to the language (as opposed to bug-fixes) in previous RFCs and were postponed then since they were not a priority leading up to 1.0. @nikomatsakis said as much in their move to FCP in the linked comment.

Therefore, I do not think that the proper process was followed and I will insist that an RFC is written prior to stabilization because I want to avoid the precedent of allowing larger and larger language changes to be accepted with no RFC.

I want to emphasize that this is not about this particular feature, which I think is an excellent idea and I have no doubt we would merge the RFC; but I do want to see the bits and pieces of text and reasoning that the RFC document provides (and I know that you're capable of writing excellent RFCs). Think of it as a stabilization report. Once we have that RFC merged, we can skip the extra FCP after it.

hcpl added a commit to hcpl/serde_mtproto that referenced this issue Oct 9, 2018

Replace an if-else chain with a pattern match
Due to a compiler limitation, exhaustive integer matches only work on
nightly for now.

See <rust-lang/rust#50907> for details.

hcpl pushed a commit to hcpl/serde_mtproto that referenced this issue Oct 9, 2018

Travis CI User
Automatic Travis documentation build
Replace an if-else chain with a pattern match

Due to a compiler limitation, exhaustive integer matches only work on
nightly for now.

See <rust-lang/rust#50907> for details.
@Centril

This comment has been minimized.

Contributor

Centril commented Nov 10, 2018

@varkor

This has been in the nightly compiler for ~80 days which approximates 12 weeks;
I think it would be appropriate to stabilize this soon.
If you could write the RFC aforementioned we can begin that process.
Also please attach an Appendix to the RFC pointing out relevant test files and such things.

@jonhoo

This comment has been minimized.

Contributor

jonhoo commented Nov 10, 2018

@Centril should this also be tagged with relnotes when stabilized?

@Centril

This comment has been minimized.

Contributor

Centril commented Nov 10, 2018

@jonhoo we tag the PR that stabilizes it with relnotes.

@Centril

This comment has been minimized.

Contributor

Centril commented Nov 10, 2018

Stabilization proposal filed in rust-lang/rfcs#2591.

bors added a commit that referenced this issue Dec 4, 2018

Auto merge of #56362 - varkor:stabilise-exhaustive-integer-patterns, …
…r=nikomatsakis

Stabilise exhaustive integer patterns

This is dependent on the FCP for rust-lang/rfcs#2591 being completed, but that should happen tomorrow, so there's little harm in opening this PR early.

Closes #50907.

@bors bors closed this in e57ed0d Dec 6, 2018

@varkor

This comment has been minimized.

Member

varkor commented Dec 6, 2018

This has been stabilised in #56557.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment