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

Missing size_hint overrides in some Iterators implementations #23708

Closed
SimonSapin opened this issue Mar 25, 2015 · 7 comments
Closed

Missing size_hint overrides in some Iterators implementations #23708

SimonSapin opened this issue Mar 25, 2015 · 7 comments
Labels
C-enhancement Category: An issue proposing an enhancement or a PR with one. I-slow Issue: Problems and improvements with respect to performance of generated code. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@SimonSapin
Copy link
Contributor

Some implementations of Iterator don’t override the size_hint method (and keep the conservative (0, None) default), but should. This includes at least:

impl<A: Int> Iterator for StepBy<A, ::ops::Range<A>>
impl<A: Int> Iterator for ::ops::RangeFrom<A>
impl<A: Int> Iterator for RangeStepInclusive<A>

CC @alexcrichton

@alexcrichton
Copy link
Member

cc @aturon, this may some day tie into std::num stabilization and how Int is going away.

@SimonSapin
Copy link
Contributor Author

Yes, these implementations (and other) will need to change if Int is going away, but isn’t that orthogonal to size_hint?

@alexcrichton
Copy link
Member

The bounds may change somewhat if we expect to be able to define size_hint, but it is largely orthogonal, yes.

bors added a commit that referenced this issue May 1, 2015
`Iterator::size_hint` can be easily implemented for `StepBy`.
#23708
@steveklabnik steveklabnik added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Nov 15, 2016
@Mark-Simulacrum
Copy link
Member

Mark-Simulacrum commented Jun 16, 2017

I've made a list of the things which are currently missing size_hint overrides. Some of these may be fine with the default.

List found with rg 'impl.*\bIterator for ', though some items have been eliminated.

@Mark-Simulacrum Mark-Simulacrum added C-enhancement Category: An issue proposing an enhancement or a PR with one. I-slow Issue: Problems and improvements with respect to performance of generated code. labels Jul 22, 2017
Phlosioneer added a commit to Phlosioneer/rust that referenced this issue Mar 20, 2018
This also implements ExactSizeIterator where applicable.

Addresses most of the Iterator traits mentioned in rust-lang#23708.
@Phlosioneer
Copy link
Contributor

CycleIter is used just for testing, so it's irrelevant.
There is no sensible implementation of size_hint for $forward_iterator and $reverse_iterator, as far as I can tell.
The rest are addressed in my PR.

I've compiled a more complete list of iterators without size_hints, and will make a new tracking issue for them.

@Phlosioneer
Copy link
Contributor

New tracking issue for expanded list: #49205

bors added a commit that referenced this issue Mar 31, 2018
Implement some trivial size_hints for various iterators

This also implements ExactSizeIterator where applicable.

Addresses most of the Iterator traits mentioned in #23708.

I intend to do more, but I don't want to make the PR too large.
@SimonSapin
Copy link
Contributor Author

Closing for #49205.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: An issue proposing an enhancement or a PR with one. I-slow Issue: Problems and improvements with respect to performance of generated code. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

5 participants