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

Tracking issue for std::iter::repeat_with #48169

Closed
Centril opened this Issue Feb 12, 2018 · 11 comments

Comments

Projects
None yet
6 participants
@Centril
Copy link
Contributor

Centril commented Feb 12, 2018

This is the tracking issue for std::iter::repeat_with.
Currently in nightly, PR = #48156.

Centril added a commit to Centril/rust that referenced this issue Feb 12, 2018

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Feb 17, 2018

Rollup merge of rust-lang#48282 - Centril:spelling-fix/iter-repeat-wi…
…th, r=kennytm

Fix spelling in core::iter::repeat_with: s/not/note

Fixes spelling error in rust-lang#48156 (comment).
Tracking issue: rust-lang#48169

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Feb 18, 2018

Rollup merge of rust-lang#48282 - Centril:spelling-fix/iter-repeat-wi…
…th, r=kennytm

Fix spelling in core::iter::repeat_with: s/not/note

Fixes spelling error in rust-lang#48156 (comment).
Tracking issue: rust-lang#48169
@Centril

This comment has been minimized.

Copy link
Contributor Author

Centril commented May 3, 2018

@SimonSapin Would stabilizing this be appropriate now?

@SimonSapin

This comment has been minimized.

Copy link
Contributor

SimonSapin commented May 3, 2018

The iterator_repeat_with feature covers these APIs in std::iter:

pub fn repeat_with<A, F: FnMut() -> A>(repeater: F) -> RepeatWith<F> { /*…*/ }
pub struct RepeatWith<F> { /*…*/ }
impl<A, F: FnMut() -> A> Iterator for RepeatWith<F> { type Item = A; /* … */ }
impl<A, F: FnMut() -> A> DoubleEndedIterator for RepeatWith<F> { /*…*/ }
impl<A, F: FnMut() -> A> FusedIterator for RepeatWith<F> {}
unsafe impl<A, F: FnMut() -> A> TrustedLen for RepeatWith<F> {}

They’ve been in Nightly since February 15 and seem fine to me to stabilize.

@rfcbot fcp merge

@rfcbot

This comment has been minimized.

Copy link

rfcbot commented May 3, 2018

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

Concerns:

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.

@dtolnay

This comment has been minimized.

Copy link
Member

dtolnay commented May 4, 2018

@rfcbot concern DEI

The DoubleEndedIterator impl is somewhat surprising. I see this was brought up in #48156 (review) as well. While this has been in nightly has anyone observed use cases that benefit from having the DoubleEndedIterator impl?

@Centril

This comment has been minimized.

Copy link
Contributor Author

Centril commented May 5, 2018

@dtolnay I haven't; DEI for RepeatWith doesn't seem useful on its own; and given
repeat_with(|| ..).zip(my_dei).rev() you can reorder as repeat_with(|| ..).zip(my_dei.rev()). This also applies to .chain(..). However, I think forcing the user to re-order might not be such a good thing from an ergonomics perspective. But... We can always get rid of the DEI impl now and re-add it if anyone asks for it.

@dtolnay

This comment has been minimized.

Copy link
Member

dtolnay commented May 5, 2018

I would accept this without a DoubleEndedIterator impl and then follow up when someone can explain their concrete use case for it.

@Centril

This comment has been minimized.

Copy link
Contributor Author

Centril commented May 6, 2018

@dtolnay Fair enough, sounds like a plan 👍.

@dtolnay

This comment has been minimized.

Copy link
Member

dtolnay commented May 6, 2018

@rfcbot resolved DEI

@Kimundi

This comment has been minimized.

Copy link
Member

Kimundi commented May 16, 2018

Removing the DEI impl seems like the best course of action, yeah.

@rfcbot

This comment has been minimized.

Copy link

rfcbot commented May 16, 2018

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

@rfcbot

This comment has been minimized.

Copy link

rfcbot commented May 26, 2018

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

tmccombs added a commit to tmccombs/rust that referenced this issue May 30, 2018

tmccombs added a commit to tmccombs/rust that referenced this issue Jun 2, 2018

bors added a commit that referenced this issue Jun 10, 2018

Auto merge of #51200 - tmccombs:stable-iter-repeat-with, r=Centril,ke…
…nnytm

Stabilize iterator_repeat_with

Fixes #48169

@bors bors closed this in #51200 Jun 10, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.