-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
RFC for inclusive ranges with ... #1192
Merged
Merged
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,79 @@ | ||
- Feature Name: inclusive_range_syntax | ||
- Start Date: 2015-07-07 | ||
- RFC PR: (leave this empty) | ||
- Rust Issue: (leave this empty) | ||
|
||
# Summary | ||
|
||
Allow a `x...y` expression to create an inclusive range. | ||
|
||
# Motivation | ||
|
||
There are several use-cases for inclusive ranges, that semantically | ||
include both end-points. For example, iterating from `0_u8` up to and | ||
including some number `n` can be done via `for _ in 0..n + 1` at the | ||
moment, but this will fail if `n` is `255`. Furthermore, some iterable | ||
things only have a successor operation that is sometimes sensible, | ||
e.g., `'a'..'{'` is equivalent to the inclusive range `'a'...'z'`: | ||
there's absolutely no reason that `{` is after `z` other than a quirk | ||
of the representation. | ||
|
||
The `...` syntax mirrors the current `..` used for exclusive ranges: | ||
more dots means more elements. | ||
|
||
# Detailed design | ||
|
||
`std::ops` defines | ||
|
||
```rust | ||
pub struct RangeInclusive<T> { | ||
pub start: T, | ||
pub end: T, | ||
} | ||
``` | ||
|
||
Writing `a...b` in an expression desugars to `std::ops::RangeInclusive | ||
{ start: a, end: b }`. | ||
|
||
This struct implements the standard traits (`Clone`, `Debug` etc.), | ||
but, unlike the other `Range*` types, does not implement `Iterator` | ||
directly, since it cannot do so correctly without more internal | ||
state. It can implement `IntoIterator` that converts it into an | ||
iterator type that contains the necessary state. | ||
|
||
The use of `...` in a pattern remains as testing for inclusion | ||
within that range, *not* a struct match. | ||
|
||
The author cannot forsee problems with breaking backward | ||
compatibility. In particular, one tokenisation of syntax like `1...` | ||
now would be `1. ..` i.e. a floating point number on the left, however, fortunately, | ||
it is actually tokenised like `1 ...`, and is hence an error. | ||
|
||
# Drawbacks | ||
|
||
There's a mismatch between pattern-`...` and expression-`...`, in that | ||
the former doesn't undergo the same desugaring as the | ||
latter. (Although they represent essentially the same thing | ||
semantically.) | ||
|
||
The `...` vs. `..` distinction is the exact inversion of Ruby's syntax. | ||
|
||
Only implementing `IntoIterator` means uses of it in iterator chains | ||
look like `(a...b).into_iter().collect()` instead of | ||
`(a..b).collect()` as with exclusive ones (although this doesn't | ||
affect `for` loops: `for _ in a...b` works fine). | ||
|
||
# Alternatives | ||
|
||
An alternate syntax could be used, like | ||
`..=`. [There has been discussion][discuss], but there wasn't a clear | ||
winner. | ||
|
||
[discuss]: https://internals.rust-lang.org/t/vs-for-inclusive-ranges/1539 | ||
|
||
This RFC doesn't propose non-double-ended syntax, like `a...`, `...b` | ||
or `...` since it isn't clear that this is so useful. Maybe it is. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually, it now does. |
||
|
||
# Unresolved questions | ||
|
||
None so far. |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you clarify why more state is needed? Why not just increment start while start is less than or equal to end?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gsingh93 The problem is that
0...u8::MAX
has 257 states: Some(i) for all i; and None.So you can't modify only one bound and get all the states. You have two options:
next_back
)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so couldn't RangeInclusive simply get another field that is set during desugaring?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I discussed it with a few people while writing the API, but got such negative feedback that I didn't even think to put it as an alternative. However, several people seem to be in support of it.