Tracking issue Iterator map_while

[Mark-Simulacrum]

Added in #66577.

Open questions:

• Implement DoubleEndedIterator, FusedIterator
• Debug output

[WaffleLapkin]

@ollie27 noticed in #66577 (comment) that MapWhile doesn't need to be FusedIterator (and so we can remove the finished flag). However TakeWhile is already FusedIterator and we can't change that since it would be a breaking change. It may be confusing to users that MapWhile and TakeWhile works a bit differently.

How do we choose? Remove the finished flag and probably confuse some users or make less flexible implementation?

Personally I would prefer removing the finished flag, but I'm not sure.

[Boscop]

@WaffleLapkin So the reasoning that it shouldn't impl FusedIterator is that the user might want to continue consuming this iterator after it first returned None (e.g. find multiple "sections" in the original iterator), right?
Then I think removing the finished flag is the right thing to do.

Btw, since TakeWhile impls FusedIterator shouldn't we also want to add a version of it that isn't fused?

[WaffleLapkin]

@Boscop Yes, one of the reasons is that user might want to continue. But it is a rare case, so it's not so important.
In my opinion, we need to remove the finished flag because there is no need for it to exist. When you need to guarantee Nones after the first None, you must call .fuse() (there is even a note in the docs that says that you should prefer .fuse() over T: FusedIterator) and in most cases, you don't need to worry because everything (for loop, fold, for_each, collect...) stops on the first None. That way MapWhile is just more zero-cost.

Not sure if we need unfused TakeWhile since

1. we can't change TakeWhile (this would be perfect, but it's breaking change)
2. all cases of unfused TakeWhile are covered by MapWhile. E.g. .unfused_take_while(pred) == .map_while(|it| if pred(&it) { Some(it) } else { None })

[camsteffen]

Ready to stabilize?

[197g]

This method overall is very, very similar to scan.

// These two yield the same items.
iter.map_while(predicate)
iter.scan((), |(), item| predicate(item)).fuse()

Is there any impl (unsafe or not) that could be added to scan+fuse but not to scan and fuse alone by composition? This would be one possible way to distinguish these two methods, otherwise I see little point in adding map_while here instead of itertools.

[WaffleLapkin]

Wow, I haven't thought that there is such a similar adapter...

@HeroicKatora please note that after #68820 MapWhile does not implement FusedIteraror and thus, iter.map_while(predicate) is analog to just iter.scan((), |(), item| predicate(item)).

However, I'm still in favour of adding map_while to std, because (in my opinion) there are a lot of situations where map_while is more readable than scan (()s are a bit nasty) and also scan doesn't come to mind in such situations (nobody has mentioned it for 7 months after the original PR :D).

[WaffleLapkin]

Oh, by the way, @Mark-Simulacrum , as #68820 is merged, could you remove current "open questions" from the issue?

Open questions:

• Implement DoubleEndedIterator, FusedIterator
• Debug output

[Boscop]

I agree with @WaffleLapkin that the scan solution is very non-obvious and also harder to read for others who read the code. So I'd really prefer to use map_while.
The impl of the map_while method should maybe be written using the scan solution.

[kamulos]

Is there any status update on stabilization? This would be such a valuable function for some cases: I currently have the exact same case that is in the documentation:

.take_while(|x| x.is_some())
.map(|x| x.unwrap());

and I am not comfortable having unwrap() in the code

[WaffleLapkin]

@kamulos since this method was added almost a year ago and all unresolved questions were since resolved, I believe we can start the stabilization process. Someone from the libs team needs to start an FCP.

[m-ou-se]

@rfcbot merge

[rfcbot]

Team member @m-ou-se has proposed to merge this. The next step is review by the rest of the tagged team members:

• @Amanieu
• @BurntSushi
• @dtolnay
• @joshtriplett
• @m-ou-se
• @yaahc

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), 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]

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

[Stargateur]

This method overall is very, very similar to scan.

// These two yield the same items.
iter.map_while(predicate)
iter.scan((), |(), item| predicate(item)).fuse()

Is there any impl (unsafe or not) that could be added to scan+fuse but not to scan and fuse alone by composition? This would be one possible way to distinguish these two methods, otherwise I see little point in adding map_while here instead of itertools.

scan can do everything... that not a good argument in this context.

[Stargateur]

.take_while(|x| x.is_some())
.map(|x| x.unwrap());

@kamulos BY the way .take_while(|x| x.is_some()).flatten()

[rfcbot]

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

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

The RFC will be merged soon.

[matklad]

This method overall is very, very similar to scan.

scan and map_while are equivalent:

scan(init, |state, item| exr)

// is equivalent to

let state = &mut init;
map_whie(|item| expr)

map_while even is more general, as you can get the final state after the iteration is finished.

So, I'd say map_while is what we should have had from the begging, and scan is just an overly complicated version of that with an extra state argument, which makes sense in a purely functional language, but is unnecessary in Rust.

[m-ou-se]

Stabilized in #89086.