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

impl<T> IntoIterator for [T; $N] #32871

Closed
wants to merge 2 commits 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
129 changes: 125 additions & 4 deletions src/libcore/array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,16 @@

use borrow::{Borrow, BorrowMut};
use clone::Clone;
use cmp::{PartialEq, Eq, PartialOrd, Ord, Ordering};
use cmp::{PartialEq, Eq, PartialOrd, Ord, Ordering, self};
use convert::{AsRef, AsMut};
use default::Default;
use fmt;
use hash::{Hash, self};
use iter::IntoIterator;
use marker::{Copy, Sized, Unsize};
use option::Option;
use iter::{IntoIterator, Iterator, DoubleEndedIterator, ExactSizeIterator};
use marker::{Copy, Sized, Unsize, PhantomData};
use ops::Drop;
use option::Option::{Some, None, self};
use ptr;
use slice::{Iter, IterMut, SliceExt};

/// Utility trait implemented only on arrays of fixed size
Expand Down Expand Up @@ -62,6 +64,111 @@ unsafe impl<T, A: Unsize<[T]>> FixedSizeArray<T> for A {
}
}

/// An iterator that moves out of an array.
#[derive(Debug)]
pub struct IntoIter<T, A: FixedSizeArray<T>> {
// Invariants: index <= index_back <= array.len()
// Only values in array[index..index_back] are alive at any given time.
// Values from array[..index] and array[index_back..] are already moved/dropped.
array: Option<A>,
index: usize,
index_back: usize,
_marker: PhantomData<T>,
}

impl<T, A: FixedSizeArray<T>> Drop for IntoIter<T, A> {
#[inline]
fn drop(&mut self) {
// Drop values that are still alive.
if let Some(array) = self.array.as_mut() {
let slice = array.as_mut_slice();
for p in &mut slice[self.index..self.index_back] {
unsafe { ptr::drop_in_place(p); }
}
}

// Prevent the array as a whole from dropping.
unsafe { ptr::write(&mut self.array, None); }
Copy link
Member Author

Choose a reason for hiding this comment

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

This actually has the same hazard you raised for nth -- if there's a panic in one of the drop_in_place calls, we won't get to clobber self.array. And AFAICT a panic in a drop function will still try to drop the struct members, or even locals if I were to take() the array out early.

I think this will need drop-prevention to be an additional wrapper on the array, to be unwound on its own.

Copy link
Member

Choose a reason for hiding this comment

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

Arrayvec has experienced this hazard too, and it uses two drop flags for this reason (nested types).

}
}

impl<T, A: FixedSizeArray<T>> Iterator for IntoIter<T, A> {
type Item = T;

#[inline]
fn next(&mut self) -> Option<T> {
if self.len() > 0 {
if let Some(array) = self.array.as_ref() {
Copy link
Member

Choose a reason for hiding this comment

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

It seems to me that self.array should never be None here (i.e. it is a bug for it to be None), so maybe unwrap is more appropriate?

Copy link
Member Author

Choose a reason for hiding this comment

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

Right, it should never be None. I suppose unwrap expresses that better, but it will introduce a panic into codegen, whereas this currently would act like just the end of iteration. I don't mind changing this though.

let slice = array.as_slice();
let p = unsafe { slice.get_unchecked(self.index) };
self.index += 1;
return Some(unsafe { ptr::read(p) })
}
}
None
}

#[inline]
fn size_hint(&self) -> (usize, Option<usize>) {
let len = self.len();
(len, Some(len))
}

#[inline]
fn count(self) -> usize {
self.len()
}

#[inline]
fn nth(&mut self, n: usize) -> Option<T> {
let len = self.len();
if len > 0 {
// Drop values prior to the nth.
if let Some(array) = self.array.as_mut() {
let ndrop = cmp::min(n, len);
let slice = array.as_mut_slice();
for p in &mut slice[self.index..self.index + ndrop] {
unsafe { ptr::drop_in_place(p); }
Copy link
Member

Choose a reason for hiding this comment

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

What happens if one of these destructors panics? self.index probably needs to be updated either before the loop ("leak amplification") or inside the loop.

Copy link
Member Author

Choose a reason for hiding this comment

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

That's a good point! I think I prefer inside the loop, just to leak as little as possible, but I could go either way.

}
self.index += ndrop;
}
}
self.next()
}

#[inline]
fn last(mut self) -> Option<T> {
let len = self.len();
if len > 0 {
self.nth(len - 1)
} else {
None
}
}
}

impl<T, A: FixedSizeArray<T>> DoubleEndedIterator for IntoIter<T, A> {
#[inline]
fn next_back(&mut self) -> Option<T> {
if self.len() > 0 {
if let Some(array) = self.array.as_mut() {
self.index_back -= 1;
let slice = array.as_slice();
let p = unsafe { slice.get_unchecked(self.index_back) };
return Some(unsafe { ptr::read(p) })
}
}
None
}
}

