Skip to content

Commit dae8cb7

Browse files
author
Ulrik Sverdrup
committed
collections: Implement vec::drain(range) according to RFC 574
Old `.drain()` on vec is performed using `.drain(..)` now. [breaking-change]
1 parent 4dbb8e7 commit dae8cb7

File tree

4 files changed

+138
-86
lines changed

4 files changed

+138
-86
lines changed

src/libcollections/binary_heap.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -546,7 +546,7 @@ impl<T: Ord> BinaryHeap<T> {
546546
#[unstable(feature = "collections",
547547
reason = "matches collection reform specification, waiting for dust to settle")]
548548
pub fn drain(&mut self) -> Drain<T> {
549-
Drain { iter: self.data.drain() }
549+
Drain { iter: self.data.drain(..) }
550550
}
551551

552552
/// Drops all items from the binary heap.

src/libcollections/vec.rs

Lines changed: 103 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,8 @@ use core::usize;
6969

7070
use borrow::{Cow, IntoCow};
7171

72+
use super::range::RangeArgument;
73+
7274
// FIXME- fix places which assume the max vector allowed has memory usize::MAX.
7375
static MAX_MEMORY_SIZE: usize = isize::MAX as usize;
7476

@@ -718,36 +720,59 @@ impl<T> Vec<T> {
718720
unsafe { other.set_len(0); }
719721
}
720722

721-
/// Creates a draining iterator that clears the `Vec` and iterates over
722-
/// the removed items from start to end.
723+
/// Create a draining iterator that removes the specified range in the vector
724+
/// and yields the removed items from start to end. The element range is
725+
/// removed even if the iterator is not consumed until the end.
726+
///
727+
/// Note: It is unspecified how many elements are removed from the vector,
728+
/// if the `Drain` value is leaked.
729+
///
730+
/// # Panics
731+
///
732+
/// Panics if the starting point is greater than the end point or if
733+
/// the end point is greater than the length of the vector.
723734
///
724735
/// # Examples
725736
///
726737
/// ```
727-
/// # #![feature(collections)]
728-
/// let mut v = vec!["a".to_string(), "b".to_string()];
729-
/// for s in v.drain() {
730-
/// // s has type String, not &String
731-
/// println!("{}", s);
732-
/// }
733-
/// assert!(v.is_empty());
738+
/// // Draining using `..` clears the whole vector.
739+
/// let mut v = vec![1, 2, 3];
740+
/// let u: Vec<_> = v.drain(..).collect();
741+
/// assert_eq!(v, &[]);
742+
/// assert_eq!(u, &[1, 2, 3]);
734743
/// ```
735-
#[inline]
736744
#[unstable(feature = "collections",
737-
reason = "matches collection reform specification, waiting for dust to settle")]
738-
pub fn drain(&mut self) -> Drain<T> {
745+
reason = "recently added, matches RFC")]
746+
pub fn drain<R>(&mut self, range: R) -> Drain<T> where
747+
R: RangeArgument<usize>,
748+
{
749+
// Memory safety
750+
//
751+
// When the Drain is first created, it shortens the length of
752+
// the source vector to make sure no uninitalized or moved-from elements
753+
// are accessible at all if the Drain's destructor never gets to run.
754+
//
755+
// Drain will ptr::read out the values to remove.
756+
// When finished, remaining tail of the vec is copied back to cover
757+
// the hole, and the vector length is restored to the new length.
758+
//
759+
let len = self.len();
760+
let start = *range.start().unwrap_or(&0);
761+
let end = *range.end().unwrap_or(&len);
762+
assert!(start <= end);
763+
assert!(end <= len);
764+
739765
unsafe {
740-
let begin = *self.ptr as *const T;
741-
let end = if mem::size_of::<T>() == 0 {
742-
(*self.ptr as usize + self.len()) as *const T
743-
} else {
744-
(*self.ptr).offset(self.len() as isize) as *const T
745-
};
746-
self.set_len(0);
766+
// set self.vec length's to start, to be safe in case Drain is leaked
767+
self.set_len(start);
768+
let range_slice = slice::from_raw_parts(self.as_ptr().offset(start as isize),
769+
end - start);
747770
Drain {
748-
ptr: begin,
749-
end: end,
750-
marker: PhantomData,
771+
tail_start: end,
772+
tail_len: len - end,
773+
iter: range_slice.iter(),
774+
vec: self as *mut _,
775+
_marker: PhantomData,
751776
}
752777
}
753778
}
@@ -1799,95 +1824,92 @@ impl<T> Drop for IntoIter<T> {
17991824
}
18001825
}
18011826

