From 4131a127cc1b5cadf6182127734ab3f5c00505ec Mon Sep 17 00:00:00 2001 From: soruh Date: Sun, 26 Jul 2026 14:28:30 +0200 Subject: [PATCH] continue refactor --- src/guard.rs | 12 +- src/lib.rs | 49 ++++--- src/send.rs | 6 +- src/tests.rs | 371 ++++++++++++++++++++++++++++++++++---------------- src/vtable.rs | 47 +++---- 5 files changed, 310 insertions(+), 175 deletions(-) diff --git a/src/guard.rs b/src/guard.rs index d98aafe..1b5879f 100644 --- a/src/guard.rs +++ b/src/guard.rs @@ -21,13 +21,7 @@ impl<'vec, T> ContentGuard<'vec, T> { let erased = ManuallyDrop::new(erased); // SAFETY: The erased pointer, length, and capacity are valid for a Vec. // ManuallyDrop prevents the old TypeErasedVec from double-freeing the allocation. - unsafe { - Vec::from_raw_parts( - erased.parts.ptr.as_ptr().cast::(), - erased.parts.len, - erased.parts.cap, - ) - } + unsafe { erased.parts.into_vec() } } pub fn with(&mut self, f: impl FnOnce(&mut Vec) -> R) -> R { @@ -41,7 +35,7 @@ impl<'vec, T> ContentGuard<'vec, T> { } pub fn clear(&mut self) { - self.with(|vec| vec.clear()); + self.with(Vec::clear); } pub fn reserve(&mut self, additional: usize) { @@ -63,6 +57,7 @@ impl<'vec, T> ContentGuard<'vec, T> { self.erased.is_empty() } + #[must_use] pub fn as_slice(&self) -> &[T] { // SAFETY: The pointer and length correctly represent the currently initialized elements. unsafe { @@ -70,6 +65,7 @@ impl<'vec, T> ContentGuard<'vec, T> { } } + #[must_use] pub fn as_slice_mut(&mut self) -> &mut [T] { // SAFETY: The pointer and length correctly represent the currently initialized elements. unsafe { diff --git a/src/lib.rs b/src/lib.rs index f0c125b..bbf429e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -22,7 +22,7 @@ pub(crate) struct VecParts { } impl VecParts { - pub(crate) fn new(vec: Vec) -> Self { + pub(crate) fn from_vec(vec: Vec) -> Self { let (ptr, len, cap) = vec.into_raw_parts(); // SAFETY: Vec guarantees its underlying pointer is non-null. @@ -30,6 +30,10 @@ impl VecParts { Self { ptr, len, cap } } + + pub(crate) unsafe fn into_vec(self) -> Vec { + unsafe { Vec::from_raw_parts(self.ptr.as_ptr().cast::(), self.len, self.cap) } + } } /// Stores the capacity of a `Vec` for later reuse as a `Vec` where `V` shares the same `Layout` as `U`. @@ -52,7 +56,7 @@ impl TypeErasedVec { #[must_use] pub fn new(vec: Vec) -> Self { Self { - parts: VecParts::new(vec), + parts: VecParts::from_vec(vec), layout: Layout::new::(), vtable: TypeErasedVecVtable::new::(), } @@ -61,13 +65,13 @@ impl TypeErasedVec { /// Clear any remaining elements in the vector, dropping them. pub fn clear(&mut self) { // SAFETY: The clear function correctly targets the currently stored type elements. - self.parts = unsafe { (self.vtable.clear_vector)(self.parts) }; + self.parts = unsafe { (self.vtable.clear)(self.parts) }; } /// Reserve additional elements in the vector. pub fn reserve(&mut self, additional: usize) { // SAFETY: The reserve function correctly targets the currently stored type elements. - self.parts = unsafe { (self.vtable.reserve_vector)(self.parts, additional) }; + self.parts = unsafe { (self.vtable.reserve)(self.parts, additional) }; } #[must_use] @@ -101,28 +105,35 @@ impl TypeErasedVec { /// # Safety /// The caller must ensure that the existing elements can be safely transmuted /// into the new type `T` and that dropping them as `T` is sound. - pub unsafe fn try_cast_type<'vec, T>(&'vec mut self) -> Option> { - if self.layout != Layout::new::() { - return None; - } + /// + /// The `Layout` of the new type `T` does not match the `Layout` of the allocated capacity + pub unsafe fn cast_type(&mut self) -> ContentGuard<'_, T> { + debug_assert_eq!(self.layout, Layout::new::()); // Overwrite the vtable completely to target the new type. // This guarantees that any subsequent drop or clear invokes the correct destructors. self.vtable = TypeErasedVecVtable::new::(); - Some(ContentGuard::new(self)) + ContentGuard::new(self) } /// Access the erased capacity with a temporarily fixed type. /// Clears all elements currently stored in the `TypeErasedVec`. - pub fn try_to_type<'vec, T>(&'vec mut self) -> Option> { + /// + /// Will fail if `T` does not have the same `Layout` as the underlying capacity. + pub fn try_to_type(&mut self) -> Option> { self.clear(); // SAFETY: The vector has been cleared, meaning there are no existing elements // that could be invalidated or improperly dropped by the cast. - unsafe { self.try_cast_type() } + (self.layout == Layout::new::()).then(|| ContentGuard::new(self)) } - pub fn to_type<'vec, T>(&'vec mut self) -> ContentGuard<'vec, T> { + /// Access the erased capacity with a temporarily fixed type. + /// Clears all elements currently stored in the `TypeErasedVec`. + /// + /// # Panics + /// Panics if `T` does not have the same `Layout` as the underlying capacity. + pub fn to_type(&mut self) -> ContentGuard<'_, T> { let layout = self.layout; self.try_to_type().unwrap_or_else(|| { panic!( @@ -134,9 +145,12 @@ impl TypeErasedVec { }) } + /// Access the erased capacity with a temporarily fixed type. + /// Clears all elements currently stored in the `TypeErasedVec`. + /// /// # Safety - /// `T` must perfectly match the `Layout` of the erased allocation. - pub unsafe fn to_type_unchecked<'vec, T>(&'vec mut self) -> ContentGuard<'vec, T> { + /// `T` must have the same `Layout` as the underlying capacity. + pub unsafe fn to_type_unchecked(&mut self) -> ContentGuard<'_, T> { let layout = self.layout; let res = self.try_to_type(); @@ -185,17 +199,16 @@ impl TypeErasedVec { } /// Clear a type erased vec allowing it be safely shared across threads, preserving the capacity + #[must_use] pub fn send(mut self) -> SendableTypeErasedVec { self.clear(); - SendableTypeErasedVec::new(self) + unsafe { SendableTypeErasedVec::from_empty(self) } } } impl Drop for TypeErasedVec { fn drop(&mut self) { // SAFETY: The correct drop vector executes the destructors and frees the allocation. - unsafe { - (self.vtable.drop_vector)(self.parts); - } + unsafe { (self.vtable.drop)(self.parts) }; } } diff --git a/src/send.rs b/src/send.rs index 9ac6579..923ec2a 100644 --- a/src/send.rs +++ b/src/send.rs @@ -3,10 +3,14 @@ use crate::TypeErasedVec; pub struct SendableTypeErasedVec(TypeErasedVec); impl SendableTypeErasedVec { - pub(crate) fn new(vec: TypeErasedVec) -> Self { + /// # SAFETY + /// the `vec` must not contain any initialized elements + pub(crate) unsafe fn from_empty(vec: TypeErasedVec) -> Self { + debug_assert!(vec.is_empty()); Self(vec) } + #[must_use] pub fn unpack(self) -> TypeErasedVec { self.0 } diff --git a/src/tests.rs b/src/tests.rs index 691e8f2..f240a46 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -1,165 +1,300 @@ use super::*; +use std::alloc::Layout; +use std::cell::Cell; +use std::panic; use std::rc::Rc; -#[test] -fn test_basic_roundtrip() { - let mut v = Vec::::with_capacity(10); - v.push(1); - v.push(2); - let cap = v.capacity(); +struct DropTracker { + counter: Rc>, +} - let erased = TypeErasedVec::new(v); - let recovered: Vec = erased.into_vec(); +impl DropTracker { + fn new(counter: Rc>) -> Self { + Self { counter } + } +} - assert_eq!(recovered.len(), 0); - assert_eq!(recovered.capacity(), cap); +impl Drop for DropTracker { + fn drop(&mut self) { + self.counter.set(self.counter.get() + 1); + } +} + +#[track_caller] +fn assert_erased_state( + erased: &TypeErasedVec, + expected_len: usize, + expected_min_cap: usize, + expected_layout: Layout, +) { + assert_eq!(erased.length(), expected_len, "Length mismatch"); + assert!(erased.capacity() >= expected_min_cap, "Capacity too small"); + assert_eq!(erased.is_empty(), expected_len == 0, "is_empty mismatch"); + assert_eq!(erased.layout(), expected_layout, "Layout mismatch"); + assert_eq!( + erased.capacity_bytes(), + erased.capacity() * expected_layout.size(), + "Capacity bytes mismatch" + ); +} + +#[track_caller] +fn assert_guard_state( + guard: &ContentGuard<'_, T>, + expected_len: usize, + expected_min_cap: usize, +) { + assert_eq!(guard.length(), expected_len, "Guard length mismatch"); + assert!( + guard.capacity() >= expected_min_cap, + "Guard capacity too small" + ); + assert_eq!( + guard.is_empty(), + expected_len == 0, + "Guard is_empty mismatch" + ); } #[test] -fn test_compatible_types() { - let mut v = Vec::::with_capacity(42); - v.push(100); - let cap = v.capacity(); +fn test_sendable_is_send_sync() { + fn assert_send() {} + fn assert_sync() {} - let erased = TypeErasedVec::new(v); - - // Convert to a different type with the exact same Layout - let recovered: Vec = erased.try_into_vec().ok().unwrap(); - - assert_eq!(recovered.len(), 0); - assert_eq!(recovered.capacity(), cap); + assert_send::(); + assert_sync::(); } #[test] -fn test_incompatible_size() { - let v = Vec::::with_capacity(10); - let erased = TypeErasedVec::new(v); +fn test_new_and_basic_properties() { + let mut vec = Vec::::with_capacity(10); + vec.extend_from_slice(&[1, 2, 3, 4, 5]); + let initial_cap = vec.capacity(); - let result: Result, TypeErasedVec> = erased.try_into_vec(); + let erased = TypeErasedVec::new(vec); - // Result should be Err containing the original erased vec - let Err(erased) = result else { - panic!("Expected conversion from u64 to u8 to fail"); + assert_erased_state(&erased, 5, initial_cap, Layout::new::()); +} + +#[test] +fn test_clear_drops_elements() { + let drop_count = Rc::new(Cell::new(0)); + let mut vec = Vec::with_capacity(5); + + for _ in 0..5 { + vec.push(DropTracker::new(drop_count.clone())); + } + + let mut erased = TypeErasedVec::new(vec); + assert_erased_state(&erased, 5, 5, Layout::new::()); + assert_eq!(drop_count.get(), 0); + + erased.clear(); + + assert_erased_state(&erased, 0, 5, Layout::new::()); + assert_eq!(drop_count.get(), 5); +} + +#[test] +fn test_reserve_increases_capacity() { + let vec = Vec::::with_capacity(2); + let mut erased = TypeErasedVec::new(vec); + + assert_erased_state(&erased, 0, 2, Layout::new::()); + + erased.reserve(100); + + assert_erased_state(&erased, 0, 100, Layout::new::()); +} + +#[test] +fn test_try_cast_type_success_and_mutation() { + let mut vec = Vec::::with_capacity(10); + vec.push(-1); + vec.push(-2); + let initial_cap = vec.capacity(); + + let mut erased = TypeErasedVec::new(vec); + assert_erased_state(&erased, 2, initial_cap, Layout::new::()); + + let mut guard = unsafe { erased.try_cast_type::().expect("Layouts match") }; + assert_guard_state(&guard, 2, initial_cap); + + let slice = guard.as_slice_mut(); + slice[0] = 42; + + let slice = guard.as_slice(); + assert_eq!(slice, &[42, 4294967294]); + + let restored = guard.take(); + assert_eq!(restored, vec![42, 4294967294]); + assert_guard_state(&guard, 0, 0); +} + +#[test] +fn test_try_cast_type_failure() { + let vec = Vec::::with_capacity(5); + let initial_cap = vec.capacity(); + let mut erased = TypeErasedVec::new(vec); + + assert_erased_state(&erased, 0, initial_cap, Layout::new::()); + + let guard = unsafe { erased.try_cast_type::() }; + assert!(guard.is_none(), "Should fail due to layout mismatch"); + assert_erased_state(&erased, 0, initial_cap, Layout::new::()); +} + +#[test] +fn test_try_to_type_clears_elements() { + let drop_count = Rc::new(Cell::new(0)); + let mut vec = Vec::with_capacity(5); + + for _ in 0..3 { + vec.push(DropTracker::new(drop_count.clone())); + } + + let mut erased = TypeErasedVec::new(vec); + assert_erased_state(&erased, 3, 5, Layout::new::()); + + let guard = erased.try_to_type::().expect("Layouts match"); + + assert_guard_state(&guard, 0, 5); + assert_eq!(drop_count.get(), 3); +} + +#[test] +fn test_guard_with_unwind_safety() { + let drop_count = Rc::new(Cell::new(0)); + let mut vec = Vec::with_capacity(10); + vec.push(DropTracker::new(drop_count.clone())); + + let mut erased = TypeErasedVec::new(vec); + assert_erased_state(&erased, 1, 10, Layout::new::()); + + let res = panic::catch_unwind(panic::AssertUnwindSafe(|| { + let mut guard = unsafe { erased.try_cast_type::().unwrap() }; + guard.with(|v| { + v.push(DropTracker::new(drop_count.clone())); + panic!("Simulated panic inside with()"); + }); + })); + + assert!(res.is_err()); + + assert_erased_state(&erased, 0, 0, Layout::new::()); + assert_eq!(drop_count.get(), 2); +} + +#[test] +fn test_conversion_methods() { + let mut vec = Vec::::with_capacity(5); + vec.push("test".to_string()); + let initial_cap = vec.capacity(); + + let erased = TypeErasedVec::new(vec); + assert_erased_state(&erased, 1, initial_cap, Layout::new::()); + + let Ok(restored) = erased.try_into_vec::() else { + panic!("try_into_vec failed despite matching layouts"); }; - assert_eq!(erased.capacity(), 10); - assert_eq!(erased.into_vec::().capacity(), 10); + assert_eq!(restored.len(), 0); + assert!(restored.capacity() >= initial_cap); + + let mut erased2 = TypeErasedVec::new(restored); + let mut guard = erased2.to_type::(); + guard.reserve(20); + let new_cap = guard.capacity(); + + let restored2 = guard.take(); + assert_eq!(restored2.capacity(), new_cap); } #[test] -fn test_incompatible_alignment() { - #[repr(align(16))] - struct Align16(#[allow(unused)] u8); +fn test_try_into_vec_failure_and_recovery() { + let vec = Vec::::with_capacity(5); + let initial_cap = vec.capacity(); - #[repr(align(8))] - struct Align8(#[allow(unused)] u8); + let erased = TypeErasedVec::new(vec); - let v = Vec::::with_capacity(10); - let erased = TypeErasedVec::new(v); - - // Sizes might be compatible or both be wrapped in padding, but alignment differs - let result: Result, TypeErasedVec> = erased.try_into_vec(); - - let Err(erased) = result else { - panic!("Expected conversion from u64 to u8 to fail"); + let Err(recovered_erased) = erased.try_into_vec::() else { + panic!("try_into_vec succeeded with mismatched layout"); }; - assert_eq!(erased.capacity(), 10); - assert_eq!(erased.into_vec::().capacity(), 10); + assert_erased_state(&recovered_erased, 0, initial_cap, Layout::new::()); + + let Ok(restored) = recovered_erased.try_into_vec::() else { + panic!("try_into_vec failed after recovery"); + }; + + assert!(restored.capacity() >= initial_cap); } #[test] #[should_panic(expected = "Target type layout must exactly match")] -fn test_into_vec_panic_on_mismatch() { - let v = Vec::::with_capacity(10); - let erased = TypeErasedVec::new(v); - - // This should panic due to Layout mismatch - let _panic: Vec = erased.into_vec(); +fn test_to_type_panics_on_layout_mismatch() { + let vec = Vec::::new(); + let mut erased = TypeErasedVec::new(vec); + let _ = erased.to_type::(); } #[test] -fn test_elements_are_dropped() { - let counter = Rc::new(()); - let mut v = Vec::new(); - - v.push(Rc::clone(&counter)); - v.push(Rc::clone(&counter)); - - assert_eq!(Rc::strong_count(&counter), 3); - - let erased = TypeErasedVec::new(v); - - // Elements should have been dropped during `TypeErasedVec::new` by `vec.clear()` - assert_eq!(Rc::strong_count(&counter), 1); - - std::mem::drop(erased); - - assert_eq!(Rc::strong_count(&counter), 1); +#[should_panic(expected = "Target type layout must exactly match")] +fn test_into_vec_panics_on_layout_mismatch() { + let vec = Vec::::new(); + let erased = TypeErasedVec::new(vec); + let _ = erased.into_vec::(); } #[test] -fn test_zst_handling() { - let mut v = Vec::<()>::with_capacity(10); - v.push(()); - v.push(()); - - let erased = TypeErasedVec::new(v); - let recovered: Vec<()> = erased.into_vec(); - - assert_eq!(recovered.len(), 0); +#[cfg(debug_assertions)] +#[should_panic(expected = "Calling `to_type_unchecked` with an incompatible layout is UB!")] +fn test_to_type_unchecked_panics_on_layout_mismatch_in_debug() { + let vec = Vec::::new(); + let mut erased = TypeErasedVec::new(vec); + unsafe { + let _ = erased.to_type_unchecked::(); + } } #[test] -fn test_zero_capacity() { - let v = Vec::::new(); - let erased = TypeErasedVec::new(v); - - let recovered: Vec = erased.into_vec(); - - assert_eq!(recovered.len(), 0); - assert_eq!(recovered.capacity(), 0); +#[cfg(debug_assertions)] +#[should_panic(expected = "Calling `to_type_unchecked` with an incompatible layout is UB!")] +fn test_into_vec_unchecked_panics_on_layout_mismatch_in_debug() { + let vec = Vec::::new(); + let erased = TypeErasedVec::new(vec); + unsafe { + let _ = erased.into_vec_unchecked::(); + } } #[test] -fn test_drop_erased_memory_leak() { - let v = Vec::::with_capacity(100); - let erased = TypeErasedVec::new(v); +fn test_sendable_clears_and_unpacks() { + let drop_count = Rc::new(Cell::new(0)); + let mut vec = Vec::with_capacity(5); + vec.push(DropTracker::new(drop_count.clone())); - // Miri will flag this test as failing if `erased` doesn't properly deallocate - // the underlying memory upon being dropped. - drop(erased); + let erased = TypeErasedVec::new(vec); + assert_erased_state(&erased, 1, 5, Layout::new::()); + + let sendable = erased.send(); + assert_eq!(drop_count.get(), 1); + + let unpacked = sendable.unpack(); + assert_erased_state(&unpacked, 0, 5, Layout::new::()); } #[test] -fn test_into_vec_unchecked_success() { - let mut v = Vec::with_capacity(15); - v.push(42); - let cap = v.capacity(); +fn test_drop_cleans_up_allocation() { + let drop_count = Rc::new(Cell::new(0)); + let mut vec = Vec::with_capacity(5); + vec.push(DropTracker::new(drop_count.clone())); - let erased = TypeErasedVec::new(v); - - let recovered: Vec = unsafe { erased.into_vec_unchecked() }; - - assert_eq!(recovered.len(), 0); - assert_eq!(recovered.capacity(), cap); -} - -#[test] -fn test_complex_struct_drop() { - struct Droppy { - _a: String, - _b: Vec, + { + let erased = TypeErasedVec::new(vec); + assert_erased_state(&erased, 1, 5, Layout::new::()); + assert_eq!(drop_count.get(), 0); } - let mut v = Vec::with_capacity(5); - v.push(Droppy { - _a: String::from("Hello"), - _b: vec![1, 2, 3], - }); - - let erased = TypeErasedVec::new(v); - - // Ensures `TypeErasedVec` properly drops via the generic drop function - drop(erased); + assert_eq!(drop_count.get(), 1); } diff --git a/src/vtable.rs b/src/vtable.rs index 239571c..8a7efeb 100644 --- a/src/vtable.rs +++ b/src/vtable.rs @@ -3,39 +3,31 @@ use std::ptr::NonNull; pub(crate) struct TypeErasedVecVtable { /// Function pointer to reserve additional capacity in the underlying Vec - pub reserve_vector: unsafe fn(VecParts, usize) -> VecParts, + pub reserve: unsafe fn(parts: VecParts, additional: usize) -> VecParts, /// Function pointer to clear the underlying capacity of its old elements - pub clear_vector: unsafe fn(VecParts) -> VecParts, + pub clear: unsafe fn(parts: VecParts) -> VecParts, /// Function pointer to the original type's drop logic. - pub drop_vector: unsafe fn(VecParts), + pub drop: unsafe fn(parts: VecParts), } impl TypeErasedVecVtable { pub fn new() -> Self { unsafe fn drop_vec(parts: VecParts) { // SAFETY: We reconstruct the Vec to let its Drop impl handle deallocation. - _ = unsafe { - Vec::from_raw_parts(parts.ptr.as_ptr().cast::(), parts.len, parts.cap) - }; + _ = unsafe { parts.into_vec::() }; } unsafe fn clear_vec(parts: VecParts) -> VecParts { // SAFETY: We reconstruct the Vec to call clear, dropping only the elements. // Using into_raw_parts ensures we don't accidentally drop the allocation itself. unsafe { - let mut vec = - Vec::from_raw_parts(parts.ptr.as_ptr().cast::(), parts.len, parts.cap); + let mut vec = parts.into_vec::(); vec.clear(); - let (new_ptr, new_len, new_cap) = Vec::into_raw_parts(vec); - debug_assert!(std::ptr::eq(new_ptr, parts.ptr.as_ptr().cast())); - debug_assert_eq!(new_cap, parts.cap); - debug_assert_eq!(new_len, 0); - - VecParts { - ptr: NonNull::new_unchecked(new_ptr).cast(), - len: new_len, - cap: new_cap, - } + let new_parts = VecParts::from_vec(vec); + debug_assert!(std::ptr::eq(new_parts.ptr.as_ptr(), parts.ptr.as_ptr())); + debug_assert_eq!(new_parts.cap, parts.cap); + debug_assert_eq!(new_parts.len, 0); + new_parts } } @@ -43,24 +35,19 @@ impl TypeErasedVecVtable { // SAFETY: We reconstruct the Vec to trigger a reserve. // Capacity might change upon reallocation. unsafe { - let mut vec = - Vec::from_raw_parts(parts.ptr.as_ptr().cast::(), parts.len, parts.cap); + let mut vec = parts.into_vec::(); vec.reserve(additional); - let (new_ptr, new_len, new_cap) = Vec::into_raw_parts(vec); - debug_assert_eq!(new_len, parts.len); + let new_parts = VecParts::from_vec(vec); + debug_assert_eq!(new_parts.len, parts.len); - VecParts { - ptr: NonNull::new_unchecked(new_ptr).cast(), - len: new_len, - cap: new_cap, - } + new_parts } } Self { - reserve_vector: reserve_vec::, - clear_vector: clear_vec::, - drop_vector: drop_vec::, + reserve: reserve_vec::, + clear: clear_vec::, + drop: drop_vec::, } } }