Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Document RangeFrom patterns #900

Merged
merged 5 commits into from
Sep 17, 2021
Merged

Document RangeFrom patterns #900

merged 5 commits into from
Sep 17, 2021

Conversation

workingjubilee
Copy link
Member

This PR documents RangeFrom-style patterns as a prelude to stabilizing rust-lang/rust#67264, per the stabilization policy.

@ehuss ehuss added the S-waiting-on-stabilization Waiting for a stabilization PR to be merged in the main Rust repository label Nov 9, 2020
ehuss
ehuss reviewed Nov 9, 2020
View reviewed changes
Copy link
Contributor

@ehuss ehuss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this more formally define the meaning of the half-open range? Generally the definition shouldn't be given as an example.

Additionally, can you add a small example to the example block below?

Can you also update the Punctuation section to include a link for .. to the "Range patterns"?

src/patterns.md Outdated Show resolved Hide resolved
@workingjubilee
Copy link
Member Author

Whoops, sorry for falling off the face of the planet.
Need to think of a good example still, but was this more what you had in mind re: writing style?

@workingjubilee workingjubilee mentioned this pull request Apr 6, 2021
Merged
@workingjubilee workingjubilee force-pushed the rangefrom-pat branch 2 times, most recently from 6b1d50c to 573e062 Compare April 6, 2021 22:46
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 11, 2021
@bors
Auto merge of rust-lang#83918 - workingjubilee:stable-rangefrom-pat, …
0d76b73 
…r=joshtriplett

Stabilize "RangeFrom" patterns in 1.55

Implements a partial stabilization of rust-lang#67264 and rust-lang#37854.
Reference PR: rust-lang/reference#900

# Stabilization Report

This stabilizes the `X..` pattern, shown as such, offering an exhaustive match for unsigned integers:
```rust
match x as u32 {
      0 => println!("zero!"),
      1.. => println!("positive number!"),
}
```

Currently if a Rust author wants to write such a match on an integer, they must use `1..={integer}::MAX` . By allowing a "RangeFrom" style pattern, this simplifies the match to not require the MAX path and thus not require specifically repeating the type inside the match, allowing for easier refactoring. This is particularly useful for instances like the above case, where different behavior on "0" vs. "1 or any positive number" is desired, and the actual MAX is unimportant.

Notably, this excepts slice patterns which include half-open ranges from stabilization, as the wisdom of those is still subject to some debate.

## Practical Applications

Instances of this specific usage have appeared in the compiler:
https://github.com/rust-lang/rust/blob/16143d10679537d3fde4247e15334e78ad9d55b9/compiler/rustc_middle/src/ty/inhabitedness/mod.rs#L219
https://github.com/rust-lang/rust/blob/673d0db5e393e9c64897005b470bfeb6d5aec61b/compiler/rustc_ty_utils/src/ty.rs#L524

And I have noticed there are also a handful of "in the wild" users who have deployed it to similar effect, especially in the case of rejecting any value of a certain number or greater. It simply makes it much more ergonomic to write an irrefutable match, as done in Katholieke Universiteit Leuven's [SCALE and MAMBA project](https://github.com/KULeuven-COSIC/SCALE-MAMBA/blob/05e5db00d553573534258585651c525d0da5f83f/WebAssembly/scale_std/src/fixed_point.rs#L685-L695).

## Tests
There were already many tests in [src/test/ui/half-open-range/patterns](https://github.com/rust-lang/rust/tree/90a2e5e3fe59a254d4d707aa291517b3791ea5a6/src/test/ui/half-open-range-patterns), as well as [generic pattern tests that test the `exclusive_range_pattern` feature](https://github.com/rust-lang/rust/blob/673d0db5e393e9c64897005b470bfeb6d5aec61b/src/test/ui/pattern/usefulness/integer-ranges/reachability.rs), many dating back to the feature's introduction and remaining standing to this day. However, this stabilization comes with some additional tests to explore the... sometimes interesting behavior of interactions with other patterns. e.g. There is, at least, a mild diagnostic improvement in some edge cases, because before now, the pattern `0..=(5+1)` encounters the `half_open_range_patterns` feature gate and can thus emit the request to enable the feature flag, while also emitting the "inclusive range with no end" diagnostic. There is no intent to allow an `X..=` pattern that I am aware of, so removing the flag request is a strict improvement. The arrival of the `J | K` "or" pattern also enables some odd formations.

Some of the behavior tested for here is derived from experiments in this [Playground](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=58777b3c715c85165ac4a70d93efeefc) example, linked at rust-lang#67264 (comment), which may be useful to reference to observe the current behavior more closely.

In addition tests constituting an explanation of the "slicing range patterns" syntax issue are included in this PR.

## Desiderata

The exclusive range patterns and half-open range patterns are fairly strongly requested by many authors, as they make some patterns much more natural to write, but there is disagreement regarding the "closed" exclusive range pattern or the "RangeTo" pattern, especially where it creates "off by one" gaps in the presence of a "catch-all" wildcard case. Also, there are obviously no range analyses in place that will force diagnostics for e.g. highly overlapping matches. I believe these should be warned on, ideally, and I think it would be reasonable to consider such a blocker to stabilizing this feature, but there is no technical issue with the feature as-is from the purely syntactic perspective as such overlapping or missed matches can already be generated today with such a catch-all case. And part of the "point" of the feature, at least from my view, is to make it easier to omit wildcard matches: a pattern with such an "open" match produces an irrefutable match and does not need the wild card case, making it easier to benefit from exhaustiveness checking.

## History

- Implemented:
  - Partially via exclusive ranges: rust-lang#35712
  - Fully with half-open ranges: rust-lang#67258
- Unresolved Questions:
  - The precedence concerns of rust-lang#48501 were considered as likely requiring adjustment but probably wanting a uniform consistent change across all pattern styles, given rust-lang#67264 (comment), but it is still unknown what changes might be desired
  - How we want to handle slice patterns in ranges seems to be an open question still, as witnessed in the discussion of this PR!

I checked but I couldn't actually find an RFC for this, and given "approved provisionally by lang team without an RFC", I believe this might require an RFC before it can land? Unsure of procedure here, on account of this being stabilizing a subset of a feature of syntax.

r? `@scottmcm`
ehuss
ehuss requested changes Jul 12, 2021
View reviewed changes
Copy link
Contributor

@ehuss ehuss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like there were some changes in the stabilization PR. Do you think you can update with those changes? Was it just the restriction of slice patterns, or were there other changes?

src/patterns.md Outdated Show resolved Hide resolved
@workingjubilee
Copy link
Member Author

Just the restriction on slice patterns, but will update. Blargh!

@workingjubilee @ehuss
add spacing in src/patterns.md
da75d92 
Co-authored-by: Eric Huss <eric@huss.org>
@ehuss
Copy link
Contributor

ehuss commented Aug 5, 2021

Can you update with the restrictions on slice patterns?

@workingjubilee
Copy link
Member Author

Only if I can remember what I was doing, unfortunately.

Does the grammar look right?

ehuss
ehuss reviewed Sep 14, 2021
View reviewed changes
src/patterns.md Outdated
Comment on lines 719 to 723
> &nbsp;&nbsp; _InSlicePattern_ \(`,` _InSlicePattern_)<sup>\*</sup> `,`<sup>?</sup>
>
> _InSlicePattern_ :\
> &nbsp;&nbsp; &nbsp;&nbsp; [_PatternWithoutRange_]\
> &nbsp;&nbsp; | [_ClosedRangePattern_]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe the restriction is done after parsing, so this shouldn't be expressed as part of the grammar. That is, [0..] is syntactically valid as a pattern, but is then later rejected. I think it would be fine just to list that as a restriction below in prose.

(and, fwiw, ClosedRangePattern isn't defined)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough then.
Whoops.
Alright, let's see...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also a good point because: I forgot that it was only the version without parentheses that is currently "up in the air".

ehuss
ehuss approved these changes Sep 17, 2021
View reviewed changes
Copy link
Contributor

@ehuss ehuss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@ehuss ehuss merged commit df5799c into rust-lang:master Sep 17, 2021
@workingjubilee
Copy link
Member Author

Thank you.

@workingjubilee workingjubilee deleted the rangefrom-pat branch September 17, 2021 05:10
@ehuss ehuss mentioned this pull request Sep 29, 2021
Merged
ehuss added a commit to ehuss/rust that referenced this pull request Sep 30, 2021
@ehuss
Rollup merge of rust-lang#89378 - ehuss:update-books, r=ehuss
20c9530 
Update books

## nomicon

1 commits in fe6227eb3c8533200c52dffa42ef1b6f2f02c40e..2747c4bb2cbc0639b733793ddb0bf4e9daa2634e
2021-08-31 05:42:38 +0900 to 2021-09-19 17:33:32 +0900
- Clarify a bit of wording (rust-lang/nomicon#310)

## reference

2 commits in 0e5ed7a4bec065f0cc18c35d1c904639e095314d..13747275bd14c2d2b453100498532f9ae5504769
2021-08-29 17:33:21 +0900 to 2021-09-24 17:44:04 +0900
-  (rust-lang/reference#1087)
- Document RangeFrom patterns (rust-lang/reference#900)

## book

4 commits in fcb5e0ea68112d85a1d29a7a7335978ef2a02181..eb1282ec444db94055fa9531b6f3f803e86bb382
2021-08-31 21:26:19 -0400 to 2021-09-16 21:17:09 -0400
- Rust 1.55 (rust-lang/book#2890)
- Chapter 3.3: Remove unused variable (rust-lang/book#2877)
- Appendix (operators and symbols): remove extra space (rust-lang/book#2876)
- Chapter 17.1: add missing comma (rust-lang/book#2867)

## rust-by-example

2 commits in 9d4132b56c4999cd3ce1aeca5f1b2f2cb0d11c24..28aca4a36962c709bce301c03114b5589381dfb8
2021-09-14 06:56:00 -0300 to 2021-09-25 08:19:51 -0300
- fix formatting (rust-lang/rust-by-example#1463)
- involving === verb, involved === adjective (rust-lang/rust-by-example#1461)

## rustc-dev-guide

7 commits in 9198465..d1f03cb
2021-09-12 11:50:44 -0500 to 2021-09-24 12:00:29 +0900
- Fix a typo on closure.md (rust-lang/rustc-dev-guide#1218)
- const generics update
- s/--blessed/--bless/
- Make indentation consistent in example vscode config file
- Suggests `--edition=2018` argument when using stage0/bin/rustfmt directly
- `ty::Unevaluated`: dealing with unused substs (rust-lang/rustc-dev-guide#1190)
- Parallel codegen (rust-lang/rustc-dev-guide#1206)
@dtolnay dtolnay mentioned this pull request Feb 7, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ehuss ehuss ehuss approved these changes

Assignees
No one assigned
Labels
S-waiting-on-stabilization Waiting for a stabilization PR to be merged in the main Rust repository
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@workingjubilee @ehuss