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.

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
wants to merge 4 commits into from
Closed
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 54 additions & 0 deletions text/0000-iterator-len-hint.md
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.
Copy link
Member

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 to size_hint while adding a #[deprecated] tag on the size_hint method.

Copy link
Contributor Author

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.)

Copy link
Member

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.

Copy link
Contributor Author

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.)

Copy link
Member

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.

Copy link
Contributor Author

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.)

Copy link
Member

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?

Copy link
Contributor Author

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:

Note that there were some changes that were "technically breaking" but didn't make it into the beta.

And I believe yes, it will be removed in 1.0 final.

Renaming size_hint is a bit more involving than removing PhantomFn, though. As the latter only involves a removal, while the former can be seen as a removal plus an addition. Also, I expect size_hint to be more frequently used than PhantomFn.

Copy link
Contributor Author

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".


# 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.