-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Add is_empty()
to Iterator
#90676
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
Closed
Closed
Add is_empty()
to Iterator
#90676
Changes from all commits
Commits
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 hidden or 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
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.
There's no requirement that the
size_hint
be this precise (unless it'sExactSizeIterator
orTrustedLen
), so I don't think this implementation is ok. For example, this won't work withfrom_fn
: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=0fce510cb4a02171b289d6d3bd0e8808This would either need to be
-> Option<bool>
(so it can returnNone
if the size_hint is inconclusive), or be on a different trait (as you mention).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.
from_fn
obviously doesn't work, but I don't see why this is bad. This method is not always correct when it returnsfalse
(return value offalse
does not mean the iterator has items), but it should be correct when it returnstrue
.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's not what I'd expect it to do, at least. I'd expect it, since Rust has all the type system machinery it needs to do this, to only compile when it works -- same as
.len()
isn't "well, here's what it might be".Someone who wants unreliable checks can always just do
.size_hint().0 > 0
or.size_hint().1 == Some(0)
, depending on which direction they want; I don't think convenience methods for that are appropriate.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.
ESI is a safe trait, and to me
len()
is also "here's what it might be" since we can't say not reporting accurate size is UB.is_empty
doesn't have to reside in another trait to me, but we could add a trait for users to require iterators to produce accurateis_empty
results, likeFusedIterator
.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.
UB is not the only kind of forbidden behavior. Safe traits can also make promises where breaking these promises is a bug that can cascade into further downstream misbehavior such as infinite loops, panics, leaks and other API contracts being broken which easily can result in data loss or miscomputations. Sure, it's not a buffer overflow or double-free but that's little consolation when your blob storage containing petabytes of critical data suddenly become inaccessible, your account balance gets miscomputed or an airliner has an intersection event with a mountain.
So from the perspective of safe code
ExactSizeIterator::len
is guaranteed to be correct and it is free to operate on that assumption for allT
. B is correct if A is correct.is_empty
here does not meet this level of conditional correctness and would have to be weakened to the level of uselessness because it would have to explicitly mention something likeWhich renders it useless as a trait method, because then people would have to reason about concrete types rather than the trait.