From 8bd9e0c57af75ca4ef95bd09b8f4ae70d6e49a14 Mon Sep 17 00:00:00 2001 From: C Date: Wed, 16 Dec 2020 01:34:11 +0000 Subject: [PATCH 1/5] refactor: moved new out of ZipImpl --- library/core/src/iter/adapters/zip.rs | 56 ++++++++++++++++++--------- 1 file changed, 38 insertions(+), 18 deletions(-) diff --git a/library/core/src/iter/adapters/zip.rs b/library/core/src/iter/adapters/zip.rs index 2f8f504d8fcaa..6c696336a2b91 100644 --- a/library/core/src/iter/adapters/zip.rs +++ b/library/core/src/iter/adapters/zip.rs @@ -20,8 +20,9 @@ pub struct Zip { } impl Zip { pub(in crate::iter) fn new(a: A, b: B) -> Zip { - ZipImpl::new(a, b) + ZipNew::new(a, b) } + fn super_nth(&mut self, mut n: usize) -> Option<(A::Item, B::Item)> { while let Some(x) = Iterator::next(self) { if n == 0 { @@ -61,7 +62,42 @@ where A: IntoIterator, B: IntoIterator, { - ZipImpl::new(a.into_iter(), b.into_iter()) + ZipNew::new(a.into_iter(), b.into_iter()) +} + +#[doc(hidden)] +trait ZipNew { + fn new(a: A, b: B) -> Self; +} + +#[doc(hidden)] +impl ZipNew for Zip +where + A: Iterator, + B: Iterator, +{ + default fn new(a: A, b: B) -> Self { + Zip { + a, + b, + index: 0, // unused + len: 0, // unused + a_len: 0, // unused + } + } +} + +#[doc(hidden)] +impl ZipNew for Zip +where + A: TrustedRandomAccess + Iterator, + B: TrustedRandomAccess + Iterator, +{ + fn new(a: A, b: B) -> Self { + let a_len = a.size(); + let len = cmp::min(a_len, b.size()); + Zip { a, b, index: 0, len, a_len } + } } #[stable(feature = "rust1", since = "1.0.0")] @@ -114,7 +150,6 @@ where #[doc(hidden)] trait ZipImpl { type Item; - fn new(a: A, b: B) -> Self; fn next(&mut self) -> Option; fn size_hint(&self) -> (usize, Option); fn nth(&mut self, n: usize) -> Option; @@ -136,15 +171,6 @@ where B: Iterator, { type Item = (A::Item, B::Item); - default fn new(a: A, b: B) -> Self { - Zip { - a, - b, - index: 0, // unused - len: 0, // unused - a_len: 0, // unused - } - } #[inline] default fn next(&mut self) -> Option<(A::Item, B::Item)> { @@ -216,12 +242,6 @@ where A: TrustedRandomAccess + Iterator, B: TrustedRandomAccess + Iterator, { - fn new(a: A, b: B) -> Self { - let a_len = a.size(); - let len = cmp::min(a_len, b.size()); - Zip { a, b, index: 0, len, a_len } - } - #[inline] fn next(&mut self) -> Option<(A::Item, B::Item)> { if self.index < self.len { From 55e6b0b693f65d0b537dd9c38daac7fccf19abe1 Mon Sep 17 00:00:00 2001 From: C Date: Wed, 16 Dec 2020 01:38:37 +0000 Subject: [PATCH 2/5] refactor: moved next_back out of ZipImpl --- library/core/src/iter/adapters/zip.rs | 144 ++++++++++++-------------- 1 file changed, 69 insertions(+), 75 deletions(-) diff --git a/library/core/src/iter/adapters/zip.rs b/library/core/src/iter/adapters/zip.rs index 6c696336a2b91..ee590d47df2db 100644 --- a/library/core/src/iter/adapters/zip.rs +++ b/library/core/src/iter/adapters/zip.rs @@ -141,8 +141,75 @@ where B: DoubleEndedIterator + ExactSizeIterator, { #[inline] - fn next_back(&mut self) -> Option<(A::Item, B::Item)> { - ZipImpl::next_back(self) + default fn next_back(&mut self) -> Option<(A::Item, B::Item)> { + let a_sz = self.a.len(); + let b_sz = self.b.len(); + if a_sz != b_sz { + // Adjust a, b to equal length + if a_sz > b_sz { + for _ in 0..a_sz - b_sz { + self.a.next_back(); + } + } else { + for _ in 0..b_sz - a_sz { + self.b.next_back(); + } + } + } + match (self.a.next_back(), self.b.next_back()) { + (Some(x), Some(y)) => Some((x, y)), + (None, None) => None, + _ => unreachable!(), + } + } +} + +#[stable(feature = "rust1", since = "1.0.0")] +impl DoubleEndedIterator for Zip +where + A: TrustedRandomAccess + DoubleEndedIterator + ExactSizeIterator, + B: TrustedRandomAccess + DoubleEndedIterator + ExactSizeIterator, +{ + #[inline] + fn next_back(&mut self) -> Option<(A::Item, B::Item)> + where + A: DoubleEndedIterator + ExactSizeIterator, + B: DoubleEndedIterator + ExactSizeIterator, + { + if A::MAY_HAVE_SIDE_EFFECT || B::MAY_HAVE_SIDE_EFFECT { + let sz_a = self.a.size(); + let sz_b = self.b.size(); + // Adjust a, b to equal length, make sure that only the first call + // of `next_back` does this, otherwise we will break the restriction + // on calls to `self.next_back()` after calling `get_unchecked()`. + if sz_a != sz_b { + let sz_a = self.a.size(); + if A::MAY_HAVE_SIDE_EFFECT && sz_a > self.len { + for _ in 0..sz_a - self.len { + self.a.next_back(); + } + self.a_len = self.len; + } + let sz_b = self.b.size(); + if B::MAY_HAVE_SIDE_EFFECT && sz_b > self.len { + for _ in 0..sz_b - self.len { + self.b.next_back(); + } + } + } + } + if self.index < self.len { + self.len -= 1; + self.a_len -= 1; + let i = self.len; + // SAFETY: `i` is smaller than the previous value of `self.len`, + // which is also smaller than or equal to `self.a.len()` and `self.b.len()` + unsafe { + Some((self.a.__iterator_get_unchecked(i), self.b.__iterator_get_unchecked(i))) + } + } else { + None + } } } @@ -153,10 +220,6 @@ trait ZipImpl { fn next(&mut self) -> Option; fn size_hint(&self) -> (usize, Option); fn nth(&mut self, n: usize) -> Option; - fn next_back(&mut self) -> Option - where - A: DoubleEndedIterator + ExactSizeIterator, - B: DoubleEndedIterator + ExactSizeIterator; // This has the same safety requirements as `Iterator::__iterator_get_unchecked` unsafe fn get_unchecked(&mut self, idx: usize) -> ::Item where @@ -184,33 +247,6 @@ where self.super_nth(n) } - #[inline] - default fn next_back(&mut self) -> Option<(A::Item, B::Item)> - where - A: DoubleEndedIterator + ExactSizeIterator, - B: DoubleEndedIterator + ExactSizeIterator, - { - let a_sz = self.a.len(); - let b_sz = self.b.len(); - if a_sz != b_sz { - // Adjust a, b to equal length - if a_sz > b_sz { - for _ in 0..a_sz - b_sz { - self.a.next_back(); - } - } else { - for _ in 0..b_sz - a_sz { - self.b.next_back(); - } - } - } - match (self.a.next_back(), self.b.next_back()) { - (Some(x), Some(y)) => Some((x, y)), - (None, None) => None, - _ => unreachable!(), - } - } - #[inline] default fn size_hint(&self) -> (usize, Option) { let (a_lower, a_upper) = self.a.size_hint(); @@ -298,48 +334,6 @@ where self.super_nth(n - delta) } - #[inline] - fn next_back(&mut self) -> Option<(A::Item, B::Item)> - where - A: DoubleEndedIterator + ExactSizeIterator, - B: DoubleEndedIterator + ExactSizeIterator, - { - if A::MAY_HAVE_SIDE_EFFECT || B::MAY_HAVE_SIDE_EFFECT { - let sz_a = self.a.size(); - let sz_b = self.b.size(); - // Adjust a, b to equal length, make sure that only the first call - // of `next_back` does this, otherwise we will break the restriction - // on calls to `self.next_back()` after calling `get_unchecked()`. - if sz_a != sz_b { - let sz_a = self.a.size(); - if A::MAY_HAVE_SIDE_EFFECT && sz_a > self.len { - for _ in 0..sz_a - self.len { - self.a.next_back(); - } - self.a_len = self.len; - } - let sz_b = self.b.size(); - if B::MAY_HAVE_SIDE_EFFECT && sz_b > self.len { - for _ in 0..sz_b - self.len { - self.b.next_back(); - } - } - } - } - if self.index < self.len { - self.len -= 1; - self.a_len -= 1; - let i = self.len; - // SAFETY: `i` is smaller than the previous value of `self.len`, - // which is also smaller than or equal to `self.a.len()` and `self.b.len()` - unsafe { - Some((self.a.__iterator_get_unchecked(i), self.b.__iterator_get_unchecked(i))) - } - } else { - None - } - } - #[inline] unsafe fn get_unchecked(&mut self, idx: usize) -> ::Item { let idx = self.index + idx; From 69a5072fd37bfa35336cdb0973fe3ee950f9ce0c Mon Sep 17 00:00:00 2001 From: C Date: Wed, 16 Dec 2020 02:03:20 +0000 Subject: [PATCH 3/5] refactor: removed ZipImpl --- library/core/src/iter/adapters/zip.rs | 67 +++++---------------------- 1 file changed, 12 insertions(+), 55 deletions(-) diff --git a/library/core/src/iter/adapters/zip.rs b/library/core/src/iter/adapters/zip.rs index ee590d47df2db..a1070ac917995 100644 --- a/library/core/src/iter/adapters/zip.rs +++ b/library/core/src/iter/adapters/zip.rs @@ -18,7 +18,12 @@ pub struct Zip { len: usize, a_len: usize, } -impl Zip { + +impl Zip +where + A: Iterator, + B: Iterator, +{ pub(in crate::iter) fn new(a: A, b: B) -> Zip { ZipNew::new(a, b) } @@ -100,40 +105,6 @@ where } } -#[stable(feature = "rust1", since = "1.0.0")] -impl Iterator for Zip -where - A: Iterator, - B: Iterator, -{ - type Item = (A::Item, B::Item); - - #[inline] - fn next(&mut self) -> Option { - ZipImpl::next(self) - } - - #[inline] - fn size_hint(&self) -> (usize, Option) { - ZipImpl::size_hint(self) - } - - #[inline] - fn nth(&mut self, n: usize) -> Option { - ZipImpl::nth(self, n) - } - - #[inline] - unsafe fn __iterator_get_unchecked(&mut self, idx: usize) -> Self::Item - where - Self: TrustedRandomAccess, - { - // SAFETY: `ZipImpl::__iterator_get_unchecked` has same safety - // requirements as `Iterator::__iterator_get_unchecked`. - unsafe { ZipImpl::get_unchecked(self, idx) } - } -} - #[stable(feature = "rust1", since = "1.0.0")] impl DoubleEndedIterator for Zip where @@ -213,22 +184,8 @@ where } } -// Zip specialization trait -#[doc(hidden)] -trait ZipImpl { - type Item; - fn next(&mut self) -> Option; - fn size_hint(&self) -> (usize, Option); - fn nth(&mut self, n: usize) -> Option; - // This has the same safety requirements as `Iterator::__iterator_get_unchecked` - unsafe fn get_unchecked(&mut self, idx: usize) -> ::Item - where - Self: Iterator + TrustedRandomAccess; -} - -// General Zip impl -#[doc(hidden)] -impl ZipImpl for Zip +#[stable(feature = "rust1", since = "1.0.0")] +impl Iterator for Zip where A: Iterator, B: Iterator, @@ -264,7 +221,7 @@ where (lower, upper) } - default unsafe fn get_unchecked(&mut self, _idx: usize) -> ::Item + default unsafe fn __iterator_get_unchecked(&mut self, _idx: usize) -> ::Item where Self: TrustedRandomAccess, { @@ -272,8 +229,8 @@ where } } -#[doc(hidden)] -impl ZipImpl for Zip +#[stable(feature = "rust1", since = "1.0.0")] +impl Iterator for Zip where A: TrustedRandomAccess + Iterator, B: TrustedRandomAccess + Iterator, @@ -335,7 +292,7 @@ where } #[inline] - unsafe fn get_unchecked(&mut self, idx: usize) -> ::Item { + unsafe fn __iterator_get_unchecked(&mut self, idx: usize) -> ::Item { let idx = self.index + idx; // SAFETY: the caller must uphold the contract for // `Iterator::__iterator_get_unchecked`. From f6405ad647c5b2e4412b5dc1a27688f8582116d2 Mon Sep 17 00:00:00 2001 From: C Date: Wed, 16 Dec 2020 02:07:13 +0000 Subject: [PATCH 4/5] refactor: updated zip nth() to iterator nth() When the iterator nth() method was updated it was not in zip. Zip needs to implement nth() for the trusted length specialised implementation. --- library/core/src/iter/adapters/zip.rs | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/library/core/src/iter/adapters/zip.rs b/library/core/src/iter/adapters/zip.rs index a1070ac917995..5be0cafd11123 100644 --- a/library/core/src/iter/adapters/zip.rs +++ b/library/core/src/iter/adapters/zip.rs @@ -28,14 +28,9 @@ where ZipNew::new(a, b) } - fn super_nth(&mut self, mut n: usize) -> Option<(A::Item, B::Item)> { - while let Some(x) = Iterator::next(self) { - if n == 0 { - return Some(x); - } - n -= 1; - } - None + fn super_nth(&mut self, n: usize) -> Option<(A::Item, B::Item)> { + self.advance_by(n).ok()?; + self.next() } } From 4adf61575a377c56404dd145c1515c315436c8c8 Mon Sep 17 00:00:00 2001 From: DeveloperC Date: Fri, 19 Mar 2021 19:57:37 +0000 Subject: [PATCH 5/5] refactor: removing redundant variables --- library/core/src/iter/adapters/zip.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/library/core/src/iter/adapters/zip.rs b/library/core/src/iter/adapters/zip.rs index 5be0cafd11123..c720ed7daa53e 100644 --- a/library/core/src/iter/adapters/zip.rs +++ b/library/core/src/iter/adapters/zip.rs @@ -149,14 +149,13 @@ where // of `next_back` does this, otherwise we will break the restriction // on calls to `self.next_back()` after calling `get_unchecked()`. if sz_a != sz_b { - let sz_a = self.a.size(); if A::MAY_HAVE_SIDE_EFFECT && sz_a > self.len { for _ in 0..sz_a - self.len { self.a.next_back(); } self.a_len = self.len; } - let sz_b = self.b.size(); + if B::MAY_HAVE_SIDE_EFFECT && sz_b > self.len { for _ in 0..sz_b - self.len { self.b.next_back();