Add std/core::iter::repeat_with #48156

Centril · 2018-02-12T08:54:01Z

Adds an iterator primitive repeat_with which is the "lazy" version of repeat but also more flexible since you can build up state with the FnMut. The design is mostly taken from repeat.

The tracking issue is #48169.

r? @rust-lang/libs
cc @withoutboats, @scottmcm

rust-highfive · 2018-02-12T08:54:08Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @rust-lang (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

scottmcm

Good to see this; I've done repeat(()).map(|()| ...) before and been sad to do so 😆

scottmcm · 2018-02-12T10:21:56Z

src/libcore/tests/iter.rs

+}
+
+#[test]
+fn test_repeat_with_rev() {


This behaviour is surprising to me, since it produces the same sequence of different things with or without the .rev(). Perhaps it should only be DEI for F:Fn, not F:FnMut?

Yeah I agree. F: Fn() -> A seems reasonable under the assumption that it should be a pure function in the context of iterators, and we can always document that on repeat_with (the function).

It's consistent with map this way... No big opinion but I'd vote to keep it double ended with FnMut

@bluss I guess that works too if we document it well... I don't have a big opinion either.

bluss · 2018-02-12T18:14:22Z

src/libcore/iter/sources.rs

@@ -57,6 +57,12 @@ unsafe impl<A: Clone> TrustedLen for Repeat<A> {}
 ///
 /// [`take`]: trait.Iterator.html#method.take
 ///
+/// If the element type of the iterator you need does not implement `Clone`,
+/// or if you do not want to keep the repeated element in memory, you can
+/// instead use the [`repeat_with`] function.


Not necessary that we mention it, but another case is indeed to emit a different value each time the function is called, for example (stolen from itertools so it uses while_some() as well):

let mut heap = BinaryHeap::from(vec![2, 5, 3, 7, 8]); // extract each element in sorted order for element in repeat_with(|| heap.pop()).while_some() { print!("{}", element); }

Pretty neat example - let's have .while_some() in core?

Edit: we could also have a while_some(..) source? so that you can write:

for elt in while_some(|| heap.pop()) { print!("{}", elt); }

I think while_some is a pretty descriptive name as source as well.

Hopefully there's a Try version that can go well with Option and Result and friends, or perhaps a more fundamental adapter too. Possibly-related: #45594 (comment)

@scottmcm Perhaps Try<Ok = Elt, Error: Into<()>>?

alexcrichton · 2018-02-12T20:05:27Z

This also seems fine by me, thanks @Centril! Want to file a tracking issue for this and then we can r+?

Centril · 2018-02-12T20:49:03Z

@alexcrichton Cheers! Damn, T-libs is working fast today ;) The tracking issue is #48169.

alexcrichton · 2018-02-12T20:53:36Z

@bors: r+

bors · 2018-02-12T20:53:37Z

📌 Commit 91a4b90 has been approved by alexcrichton

Pazzaz · 2018-02-12T23:00:23Z

src/libcore/iter/sources.rs

+#[unstable(feature = "trusted_len", issue = "37572")]
+unsafe impl<A, F: FnMut() -> A> TrustedLen for RepeatWith<F> {}
+
+/// Creates a new that repeats elements of type `A` endlessly by


Missing word after Creates a new?

@Pazzaz Nice catch =) I'll patch it once bors is done (probably have to run bors again tho..?)

alexcrichton · 2018-02-13T15:22:34Z

@bors: r+

bors · 2018-02-13T15:22:34Z

📌 Commit db13296 has been approved by alexcrichton

@withoutboats

…h, r=alexcrichton Add std/core::iter::repeat_with Adds an iterator primitive `repeat_with` which is the "lazy" version of `repeat` but also more flexible since you can build up state with the `FnMut`. The design is mostly taken from `repeat`. r? @rust-lang/libs cc @withoutboats, @scottmcm

Rollup of 22 pull requests - Successful merges: #47784, #47806, #47846, #48005, #48033, #48065, #48087, #48114, #48126, #48130, #48133, #48151, #48154, #48156, #48162, #48163, #48165, #48167, #48181, #48186, #48195, #48035 - Failed merges:

