From b0e5acfdb44e5931ee81d7969787ab5f7a4f8cfb Mon Sep 17 00:00:00 2001 From: Adam Cimarosti Date: Wed, 25 Sep 2024 16:28:37 +0100 Subject: [PATCH] code review feedback --- src/vec.rs | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/src/vec.rs b/src/vec.rs index 6de62d3..32ec7d8 100644 --- a/src/vec.rs +++ b/src/vec.rs @@ -119,15 +119,11 @@ impl Vec { #[inline] pub fn extend_from_slice(&mut self, slice: &[T]) -> Result<(), TryReserveError> { self.reserve(slice.len())?; - // SAFETY: we just reserved space for `slice.len()` more elements. - unsafe { - let len = self.inner.len(); - let end = self.inner.as_mut_ptr().add(len); - for (i, value) in slice.iter().enumerate() { - core::ptr::write(end.add(i), value.clone()); - } - self.inner.set_len(len + slice.len()); - } + + // Yes, we re-evaluate the capacity by delegating to the inner Vec, + // but we also gain the optimizations available via specific implementations + // for anything that supports the `Copy` trait. + self.inner.extend_from_slice(slice); Ok(()) } } @@ -367,8 +363,20 @@ mod tests { assert_eq!(vec[3], 4); } + /// A type that implements `Clone` but not `Copy`. + #[derive(Clone, Eq, PartialEq)] + struct Cloneable(i32); + + #[test] + fn test_extend_from_slice_clone() { + let wma = WatermarkAllocator::new(32); + let mut vec = Vec::new_in(wma); + vec.extend_from_slice(&[Cloneable(1), Cloneable(2), Cloneable(3), Cloneable(4)]) + .unwrap(); + } + #[test] - fn test_extend_from_slice() { + fn test_extend_from_slice_copy() { let wma = WatermarkAllocator::new(32); let mut vec = Vec::new_in(wma); vec.extend_from_slice(&[1, 2, 3, 4]).unwrap();