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

Add is_empty function to ExactSizeIterator #34357

Merged
merged 4 commits into from
Jul 19, 2016

Conversation

tbu-
Copy link
Contributor

@tbu- tbu- commented Jun 19, 2016

All other types implementing a len functions have is_empty already.

All other types implementing a `len` functions have `is_empty` already.
@rust-highfive
Copy link
Collaborator

r? @brson

(rust_highfive has picked a reviewer for you, use r? to override)

@@ -519,6 +519,31 @@ pub trait ExactSizeIterator: Iterator {
assert_eq!(upper, Some(lower));
lower
}

///
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rustdoc might not like this leading empty doc comment line :)

@alexcrichton alexcrichton added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. S-waiting-on-crater Status: Waiting on a crater run to be completed. labels Jun 19, 2016
@alexcrichton
Copy link
Member

For adding a method to many iterators we'll likely want to run this through crater to ensure that there's no unexpected fallout if we also decide to merge.

@brson
Copy link
Contributor

brson commented Jun 20, 2016

Didn't we see a very similar PR to this one recently (adding is_empty to ... something) and decline to add it? I don't see it by searching old pulls.

@tbu-
Copy link
Contributor Author

tbu- commented Jun 20, 2016

Maybe you mean #31877?

@alexcrichton
Copy link
Member

Either that or perhaps Peekable::is_empty? That was different because the signature took &mut self and didn't correspond with .len() like other containers. Conventionally this seems ok to me.

@alexcrichton
Copy link
Member

The libs team discussed this during triage yesterday and the conclusion was that this is ok to merge pending a crater run

@brson
Copy link
Contributor

brson commented Jul 11, 2016

Travis failure looks legit.

@brson
Copy link
Contributor

brson commented Jul 11, 2016

Starting a crater run.

@tbu-
Copy link
Contributor Author

tbu- commented Jul 12, 2016

@brson May I push a fix for the test failure or does that interfere with the crater run?

@brson
Copy link
Contributor

brson commented Jul 12, 2016

@tbu- You can go ahead and push it now. There's a small window where it would have interfered.

@brson
Copy link
Contributor

brson commented Jul 14, 2016

@alexcrichton
Copy link
Member

Thanks @brson! @tbu- looks like the travis error is legitimate as well

@tbu- tbu- force-pushed the pr_exact_size_is_empty branch 2 times, most recently from 7dfe1a6 to 9e2c72f Compare July 18, 2016 12:35
@brson
Copy link
Contributor

brson commented Jul 18, 2016

@bors r+

@bors
Copy link
Contributor

bors commented Jul 18, 2016

📌 Commit 7b2a03f has been approved by brson

@bors
Copy link
Contributor

bors commented Jul 18, 2016

⌛ Testing commit 7b2a03f with merge bbfcb47...

bors added a commit that referenced this pull request Jul 18, 2016
Add `is_empty` function to `ExactSizeIterator`

All other types implementing a `len` functions have `is_empty` already.
@bors bors merged commit 7b2a03f into rust-lang:master Jul 19, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-crater Status: Waiting on a crater run to be completed. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants