From 97d082c6cdc05c0ac7f37f7a0c0b3a2f9fe698e3 Mon Sep 17 00:00:00 2001 From: Thomas Garcia <thomasjgarcia1@gmail.com> Date: Thu, 21 Jul 2016 01:03:40 -0700 Subject: [PATCH 1/2] Make vec::Drain and binary_heap::Drain covariant --- src/libcollections/binary_heap.rs | 10 ++-- src/libcollections/vec.rs | 66 ++++++++++----------------- src/libcollectionstest/binary_heap.rs | 6 +++ src/libcollectionstest/vec.rs | 6 +++ 4 files changed, 40 insertions(+), 48 deletions(-) diff --git a/src/libcollections/binary_heap.rs b/src/libcollections/binary_heap.rs index b9f5c6fcab909..f6ca90234c5b4 100644 --- a/src/libcollections/binary_heap.rs +++ b/src/libcollections/binary_heap.rs @@ -1016,12 +1016,12 @@ impl<T> ExactSizeIterator for IntoIter<T> {} /// An iterator that drains a `BinaryHeap`. #[stable(feature = "drain", since = "1.6.0")] -pub struct Drain<'a, T: 'a> { - iter: vec::Drain<'a, T>, +pub struct Drain<T> { + iter: vec::Drain<T>, } #[stable(feature = "rust1", since = "1.0.0")] -impl<'a, T: 'a> Iterator for Drain<'a, T> { +impl<T> Iterator for Drain<T> { type Item = T; #[inline] @@ -1036,7 +1036,7 @@ impl<'a, T: 'a> Iterator for Drain<'a, T> { } #[stable(feature = "rust1", since = "1.0.0")] -impl<'a, T: 'a> DoubleEndedIterator for Drain<'a, T> { +impl<T> DoubleEndedIterator for Drain<T> { #[inline] fn next_back(&mut self) -> Option<T> { self.iter.next_back() @@ -1044,7 +1044,7 @@ impl<'a, T: 'a> DoubleEndedIterator for Drain<'a, T> { } #[stable(feature = "rust1", since = "1.0.0")] -impl<'a, T: 'a> ExactSizeIterator for Drain<'a, T> {} +impl<T> ExactSizeIterator for Drain<T> {} #[stable(feature = "rust1", since = "1.0.0")] impl<T: Ord> From<Vec<T>> for BinaryHeap<T> { diff --git a/src/libcollections/vec.rs b/src/libcollections/vec.rs index 518b94b5031b4..f4855cce3b4a2 100644 --- a/src/libcollections/vec.rs +++ b/src/libcollections/vec.rs @@ -74,6 +74,7 @@ use core::ops::{Index, IndexMut}; use core::ops; use core::ptr; use core::slice; +use super::vec_deque::VecDeque; use super::SpecExtend; use super::range::RangeArgument; @@ -843,20 +844,20 @@ impl<T> Vec<T> { let end = *range.end().unwrap_or(&len); assert!(start <= end); assert!(end <= len); + let mut drain_vec = VecDeque::new(); unsafe { - // set self.vec length's to start, to be safe in case Drain is leaked - self.set_len(start); - // 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 { - tail_start: end, - tail_len: len - end, - iter: range_slice.iter_mut(), - vec: self as *mut _, + for i in start..end { + let p = self.as_ptr().offset(i as isize); + drain_vec.push_back(ptr::read(p)); } + let src = self.as_ptr().offset(end as isize); + let dst = self.as_mut_ptr().offset(start as isize); + ptr::copy(src, dst, len - end); + self.set_len(len - (end - start)); + } + Drain { + deque: drain_vec } } @@ -1755,64 +1756,43 @@ impl<T> Drop for IntoIter<T> { /// [`drain`]: struct.Vec.html#method.drain /// [`Vec`]: struct.Vec.html #[stable(feature = "drain", since = "1.6.0")] -pub struct Drain<'a, T: 'a> { - /// Index of tail to preserve - tail_start: usize, - /// Length of tail - tail_len: usize, +pub struct Drain<T> { /// Current remaining range to remove - iter: slice::IterMut<'a, T>, - vec: *mut Vec<T>, + deque: VecDeque<T> } #[stable(feature = "drain", since = "1.6.0")] -unsafe impl<'a, T: Sync> Sync for Drain<'a, T> {} +unsafe impl<T: Sync> Sync for Drain<T> {} #[stable(feature = "drain", since = "1.6.0")] -unsafe impl<'a, T: Send> Send for Drain<'a, T> {} +unsafe impl<T: Send> Send for Drain<T> {} #[stable(feature = "rust1", since = "1.0.0")] -impl<'a, T> Iterator for Drain<'a, T> { +impl<T> Iterator for Drain<T> { type Item = T; #[inline] fn next(&mut self) -> Option<T> { - self.iter.next().map(|elt| unsafe { ptr::read(elt as *const _) }) + self.deque.pop_front() } fn size_hint(&self) -> (usize, Option<usize>) { - self.iter.size_hint() + (self.deque.len(), Some(self.deque.len())) } } #[stable(feature = "rust1", since = "1.0.0")] -impl<'a, T> DoubleEndedIterator for Drain<'a, T> { +impl<T> DoubleEndedIterator for Drain<T> { #[inline] fn next_back(&mut self) -> Option<T> { - self.iter.next_back().map(|elt| unsafe { ptr::read(elt as *const _) }) + self.deque.pop_back() } } #[stable(feature = "rust1", since = "1.0.0")] -impl<'a, T> Drop for Drain<'a, T> { +impl<T> Drop for Drain<T> { fn drop(&mut self) { - // exhaust self first - while let Some(_) = self.next() {} - - 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> {} +impl<T> ExactSizeIterator for Drain<T> {} diff --git a/src/libcollectionstest/binary_heap.rs b/src/libcollectionstest/binary_heap.rs index be933abe41fe2..39efc9fc22c39 100644 --- a/src/libcollectionstest/binary_heap.rs +++ b/src/libcollectionstest/binary_heap.rs @@ -9,6 +9,7 @@ // except according to those terms. use std::collections::BinaryHeap; +use std::collections::binary_heap::Drain; #[test] fn test_iterator() { @@ -292,3 +293,8 @@ fn test_extend_specialization() { assert_eq!(a.into_sorted_vec(), [-20, -10, 1, 2, 3, 3, 5, 43]); } + +#[allow(dead_code)] +fn assert_covariance() { + fn drain<'new>(d: Drain<&'static str>) -> Drain<&'new str> { d } +} diff --git a/src/libcollectionstest/vec.rs b/src/libcollectionstest/vec.rs index cb99659cc0ead..01656b44a8495 100644 --- a/src/libcollectionstest/vec.rs +++ b/src/libcollectionstest/vec.rs @@ -11,6 +11,7 @@ use std::borrow::Cow; use std::iter::{FromIterator, repeat}; use std::mem::size_of; +use std::vec::Drain; use test::Bencher; @@ -510,6 +511,11 @@ fn test_cow_from() { } } +#[allow(dead_code)] +fn assert_covariance() { + fn drain<'new>(d: Drain<&'static str>) -> Drain<&'new str> { d } +} + #[bench] fn bench_new(b: &mut Bencher) { b.iter(|| { From d1e2a935d2d35e768d0a56af7938c725f243fc28 Mon Sep 17 00:00:00 2001 From: Thomas Garcia <thomasjgarcia1@gmail.com> Date: Thu, 21 Jul 2016 20:55:19 -0700 Subject: [PATCH 2/2] Readding lifetime parameters and removing allocation --- src/libcollections/binary_heap.rs | 10 ++-- src/libcollections/vec.rs | 67 ++++++++++++++++++--------- src/libcollectionstest/binary_heap.rs | 2 +- src/libcollectionstest/vec.rs | 2 +- 4 files changed, 51 insertions(+), 30 deletions(-) diff --git a/src/libcollections/binary_heap.rs b/src/libcollections/binary_heap.rs index f6ca90234c5b4..b9f5c6fcab909 100644 --- a/src/libcollections/binary_heap.rs +++ b/src/libcollections/binary_heap.rs @@ -1016,12 +1016,12 @@ impl<T> ExactSizeIterator for IntoIter<T> {} /// An iterator that drains a `BinaryHeap`. #[stable(feature = "drain", since = "1.6.0")] -pub struct Drain<T> { - iter: vec::Drain<T>, +pub struct Drain<'a, T: 'a> { + iter: vec::Drain<'a, T>, } #[stable(feature = "rust1", since = "1.0.0")] -impl<T> Iterator for Drain<T> { +impl<'a, T: 'a> Iterator for Drain<'a, T> { type Item = T; #[inline] @@ -1036,7 +1036,7 @@ impl<T> Iterator for Drain<T> { } #[stable(feature = "rust1", since = "1.0.0")] -impl<T> DoubleEndedIterator for Drain<T> { +impl<'a, T: 'a> DoubleEndedIterator for Drain<'a, T> { #[inline] fn next_back(&mut self) -> Option<T> { self.iter.next_back() @@ -1044,7 +1044,7 @@ impl<T> DoubleEndedIterator for Drain<T> { } #[stable(feature = "rust1", since = "1.0.0")] -impl<T> ExactSizeIterator for Drain<T> {} +impl<'a, T: 'a> ExactSizeIterator for Drain<'a, T> {} #[stable(feature = "rust1", since = "1.0.0")] impl<T: Ord> From<Vec<T>> for BinaryHeap<T> { diff --git a/src/libcollections/vec.rs b/src/libcollections/vec.rs index f4855cce3b4a2..f3d31ceea1347 100644 --- a/src/libcollections/vec.rs +++ b/src/libcollections/vec.rs @@ -73,8 +73,8 @@ use core::mem; use core::ops::{Index, IndexMut}; use core::ops; use core::ptr; +use core::ptr::Shared; use core::slice; -use super::vec_deque::VecDeque; use super::SpecExtend; use super::range::RangeArgument; @@ -844,20 +844,20 @@ impl<T> Vec<T> { let end = *range.end().unwrap_or(&len); assert!(start <= end); assert!(end <= len); - let mut drain_vec = VecDeque::new(); unsafe { - for i in start..end { - let p = self.as_ptr().offset(i as isize); - drain_vec.push_back(ptr::read(p)); + // set self.vec length's to start, to be safe in case Drain is leaked + self.set_len(start); + // 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 { + tail_start: end, + tail_len: len - end, + iter: range_slice.iter(), + vec: Shared::new(self as *mut _), } - let src = self.as_ptr().offset(end as isize); - let dst = self.as_mut_ptr().offset(start as isize); - ptr::copy(src, dst, len - end); - self.set_len(len - (end - start)); - } - Drain { - deque: drain_vec } } @@ -1756,43 +1756,64 @@ impl<T> Drop for IntoIter<T> { /// [`drain`]: struct.Vec.html#method.drain /// [`Vec`]: struct.Vec.html #[stable(feature = "drain", since = "1.6.0")] -pub struct Drain<T> { +pub struct Drain<'a, T: 'a> { + /// Index of tail to preserve + tail_start: usize, + /// Length of tail + tail_len: usize, /// Current remaining range to remove - deque: VecDeque<T> + iter: slice::Iter<'a, T>, + vec: Shared<Vec<T>>, } #[stable(feature = "drain", since = "1.6.0")] -unsafe impl<T: Sync> Sync for Drain<T> {} +unsafe impl<'a, T: Sync> Sync for Drain<'a, T> {} #[stable(feature = "drain", since = "1.6.0")] -unsafe impl<T: Send> Send for Drain<T> {} +unsafe impl<'a, T: Send> Send for Drain<'a, T> {} #[stable(feature = "rust1", since = "1.0.0")] -impl<T> Iterator for Drain<T> { +impl<'a, T> Iterator for Drain<'a, T> { type Item = T; #[inline] fn next(&mut self) -> Option<T> { - self.deque.pop_front() + self.iter.next().map(|elt| unsafe { ptr::read(elt as *const _) }) } fn size_hint(&self) -> (usize, Option<usize>) { - (self.deque.len(), Some(self.deque.len())) + self.iter.size_hint() } } #[stable(feature = "rust1", since = "1.0.0")] -impl<T> DoubleEndedIterator for Drain<T> { +impl<'a, T> DoubleEndedIterator for Drain<'a, T> { #[inline] fn next_back(&mut self) -> Option<T> { - self.deque.pop_back() + self.iter.next_back().map(|elt| unsafe { ptr::read(elt as *const _) }) } } #[stable(feature = "rust1", since = "1.0.0")] -impl<T> Drop for Drain<T> { +impl<'a, T> Drop for Drain<'a, T> { fn drop(&mut self) { + // exhaust self first + while let Some(_) = self.next() {} + + 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<T> ExactSizeIterator for Drain<T> {} +impl<'a, T> ExactSizeIterator for Drain<'a, T> {} diff --git a/src/libcollectionstest/binary_heap.rs b/src/libcollectionstest/binary_heap.rs index 39efc9fc22c39..e2a57bd8d3862 100644 --- a/src/libcollectionstest/binary_heap.rs +++ b/src/libcollectionstest/binary_heap.rs @@ -296,5 +296,5 @@ fn test_extend_specialization() { #[allow(dead_code)] fn assert_covariance() { - fn drain<'new>(d: Drain<&'static str>) -> Drain<&'new str> { d } + fn drain<'new>(d: Drain<'static, &'static str>) -> Drain<'new, &'new str> { d } } diff --git a/src/libcollectionstest/vec.rs b/src/libcollectionstest/vec.rs index 01656b44a8495..7a6bd958a5f8c 100644 --- a/src/libcollectionstest/vec.rs +++ b/src/libcollectionstest/vec.rs @@ -513,7 +513,7 @@ fn test_cow_from() { #[allow(dead_code)] fn assert_covariance() { - fn drain<'new>(d: Drain<&'static str>) -> Drain<&'new str> { d } + fn drain<'new>(d: Drain<'static, &'static str>) -> Drain<'new, &'new str> { d } } #[bench]