From 3b4f7045549018e7480f6745ba0c307a89a71ee8 Mon Sep 17 00:00:00 2001 From: Michael Matloka Date: Sat, 16 Oct 2021 04:57:41 +0200 Subject: [PATCH 1/5] Propose an `ArrayVec::splice` implementation --- src/arrayvec.rs | 42 +++++++++++++++++---- src/lib.rs | 1 + src/splice.rs | 97 +++++++++++++++++++++++++++++++++++++++++++++++++ tests/tests.rs | 75 +++++++++++++++++++++++++++++++++++++- 4 files changed, 207 insertions(+), 8 deletions(-) create mode 100644 src/splice.rs diff --git a/src/arrayvec.rs b/src/arrayvec.rs index 6b4faf8a..4bb98422 100644 --- a/src/arrayvec.rs +++ b/src/arrayvec.rs @@ -23,6 +23,7 @@ use serde::{Serialize, Deserialize, Serializer, Deserializer}; use crate::LenUint; use crate::errors::CapacityError; use crate::arrayvec_impl::ArrayVecImpl; +use crate::splice::Splice; use crate::utils::MakeMaybeUninit; /// A vector with a fixed capacity. @@ -41,8 +42,8 @@ use crate::utils::MakeMaybeUninit; /// available. The ArrayVec can be converted into a by value iterator. pub struct ArrayVec { // the `len` first elements of the array are initialized - xs: [MaybeUninit; CAP], - len: LenUint, + pub(super) xs: [MaybeUninit; CAP], + pub(super) len: LenUint, } impl Drop for ArrayVec { @@ -253,7 +254,7 @@ impl ArrayVec { /// Get pointer to where element at `index` would be - unsafe fn get_unchecked_ptr(&mut self, index: usize) -> *mut T { + pub(super) unsafe fn get_unchecked_ptr(&mut self, index: usize) -> *mut T { self.as_mut_ptr().add(index) } @@ -562,6 +563,33 @@ impl ArrayVec { Ok(()) } + /// Creates a splicing iterator that replaces the specified range in the vector with the given `replace_with` iterator and yields the removed items. `replace_with` does not need to be the same length as `range`. + /// + /// `range` is removed even if the iterator is not consumed until the end. + /// + /// It is unspecified how many elements are removed from the vector if the `Splice` value is leaked. + /// + /// The input iterator `replace_with` is only consumed when the `Splice` value is dropped. + /// + /// ``` + /// use std::iter::FromIterator; + /// use arrayvec::ArrayVec; + /// + /// let mut vec: ArrayVec = ArrayVec::from_iter((0..4)); + /// let elements_popped: Vec<_> = vec.splice(1..3, [7, 9]).into_iter().collect(); + /// assert_eq!(&vec[..], &[0, 7, 9, 3]); + /// assert_eq!(&elements_popped[..], &[1, 2]); + /// ``` + /// + /// ***Panics*** if splicing the vector exceeds its capacity. + pub fn splice(&mut self, range: R, replace_with: I) -> Splice<'_, I::IntoIter, CAP> + where + R: RangeBounds, + I: IntoIterator, + { + Splice { drain: self.drain(range), replace_with: replace_with.into_iter() } + } + /// 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. @@ -908,12 +936,12 @@ where /// A draining iterator for `ArrayVec`. pub struct Drain<'a, T: 'a, const CAP: usize> { /// Index of tail to preserve - tail_start: usize, + pub(super) tail_start: usize, /// Length of tail - tail_len: usize, + pub(super) tail_len: usize, /// Current remaining range to remove - iter: slice::Iter<'a, T>, - vec: *mut ArrayVec, + pub(super) iter: slice::Iter<'a, T>, + pub(super) vec: *mut ArrayVec, } unsafe impl<'a, T: Sync, const CAP: usize> Sync for Drain<'a, T, CAP> {} diff --git a/src/lib.rs b/src/lib.rs index 5dc0273a..4e869e19 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -48,6 +48,7 @@ macro_rules! assert_capacity_limit_const { mod arrayvec_impl; mod arrayvec; +mod splice; mod array_string; mod char; mod errors; diff --git a/src/splice.rs b/src/splice.rs new file mode 100644 index 00000000..4c6c6802 --- /dev/null +++ b/src/splice.rs @@ -0,0 +1,97 @@ +use core::ptr; +use core::slice; + +use crate::Drain; + +/// A splicing iterator adapter for `ArrayVec` from `Vec`. +pub struct Splice< + 'a, + I: Iterator + 'a, + const CAP: usize +> { + pub(super) drain: Drain<'a, I::Item, CAP>, + pub(super) replace_with: I, +} + +impl Iterator for Splice<'_, I, CAP> { + type Item = I::Item; + + fn next(&mut self) -> Option { + self.drain.next() + } + + fn size_hint(&self) -> (usize, Option) { + self.drain.size_hint() + } +} + +impl DoubleEndedIterator for Splice<'_, I, CAP> { + fn next_back(&mut self) -> Option { + self.drain.next_back() + } +} + +impl ExactSizeIterator for Splice<'_, I, CAP> {} + +impl Drop for Splice<'_, I, CAP> { + fn drop(&mut self) { + self.drain.by_ref().for_each(drop); + + unsafe { + if self.drain.tail_len == 0 { + let target_vec = &mut *self.drain.vec; + target_vec.extend(self.replace_with.by_ref()); + return; + } + + // First fill the range left by drain(). + if !self.drain.fill(&mut self.replace_with) { + return; + } + + // There may be more elements. Use the lower bound as an estimate. + // FIXME: Is the upper bound a better guess? Or something else? + let (lower_bound, _upper_bound) = self.replace_with.size_hint(); + if lower_bound > 0 { + if !self.drain.fill(&mut self.replace_with) { + return; + } + } + + // Collect any remaining elements. + // This is a zero-length vector which does not allocate if `lower_bound` was exact. + let mut collected = self.replace_with.by_ref().collect::>().into_iter(); + // Now we have an exact count. + if collected.len() > 0 { + let filled = self.drain.fill(&mut collected); + debug_assert!(filled); + debug_assert_eq!(collected.len(), 0); + } + } + // Let `Drain::drop` move the tail back if necessary and restore `vec.len`. + } +} + +/// Private helper methods for `Splice::drop` +impl Drain<'_, T, CAP> { + /// The range from `self.vec.len` to `self.tail_start` contains elements + /// that have been moved out. + /// Fill that range as much as possible with new elements from the `replace_with` iterator. + /// Returns `true` if we filled the entire range. (`replace_with.next()` didn’t return `None`.) + unsafe fn fill>(&mut self, replace_with: &mut I) -> bool { + let vec = &mut *self.vec; + let range_start = vec.len as usize; + let range_end = self.tail_start; + let range_slice = slice::from_raw_parts_mut(vec.get_unchecked_ptr(range_start), range_end - range_start); + + for place in range_slice { + if let Some(new_item) = replace_with.next() { + ptr::write(place, new_item); + vec.len += 1; + } else { + return false; + } + } + true + } +} diff --git a/tests/tests.rs b/tests/tests.rs index 2f8a5ef5..42f073ee 100644 --- a/tests/tests.rs +++ b/tests/tests.rs @@ -65,6 +65,79 @@ fn test_extend_from_slice_error() { assert_matches!(res, Err(_)); } +#[test] +fn test_splice_works_when_it_should() { + // Comparing behavior with std::vec::Vec to make sure it's analogous + let mut vec: Vec<_> = (0..5).collect(); + let mut array: ArrayVec<_, 5> = (0..5).collect(); + assert_eq!(&vec[..], &[0, 1, 2, 3, 4]); + assert_eq!(&array[..], &[0, 1, 2, 3, 4]); + // Typical case + let vec_popped: Vec<_> = vec.splice( 1..4, [11, 12, 13]).into_iter().collect(); + assert_eq!(&vec[..], &[0, 11, 12, 13, 4]); + assert_eq!(&vec_popped, &[1, 2, 3]); + let array_popped: Vec<_> = array.splice( 1..4, [11, 12, 13]).into_iter().collect(); + assert_eq!(&array[..], &vec[..]); + assert_eq!(&array_popped, &vec_popped); + // `replace_with` shorter than `range` + let vec_popped: Vec<_> = vec.splice( 2..5, [21, 22]).into_iter().collect(); + assert_eq!(&vec[..], &[0, 11, 21, 22]); + assert_eq!(&vec_popped, &[12, 13, 4]); + let array_popped: Vec<_> = array.splice( 2..5, [21, 22]).into_iter().collect(); + assert_eq!(&array[..], &vec[..]); + assert_eq!(&array_popped, &vec_popped); + // `range` shorter than `replace_with` + let vec_popped: Vec<_> = vec.splice( 3..4, [31, 32]).into_iter().collect(); + assert_eq!(&vec[..], &[0, 11, 21, 31, 32]); + assert_eq!(&vec_popped, &[22]); + let array_popped: Vec<_> = array.splice( 3..4, [31, 32]).into_iter().collect(); + assert_eq!(&array[..], &vec[..]); + assert_eq!(&array_popped, &vec_popped); + //`replace_with` shorter than open `range` + let vec_popped: Vec<_> = vec.splice( 1.., [41, 42]).into_iter().collect(); + assert_eq!(&vec[..], &[0, 41, 42]); + assert_eq!(&vec_popped, &[11, 21, 31, 32]); + let array_popped: Vec<_> = array.splice( 1.., [41, 42]).into_iter().collect(); + assert_eq!(&array[..], &vec[..]); + assert_eq!(&array_popped, &vec_popped); + // `range` shorter than open `replace_with` + let vec_popped: Vec<_> = vec.splice( 3.., [51, 52]).into_iter().collect(); + assert_eq!(&vec[..], &[0, 41, 42, 51, 52]); + assert_eq!(&vec_popped, &[]); + let array_popped: Vec<_> = array.splice( 3.., [51, 52]).into_iter().collect(); + assert_eq!(&array[..], &vec[..]); + assert_eq!(&array_popped, &vec_popped); +} + + +#[test] +#[should_panic] +fn test_splice_fails_on_overflowing_range() { + // Comparing behavior with std::vec::Vec to make sure it's analogous + let mut vec: Vec<_> = (0..5).collect(); + let mut array: ArrayVec<_, 5> = (0..5).collect(); + let vec_popped: Vec<_> = vec.splice( 4..6, [11]).into_iter().collect(); + // This is actually less than 5! + // Maybe this would be fine, but we don't want to allow using overflowing ranges with ArrayVec anyway + assert_eq!(&vec[..], &[0, 1, 2, 11]); + assert_eq!(&vec_popped, &[3, 4]); + array.splice(4..6, [11]); +} + +#[test] +#[should_panic] +fn test_splice_fails_if_operation_would_enlarge_vec() { + // Comparing behavior with std::vec::Vec to make sure it's analogous + let mut vec: Vec<_> = (0..5).collect(); + let mut array: ArrayVec<_, 5> = (0..5).collect(); + let vec_popped: Vec<_> = vec.splice( 4..5, [11, 12, 13]).into_iter().collect(); + // This is now 6 instead of 5! + // No way for this ArrayVec to fit this in + assert_eq!(&vec[..], &[0, 1, 2, 11, 12, 13]); + assert_eq!(&vec_popped, &[3, 4]); + array.splice(4..5, [11, 12, 13]); +} + #[test] fn test_try_from_slice_error() { use arrayvec::ArrayVec; @@ -790,4 +863,4 @@ fn test_arraystring_zero_filled_has_some_sanity_checks() { let string = ArrayString::<4>::zero_filled(); assert_eq!(string.as_str(), "\0\0\0\0"); assert_eq!(string.len(), 4); -} \ No newline at end of file +} From 1160de87d1b5c8347f7ea4a77196430334fb8024 Mon Sep 17 00:00:00 2001 From: Michael Matloka Date: Sat, 16 Oct 2021 05:02:24 +0200 Subject: [PATCH 2/5] Fix comment typo --- src/splice.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/splice.rs b/src/splice.rs index 4c6c6802..794e44d9 100644 --- a/src/splice.rs +++ b/src/splice.rs @@ -3,7 +3,7 @@ use core::slice; use crate::Drain; -/// A splicing iterator adapter for `ArrayVec` from `Vec`. +/// A splicing iterator adapted for `ArrayVec` from `Vec`. pub struct Splice< 'a, I: Iterator + 'a, From 57054c9cce73ed447d9d3dd5c5e98c274f4dd9b0 Mon Sep 17 00:00:00 2001 From: Michael Matloka Date: Thu, 28 Oct 2021 19:59:19 +0200 Subject: [PATCH 3/5] Avoid moving items from array --- tests/tests.rs | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/tests/tests.rs b/tests/tests.rs index 42f073ee..29e8e676 100644 --- a/tests/tests.rs +++ b/tests/tests.rs @@ -73,38 +73,38 @@ fn test_splice_works_when_it_should() { assert_eq!(&vec[..], &[0, 1, 2, 3, 4]); assert_eq!(&array[..], &[0, 1, 2, 3, 4]); // Typical case - let vec_popped: Vec<_> = vec.splice( 1..4, [11, 12, 13]).into_iter().collect(); + let vec_popped: Vec<_> = vec.splice( 1..4, vec![11, 12, 13]).into_iter().collect(); assert_eq!(&vec[..], &[0, 11, 12, 13, 4]); assert_eq!(&vec_popped, &[1, 2, 3]); - let array_popped: Vec<_> = array.splice( 1..4, [11, 12, 13]).into_iter().collect(); + let array_popped: Vec<_> = array.splice( 1..4, vec![11, 12, 13]).into_iter().collect(); assert_eq!(&array[..], &vec[..]); assert_eq!(&array_popped, &vec_popped); // `replace_with` shorter than `range` - let vec_popped: Vec<_> = vec.splice( 2..5, [21, 22]).into_iter().collect(); + let vec_popped: Vec<_> = vec.splice( 2..5, vec![21, 22]).into_iter().collect(); assert_eq!(&vec[..], &[0, 11, 21, 22]); assert_eq!(&vec_popped, &[12, 13, 4]); - let array_popped: Vec<_> = array.splice( 2..5, [21, 22]).into_iter().collect(); + let array_popped: Vec<_> = array.splice( 2..5, vec![21, 22]).into_iter().collect(); assert_eq!(&array[..], &vec[..]); assert_eq!(&array_popped, &vec_popped); // `range` shorter than `replace_with` - let vec_popped: Vec<_> = vec.splice( 3..4, [31, 32]).into_iter().collect(); + let vec_popped: Vec<_> = vec.splice( 3..4, vec![31, 32]).into_iter().collect(); assert_eq!(&vec[..], &[0, 11, 21, 31, 32]); assert_eq!(&vec_popped, &[22]); - let array_popped: Vec<_> = array.splice( 3..4, [31, 32]).into_iter().collect(); + let array_popped: Vec<_> = array.splice( 3..4, vec![31, 32]).into_iter().collect(); assert_eq!(&array[..], &vec[..]); assert_eq!(&array_popped, &vec_popped); //`replace_with` shorter than open `range` - let vec_popped: Vec<_> = vec.splice( 1.., [41, 42]).into_iter().collect(); + let vec_popped: Vec<_> = vec.splice( 1.., vec![41, 42]).into_iter().collect(); assert_eq!(&vec[..], &[0, 41, 42]); assert_eq!(&vec_popped, &[11, 21, 31, 32]); - let array_popped: Vec<_> = array.splice( 1.., [41, 42]).into_iter().collect(); + let array_popped: Vec<_> = array.splice( 1.., vec![41, 42]).into_iter().collect(); assert_eq!(&array[..], &vec[..]); assert_eq!(&array_popped, &vec_popped); // `range` shorter than open `replace_with` - let vec_popped: Vec<_> = vec.splice( 3.., [51, 52]).into_iter().collect(); + let vec_popped: Vec<_> = vec.splice( 3.., vec![51, 52]).into_iter().collect(); assert_eq!(&vec[..], &[0, 41, 42, 51, 52]); assert_eq!(&vec_popped, &[]); - let array_popped: Vec<_> = array.splice( 3.., [51, 52]).into_iter().collect(); + let array_popped: Vec<_> = array.splice( 3.., vec![51, 52]).into_iter().collect(); assert_eq!(&array[..], &vec[..]); assert_eq!(&array_popped, &vec_popped); } @@ -116,12 +116,12 @@ fn test_splice_fails_on_overflowing_range() { // Comparing behavior with std::vec::Vec to make sure it's analogous let mut vec: Vec<_> = (0..5).collect(); let mut array: ArrayVec<_, 5> = (0..5).collect(); - let vec_popped: Vec<_> = vec.splice( 4..6, [11]).into_iter().collect(); + let vec_popped: Vec<_> = vec.splice( 4..6, vec![11]).into_iter().collect(); // This is actually less than 5! // Maybe this would be fine, but we don't want to allow using overflowing ranges with ArrayVec anyway assert_eq!(&vec[..], &[0, 1, 2, 11]); assert_eq!(&vec_popped, &[3, 4]); - array.splice(4..6, [11]); + array.splice(4..6, vec![11]); } #[test] @@ -130,12 +130,12 @@ fn test_splice_fails_if_operation_would_enlarge_vec() { // Comparing behavior with std::vec::Vec to make sure it's analogous let mut vec: Vec<_> = (0..5).collect(); let mut array: ArrayVec<_, 5> = (0..5).collect(); - let vec_popped: Vec<_> = vec.splice( 4..5, [11, 12, 13]).into_iter().collect(); + let vec_popped: Vec<_> = vec.splice( 4..5, vec![11, 12, 13]).into_iter().collect(); // This is now 6 instead of 5! // No way for this ArrayVec to fit this in assert_eq!(&vec[..], &[0, 1, 2, 11, 12, 13]); assert_eq!(&vec_popped, &[3, 4]); - array.splice(4..5, [11, 12, 13]); + array.splice(4..5, vec![11, 12, 13]); } #[test] From 9575846a0aa2e321f2be6ac42aeb61c6be470e58 Mon Sep 17 00:00:00 2001 From: Michael Matloka Date: Thu, 28 Oct 2021 20:11:14 +0200 Subject: [PATCH 4/5] Remove extra newline --- tests/tests.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/tests.rs b/tests/tests.rs index 29e8e676..0b391f74 100644 --- a/tests/tests.rs +++ b/tests/tests.rs @@ -109,7 +109,6 @@ fn test_splice_works_when_it_should() { assert_eq!(&array_popped, &vec_popped); } - #[test] #[should_panic] fn test_splice_fails_on_overflowing_range() { From ee3a8c293baec8c44b6677222226a33f58fa95ca Mon Sep 17 00:00:00 2001 From: Michael Matloka Date: Fri, 29 Oct 2021 00:27:29 +0200 Subject: [PATCH 5/5] Fix doc test --- src/arrayvec.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/arrayvec.rs b/src/arrayvec.rs index 4bb98422..64a7b454 100644 --- a/src/arrayvec.rs +++ b/src/arrayvec.rs @@ -576,7 +576,7 @@ impl ArrayVec { /// use arrayvec::ArrayVec; /// /// let mut vec: ArrayVec = ArrayVec::from_iter((0..4)); - /// let elements_popped: Vec<_> = vec.splice(1..3, [7, 9]).into_iter().collect(); + /// let elements_popped: Vec<_> = vec.splice(1..3, vec![7, 9]).into_iter().collect(); /// assert_eq!(&vec[..], &[0, 7, 9, 3]); /// assert_eq!(&elements_popped[..], &[1, 2]); /// ```