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 `Iterator::take_while_map` #66577

Open
wants to merge 10 commits into
base: master
from
@@ -1609,6 +1609,100 @@ impl<I: Iterator, P> Iterator for TakeWhile<I, P>
}
}

/// An iterator that only accepts elements while `predicate` returns `Some(_)`.
///
/// This `struct` is created by the [`take_while_map`] method on [`Iterator`]. See its
/// documentation for more.
///
/// [`take_while_map`]: trait.Iterator.html#method.take_while
/// [`Iterator`]: trait.Iterator.html
#[must_use = "iterators are lazy and do nothing unless consumed"]
#[unstable(feature = "iter_take_while_map", reason = "recently added", issue = "0")]
#[derive(Clone)]
pub struct TakeWhileMap<I, P> {
iter: I,
flag: bool,
This conversation was marked as resolved by WaffleLapkin

This comment has been minimized.

Copy link
@AnthonyMikh

AnthonyMikh Nov 22, 2019

Contributor

Maybe not flag but finished?

This comment has been minimized.

Copy link
@WaffleLapkin

WaffleLapkin Nov 22, 2019

Author

This field was copy-pasted from TakeWhile 🤷‍♀️ But I agree that finished is more clear than flag (actually when I was first reading TakeWhile sources it took me a while to understand the meaning of the flag)

predicate: P,
}

impl<I, P> TakeWhileMap<I, P> {
pub(super) fn new(iter: I, predicate: P) -> TakeWhileMap<I, P> {
TakeWhileMap { iter, flag: false, predicate }
}
}

#[unstable(feature = "iter_take_while_map", reason = "recently added", issue = "0")]

This comment has been minimized.

Copy link
@AnthonyMikh

AnthonyMikh Nov 22, 2019

Contributor

Stability attributes currently don't work with trait impls

This comment has been minimized.

Copy link
@WaffleLapkin

WaffleLapkin Nov 22, 2019

Author

Why then there is a #[stable(feature = "rust1", since = "1.0.0")] on Iterator impl for TakeWhile?

This comment has been minimized.

Copy link
@dtolnay

dtolnay Nov 28, 2019

Member

It's unstability attributes that don't work. ;) There is no such thing as an unstable trait impl.

This comment has been minimized.

Copy link
@WaffleLapkin

WaffleLapkin Dec 12, 2019

Author

I don't understand :( You said that there is no unstable trait impl, but

  1. without #[unsatble] this code doesn't compile: image
  2. there are other #[unstable] impls, e.g.: impl TrustedLen for Rev
  3. there are #[stable] impls too, e.g.: impl Iterator for Rev
impl<I: fmt::Debug, P> fmt::Debug for TakeWhileMap<I, P> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
f.debug_struct("TakeWhileMap")
.field("iter", &self.iter)
.field("flag", &self.flag)
.finish()
}
}

#[unstable(feature = "iter_take_while_map", reason = "recently added", issue = "0")]
impl<B, I: Iterator, P> Iterator for TakeWhileMap<I, P>
where P: FnMut(I::Item) -> Option<B>
{
type Item = B;

#[inline]
fn next(&mut self) -> Option<B> {
if self.flag {
None
} else {
let x = self.iter.next()?;
match (self.predicate)(x) {
res @ Some(_) => res,
None => {
self.flag = true;
None
}
}
}
This conversation was marked as resolved by WaffleLapkin
Comment on lines +1652 to +1663

This comment has been minimized.

Copy link
@AnthonyMikh

AnthonyMikh Nov 22, 2019

Contributor

Can be simpler:

if self.flag {
    return None
}

let x = self.iter.next()?;
let ret = (self.predicate)(x);
self.flag = ret.is_none();
ret

This comment has been minimized.

Copy link
@WaffleLapkin

WaffleLapkin Nov 22, 2019

Author

I think it would be even better to use if+else instead of if+return:

if self.flag {
    None
} else {
    let x = self.iter.next()?;
    let ret = (self.predicate)(x);
    self.flag = ret.is_none();
    ret
}
}

#[inline]
fn size_hint(&self) -> (usize, Option<usize>) {
if self.flag {
(0, Some(0))
} else {
let (_, upper) = self.iter.size_hint();
(0, upper) // can't know a lower bound, due to the predicate
}
}

#[inline]
fn try_fold<Acc, Fold, R>(&mut self, init: Acc, fold: Fold) -> R where
Self: Sized, Fold: FnMut(Acc, Self::Item) -> R, R: Try<Ok=Acc>
{
fn check<'a, B, T, Acc, R: Try<Ok = Acc>>(
flag: &'a mut bool,
p: &'a mut impl FnMut(T) -> Option<B>,
mut fold: impl FnMut(Acc, B) -> R + 'a,
) -> impl FnMut(Acc, T) -> LoopState<Acc, R> + 'a {
move |acc, x| {
match p(x) {
Some(item) => LoopState::from_try(fold(acc, item)),
None => {
*flag = true;
LoopState::Break(Try::from_ok(acc))
},
}
}
}

