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

Implement Vec::drain from RFC 574 #24781

Merged
merged 2 commits into from
Apr 29, 2015
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
2 changes: 1 addition & 1 deletion src/libcollections/binary_heap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -546,7 +546,7 @@ impl<T: Ord> BinaryHeap<T> {
#[unstable(feature = "collections",
reason = "matches collection reform specification, waiting for dust to settle")]
pub fn drain(&mut self) -> Drain<T> {
Drain { iter: self.data.drain() }
Drain { iter: self.data.drain(..) }
}

/// Drops all items from the binary heap.
Expand Down
4 changes: 3 additions & 1 deletion src/libcollections/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@
#![feature(slice_patterns)]
#![feature(debug_builders)]
#![feature(utf8_error)]
#![cfg_attr(test, feature(rand, rustc_private, test, hash, collections))]
#![cfg_attr(test, feature(rand, rustc_private, test, hash, collections,
collections_drain, collections_range))]
#![cfg_attr(test, allow(deprecated))] // rand

#![feature(no_std)]
Expand Down Expand Up @@ -82,6 +83,7 @@ pub mod borrow;
pub mod enum_set;
pub mod fmt;
pub mod linked_list;
pub mod range;
pub mod slice;
pub mod str;
pub mod string;
Expand Down
45 changes: 45 additions & 0 deletions src/libcollections/range.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.
#![unstable(feature = "collections_range", reason = "was just added")]

//! Range syntax.

use core::option::Option::{self, None, Some};
use core::ops::{RangeFull, Range, RangeTo, RangeFrom};

/// **RangeArgument** is implemented by Rust's built-in range types, produced
/// by range syntax like `..`, `a..`, `..b` or `c..d`.
pub trait RangeArgument<T> {
/// Start index (inclusive)
///
/// Return start value if present, else `None`.
fn start(&self) -> Option<&T> { None }

/// End index (exclusive)
///
/// Return end value if present, else `None`.
fn end(&self) -> Option<&T> { None }
}


impl<T> RangeArgument<T> for RangeFull {}

impl<T> RangeArgument<T> for RangeFrom<T> {
fn start(&self) -> Option<&T> { Some(&self.start) }
}

impl<T> RangeArgument<T> for RangeTo<T> {
fn end(&self) -> Option<&T> { Some(&self.end) }
}

impl<T> RangeArgument<T> for Range<T> {
fn start(&self) -> Option<&T> { Some(&self.start) }
fn end(&self) -> Option<&T> { Some(&self.end) }
}
165 changes: 87 additions & 78 deletions src/libcollections/vec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@ use core::usize;

use borrow::{Cow, IntoCow};

use super::range::RangeArgument;

// FIXME- fix places which assume the max vector allowed has memory usize::MAX.
static MAX_MEMORY_SIZE: usize = isize::MAX as usize;

Expand Down Expand Up @@ -718,36 +720,61 @@ impl<T> Vec<T> {
unsafe { other.set_len(0); }
}

/// Creates a draining iterator that clears the `Vec` and iterates over
/// the removed items from start to end.
/// Create a draining iterator that removes the specified range in the vector
/// and yields the removed items from start to end. The element range is
/// removed even if the iterator is not consumed until the end.
///
/// Note: It is unspecified how many elements are removed from the vector,
/// if the `Drain` value is leaked.
///
/// # Panics
///
/// Panics if the starting point is greater than the end point or if
/// the end point is greater than the length of the vector.
///
/// # Examples
///
/// ```
/// # #![feature(collections)]
/// let mut v = vec!["a".to_string(), "b".to_string()];
/// for s in v.drain() {
/// // s has type String, not &String
/// println!("{}", s);
/// }
/// assert!(v.is_empty());
/// # #![feature(collections_drain, collections_range)]
///
/// // Draining using `..` clears the whole vector.
/// let mut v = vec![1, 2, 3];
/// let u: Vec<_> = v.drain(..).collect();
/// assert_eq!(v, &[]);
/// assert_eq!(u, &[1, 2, 3]);
/// ```
#[inline]
#[unstable(feature = "collections",
reason = "matches collection reform specification, waiting for dust to settle")]
pub fn drain(&mut self) -> Drain<T> {
#[unstable(feature = "collections_drain",
reason = "recently added, matches RFC")]
pub fn drain<R>(&mut self, range: R) -> Drain<T> where R: RangeArgument<usize> {
// Memory safety
//
// When the Drain is first created, it shortens the length of
// the source vector to make sure no uninitalized or moved-from elements
// are accessible at all if the Drain's destructor never gets to run.
//
// Drain will ptr::read out the values to remove.
// When finished, remaining tail of the vec is copied back to cover
// the hole, and the vector length is restored to the new length.
//
let len = self.len();
let start = *range.start().unwrap_or(&0);
let end = *range.end().unwrap_or(&len);
assert!(start <= end);
assert!(end <= len);

unsafe {
let begin = *self.ptr as *const T;
let end = if mem::size_of::<T>() == 0 {
(*self.ptr as usize + self.len()) as *const T
} else {
(*self.ptr).offset(self.len() as isize) as *const T
};
self.set_len(0);
// set self.vec length's to start, to be safe in case Drain is leaked
self.set_len(start);
Copy link
Member

Choose a reason for hiding this comment

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

Props for this, it's a clever solution!

// Use the borrow in the IterMut to indicate borrowing behavior of the
// whole Drain iterator (like &mut T).
let range_slice = slice::from_raw_parts_mut(
self.as_mut_ptr().offset(start as isize),
end - start);
Drain {
ptr: begin,
end: end,
marker: PhantomData,
tail_start: end,
tail_len: len - end,
iter: range_slice.iter_mut(),
vec: self as *mut _,
}
}
}
Expand Down Expand Up @@ -1799,14 +1826,16 @@ impl<T> Drop for IntoIter<T> {
}
}