impl<T, A: FixedSizeArray<T>> ExactSizeIterator for IntoIter<T, A> {
#[inline]
fn len(&self) -> usize {
self.index_back - self.index
}
}

macro_rules! __impl_slice_eq1 {
($Lhs: ty, $Rhs: ty) => {
__impl_slice_eq1! { $Lhs, $Rhs, Sized }
Expand Down Expand Up @@ -167,6 +274,20 @@ macro_rules! array_impls {
}
}

impl<T> IntoIterator for [T; $N] {
type Item = T;
type IntoIter = IntoIter<T, Self>;

fn into_iter(self) -> IntoIter<T, Self> {
IntoIter {
array: Some(self),
index: 0,
index_back: $N,
_marker: PhantomData,
}
}
}

// NOTE: some less important impls are omitted to reduce code bloat
__impl_slice_eq1! { [A; $N], [B; $N] }
__impl_slice_eq2! { [A; $N], [B] }
Expand Down
99 changes: 99 additions & 0 deletions src/libcoretest/array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,102 @@ fn fixed_size_array() {
assert_eq!(FixedSizeArray::as_mut_slice(&mut empty_array).len(), 0);
assert_eq!(FixedSizeArray::as_mut_slice(&mut empty_zero_sized).len(), 0);
}

#[test]
fn test_iterator_nth() {
let v = [0, 1, 2, 3, 4];
for i in 0..v.len() {
assert_eq!(v.clone().into_iter().nth(i).unwrap(), v[i]);
}
assert_eq!(v.clone().into_iter().nth(v.len()), None);

let mut iter = v.into_iter();
assert_eq!(iter.nth(2).unwrap(), v[2]);
assert_eq!(iter.nth(1).unwrap(), v[4]);
}

#[test]
fn test_iterator_last() {
let v = [0, 1, 2, 3, 4];
assert_eq!(v.into_iter().last().unwrap(), 4);
assert_eq!([0].into_iter().last().unwrap(), 0);
}

#[test]
fn test_iterator_count() {
let v = [0, 1, 2, 3, 4];
assert_eq!(v.clone().into_iter().count(), 5);

let mut iter2 = v.into_iter();
iter2.next();
iter2.next();
assert_eq!(iter2.count(), 3);
}

#[test]
fn test_iterator_flat_map() {
assert!((0..5).flat_map(|i| [2 * i, 2 * i + 1]).eq(0..10));
}

#[test]
fn test_iterator_drops() {
use core::cell::Cell;

struct R<'a> {
i: &'a Cell<usize>,
}

impl<'a> Drop for R<'a> {
fn drop(&mut self) {
self.i.set(self.i.get() + 1);
}
}

fn r(i: &Cell<usize>) -> R {
R {
i: i
}
}

fn v(i: &Cell<usize>) -> [R; 5] {
[r(i), r(i), r(i), r(i), r(i)]
}

let i = Cell::new(0);
{
v(&i).into_iter();
}
assert_eq!(i.get(), 5);

let i = Cell::new(0);
{
let mut iter = v(&i).into_iter();
let _x = iter.next();
assert_eq!(i.get(), 0);
assert_eq!(iter.count(), 4);
assert_eq!(i.get(), 4);
}
assert_eq!(i.get(), 5);

let i = Cell::new(0);
{
let mut iter = v(&i).into_iter();
let _x = iter.nth(2);
assert_eq!(i.get(), 2);
let _y = iter.last();
assert_eq!(i.get(), 3);
}
assert_eq!(i.get(), 5);

let i = Cell::new(0);
for (index, _x) in v(&i).into_iter().enumerate() {
assert_eq!(i.get(), index);
}
assert_eq!(i.get(), 5);

let i = Cell::new(0);
for (index, _x) in v(&i).into_iter().rev().enumerate() {
assert_eq!(i.get(), index);
}
assert_eq!(i.get(), 5);
}
2 changes: 1 addition & 1 deletion src/librustdoc/html/render.rs
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,7 @@ fn init_ids() -> HashMap<String, usize> {
"deref-methods",
"implementations",
"derived_implementations"
].into_iter().map(|id| (String::from(*id), 1)).collect()
].into_iter().map(|id| (String::from(id), 1)).collect()
}

/// This method resets the local table of used ID attributes. This is typically
Expand Down
2 changes: 1 addition & 1 deletion src/libstd/primitive_docs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ mod prim_pointer { }
///
/// - `Clone` (only if `T: Copy`)
/// - `Debug`
/// - `IntoIterator` (implemented for `&[T; N]` and `&mut [T; N]`)
/// - `IntoIterator` (implemented for `[T; N]`, `&[T; N]`, and `&mut [T; N]`)
/// - `PartialEq`, `PartialOrd`, `Ord`, `Eq`
/// - `Hash`
/// - `AsRef`, `AsMut`
Expand Down