if self.flag {
Try::from_ok(init)
} else {
let flag = &mut self.flag;
let p = &mut self.predicate;
self.iter.try_fold(init, check(flag, p, fold)).into_try()
}
}
}

#[stable(feature = "fused", since = "1.26.0")]
impl<I, P> FusedIterator for TakeWhile<I, P>
where I: FusedIterator, P: FnMut(&I::Item) -> bool {}
@@ -352,6 +352,8 @@ pub use self::adapters::StepBy;
pub use self::adapters::Flatten;
#[stable(feature = "iter_copied", since = "1.36.0")]
pub use self::adapters::Copied;
#[unstable(feature = "iter_take_while_map", reason = "recently added", issue = "0")]
pub use self::adapters::TakeWhileMap;

pub(crate) use self::adapters::{TrustedRandomAccess, process_results};

@@ -1,10 +1,16 @@
// ignore-tidy-filelength
// This file almost exclusively consists of the definition of `Iterator`. We
// can't split that into multiple files.
This conversation was marked as resolved by dtolnay
Comment on lines +2 to +3

This comment has been minimized.

Copy link
@AnthonyMikh

AnthonyMikh Nov 22, 2019

Contributor

Well, technically we can split definition of Iterator into multiple files and include! them into source...

This comment has been minimized.

Copy link
@WaffleLapkin

WaffleLapkin Nov 22, 2019

Author

But we won't do that, right?

This comment has been minimized.

Copy link
@AnthonyMikh

AnthonyMikh Nov 22, 2019

Contributor

Well, that's up to libs team to decide

This comment has been minimized.

Copy link
@dtolnay

dtolnay Nov 24, 2019

Member

I wouldn't want to do that. That seems worse.


use crate::cmp::{self, Ordering};
use crate::ops::{Add, Try};

use super::super::LoopState;
use super::super::{Chain, Cycle, Copied, Cloned, Enumerate, Filter, FilterMap, Fuse};
use super::super::{Flatten, FlatMap};
use super::super::{Inspect, Map, Peekable, Scan, Skip, SkipWhile, StepBy, Take, TakeWhile, Rev};
use super::super::{
Inspect, Map, Peekable, Scan, Skip, SkipWhile, StepBy, Take, TakeWhile, TakeWhileMap, Rev
};
use super::super::{Zip, Sum, Product, FromIterator};

fn _assert_is_object_safe(_: &dyn Iterator<Item=()>) {}
@@ -988,6 +994,101 @@ pub trait Iterator {
TakeWhile::new(self, predicate)
}

