From 9c7fe03bd526ea1652e7f89d618f4e1b7abccc0a Mon Sep 17 00:00:00 2001 From: Nathan West Date: Tue, 8 Oct 2019 12:07:46 -0700 Subject: [PATCH 1/7] Improved the use of `Extend` in a few `Iterator` methods - Reimplemented `Iterator::partition` to use a full iterator `extend` on one of the two collections, rather than pushing to both of them one element at a time. - Reimplemented `Iterator::unzip` to use a full iterator `extend` on one of the two collections, rather than pushing to both of them at the same time --- src/libcore/iter/traits/iterator.rs | 49 +++++++++++------------------ 1 file changed, 18 insertions(+), 31 deletions(-) diff --git a/src/libcore/iter/traits/iterator.rs b/src/libcore/iter/traits/iterator.rs index a272035150a15..530e136ed1ae3 100644 --- a/src/libcore/iter/traits/iterator.rs +++ b/src/libcore/iter/traits/iterator.rs @@ -1504,30 +1504,23 @@ pub trait Iterator { /// assert_eq!(odd, vec![1, 3]); /// ``` #[stable(feature = "rust1", since = "1.0.0")] - fn partition(self, f: F) -> (B, B) where + fn partition(self, mut predicate: F) -> (B, B) where Self: Sized, B: Default + Extend, F: FnMut(&Self::Item) -> bool { - #[inline] - fn extend<'a, T, B: Extend>( - mut f: impl FnMut(&T) -> bool + 'a, - left: &'a mut B, - right: &'a mut B, - ) -> impl FnMut(T) + 'a { - move |x| { - if f(&x) { - left.extend(Some(x)); - } else { - right.extend(Some(x)); - } - } - } - let mut left: B = Default::default(); let mut right: B = Default::default(); + let right_ref = &mut right; - self.for_each(extend(f, &mut left, &mut right)); + left.extend(self.filter_map(#[inline] move |item| { + if predicate(&item) { + Some(item) + } else { + right_ref.extend(Some(item)); + None + } + })); (left, right) } @@ -2378,22 +2371,16 @@ pub trait Iterator { FromB: Default + Extend, Self: Sized + Iterator, { - fn extend<'a, A, B>( - ts: &'a mut impl Extend, - us: &'a mut impl Extend, - ) -> impl FnMut((A, B)) + 'a { - move |(t, u)| { - ts.extend(Some(t)); - us.extend(Some(u)); - } - } - - let mut ts: FromA = Default::default(); - let mut us: FromB = Default::default(); + let mut lhs = FromA::default(); + let mut rhs = FromB::default(); + let rhs_ref = &mut rhs; - self.for_each(extend(&mut ts, &mut us)); + lhs.extend(self.map(#[inline] move |(a, b)| { + rhs_ref.extend(Some(b)); + a + })); - (ts, us) + (lhs, rhs) } /// Creates an iterator which copies all of its elements. From dce673446307bd5816dca5227c03ef930c5dd02d Mon Sep 17 00:00:00 2001 From: Nathan West Date: Tue, 8 Oct 2019 14:23:04 -0700 Subject: [PATCH 2/7] Switched to local functions away from closures, per #62429 --- src/libcore/iter/traits/iterator.rs | 49 +++++++++++++++++++---------- 1 file changed, 32 insertions(+), 17 deletions(-) diff --git a/src/libcore/iter/traits/iterator.rs b/src/libcore/iter/traits/iterator.rs index 530e136ed1ae3..27e484f7b2d0f 100644 --- a/src/libcore/iter/traits/iterator.rs +++ b/src/libcore/iter/traits/iterator.rs @@ -1504,27 +1504,36 @@ pub trait Iterator { /// assert_eq!(odd, vec![1, 3]); /// ``` #[stable(feature = "rust1", since = "1.0.0")] - fn partition(self, mut predicate: F) -> (B, B) where + fn partition(self, predicate: F) -> (B, B) + where Self: Sized, B: Default + Extend, - F: FnMut(&Self::Item) -> bool + F: FnMut(&Self::Item) -> bool, { let mut left: B = Default::default(); let mut right: B = Default::default(); - let right_ref = &mut right; - left.extend(self.filter_map(#[inline] move |item| { - if predicate(&item) { - Some(item) - } else { - right_ref.extend(Some(item)); - None + #[inline] + fn extend_rhs<'a, A, T: Extend, P: FnMut(&A) -> bool + 'a>( + rhs: &'a mut T, + mut predicate: P, + ) -> impl FnMut(A) -> Option + 'a { + move |item| { + if predicate(&item) { + Some(item) + } else { + rhs.extend(Some(item)); + None + } } - })); + } + + left.extend(self.filter_map(extend_rhs(&mut right, predicate))); (left, right) } + /// Reorder the elements of this iterator *in-place* according to the given predicate, /// such that all those that return `true` precede all those that return `false`. /// Returns the number of `true` elements found. @@ -2366,23 +2375,29 @@ pub trait Iterator { /// assert_eq!(right, [2, 4]); /// ``` #[stable(feature = "rust1", since = "1.0.0")] - fn unzip(self) -> (FromA, FromB) where + fn unzip(self) -> (FromA, FromB) + where FromA: Default + Extend, FromB: Default + Extend, - Self: Sized + Iterator, + Self: Sized + Iterator, { let mut lhs = FromA::default(); let mut rhs = FromB::default(); - let rhs_ref = &mut rhs; - lhs.extend(self.map(#[inline] move |(a, b)| { - rhs_ref.extend(Some(b)); - a - })); + #[inline] + fn extend_rhs<'a, A, B, T: Extend>(rhs: &'a mut T) -> impl FnMut((A, B)) -> A + 'a { + move |(a, b)| { + rhs.extend(Some(b)); + a + } + } + + lhs.extend(self.map(extend_rhs(&mut rhs))); (lhs, rhs) } + /// Creates an iterator which copies all of its elements. /// /// This is useful when you have an iterator over `&T`, but you need an From 1e9b187a9ade7aa0e9c23009af508aa88170b1b1 Mon Sep 17 00:00:00 2001 From: Nathan West Date: Tue, 8 Oct 2019 17:17:51 -0700 Subject: [PATCH 3/7] Updated `unzip` to fully consume the provided iterator --- src/libcore/iter/traits/iterator.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/libcore/iter/traits/iterator.rs b/src/libcore/iter/traits/iterator.rs index 27e484f7b2d0f..5123b7c5e281f 100644 --- a/src/libcore/iter/traits/iterator.rs +++ b/src/libcore/iter/traits/iterator.rs @@ -2392,7 +2392,13 @@ pub trait Iterator { } } - lhs.extend(self.map(extend_rhs(&mut rhs))); + lhs.extend((&mut self).map(extend_rhs(&mut rhs))); + + // lhs.extend may not have fully consumed the iterator + rhs.extend(&mut self); + + // rhs.extend may not have fully consumed the iterator + self.for_each(#[inline(always)] |_| {}) (lhs, rhs) } From c42d974b631c39629483c337e5236131e4afdf8f Mon Sep 17 00:00:00 2001 From: Nathan West Date: Wed, 9 Oct 2019 14:08:31 -0700 Subject: [PATCH 4/7] Added fuse & size_hint test to unzip --- src/libcore/iter/traits/iterator.rs | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/src/libcore/iter/traits/iterator.rs b/src/libcore/iter/traits/iterator.rs index 5123b7c5e281f..16b2455fbce85 100644 --- a/src/libcore/iter/traits/iterator.rs +++ b/src/libcore/iter/traits/iterator.rs @@ -2383,6 +2383,7 @@ pub trait Iterator { { let mut lhs = FromA::default(); let mut rhs = FromB::default(); + let mut fused = self.fuse(); #[inline] fn extend_rhs<'a, A, B, T: Extend>(rhs: &'a mut T) -> impl FnMut((A, B)) -> A + 'a { @@ -2392,13 +2393,22 @@ pub trait Iterator { } } - lhs.extend((&mut self).map(extend_rhs(&mut rhs))); + #[inline] + fn second((_a, b): (A, B)) -> B { + b + } + + lhs.extend((&mut fused).map(extend_rhs(&mut rhs))); // lhs.extend may not have fully consumed the iterator - rhs.extend(&mut self); + if fused.size_hint().1 != Some(0) { + rhs.extend((&mut fused).map(second)); - // rhs.extend may not have fully consumed the iterator - self.for_each(#[inline(always)] |_| {}) + // rhs.extend may not have fully consumed the iterator + if fused.size_hint().1 != Some(0) { + fused.for_each(drop); + } + } (lhs, rhs) } From 44fb4e9f8ac52598adaf56deb099bfc0a6349987 Mon Sep 17 00:00:00 2001 From: Nathan West Date: Wed, 9 Oct 2019 19:06:01 -0700 Subject: [PATCH 5/7] Additional small changes - Simplified unzip implementation (removed size_hint checks) - Added forwarding try_fold implementation to &mut T: Iterator --- src/libcore/iter/traits/iterator.rs | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/libcore/iter/traits/iterator.rs b/src/libcore/iter/traits/iterator.rs index 16b2455fbce85..141942198c399 100644 --- a/src/libcore/iter/traits/iterator.rs +++ b/src/libcore/iter/traits/iterator.rs @@ -2401,14 +2401,10 @@ pub trait Iterator { lhs.extend((&mut fused).map(extend_rhs(&mut rhs))); // lhs.extend may not have fully consumed the iterator - if fused.size_hint().1 != Some(0) { - rhs.extend((&mut fused).map(second)); + rhs.extend((&mut fused).map(second)); - // rhs.extend may not have fully consumed the iterator - if fused.size_hint().1 != Some(0) { - fused.for_each(drop); - } - } + // rhs.extend may not have fully consumed the iterator + fused.for_each(drop); (lhs, rhs) } @@ -2995,4 +2991,10 @@ impl Iterator for &mut I { fn nth(&mut self, n: usize) -> Option { (**self).nth(n) } + #[inline] + fn try_fold(&mut self, init: B, mut f: F) -> R where + Self: Sized, F: FnMut(B, Self::Item) -> R, R: Try + { + (**self).try_fold(init, f) + } } From bf3a2b14d78e3458c2622df8dac36616d886c05c Mon Sep 17 00:00:00 2001 From: Nathan West Date: Thu, 10 Oct 2019 02:12:06 -0700 Subject: [PATCH 6/7] Removed broked try_fold impl from &mut Iterator --- src/libcore/iter/traits/iterator.rs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/libcore/iter/traits/iterator.rs b/src/libcore/iter/traits/iterator.rs index 141942198c399..5204e2f609097 100644 --- a/src/libcore/iter/traits/iterator.rs +++ b/src/libcore/iter/traits/iterator.rs @@ -2991,10 +2991,4 @@ impl Iterator for &mut I { fn nth(&mut self, n: usize) -> Option { (**self).nth(n) } - #[inline] - fn try_fold(&mut self, init: B, mut f: F) -> R where - Self: Sized, F: FnMut(B, Self::Item) -> R, R: Try - { - (**self).try_fold(init, f) - } } From 6357ec442a5f85a902efff02296352718c18d3ab Mon Sep 17 00:00:00 2001 From: Nathan West Date: Thu, 10 Oct 2019 15:42:34 -0700 Subject: [PATCH 7/7] Added fusion and right.extend to partition. --- src/libcore/iter/traits/iterator.rs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/libcore/iter/traits/iterator.rs b/src/libcore/iter/traits/iterator.rs index 5204e2f609097..b41f19291ddf5 100644 --- a/src/libcore/iter/traits/iterator.rs +++ b/src/libcore/iter/traits/iterator.rs @@ -1504,7 +1504,7 @@ pub trait Iterator { /// assert_eq!(odd, vec![1, 3]); /// ``` #[stable(feature = "rust1", since = "1.0.0")] - fn partition(self, predicate: F) -> (B, B) + fn partition(self, mut predicate: F) -> (B, B) where Self: Sized, B: Default + Extend, @@ -1512,6 +1512,7 @@ pub trait Iterator { { let mut left: B = Default::default(); let mut right: B = Default::default(); + let mut fused = self.fuse(); #[inline] fn extend_rhs<'a, A, T: Extend, P: FnMut(&A) -> bool + 'a>( @@ -1528,7 +1529,14 @@ pub trait Iterator { } } - left.extend(self.filter_map(extend_rhs(&mut right, predicate))); + left.extend((&mut fused).filter_map(extend_rhs(&mut right, &mut predicate))); + + // left.extend may not have fully consumed self. + right.extend(fused.filter(move |item| !predicate(item))); + + // right.extend may not have fully consumed self, but in that case we + // assume that we don't care about the remaining elements of `self`, + // since both `left` and `right` finsished being extended (left, right) }