1802-
/// An iterator that drains a vector.
1803-
#[unsafe_no_drop_flag]
1804-
#[unstable(feature = "collections",
1805-
reason = "recently added as part of collections reform 2")]
1806-
pub struct Drain<'a, T:'a> {
1807-
ptr: *const T,
1808-
end: *const T,
1809-
marker: PhantomData<&'a T>,
1827+
/// A draining iterator for `Vec<T>`.
1828+
#[unstable(feature = "collections", reason = "recently added")]
1829+
pub struct Drain<'a, T: 'a>
1830+
{
1831+
/// Index of tail to preserve
1832+
tail_start: usize,
1833+
/// Length of tail
1834+
tail_len: usize,
1835+
/// Current remaining range to remove
1836+
iter: slice::Iter<'a, T>,
1837+
vec: *mut Vec<T>,
1838+
_marker: PhantomData<&'a mut Vec<T>>,
18101839
}
18111840

18121841
unsafe impl<'a, T: Sync> Sync for Drain<'a, T> {}
18131842
unsafe impl<'a, T: Send> Send for Drain<'a, T> {}
18141843

18151844
#[stable(feature = "rust1", since = "1.0.0")]
1816-
impl<'a, T> Iterator for Drain<'a, T> {
1845+
impl<'a, T> Iterator for Drain<'a, T>
1846+
{
18171847
type Item = T;
18181848

18191849
#[inline]
1820-
fn next(&mut self) -> Option<T> {
1821-
unsafe {
1822-
if self.ptr == self.end {
1823-
None
1824-
} else {
1825-
if mem::size_of::<T>() == 0 {
1826-
// purposefully don't use 'ptr.offset' because for
1827-
// vectors with 0-size elements this would return the
1828-
// same pointer.
1829-
self.ptr = mem::transmute(self.ptr as usize + 1);
1830-
1831-
// Use a non-null pointer value
1832-
Some(ptr::read(EMPTY as *mut T))
1833-
} else {
1834-
let old = self.ptr;
1835-
self.ptr = self.ptr.offset(1);
1836-
1837-
Some(ptr::read(old))
1850+
fn next(&mut self) -> Option<T>
1851+
{
1852+
match self.iter.next() {
1853+
None => None,
1854+
Some(elt) => {
1855+
unsafe {
1856+
Some(ptr::read(elt as *const _))
18381857
}
18391858
}
18401859
}
18411860
}
18421861

1843-
#[inline]
1844-
fn size_hint(&self) -> (usize, Option<usize>) {
1845-
let diff = (self.end as usize) - (self.ptr as usize);
1846-
let size = mem::size_of::<T>();
1847-
let exact = diff / (if size == 0 {1} else {size});
1848-
(exact, Some(exact))
1862+
fn size_hint(&self) -> (usize, Option<usize>)
1863+
{
1864+
self.iter.size_hint()
18491865
}
18501866
}
18511867

18521868
#[stable(feature = "rust1", since = "1.0.0")]
1853-
impl<'a, T> DoubleEndedIterator for Drain<'a, T> {
1869+
impl<'a, T> DoubleEndedIterator for Drain<'a, T>
1870+
{
18541871
#[inline]
1855-
fn next_back(&mut self) -> Option<T> {
1856-
unsafe {
1857-
if self.end == self.ptr {
1858-
None
1859-
} else {
1860-
if mem::size_of::<T>() == 0 {
1861-
// See above for why 'ptr.offset' isn't used
1862-
self.end = mem::transmute(self.end as usize - 1);
1863-
1864-
// Use a non-null pointer value
1865-
Some(ptr::read(EMPTY as *mut T))
1866-
} else {
1867-
self.end = self.end.offset(-1);
1868-
1869-
Some(ptr::read(self.end))
1872+
fn next_back(&mut self) -> Option<T>
1873+
{
1874+
match self.iter.next_back() {
1875+
None => None,
1876+
Some(elt) => {
1877+
unsafe {
1878+
Some(ptr::read(elt as *const _))
18701879
}
18711880
}
18721881
}
18731882
}
18741883
}
18751884

1876-
#[stable(feature = "rust1", since = "1.0.0")]
1877-
impl<'a, T> ExactSizeIterator for Drain<'a, T> {}
1878-
18791885
#[unsafe_destructor]
18801886
#[stable(feature = "rust1", since = "1.0.0")]
1881-
impl<'a, T> Drop for Drain<'a, T> {
1882-
fn drop(&mut self) {
1883-
// self.ptr == self.end == mem::POST_DROP_USIZE if drop has already been called,
1884-
// so we can use #[unsafe_no_drop_flag].
1885-
1886-
// destroy the remaining elements
1887-
for _x in self.by_ref() {}
1887+
impl<'a, T> Drop for Drain<'a, T>
1888+
{
1889+
fn drop(&mut self)
1890+
{
1891+
// exhaust self first
1892+
while let Some(_) = self.next() { }
1893+
1894+
if self.tail_len > 0 {
1895+
unsafe {
1896+
let source_vec = &mut *self.vec;
1897+
// memmove back untouched tail, update to new length
1898+
let start = source_vec.len();
1899+
let tail = self.tail_start;
1900+
let src = source_vec.as_ptr().offset(tail as isize);
1901+
let dst = source_vec.as_mut_ptr().offset(start as isize);
1902+
ptr::copy(src, dst, self.tail_len);
1903+
source_vec.set_len(start + self.tail_len);
1904+
}
1905+
}
18881906
}
18891907
}
18901908

1909+
1910+
#[stable(feature = "rust1", since = "1.0.0")]
1911+
impl<'a, T> ExactSizeIterator for Drain<'a, T> {}
1912+
18911913
////////////////////////////////////////////////////////////////////////////////
18921914
// Conversion from &[T] to &Vec<T>
18931915
////////////////////////////////////////////////////////////////////////////////

src/libcollections/vec_map.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -418,7 +418,7 @@ impl<V> VecMap<V> {
418418
}
419419
let filter: fn((usize, Option<V>)) -> Option<(usize, V)> = filter; // coerce to fn ptr
420420

421-
Drain { iter: self.v.drain().enumerate().filter_map(filter) }
421+
Drain { iter: self.v.drain(..).enumerate().filter_map(filter) }
422422
}
423423

424424
/// Returns the number of elements in the map.

src/libcollectionstest/vec.rs

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -460,7 +460,7 @@ fn test_move_items_zero_sized() {
460460
fn test_drain_items() {
461461
let mut vec = vec![1, 2, 3];
462462
let mut vec2 = vec![];
463-
for i in vec.drain() {
463+
for i in vec.drain(..) {
464464
vec2.push(i);
465465
}
466466
assert_eq!(vec, []);
@@ -471,7 +471,7 @@ fn test_drain_items() {
471471
fn test_drain_items_reverse() {
472472
let mut vec = vec![1, 2, 3];
473473
let mut vec2 = vec![];
474-
for i in vec.drain().rev() {
474+
for i in vec.drain(..).rev() {
475475
vec2.push(i);
476476
}
477477
assert_eq!(vec, []);
@@ -482,13 +482,43 @@ fn test_drain_items_reverse() {
482482
fn test_drain_items_zero_sized() {
483483
let mut vec = vec![(), (), ()];
484484
let mut vec2 = vec![];
485-
for i in vec.drain() {
485+
for i in vec.drain(..) {
486486
vec2.push(i);
487487
}
488488
assert_eq!(vec, []);
489489
assert_eq!(vec2, [(), (), ()]);
490490
}
491491

492+
#[test]
493+
#[should_panic]
494+
fn drain_out_of_bounds_1() {
495+
let mut v = vec![1, 2, 3, 4, 5];
496+
v.drain(5..6);
497+
}
498+
499+
#[test]
500+
fn drain_range() {
501+
let mut v = vec![1, 2, 3, 4, 5];
502+
for _ in v.drain(4..) {
503+
}
504+
assert_eq!(v, &[1, 2, 3, 4]);
505+
506+
let mut v: Vec<_> = (1..6).map(|x| x.to_string()).collect();
507+
for _ in v.drain(1..4) {
508+
}
509+
assert_eq!(v, &[1.to_string(), 5.to_string()]);
510+
511+
let mut v: Vec<_> = (1..6).map(|x| x.to_string()).collect();
512+
for _ in v.drain(1..4).rev() {
513+
}
514+
assert_eq!(v, &[1.to_string(), 5.to_string()]);
515+
516+
let mut v: Vec<_> = vec![(); 5];
517+
for _ in v.drain(1..4).rev() {
518+
}
519+
assert_eq!(v, &[(), ()]);
520+
}
521+
492522
#[test]
493523
fn test_into_boxed_slice() {
494524
let xs = vec![1, 2, 3];

0 commit comments

Comments
 (0)