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

Override Iterator::advance(_back)_by for array::IntoIter #91512

Merged
merged 1 commit into from
Dec 8, 2021
Merged
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
44 changes: 43 additions & 1 deletion library/core/src/array/iter.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
//! Defines the `IntoIter` owned iterator for arrays.

use crate::{
fmt,
cmp, fmt,
iter::{self, ExactSizeIterator, FusedIterator, TrustedLen},
mem::{self, MaybeUninit},
ops::Range,
Expand Down Expand Up @@ -150,6 +150,27 @@ impl<T, const N: usize> Iterator for IntoIter<T, N> {
fn last(mut self) -> Option<Self::Item> {
self.next_back()
}

fn advance_by(&mut self, n: usize) -> Result<(), usize> {
let len = self.len();

// The number of elements to drop. Always in-bounds by construction.
let delta = cmp::min(n, len);

let range_to_drop = self.alive.start..(self.alive.start + delta);

// Moving the start marks them as conceptually "dropped", so if anything
// goes bad then our drop impl won't double-free them.
self.alive.start += delta;

// SAFETY: These elements are currently initialized, so it's fine to drop them.
unsafe {
let slice = self.data.get_unchecked_mut(range_to_drop);
ptr::drop_in_place(MaybeUninit::slice_assume_init_mut(slice));
}
Copy link
Member

@the8472 the8472 Dec 4, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tangent: These dances seem a bit unergonomic and repeated in several places.

ptr::drop_in_place can take a *mut [T] (fat pointer), so for unsafe code it would be great if we had something like [T].as_slice_ptr().get_unchecked(range).cast() then we wouldn't have to refer to use those unwieldy associated methods on MaybeUninit since we're operating on pointers anyway. Generally there's a lack of helpers to work with *mut/const [T].

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, there's assume_init_drop but no slice version of that, so I guess there could be assume_init_drop_range like there's slice_assume_init_mut and such -- one could always pass .. to drop everything easily.

Don't think I'll do that in this PR, though.


if n > len { Err(len) } else { Ok(()) }
}
}

#[stable(feature = "array_value_iter_impls", since = "1.40.0")]
Expand All @@ -170,6 +191,27 @@ impl<T, const N: usize> DoubleEndedIterator for IntoIter<T, N> {
unsafe { self.data.get_unchecked(idx).assume_init_read() }
})
}

fn advance_back_by(&mut self, n: usize) -> Result<(), usize> {
let len = self.len();

// The number of elements to drop. Always in-bounds by construction.
let delta = cmp::min(n, len);

let range_to_drop = (self.alive.end - delta)..self.alive.end;

// Moving the end marks them as conceptually "dropped", so if anything
// goes bad then our drop impl won't double-free them.
self.alive.end -= delta;

// SAFETY: These elements are currently initialized, so it's fine to drop them.
unsafe {
let slice = self.data.get_unchecked_mut(range_to_drop);
ptr::drop_in_place(MaybeUninit::slice_assume_init_mut(slice));
}

if n > len { Err(len) } else { Ok(()) }
}
}

#[stable(feature = "array_value_iter_impls", since = "1.40.0")]
Expand Down
106 changes: 106 additions & 0 deletions library/core/tests/array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -474,3 +474,109 @@ fn array_split_array_mut_out_of_bounds() {

v.split_array_mut::<7>();
}

#[test]
fn array_intoiter_advance_by() {
use std::cell::Cell;
struct DropCounter<'a>(usize, &'a Cell<usize>);
impl Drop for DropCounter<'_> {
fn drop(&mut self) {
let x = self.1.get();
self.1.set(x + 1);
}
}

let counter = Cell::new(0);
let a: [_; 100] = std::array::from_fn(|i| DropCounter(i, &counter));
let mut it = IntoIterator::into_iter(a);

let r = it.advance_by(1);
assert_eq!(r, Ok(()));
assert_eq!(it.len(), 99);
assert_eq!(counter.get(), 1);

let r = it.advance_by(0);
assert_eq!(r, Ok(()));
assert_eq!(it.len(), 99);
assert_eq!(counter.get(), 1);

let r = it.advance_by(11);
assert_eq!(r, Ok(()));
assert_eq!(it.len(), 88);
assert_eq!(counter.get(), 12);

let x = it.next();
assert_eq!(x.as_ref().map(|x| x.0), Some(12));
assert_eq!(it.len(), 87);
assert_eq!(counter.get(), 12);
drop(x);
assert_eq!(counter.get(), 13);

let r = it.advance_by(123456);
assert_eq!(r, Err(87));
assert_eq!(it.len(), 0);
assert_eq!(counter.get(), 100);

let r = it.advance_by(0);
assert_eq!(r, Ok(()));
assert_eq!(it.len(), 0);
assert_eq!(counter.get(), 100);

let r = it.advance_by(10);
assert_eq!(r, Err(0));
assert_eq!(it.len(), 0);
assert_eq!(counter.get(), 100);
}

#[test]
fn array_intoiter_advance_back_by() {
use std::cell::Cell;
struct DropCounter<'a>(usize, &'a Cell<usize>);
impl Drop for DropCounter<'_> {
fn drop(&mut self) {
let x = self.1.get();
self.1.set(x + 1);
}
}

let counter = Cell::new(0);
let a: [_; 100] = std::array::from_fn(|i| DropCounter(i, &counter));
let mut it = IntoIterator::into_iter(a);

let r = it.advance_back_by(1);
assert_eq!(r, Ok(()));
assert_eq!(it.len(), 99);
assert_eq!(counter.get(), 1);

let r = it.advance_back_by(0);
assert_eq!(r, Ok(()));
assert_eq!(it.len(), 99);
assert_eq!(counter.get(), 1);

let r = it.advance_back_by(11);
assert_eq!(r, Ok(()));
assert_eq!(it.len(), 88);
assert_eq!(counter.get(), 12);

let x = it.next_back();
assert_eq!(x.as_ref().map(|x| x.0), Some(87));
assert_eq!(it.len(), 87);
assert_eq!(counter.get(), 12);
drop(x);
assert_eq!(counter.get(), 13);

let r = it.advance_back_by(123456);
assert_eq!(r, Err(87));
assert_eq!(it.len(), 0);
assert_eq!(counter.get(), 100);

let r = it.advance_back_by(0);
assert_eq!(r, Ok(()));
assert_eq!(it.len(), 0);
assert_eq!(counter.get(), 100);

let r = it.advance_back_by(10);
assert_eq!(r, Err(0));
assert_eq!(it.len(), 0);
assert_eq!(counter.get(), 100);
}