Skip to content

[experiment] break as many Zip side-effects as we can #83796

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

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 0 additions & 47 deletions library/core/src/iter/adapters/zip.rs
Original file line number Diff line number Diff line change
Expand Up @@ -231,16 +231,6 @@ where
unsafe {
Some((self.a.__iterator_get_unchecked(i), self.b.__iterator_get_unchecked(i)))
}
} else if A::MAY_HAVE_SIDE_EFFECT && self.index < self.a_len {
let i = self.index;
self.index += 1;
self.len += 1;
// match the base implementation's potential side effects
// SAFETY: we just checked that `i` < `self.a.len()`
unsafe {
self.a.__iterator_get_unchecked(i);
}
None
} else {
None
}
Expand All @@ -257,22 +247,7 @@ where
let delta = cmp::min(n, self.len - self.index);
let end = self.index + delta;
while self.index < end {
let i = self.index;
self.index += 1;
if A::MAY_HAVE_SIDE_EFFECT {
// SAFETY: the usage of `cmp::min` to calculate `delta`
// ensures that `end` is smaller than or equal to `self.len`,
// so `i` is also smaller than `self.len`.
unsafe {
self.a.__iterator_get_unchecked(i);
}
}
if B::MAY_HAVE_SIDE_EFFECT {
// SAFETY: same as above.
unsafe {
self.b.__iterator_get_unchecked(i);
}
}
}

self.super_nth(n - delta)
Expand All @@ -284,28 +259,6 @@ 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;
Expand Down
77 changes: 71 additions & 6 deletions library/core/src/iter/traits/double_ended.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use super::super::{TrustedLen, TrustedRandomAccess};
use crate::ops::{ControlFlow, Try};

/// An iterator able to yield elements from both ends.
Expand Down Expand Up @@ -278,16 +279,12 @@ pub trait DoubleEndedIterator: Iterator {
/// ```
#[inline]
#[stable(feature = "iter_rfold", since = "1.27.0")]
fn rfold<B, F>(mut self, init: B, mut f: F) -> B
fn rfold<B, F>(self, init: B, f: F) -> B
where
Self: Sized,
F: FnMut(B, Self::Item) -> B,
{
let mut accum = init;
while let Some(x) = self.next_back() {
accum = f(accum, x);
}
accum
self.rfold_spec(init, f)
}

/// Searches for an element of an iterator from the back that satisfies a predicate.
Expand Down Expand Up @@ -361,3 +358,71 @@ impl<'a, I: DoubleEndedIterator + ?Sized> DoubleEndedIterator for &'a mut I {
(**self).nth_back(n)
}
}

trait SpecIteratorDefaultImpls: DoubleEndedIterator {
fn rfold_spec<B, F>(self, init: B, f: F) -> B
where
Self: Sized,
F: FnMut(B, Self::Item) -> B;
}

impl<T> SpecIteratorDefaultImpls for T
where
T: DoubleEndedIterator,
{
#[inline]
default fn rfold_spec<B, F>(mut self, init: B, mut f: F) -> B
where
Self: Sized,
F: FnMut(B, Self::Item) -> B,
{
let mut accum = init;
while let Some(x) = self.next_back() {
accum = f(accum, x);
}
accum
}
}

impl<T> SpecIteratorDefaultImpls for T
where
T: DoubleEndedIterator + Sized + TrustedLen,
{
#[inline]
default fn rfold_spec<B, F>(mut self, init: B, mut f: F) -> B
where
Self: Sized,
F: FnMut(B, Self::Item) -> B,
{
let mut accum = init;
while self.size_hint().1.is_none() {
accum = f(accum, self.next_back().unwrap())
}
for _ in 0..self.size_hint().1.unwrap() {
accum = f(accum, self.next_back().unwrap());
}
accum
}
}

impl<T> SpecIteratorDefaultImpls for T
where
T: DoubleEndedIterator + Sized + TrustedLen + TrustedRandomAccess,
{
#[inline]
fn rfold_spec<B, F>(mut self, init: B, mut f: F) -> B
where
Self: Sized,
F: FnMut(B, Self::Item) -> B,
{
let mut accum = init;
// SAFETY: every element is only read once, as required by the
// TrustedRandomAccess contract
unsafe {
for i in (0..self.size()).rev() {
accum = f(accum, self.__iterator_get_unchecked(i))
}
}
accum
}
}
78 changes: 71 additions & 7 deletions library/core/src/iter/traits/iterator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,13 @@
use crate::cmp::{self, Ordering};
use crate::ops::{ControlFlow, Try};

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

fn _assert_is_object_safe(_: &dyn Iterator<Item = ()>) {}

Expand Down Expand Up @@ -2125,16 +2125,12 @@ pub trait Iterator {
#[doc(alias = "inject")]
#[inline]
#[stable(feature = "rust1", since = "1.0.0")]
fn fold<B, F>(mut self, init: B, mut f: F) -> B
fn fold<B, F>(self, init: B, f: F) -> B
where
Self: Sized,
F: FnMut(B, Self::Item) -> B,
{
let mut accum = init;
while let Some(x) = self.next() {
accum = f(accum, x);
}
accum
self.fold_spec(init, f)
}

/// Reduces the elements to a single one, by repeatedly applying a reducing
Expand Down Expand Up @@ -3420,3 +3416,71 @@ impl<I: Iterator + ?Sized> Iterator for &mut I {
(**self).nth(n)
}
}

trait SpecIteratorDefaultImpls: Iterator {
fn fold_spec<B, F>(self, init: B, f: F) -> B
where
Self: Sized,
F: FnMut(B, Self::Item) -> B;
}

impl<T> SpecIteratorDefaultImpls for T
where
T: Iterator,
{
#[inline]
default fn fold_spec<B, F>(mut self, init: B, mut f: F) -> B
where
Self: Sized,
F: FnMut(B, Self::Item) -> B,
{
let mut accum = init;
while let Some(x) = self.next() {
accum = f(accum, x);
}
accum
}
}

impl<T> SpecIteratorDefaultImpls for T
where
T: Iterator + Sized + TrustedLen,
{
#[inline]
default fn fold_spec<B, F>(mut self, init: B, mut f: F) -> B
where
Self: Sized,
F: FnMut(B, Self::Item) -> B,
{
let mut accum = init;
while self.size_hint().1.is_none() {
accum = f(accum, self.next().unwrap());
}
for _ in 0..self.size_hint().1.unwrap() {
accum = f(accum, self.next().unwrap())
}
accum
}
}

impl<T> SpecIteratorDefaultImpls for T
where
T: Iterator + Sized + TrustedLen + TrustedRandomAccess,
{
#[inline]
fn fold_spec<B, F>(mut self, init: B, mut f: F) -> B
where
Self: Sized,
F: FnMut(B, Self::Item) -> B,
{
let mut accum = init;
// SAFETY: every element is only read once, as required by the
// TrustedRandomAccess contract
unsafe {
for i in 0..self.size() {
accum = f(accum, self.__iterator_get_unchecked(i))
}
}
accum
}
}
17 changes: 0 additions & 17 deletions library/core/tests/iter/adapters/cloned.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,23 +17,6 @@ fn test_cloned() {
assert_eq!(it.next_back(), None);
}

#[test]
fn test_cloned_side_effects() {
let mut count = 0;
{
let iter = [1, 2, 3]
.iter()
.map(|x| {
count += 1;
x
})
.cloned()
.zip(&[1]);
for _ in iter {}
}
assert_eq!(count, 2);
}

#[test]
fn test_cloned_try_folds() {
let a = [1, 2, 3, 4, 5, 6, 7, 8, 9];
Expand Down
26 changes: 0 additions & 26 deletions library/core/tests/iter/adapters/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@ mod take;
mod take_while;
mod zip;

use core::cell::Cell;

/// An iterator that panics whenever `next` or next_back` is called
/// after `None` has already been returned. This does not violate
/// `Iterator`'s contract. Used to test that iterator adaptors don't
Expand Down Expand Up @@ -159,27 +157,3 @@ impl<'a, T> Iterator for CycleIter<'a, T> {
elt
}
}

#[derive(Debug)]
struct CountClone(Cell<i32>);

impl CountClone {
pub fn new() -> Self {
Self(Cell::new(0))
}
}

impl PartialEq<i32> for CountClone {
fn eq(&self, rhs: &i32) -> bool {
self.0.get() == *rhs
}
}

impl Clone for CountClone {
fn clone(&self) -> Self {
let ret = CountClone(self.0.clone());
let n = self.0.get();
self.0.set(n + 1);
ret
}
}
Loading