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

Fix size_hint on Skip #8375

Closed
wants to merge 1 commit into from
Closed

Conversation

lilyball
Copy link
Contributor

@lilyball lilyball commented Aug 7, 2013

If the upper bound of the wrapped iterator has saturated at
uint::max_value, keep it there. It may actually represent an upper bound
that is higher.

r? @thestinger

If the upper bound of the wrapped iterator has saturated at
uint::max_value, keep it there. It may actually represent an upper bound
that is higher.
@bluss
Copy link
Member

bluss commented Aug 7, 2013

I thought infinite iterators should use None for upper bound (like Counter). Does Some(uint::max_value) mean anything else than the literal value?

@thestinger
Copy link
Contributor

@blake2-ppc: you're right - if they can't represent the upper bound as uint it should be None, it's the lower bound that gets clamped to uint::max_value

@thestinger thestinger closed this Aug 7, 2013
@lilyball
Copy link
Contributor Author

lilyball commented Aug 7, 2013

@thestinger I thought we had decided that None meant unknown, not unbounded.

I've been giving some thought as to whether we should actually use an enum for this case, to distinguish Unknown, Unbounded, and Bounded(uint), but I've been holding off because I wasn't sure it was worth the additional complexity.

@thestinger
Copy link
Contributor

The lower and upper bounds are limits, so if it can't be represented in a uint it's unknown because it's not bounded by uint::max_value.

@lilyball
Copy link
Contributor Author

lilyball commented Aug 7, 2013

Fair enough. I don't remember anymore what led me to believe we had decided that uint::max_value should be pegged. It's possible I was just mis-remembering something else.

@thestinger
Copy link
Contributor

@kballard: the lower bound should be pegged to uint::max_value, I think that's what we discussed before

@lilyball
Copy link
Contributor Author

lilyball commented Aug 7, 2013

@thestinger I feel like the idea was that when adapting an iterator in a fashion that would raise the upper bound (e.g. chain()), the upper bound would be saturated at uint::max_value. And because of that, anything that reduces the upper bound would treat uint::max_value specially as that may be an indication of saturation rather than an exact bound.

In any case, since you don't think this is the approach we should take, then we may have to fix the iterators that increase the upper bound to detect overflow and flip to None instead of merely saturating. This needs the .checked_add() that @huonw was talking about in IRC yesterday (which we can implement trivially for primitive integral types the same way saturating_add() is implemented; in fact I was originally considering making .saturating_add() return (Self, bool) but decided not to in order to make it simpler to use).

@thestinger
Copy link
Contributor

It's not necessarily that I don't think it's the approach we should take, but right now the second element is defined as the upper bound and if the iterator yields more elements it's not a bound.

flip1995 pushed a commit to flip1995/rust that referenced this pull request Apr 21, 2022
… r=giraffate

Take over: New lint bytes count to len

take over rust-lang#8375
close rust-lang#8083

This PR adds new lint about  considering replacing `.bytes().count()` with `.len()`.

Thank you in advance.

---

r! `@Manishearth`

changelog: adds new lint [`bytes_count_to_len`] to consider replacing `.bytes().count()` with `.len()`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants