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

Stabilise exhaustive integer pattern matching #2591

Merged
merged 6 commits into from Nov 30, 2018

Conversation

@varkor
Member

varkor commented Nov 10, 2018

Extend Rust's pattern matching exhaustiveness checks to cover the integer types: u8, u16, u32, u64, u128, usize, i8, i16, i32, i64, i128, isize and char.

Rendered.

fn matcher_full(x: u8) {
  match x { // ok
    0 ..= 31 => { /* ... */ }
    32 => { /* ... */ }
    33 ..= 255 => { /* ... */ }
  }
}

fn matcher_incomplete(x: u8) {
  match x { //~ ERROR: non-exhaustive patterns: `32u8..=255u8` not covered
    0 ..= 31 => { /* ... */ }
  }
}

This feature has already been implemented behind the feature flag exhaustive_integer_patterns, so this RFC is viewed as a motion to stabilise the feature. See rust-lang/rust#50912 (comment) for the previous motion to (unstably) approve this feature.

@Centril Centril added the T-lang label Nov 10, 2018

@Centril Centril changed the title from Exhaustive integer pattern matching to Stabilize Exhaustive integer pattern matching Nov 10, 2018

@Centril Centril referenced this pull request Nov 10, 2018

Closed

Exhaustive integer patterns tracking issue #50907

2 of 2 tasks complete
@rkruppe

This comment was marked as resolved.

Contributor

rkruppe commented Nov 10, 2018

AFAIK guards are not taken into account at all for these exhaustiveness checks (as for all other current exhaustiveness checks), for completeness that should be mentioned in the text or in an example.

@varkor varkor changed the title from Stabilize Exhaustive integer pattern matching to Stabilise exhaustive integer pattern matching Nov 10, 2018

Add reference to guarded arms
Also update examples to not rely on `exclusive_range_pattern`.
@Centril

This comment has been minimized.

Contributor

Centril commented Nov 10, 2018

@rfcbot fcp merge

Feature name: #![feature(exhaustive_integer_patterns)]
Version target: 1.32 (2019-01-18)
Tracking issue: rust-lang/rust#50907

The feature exhaustive_integer_patterns feature has baked in the nightly compiler for approximately 12 weeks which is sufficient amount of time. Originally, the feature did not pass through an RFC but it was accepted by an FCP-merge over at rust-lang/rust#50912 (comment).

This RFC serves as:

  • An introduction for people who were unaware of this new capability,
  • A stabilization report,
  • As assurance that the proper RFC process is followed.

Given the clear motivation in improving code correctness and to fix bugs in the language, I propose that we accept this RFC and stabilize exhaustive_integer_patterns as per the stabilization report given in this RFC. Once this RFC is accepted, and if no further issues are found, the feature will move to immediate stabilization without further delay.

@rfcbot

This comment has been minimized.

rfcbot commented Nov 10, 2018

Team member @Centril has proposed to merge this. The next step is review by the rest of the tagged teams:

Concerns:

Once a majority of reviewers approve (and none object), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@scottmcm

This comment has been minimized.

Member

scottmcm commented Nov 11, 2018

I love the more specific error messages that come with this!

@StyMaar

This comment was marked as resolved.

StyMaar commented Nov 14, 2018

I love this, but I'm wondering: how can that work for usize and isize ?

@eddyb

This comment was marked as resolved.

Member

eddyb commented Nov 14, 2018

@StyMaar usize / isize are pointer-sized integers, so e.g. on a target with 64-bit pointers, the range is identical to that of u64, or i64, respectively.

@rkruppe

This comment was marked as resolved.

Contributor

rkruppe commented Nov 14, 2018

That seems like a noteworth and potentially objectionable portability hazard.

@RalfJung

This comment was marked as resolved.

Member

RalfJung commented Nov 14, 2018

We don't have implicit conversion between usize/isize and u64/i64 on 64bit platforms, and for good reasons.

I think for the same reasons, exhaustiveness checking should not make any assumptions about the maximal value of usize and the minimal/maximal value of isize. So e.g. matching a usize against 0..4 and 4.. is okay, but no upper bound must be given.

@StyMaar

This comment was marked as resolved.

StyMaar commented Nov 14, 2018

on a target with 64-bit pointers

@eddyb but then, how do we manage targets with different pointer size?

I think this point deserves to be talked about in the RFC at least.

IMHO it would make sense to enable exhaustive matching of usize and isize only inside a block with a specified #[cfg(target_pointer_width=X)], but I'm not sure it's possible.

@rkruppe

This comment was marked as resolved.

Contributor

rkruppe commented Nov 14, 2018

@RalfJung There are no implicit conversions between any pair of distinct integer types. A better analogy would be explicit exact conversions in the form of From, which currently are mostly non-existent for usize/usize. However, I believe that's intended to change once we have some sort of portability lint; presumably exhaustiveness would be affected in a similar way.

@RalfJung

This comment was marked as resolved.

Member

RalfJung commented Nov 14, 2018

@rkruppe Sure, but that's because no pair of distinct integer types ever has the same value range.

But yes, once we decide that e.g. usize will not ever be more than u128 and implement the appropriate From, the exhaustiveness check could use that.

@rkruppe

This comment was marked as resolved.

Contributor

rkruppe commented Nov 14, 2018

Value range is not the sole issue, we do not have implicit conversion to wider types either. At minimum quite a few people have strong feelings about not having any implicit integer conversions period.

@Centril

This comment has been minimized.

Contributor

Centril commented Nov 14, 2018

@rfcbot concern isize-usize-exhaustiveness

Seems like a valid concern re. usize and isize for reasons aforementioned (e.g. in #2591 (comment)); I'd like to see RFC amended to extract assumptions about the max value of usize and min+max values of isize out to a separate feature gate exhaustive_usize_patterns as a conservative choice that can be changed in the future if so desired. The extraction can be done as part of the stabilization PR.

@Centril Centril self-assigned this Nov 14, 2018

@Centril

This comment has been minimized.

Contributor

Centril commented Nov 19, 2018

@varkor Thoughts on / mind making the adjustments per #2591 (comment)?

@varkor

This comment has been minimized.

Member

varkor commented Nov 19, 2018

Exhaustive integer pattern matching will be very much complimented by #947, so it's probably ripe time to start a new RFC for open-ended range patterns.

@Centril

This comment has been minimized.

Contributor

Centril commented Nov 20, 2018

Given the latest changes...

@rfcbot resolve isize-usize-exhaustiveness

@rfcbot

This comment has been minimized.

rfcbot commented Nov 20, 2018

🔔 This is now entering its final comment period, as per the review above. 🔔

@rfcbot

This comment has been minimized.

rfcbot commented Nov 30, 2018

The final comment period, with a disposition to merge, as per the review above, is now complete.

@Centril Centril merged commit 58cf090 into rust-lang:master Nov 30, 2018

@Centril

This comment has been minimized.

Contributor

Centril commented Nov 30, 2018

🎉 Huzzah! This RFC has been merged! 🎉

Tracking issue: rust-lang/rust#50907
Stabilization PR: rust-lang/rust#56362

@varkor varkor deleted the varkor:exhaustive-integer-pattern-matching branch Nov 30, 2018

bors added a commit to rust-lang/rust that referenced this pull request 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.

pietroalbini added a commit to pietroalbini/rust that referenced this pull request Dec 6, 2018

Rollup merge of rust-lang#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 rust-lang#50907.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment