Skip to content

Implement IntoIterator for Receiver#24633

Merged
bors merged 1 commit into
rust-lang:masterfrom
rapha:master
Apr 24, 2015
Merged

Implement IntoIterator for Receiver#24633
bors merged 1 commit into
rust-lang:masterfrom
rapha:master

Conversation

@rapha

@rapha rapha commented Apr 20, 2015

Copy link
Copy Markdown
Contributor

No description provided.

@rust-highfive

Copy link
Copy Markdown
Contributor

r? @brson

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

@alexcrichton

Copy link
Copy Markdown
Member

This seems like a good idea to me, so thanks @rapha! This does, however, probably force our hand in having this new functionality be insta-#[stable]. I'm personally OK with that, however.

Could you change the imports of core::iter::IntoIterator to just iter::IntoIterator as well?

@rapha

rapha commented Apr 20, 2015

Copy link
Copy Markdown
Contributor Author

Thanks. I added doc and stability annotations to match those on the Iter type, adding that it was an "owning" iterator.

@rapha

rapha commented Apr 22, 2015

Copy link
Copy Markdown
Contributor Author

Do you need me to do anything more on this?

@alexcrichton alexcrichton assigned alexcrichton and unassigned brson Apr 22, 2015
@alexcrichton

Copy link
Copy Markdown
Member

Ah thanks for the reminder @rapha! I intended on just running this by some others to get their opinion as well. I believe now this is good to go! In the meantime, however, we've bumped the version number, so could you update the since tag here to 1.1.0 instead of 1.0.0? After that I'll r+

@bluss

bluss commented Apr 22, 2015

Copy link
Copy Markdown
Contributor

Shouldn't it have another IntoIterator impl to match the fn iter(&self) method too?

@alexcrichton

Copy link
Copy Markdown
Member

@bluss in theory, yes, but it's not necessarily required one way or another as it's backwards compatible to add. @rapha would you be interested in adding this, however?

@rapha ah I forgot this part, but you'll also need to choose a unique feature name for this as the same feature name can't have multiple versions it became stable in (e.g. the rust1 feature here was stable as of 1.0.0, not 1.1.0

@rapha

rapha commented Apr 23, 2015

Copy link
Copy Markdown
Contributor Author

@bluss Is this the implementation of IntoIterator for &Receiver what you had in mind?

Comment thread src/libstd/sync/mpsc/mod.rs Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ah sorry by new feature name I meant more something along the lines of receiver_into_iter or something like that

@alexcrichton

Copy link
Copy Markdown
Member

@rapha looks good to me! Could you also squash the commits together?

@rapha

rapha commented Apr 23, 2015

Copy link
Copy Markdown
Contributor Author

Done

Comment thread src/libstd/sync/mpsc/mod.rs Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

One last instance of rust1_1

@bluss

bluss commented Apr 23, 2015

Copy link
Copy Markdown
Contributor

Yes that's great. It's consistent now that the .iter() method has a corresponding IntoIterator impl, thank you!

@alexcrichton

Copy link
Copy Markdown
Member

@bors: r+ 47ebc48

bors added a commit that referenced this pull request Apr 23, 2015
@bors

bors commented Apr 23, 2015

Copy link
Copy Markdown
Collaborator

⌛ Testing commit 47ebc48 with merge 21f278a...

@bors

bors commented Apr 24, 2015

Copy link
Copy Markdown
Collaborator

@bors bors merged commit 47ebc48 into rust-lang:master Apr 24, 2015
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.

6 participants