From a8340413fe8aa29864ce05875cc63f29af305e31 Mon Sep 17 00:00:00 2001 From: Waffle Date: Tue, 4 Feb 2020 11:38:07 +0300 Subject: [PATCH 1/3] Remove `finished` flag from `MapWhile` --- src/libcore/iter/adapters/mod.rs | 68 +++++++++-------------------- src/libcore/iter/traits/iterator.rs | 15 +++++++ 2 files changed, 36 insertions(+), 47 deletions(-) diff --git a/src/libcore/iter/adapters/mod.rs b/src/libcore/iter/adapters/mod.rs index 7d10ef3d28219..177e48b05b3db 100644 --- a/src/libcore/iter/adapters/mod.rs +++ b/src/libcore/iter/adapters/mod.rs @@ -1752,6 +1752,14 @@ where } } +#[stable(feature = "fused", since = "1.26.0")] +impl FusedIterator for TakeWhile +where + I: FusedIterator, + P: FnMut(&I::Item) -> bool, +{ +} + /// An iterator that only accepts elements while `predicate` returns `Some(_)`. /// /// This `struct` is created by the [`map_while`] method on [`Iterator`]. See its @@ -1764,20 +1772,19 @@ where #[derive(Clone)] pub struct MapWhile { iter: I, - finished: bool, predicate: P, } impl MapWhile { pub(super) fn new(iter: I, predicate: P) -> MapWhile { - MapWhile { iter, finished: false, predicate } + MapWhile { iter, predicate } } } #[unstable(feature = "iter_map_while", reason = "recently added", issue = "68537")] impl fmt::Debug for MapWhile { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.debug_struct("MapWhile").field("iter", &self.iter).field("flag", &self.finished).finish() + f.debug_struct("MapWhile").field("iter", &self.iter).finish() } } @@ -1790,65 +1797,32 @@ where #[inline] fn next(&mut self) -> Option { - if self.finished { - None - } else { - let x = self.iter.next()?; - let ret = (self.predicate)(x); - self.finished = ret.is_none(); - ret - } + let x = self.iter.next()?; + (self.predicate)(x) } #[inline] fn size_hint(&self) -> (usize, Option) { - if self.finished { - (0, Some(0)) - } else { - let (_, upper) = self.iter.size_hint(); - (0, upper) // can't know a lower bound, due to the predicate - } + let (_, upper) = self.iter.size_hint(); + (0, upper) // can't know a lower bound, due to the predicate } #[inline] - fn try_fold(&mut self, init: Acc, fold: Fold) -> R + fn try_fold(&mut self, init: Acc, mut fold: Fold) -> R where Self: Sized, Fold: FnMut(Acc, Self::Item) -> R, R: Try, { - fn check<'a, B, T, Acc, R: Try>( - flag: &'a mut bool, - p: &'a mut impl FnMut(T) -> Option, - mut fold: impl FnMut(Acc, B) -> R + 'a, - ) -> impl FnMut(Acc, T) -> LoopState + '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.finished { - Try::from_ok(init) - } else { - let flag = &mut self.finished; - let p = &mut self.predicate; - self.iter.try_fold(init, check(flag, p, fold)).into_try() - } + let Self { iter, predicate } = self; + iter.try_fold(init, |acc, x| match predicate(x) { + Some(item) => LoopState::from_try(fold(acc, item)), + None => LoopState::Break(Try::from_ok(acc)), + }) + .into_try() } } -#[stable(feature = "fused", since = "1.26.0")] -impl FusedIterator for TakeWhile -where - I: FusedIterator, - P: FnMut(&I::Item) -> bool, -{ -} - /// An iterator that skips over `n` elements of `iter`. /// /// This `struct` is created by the [`skip`] method on [`Iterator`]. See its diff --git a/src/libcore/iter/traits/iterator.rs b/src/libcore/iter/traits/iterator.rs index 1d055676c7708..3b1f3b3a4ead6 100644 --- a/src/libcore/iter/traits/iterator.rs +++ b/src/libcore/iter/traits/iterator.rs @@ -1114,8 +1114,23 @@ pub trait Iterator { /// 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. /// + /// Note that unlike [`take_while`] this iterator is **not** fused: + /// + /// ``` + /// #![feature(iter_map_while)] + /// use std::convert::identity; + /// + /// let mut iter = vec![Some(0), None, Some(1)].into_iter().map_while(identity); + /// assert_eq!(iter.next(), Some(0)); + /// assert_eq!(iter.next(), None); + /// assert_eq!(iter.next(), Some(1)); + /// ``` + /// + /// If you need fused iterator, use [`fuse`]. + /// /// [`Some`]: ../../std/option/enum.Option.html#variant.Some /// [`None`]: ../../std/option/enum.Option.html#variant.None + /// [`fuse`]: #method.fuse #[inline] #[unstable(feature = "iter_map_while", reason = "recently added", issue = "68537")] fn map_while(self, predicate: P) -> MapWhile From 605bc1b19b45534b0860fad4b1d44678bc8b34e9 Mon Sep 17 00:00:00 2001 From: Waffle Date: Mon, 24 Feb 2020 10:30:41 +0300 Subject: [PATCH 2/3] adjuste doc of `map_while` Fix doc of `Iterator::map_while` so it would be clearer that it isn't fused. --- src/libcore/iter/traits/iterator.rs | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/src/libcore/iter/traits/iterator.rs b/src/libcore/iter/traits/iterator.rs index 3b1f3b3a4ead6..38882980b0d1d 100644 --- a/src/libcore/iter/traits/iterator.rs +++ b/src/libcore/iter/traits/iterator.rs @@ -1036,9 +1036,6 @@ pub trait Iterator { /// closure on each element of the iterator, and yield elements /// while it returns [`Some(_)`][`Some`]. /// - /// After [`None`] is returned, `map_while()`'s job is over, and the - /// rest of the elements are ignored. - /// /// # Examples /// /// Basic usage: @@ -1078,15 +1075,14 @@ pub trait Iterator { /// #![feature(iter_map_while)] /// use std::convert::TryFrom; /// - /// let a = [0, -1, 1, -2]; + /// let a = [0, 1, 2, -3, 4, 5, -6]; /// - /// let mut iter = a.iter().map_while(|x| u32::try_from(*x).ok()); + /// let iter = a.iter().map_while(|x| u32::try_from(*x).ok()); + /// let vec = iter.collect::>(); /// - /// assert_eq!(iter.next(), Some(0u32)); - /// - /// // We have more elements that are fit in u32, but since we already - /// // got a None, map_while() isn't used any more - /// assert_eq!(iter.next(), None); + /// // We have more elements which could fit in u32 (4, 5), but `map_while` + /// // has stopped on the first `None` from predicate (when working with `-3`) + /// assert_eq!(vec, vec![0, 1, 2]); /// ``` /// /// Because `map_while()` needs to look at the value in order to see if it From e964d7180ca4164dfa3d6c6dee6026889c604812 Mon Sep 17 00:00:00 2001 From: Waffle Date: Sat, 21 Mar 2020 18:40:28 +0300 Subject: [PATCH 3/3] slightly change the `Iterator::map_while` docs --- src/libcore/iter/traits/iterator.rs | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/src/libcore/iter/traits/iterator.rs b/src/libcore/iter/traits/iterator.rs index 38882980b0d1d..b8becb68008fa 100644 --- a/src/libcore/iter/traits/iterator.rs +++ b/src/libcore/iter/traits/iterator.rs @@ -1080,8 +1080,8 @@ pub trait Iterator { /// let iter = a.iter().map_while(|x| u32::try_from(*x).ok()); /// let vec = iter.collect::>(); /// - /// // We have more elements which could fit in u32 (4, 5), but `map_while` - /// // has stopped on the first `None` from predicate (when working with `-3`) + /// // We have more elements which could fit in u32 (4, 5), but `map_while` returned `None` for `-3` + /// // (as the `predicate` returned `None`) and `collect` stops at the first `None` entcountered. /// assert_eq!(vec, vec![0, 1, 2]); /// ``` /// @@ -1110,18 +1110,8 @@ pub trait Iterator { /// 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. /// - /// Note that unlike [`take_while`] this iterator is **not** fused: - /// - /// ``` - /// #![feature(iter_map_while)] - /// use std::convert::identity; - /// - /// let mut iter = vec![Some(0), None, Some(1)].into_iter().map_while(identity); - /// assert_eq!(iter.next(), Some(0)); - /// assert_eq!(iter.next(), None); - /// assert_eq!(iter.next(), Some(1)); - /// ``` - /// + /// Note that unlike [`take_while`] this iterator is **not** fused. + /// It is also not specified what this iterator returns after the first` None` is returned. /// If you need fused iterator, use [`fuse`]. /// /// [`Some`]: ../../std/option/enum.Option.html#variant.Some