/// An iterator that drains a vector.
#[unsafe_no_drop_flag]
#[unstable(feature = "collections",
reason = "recently added as part of collections reform 2")]
pub struct Drain<'a, T:'a> {
ptr: *const T,
end: *const T,
marker: PhantomData<&'a T>,
/// A draining iterator for `Vec<T>`.
#[unstable(feature = "collections_drain", reason = "recently added")]
pub struct Drain<'a, T: 'a> {
/// Index of tail to preserve
tail_start: usize,
/// Length of tail
tail_len: usize,
Copy link
Member

Choose a reason for hiding this comment

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

In the interest of saving space, either length or start can be derived from the other, right?

Copy link
Member

Choose a reason for hiding this comment

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

Oh wait nevermind, the vector's length is "inaccurate" so it can't be used!

Copy link
Member Author

Choose a reason for hiding this comment

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

The vector's temporary length stores another necessary field -- the original starting point of the range we are removing.

Copy link
Member Author

Choose a reason for hiding this comment

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

Space that can be saved: The drop flag.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not going to attempt the drop flag without advice because I'm not up to date with how filling drop interacts with that.

/// Current remaining range to remove
iter: slice::IterMut<'a, T>,
vec: *mut Vec<T>,
}

unsafe impl<'a, T: Sync> Sync for Drain<'a, T> {}
Expand All @@ -1818,76 +1847,56 @@ impl<'a, T> Iterator for Drain<'a, T> {

#[inline]
fn next(&mut self) -> Option<T> {
unsafe {
if self.ptr == self.end {
None
} else {
if mem::size_of::<T>() == 0 {
// purposefully don't use 'ptr.offset' because for
// vectors with 0-size elements this would return the
// same pointer.
self.ptr = mem::transmute(self.ptr as usize + 1);

// Use a non-null pointer value
Some(ptr::read(EMPTY as *mut T))
} else {
let old = self.ptr;
self.ptr = self.ptr.offset(1);

Some(ptr::read(old))
}
self.iter.next().map(|elt|
unsafe {
ptr::read(elt as *const _)
}
}
)
}

#[inline]
fn size_hint(&self) -> (usize, Option<usize>) {
let diff = (self.end as usize) - (self.ptr as usize);
let size = mem::size_of::<T>();
let exact = diff / (if size == 0 {1} else {size});
(exact, Some(exact))
self.iter.size_hint()
}
}

#[stable(feature = "rust1", since = "1.0.0")]
impl<'a, T> DoubleEndedIterator for Drain<'a, T> {
#[inline]
fn next_back(&mut self) -> Option<T> {
unsafe {
if self.end == self.ptr {
None
} else {
if mem::size_of::<T>() == 0 {
// See above for why 'ptr.offset' isn't used
self.end = mem::transmute(self.end as usize - 1);

// Use a non-null pointer value
Some(ptr::read(EMPTY as *mut T))
} else {
self.end = self.end.offset(-1);

Some(ptr::read(self.end))
}
self.iter.next_back().map(|elt|
unsafe {
ptr::read(elt as *const _)
}
}
)
}
}

#[stable(feature = "rust1", since = "1.0.0")]
impl<'a, T> ExactSizeIterator for Drain<'a, T> {}

#[unsafe_destructor]
#[stable(feature = "rust1", since = "1.0.0")]
impl<'a, T> Drop for Drain<'a, T> {
fn drop(&mut self) {
// self.ptr == self.end == mem::POST_DROP_USIZE if drop has already been called,
// so we can use #[unsafe_no_drop_flag].
// exhaust self first
while let Some(_) = self.next() { }

// destroy the remaining elements
for _x in self.by_ref() {}
if self.tail_len > 0 {
unsafe {
let source_vec = &mut *self.vec;
// memmove back untouched tail, update to new length
let start = source_vec.len();
let tail = self.tail_start;
let src = source_vec.as_ptr().offset(tail as isize);
let dst = source_vec.as_mut_ptr().offset(start as isize);
ptr::copy(src, dst, self.tail_len);
source_vec.set_len(start + self.tail_len);
}
}
}
}


#[stable(feature = "rust1", since = "1.0.0")]
impl<'a, T> ExactSizeIterator for Drain<'a, T> {}

////////////////////////////////////////////////////////////////////////////////
// Conversion from &[T] to &Vec<T>
////////////////////////////////////////////////////////////////////////////////
Expand Down
2 changes: 1 addition & 1 deletion src/libcollections/vec_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -418,7 +418,7 @@ impl<V> VecMap<V> {
}
let filter: fn((usize, Option<V>)) -> Option<(usize, V)> = filter; // coerce to fn ptr

Drain { iter: self.v.drain().enumerate().filter_map(filter) }
Drain { iter: self.v.drain(..).enumerate().filter_map(filter) }
}

/// Returns the number of elements in the map.
Expand Down
1 change: 1 addition & 0 deletions src/libcollectionstest/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

#![feature(box_syntax)]
#![feature(collections)]
#![feature(collections_drain)]
#![feature(core)]
#![feature(hash)]
#![feature(rand)]
Expand Down
36 changes: 33 additions & 3 deletions src/libcollectionstest/vec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -460,7 +460,7 @@ fn test_move_items_zero_sized() {
fn test_drain_items() {
let mut vec = vec![1, 2, 3];
let mut vec2 = vec![];
for i in vec.drain() {
for i in vec.drain(..) {
vec2.push(i);
}
assert_eq!(vec, []);
Expand All @@ -471,7 +471,7 @@ fn test_drain_items() {
fn test_drain_items_reverse() {
let mut vec = vec![1, 2, 3];
let mut vec2 = vec![];
for i in vec.drain().rev() {
for i in vec.drain(..).rev() {
vec2.push(i);
}
assert_eq!(vec, []);
Expand All @@ -482,13 +482,43 @@ fn test_drain_items_reverse() {
fn test_drain_items_zero_sized() {
let mut vec = vec![(), (), ()];
let mut vec2 = vec![];
for i in vec.drain() {
for i in vec.drain(..) {
vec2.push(i);
}
assert_eq!(vec, []);
assert_eq!(vec2, [(), (), ()]);
}

#[test]
#[should_panic]
fn test_drain_out_of_bounds() {
let mut v = vec![1, 2, 3, 4, 5];
v.drain(5..6);
}

#[test]
fn test_drain_range() {
let mut v = vec![1, 2, 3, 4, 5];
for _ in v.drain(4..) {
}
assert_eq!(v, &[1, 2, 3, 4]);

let mut v: Vec<_> = (1..6).map(|x| x.to_string()).collect();
for _ in v.drain(1..4) {
}
assert_eq!(v, &[1.to_string(), 5.to_string()]);

let mut v: Vec<_> = (1..6).map(|x| x.to_string()).collect();
for _ in v.drain(1..4).rev() {
}
assert_eq!(v, &[1.to_string(), 5.to_string()]);

let mut v: Vec<_> = vec![(); 5];
for _ in v.drain(1..4).rev() {
}
assert_eq!(v, &[(), ()]);
}

#[test]
fn test_into_boxed_slice() {
let xs = vec![1, 2, 3];
Expand Down
4 changes: 3 additions & 1 deletion src/test/run-pass/sync-send-iterators-in-libcollections.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

#![allow(unused_mut)]
#![feature(collections)]
#![feature(collections_drain)]

extern crate collections;

Expand Down Expand Up @@ -96,5 +97,6 @@ fn main() {

all_sync_send!(VecMap::<usize>::new(), iter, iter_mut, drain, into_iter, keys, values);

all_sync_send!(Vec::<usize>::new(), into_iter, drain);
all_sync_send!(Vec::<usize>::new(), into_iter);
is_sync_send!(Vec::<usize>::new(), drain(..));
}