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 implementation of Extend for () #50234

Merged
merged 1 commit into from
May 20, 2018
Merged

Add implementation of Extend for () #50234

merged 1 commit into from
May 20, 2018

Conversation

cramertj
Copy link
Member

This is useful in some generic code which wants to collect iterators of items into a result.

@rust-highfive
Copy link
Collaborator

r? @Kimundi

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

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 25, 2018
@rust-lang rust-lang deleted a comment from rust-highfive Apr 25, 2018
@rust-lang rust-lang deleted a comment from rust-highfive Apr 25, 2018
@pietroalbini
Copy link
Member

Ping from triage @Kimundi! This PR needs your review.

@Kimundi
Copy link
Member

Kimundi commented Apr 30, 2018

@rfcbot fcp merge

@kennytm kennytm added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label May 1, 2018
@kennytm
Copy link
Member

kennytm commented May 1, 2018

@rust-lang/libs Please summon rfcbot again, thanks.

@kennytm kennytm added S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). relnotes Marks issues that should be documented in the release notes of the next release. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 1, 2018
@alexcrichton
Copy link
Member

@rfcbot fcp merge

@rfcbot
Copy link

rfcbot commented May 1, 2018

Team member @alexcrichton has proposed to merge this. The next step is review by the rest of the tagged teams:

No concerns currently listed.

Once a majority of reviewers approve (and none object), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels May 1, 2018
@withoutboats
Copy link
Contributor

This is useful in some generic code which wants to collect iterators of items into a result.

Could you show an example?

@cramertj
Copy link
Member Author

cramertj commented May 2, 2018

@withoutboats The specific case where I hit this was wanting to StreamExt::collect a stream into () so that all the elements will be evaluated and an error will be returned if any of them failed.

#[stable(feature = "extend_for_unit", since = "1.27.0")]
impl Extend<()> for () {
fn extend<T: IntoIterator<Item = ()>>(&mut self, iter: T) {
for _ in iter {}
Copy link
Member

Choose a reason for hiding this comment

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

I believe iter.for_each(drop) can be faster for some iterators like chain.

Copy link
Member Author

Choose a reason for hiding this comment

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

Huh, interesting.

@rust-highfive

This comment has been minimized.

@rfcbot rfcbot added final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. labels May 4, 2018
@rfcbot
Copy link

rfcbot commented May 4, 2018

🔔 This is now entering its final comment period, as per the review above. 🔔

@@ -352,6 +352,13 @@ pub trait Extend<A> {
fn extend<T: IntoIterator<Item=A>>(&mut self, iter: T);
}

#[stable(feature = "extend_for_unit", since = "1.27.0")]
Copy link
Member

Choose a reason for hiding this comment

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

This should become 1.28.0.

@rfcbot
Copy link

rfcbot commented May 14, 2018

The final comment period, with a disposition to merge, as per the review above, is now complete.

@rfcbot rfcbot added the finished-final-comment-period The final comment period is finished for this PR / Issue. label May 14, 2018
@rfcbot rfcbot removed the final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. label May 14, 2018
@kennytm kennytm added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). labels May 14, 2018
@alexcrichton
Copy link
Member

@bors: r+

@bors
Copy link
Contributor

bors commented May 14, 2018

📌 Commit 5028b9c has been approved by alexcrichton

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 14, 2018
@alexcrichton
Copy link
Member

@bors: r-

er oops, mind updating the stability attribute?

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels May 14, 2018
@alexcrichton
Copy link
Member

Also while you're at it, mind adding a small test?

@kennytm kennytm added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 20, 2018
@kennytm
Copy link
Member

kennytm commented May 20, 2018

@bors r=alexcrichton rollup

@bors
Copy link
Contributor

bors commented May 20, 2018

📌 Commit a74e168 has been approved by alexcrichton

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 20, 2018
@bors
Copy link
Contributor

bors commented May 20, 2018

⌛ Testing commit a74e168 with merge a1d4a95...

bors added a commit that referenced this pull request May 20, 2018
Add implementation of Extend for ()

This is useful in some generic code which wants to collect iterators of items into a result.
@bors
Copy link
Contributor

bors commented May 20, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing a1d4a95 to master...

@bors bors merged commit a74e168 into rust-lang:master May 20, 2018
@cramertj cramertj deleted the extend branch May 21, 2018 20:02
cuviper added a commit to cuviper/rayon that referenced this pull request Aug 15, 2019
bors bot added a commit to rayon-rs/rayon that referenced this pull request Aug 22, 2019
683: impl ParallelExtend<()> for () r=cuviper a=cuviper

Mirrors rust-lang/rust#50234

Co-authored-by: Josh Stone <cuviper@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. relnotes Marks issues that should be documented in the release notes of the next release. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. 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.

10 participants