Rollup of 23 pull requests - Successful merges: #47784, #47806, #47846, #48005, #48033, #48065, #48087, #48114, #48126, #48130, #48133, #48151, #48154, #48156, #48162, #48163, #48165, #48167, #48181, #48186, #48195, #48035, #48210 - Failed merges:

therealprof · 2018-02-16T10:18:44Z

src/libcore/iter/sources.rs

+/// [`repeat`]: fn.repeat.html
+///
+/// An iterator produced by `repeat_with()` is a `DoubleEndedIterator`.
+/// It is important to not that reversing `repeat_with(f)` will produce


I think that not should be a note instead.

Fixed in #48282.

…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 added 6 commits February 12, 2018 08:05

add core::iter::repeat_with

0f789aa

core::iter::repeat_with: general fixes

c4099ca

core::iter::repeat_with: derive Copy, Clone, Debug

1af9ee1

core::iter::repeat_with: fix tests

f025eff

core::iter::repeat_with: fix tests some more

55c669c

core::iter::repeat_with: fix doc tests

efa3320

Centril added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 12, 2018

Centril requested a review from bluss February 12, 2018 08:57

scottmcm reviewed Feb 12, 2018

View reviewed changes

core::iter::repeat_with: document DoubleEndedIterator behavior

9cee79a

bluss reviewed Feb 12, 2018

View reviewed changes

bluss approved these changes Feb 12, 2018

View reviewed changes

Centril mentioned this pull request Feb 12, 2018

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

Closed

core::iter::repeat_with: tracking issue is rust-lang#48169

91a4b90

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 Feb 12, 2018

Pazzaz reviewed Feb 12, 2018

View reviewed changes

core::iter::repeat_with: fix missing word, see @Pazzaz's review

db13296

kennytm mentioned this pull request Feb 14, 2018

Rollup of 23 pull requests #48203

Merged

kennytm merged commit db13296 into rust-lang:master Feb 15, 2018

Centril deleted the feature/iterator_repeat_with branch February 15, 2018 21:58

therealprof reviewed Feb 16, 2018

View reviewed changes

Centril mentioned this pull request Feb 16, 2018

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

Merged

bluss mentioned this pull request Feb 21, 2018

The cutting block: These features will be removed from itertools eventually rust-itertools/itertools#223

Closed

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add std/core::iter::repeat_with #48156

Add std/core::iter::repeat_with #48156

Centril commented Feb 12, 2018 •

edited

Loading

rust-highfive commented Feb 12, 2018

scottmcm left a comment

scottmcm Feb 12, 2018

Centril Feb 12, 2018 •

edited

Loading

bluss Feb 12, 2018

Centril Feb 12, 2018

bluss Feb 12, 2018

Centril Feb 12, 2018 •

edited

Loading

scottmcm Feb 12, 2018 •

edited

Loading

Centril Feb 12, 2018

alexcrichton commented Feb 12, 2018

Centril commented Feb 12, 2018

alexcrichton commented Feb 12, 2018

bors commented Feb 12, 2018

Pazzaz Feb 12, 2018

Centril Feb 12, 2018

alexcrichton commented Feb 13, 2018

bors commented Feb 13, 2018

therealprof Feb 16, 2018

Centril Feb 16, 2018

Add std/core::iter::repeat_with #48156

Add std/core::iter::repeat_with #48156

Conversation

Centril commented Feb 12, 2018 • edited Loading

rust-highfive commented Feb 12, 2018

scottmcm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Centril Feb 12, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Centril Feb 12, 2018 • edited Loading

Choose a reason for hiding this comment

scottmcm Feb 12, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexcrichton commented Feb 12, 2018

Centril commented Feb 12, 2018

alexcrichton commented Feb 12, 2018

bors commented Feb 12, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexcrichton commented Feb 13, 2018

bors commented Feb 13, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Centril commented Feb 12, 2018 •

edited

Loading

Centril Feb 12, 2018 •

edited

Loading

Centril Feb 12, 2018 •

edited

Loading

scottmcm Feb 12, 2018 •

edited

Loading