/// Creates an iterator that both yields elements based on a predicate and maps.
///
/// `take_while_map()` takes a closure as an argument. It will call this
/// closure on each element of the iterator, and yield elements
/// while it returns [`Some(_)`][`Some`].
///
/// After [`None`] is returned, `take_while_map()`'s job is over, and the
/// rest of the elements are ignored.
///
/// # Examples
///
/// Basic usage:
///
/// ```
/// #![feature(iter_take_while_map)]
/// let a = [-1i32, 4, 0, 1];
///
/// let mut iter = a.iter().take_while_map(|x| 16i32.checked_div(*x));
///
/// assert_eq!(iter.next(), Some(-16));
/// assert_eq!(iter.next(), Some(4));
/// assert_eq!(iter.next(), None);
/// ```
///
/// Here's the same example, but with [`take_while`] and [`map`]:
///
/// [`take_while`]: #method.take_while
/// [`map`]: #method.map
///
/// ```
/// #![feature(iter_take_while_map)]
/// let a = [-1i32, 4, 0, 1];
///
/// let mut iter = a.iter()
/// .map(|x| 16i32.checked_div(*x))
/// .take_while(|x| x.is_some())
/// .map(|x| x.unwrap());
///
/// assert_eq!(iter.next(), Some(-16));
/// assert_eq!(iter.next(), Some(4));
/// assert_eq!(iter.next(), None);
/// ```
///
/// Stopping after an initial [`None`]:
///
/// ```
/// #![feature(iter_take_while_map)]
/// use std::convert::TryFrom;
///
/// let a = [0, -1, 1, -2];
///
/// let mut iter = a.iter().take_while_map(|x| u32::try_from(*x).ok());
///
/// assert_eq!(iter.next(), Some(0u32));
///
/// // We have more elements that are fit in u32, but since we already
/// // got a None, take_while_map() isn't used any more
/// assert_eq!(iter.next(), None);
/// ```
///
/// Because `take_while_map()` needs to look at the value in order to see if it
/// should be included or not, consuming iterators will see that it is
/// removed:
///
/// ```
/// #![feature(iter_take_while_map)]
/// use std::convert::TryFrom;
///
/// let a = [1, 2, -3, 4];
/// let mut iter = a.iter();
///
/// let result: Vec<u32> = iter.by_ref()
/// .take_while_map(|n| u32::try_from(*n).ok())
/// .collect();
///
/// assert_eq!(result, &[1, 2]);
///
/// let result: Vec<i32> = iter.cloned().collect();
///
/// assert_eq!(result, &[4]);
/// ```
///
/// The `-3` is no longer there, because it was consumed in order to see if
/// the iteration should stop, but wasn't placed back into the iterator.
///
/// [`Some`]: ../../std/option/enum.Option.html#variant.Some
/// [`None`]: ../../std/option/enum.Option.html#variant.None
#[inline]
#[unstable(feature = "iter_take_while_map", reason = "recently added", issue = "0")]
fn take_while_map<B, P>(self, predicate: P) -> TakeWhileMap<Self, P> where
Self: Sized, P: FnMut(Self::Item) -> Option<B>,
{
TakeWhileMap::new(self, predicate)
}

/// Creates an iterator that skips the first `n` elements.
///
/// After they have been consumed, the rest of the elements are yielded.
@@ -1479,6 +1479,7 @@ fn test_iterator_size_hint() {
assert_eq!(c.clone().take(5).size_hint(), (5, Some(5)));
assert_eq!(c.clone().skip(5).size_hint().1, None);
assert_eq!(c.clone().take_while(|_| false).size_hint(), (0, None));
assert_eq!(c.clone().take_while_map(|_| None::<()>).size_hint(), (0, None));
assert_eq!(c.clone().skip_while(|_| false).size_hint(), (0, None));
assert_eq!(c.clone().enumerate().size_hint(), (usize::MAX, None));
assert_eq!(c.clone().chain(vi.clone().cloned()).size_hint(), (usize::MAX, None));
@@ -1493,6 +1494,7 @@ fn test_iterator_size_hint() {
assert_eq!(vi.clone().skip(3).size_hint(), (7, Some(7)));
assert_eq!(vi.clone().skip(12).size_hint(), (0, Some(0)));
assert_eq!(vi.clone().take_while(|_| false).size_hint(), (0, Some(10)));
assert_eq!(vi.clone().take_while_map(|_| None::<()>).size_hint(), (0, Some(10)));
assert_eq!(vi.clone().skip_while(|_| false).size_hint(), (0, Some(10)));
assert_eq!(vi.clone().enumerate().size_hint(), (10, Some(10)));
assert_eq!(vi.clone().chain(v2).size_hint(), (13, Some(13)));
@@ -36,6 +36,7 @@
#![feature(iter_is_partitioned)]
#![feature(iter_order_by)]
#![feature(cmp_min_max_by)]
#![feature(iter_take_while_map)]

extern crate test;

ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.