-
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: Deprecate Iterator::size_hint
and ExactSizeIterator
for better named alternatives.
#1034
Closed
Closed
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
909bcf5
RFC: Rename `Iterator::size_hint` to `Iterator::len_hint`
CloudiDust e5dd2c3
Add discussions about a possible new beta and the timing of the change.
CloudiDust 1bddb7c
Promote the "deprecate but retain" alternative and propose deprecatin…
CloudiDust 4c706a2
Adjust wording.
CloudiDust 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,54 @@ | ||
- Feature Name: iterator_len_hint | ||
- Start Date: 2015-04-05 | ||
- RFC PR: (leave this empty) | ||
- Rust Issue: (leave this empty) | ||
|
||
# Summary | ||
|
||
Rename the `size_hint` method of the `Iterator` trait to `len_hint`. | ||
|
||
# Motivation | ||
|
||
Currently, methods returning the numbers of elements in containers are conventionally named `len` in Rust, even for non-linear collections like [`BTreeMap`](http://doc.rust-lang.org/1.0.0-beta/std/collections/btree_map/struct.BTreeMap.html#method.len). But the `Iterator` trait, which represents entities each yielding a sequence of elements, has a `size_hint` method, instead of the more consistent `len_hint`. | ||
|
||
The standard library documentation says about [`size_hint`](http://doc.rust-lang.org/1.0.0-beta/std/iter/trait.Iterator.html#tymethod.size_hint): | ||
|
||
> Returns a lower and upper bound on the remaining *length* of the iterator. | ||
|
||
This method should be renamed. | ||
|
||
# Detailed design | ||
|
||
Rename the `size_hint` method of `std::iter::Iterator` to `len_hint` during the Rust 1.0 beta cycle. | ||
|
||
# Drawbacks | ||
|
||
This is a late breaking change (though only a minor naming correction at that). | ||
|
||
# Alternatives | ||
|
||
#### A. Add `len_hint` as a synonym for `size_hint`, and deprecate `size_hint`. | ||
|
||
A `len_hint` method would be added to the `std::iter::Iterator` trait, which has the following inlined default implementation: | ||
|
||
```rust | ||
fn len_hint(&self) -> (usize, Option<usize>) { | ||
self.size_hint() | ||
} | ||
``` | ||
|
||
The `size_hint` method would be marked `deprecated` now. | ||
|
||
In Rust 2.x, `len_hint` would become a required method and `size_hint` would be removed. | ||
|
||
The advantage of this alternative is that it is not a breaking change. | ||
|
||
However, Rust 1.0 final is yet to be released, having something deprecated now but not removed until 2.x is weird. | ||
|
||
#### B. Keep the status quo. | ||
|
||
`size_hint` is not too bad after all, though it is inconsistent, especially when even non-linear collections use methods named `len` instead of `size`. | ||
|
||
# Unresolved questions | ||
|
||
None. |
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.
Could you elaborate a little more on the deprecation plan and migration plans here as well? I would personally vote for the addition of a default
len_hint
method which delegates tosize_hint
while adding a#[deprecated]
tag on thesize_hint
method.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.
@alexcrichton, one way to do the deprecation and migration is to publish a new beta release (beta2) that is not compatible with beta, but compatible with final.
Note that there were some changes that were "technically breaking" but didn't make it into the beta. Also there is #1030. And we may find other problems in beta that requires breaking change. So a new beta may be a good idea anyway.
I think "three weeks later" is a good time to release a new beta. Rust is still pre-1.0-final, so it may be fine to do some special casing.
(Currently I cannot update the RFC, I'll do so when I can.)
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.
@alexcrichton I'm hoping you guys don't release a 1.0 that has any deprecations.
@CloudiDust I think it's okay to break stuff in this Beta cycle without releasing Beta2. These breaking changes are very minor.
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.
@tshepang, without beta 2, lib authors affected by late breaking changes would have to maintain at least two branchs, one for beta (because many people would be using beta) and one for the yet-to-be-released final. But if we release a new beta right after applying breaking changes, the lib authors would not have to branch their libs as people will quickly switch to beta 2.
Or we can apply the changes right before the final release, but that may be a bit surprising and feel rushed.
I think whether beta 2 is necessary depends on how many "quite visible" breaking changes we want to apply. (I expect most late breaking changes to be of the "technically breaking" kind, but
size_hint -> len_hint
is "quite visible", though minor and easy to deal with.)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.
@CloudiDust you can have people update their code either 3 or 6 weeks after Beta. How is 3 weeks better? It's like you are assuming people should have 1.0-compatible libraries as soon as 1.0 is out. And even then, the breaking changes until 1.0 should still be minor, so the updates should be minimal.
Note that even if 3 weeks was chosen, there could still be breaking changes after Beta 2, and that's still preferable to getting stuck with something that's not ideal for all of 1.x.
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.
@tshepang, "three weeks" is better because more people are likely to do feedback about the breaking changes if there is another beta. Though, if the only “quite visible" breaking change is this
len_hint
one, then a new beta may not be necessary. (But some libs would have to be branched if they want to support both beta and nighty, once such a breaking change lands.)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.
@CloudiDust I think there is already some precedent. e.g.,
PhantomFn
has been deprecated. So you'll get warnings on nightly, but things still work. I guess it will be removed For Real once 1.0 hits?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.
@BurntSushi,
PhantomFn
was what I had in mind when I said:And I believe yes, it will be removed in 1.0 final.
Renaming
size_hint
is a bit more involving than removingPhantomFn
, though. As the latter only involves a removal, while the former can be seen as a removal plus an addition. Also, I expectsize_hint
to be more frequently used thanPhantomFn
.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.
That said, I still prefer the renaming (after all it is still a simple change). Failing that, I'd like to go with "deprecate but retain
size_hint
".