From 0f85bc691849ab0a4305d527b11ff07b49341ee1 Mon Sep 17 00:00:00 2001 From: overlookmotel <557937+overlookmotel@users.noreply.github.com> Date: Wed, 22 Jan 2025 13:17:57 +0000 Subject: [PATCH] refactor(allocator): reduce repeat code to prevent `Drop` types in arena (#8655) Pure refactor. Reduce repeated code. --- crates/oxc_allocator/src/boxed.rs | 20 ++++++++++---------- crates/oxc_allocator/src/hash_map.rs | 25 ++++++++++++++++--------- crates/oxc_allocator/src/lib.rs | 6 +----- crates/oxc_allocator/src/vec.rs | 23 ++++++++++------------- 4 files changed, 37 insertions(+), 37 deletions(-) diff --git a/crates/oxc_allocator/src/boxed.rs b/crates/oxc_allocator/src/boxed.rs index e78d7191b7774..e14206d23eaaa 100644 --- a/crates/oxc_allocator/src/boxed.rs +++ b/crates/oxc_allocator/src/boxed.rs @@ -7,7 +7,6 @@ use std::{ fmt::{self, Debug, Formatter}, hash::{Hash, Hasher}, marker::PhantomData, - mem::needs_drop, ops::{self, Deref}, ptr::{self, NonNull}, }; @@ -31,6 +30,13 @@ use crate::Allocator; /// with a [`Drop`] type. pub struct Box<'alloc, T: ?Sized>(NonNull, PhantomData<(&'alloc (), T)>); +impl Box<'_, T> { + /// Const assertion that `T` is not `Drop`. + /// Must be referenced in all methods which create a `Box`. + const ASSERT_T_IS_NOT_DROP: () = + assert!(!std::mem::needs_drop::(), "Cannot create a Box where T is a Drop type"); +} + impl Box<'_, T> { /// Put a `value` into a memory arena and get back a [`Box`] with ownership /// to the allocation. @@ -48,9 +54,7 @@ impl Box<'_, T> { #[expect(clippy::inline_always)] #[inline(always)] pub fn new_in(value: T, allocator: &Allocator) -> Self { - const { - assert!(!needs_drop::(), "Cannot create a Box where T is a Drop type"); - } + const { Self::ASSERT_T_IS_NOT_DROP }; Self(NonNull::from(allocator.alloc(value)), PhantomData) } @@ -62,9 +66,7 @@ impl Box<'_, T> { /// Only purpose is for mocking types without allocating for const assertions. #[allow(unsafe_code, clippy::missing_safety_doc)] pub const unsafe fn dangling() -> Self { - const { - assert!(!needs_drop::(), "Cannot create a Box where T is a Drop type"); - } + const { Self::ASSERT_T_IS_NOT_DROP }; Self(NonNull::dangling(), PhantomData) } @@ -115,9 +117,7 @@ impl Box<'_, T> { #[expect(clippy::inline_always)] #[inline(always)] pub(crate) const unsafe fn from_non_null(ptr: NonNull) -> Self { - const { - assert!(!needs_drop::(), "Cannot create a Box where T is a Drop type"); - } + const { Self::ASSERT_T_IS_NOT_DROP }; Self(ptr, PhantomData) } diff --git a/crates/oxc_allocator/src/hash_map.rs b/crates/oxc_allocator/src/hash_map.rs index 837bedcec3013..0481009404467 100644 --- a/crates/oxc_allocator/src/hash_map.rs +++ b/crates/oxc_allocator/src/hash_map.rs @@ -9,7 +9,7 @@ use std::{ hash::Hash, - mem::{needs_drop, ManuallyDrop}, + mem::ManuallyDrop, ops::{Deref, DerefMut}, }; @@ -59,16 +59,26 @@ unsafe impl Sync for HashMap<'_, K, V> {} // Wrap them in `ManuallyDrop` to prevent that. impl<'alloc, K, V> HashMap<'alloc, K, V> { + /// Const assertions that `K` and `V` are not `Drop`. + /// Must be referenced in all methods which create a `HashMap`. + const ASSERT_K_AND_V_ARE_NOT_DROP: () = { + assert!( + !std::mem::needs_drop::(), + "Cannot create a HashMap where K is a Drop type" + ); + assert!( + !std::mem::needs_drop::(), + "Cannot create a HashMap where V is a Drop type" + ); + }; + /// Creates an empty [`HashMap`]. It will be allocated with the given allocator. /// /// The hash map is initially created with a capacity of 0, so it will not allocate /// until it is first inserted into. #[inline(always)] pub fn new_in(allocator: &'alloc Allocator) -> Self { - const { - assert!(!needs_drop::(), "Cannot create a HashMap where K is a Drop type"); - assert!(!needs_drop::(), "Cannot create a HashMap where V is a Drop type"); - } + const { Self::ASSERT_K_AND_V_ARE_NOT_DROP }; let inner = FxHashMap::with_hasher_in(FxBuildHasher, allocator.bump()); Self(ManuallyDrop::new(inner)) @@ -80,10 +90,7 @@ impl<'alloc, K, V> HashMap<'alloc, K, V> { /// If capacity is 0, the hash map will not allocate. #[inline(always)] pub fn with_capacity_in(capacity: usize, allocator: &'alloc Allocator) -> Self { - const { - assert!(!needs_drop::(), "Cannot create a HashMap where K is a Drop type"); - assert!(!needs_drop::(), "Cannot create a HashMap where V is a Drop type"); - } + const { Self::ASSERT_K_AND_V_ARE_NOT_DROP }; let inner = FxHashMap::with_capacity_and_hasher_in(capacity, FxBuildHasher, allocator.bump()); diff --git a/crates/oxc_allocator/src/lib.rs b/crates/oxc_allocator/src/lib.rs index add3cbd7f8cd9..aeb5c777cbb6f 100644 --- a/crates/oxc_allocator/src/lib.rs +++ b/crates/oxc_allocator/src/lib.rs @@ -14,8 +14,6 @@ #![warn(missing_docs)] -use std::mem::needs_drop; - use bumpalo::Bump; mod address; @@ -291,9 +289,7 @@ impl Allocator { #[expect(clippy::inline_always)] #[inline(always)] pub fn alloc(&self, val: T) -> &mut T { - const { - assert!(!needs_drop::(), "Cannot allocate Drop type in arena"); - } + const { assert!(!std::mem::needs_drop::(), "Cannot allocate Drop type in arena") }; self.bump.alloc(val) } diff --git a/crates/oxc_allocator/src/vec.rs b/crates/oxc_allocator/src/vec.rs index dc798bdf98e8d..5ff17ce1400f5 100644 --- a/crates/oxc_allocator/src/vec.rs +++ b/crates/oxc_allocator/src/vec.rs @@ -9,7 +9,7 @@ use std::{ self, fmt::{self, Debug}, hash::{Hash, Hasher}, - mem::{needs_drop, ManuallyDrop}, + mem::ManuallyDrop, ops, ptr::NonNull, slice::SliceIndex, @@ -43,6 +43,11 @@ unsafe impl Send for Vec<'_, T> {} unsafe impl Sync for Vec<'_, T> {} impl<'alloc, T> Vec<'alloc, T> { + /// Const assertion that `T` is not `Drop`. + /// Must be referenced in all methods which create a `Vec`. + const ASSERT_T_IS_NOT_DROP: () = + assert!(!std::mem::needs_drop::(), "Cannot create a Vec where T is a Drop type"); + /// Constructs a new, empty `Vec`. /// /// The vector will not allocate until elements are pushed onto it. @@ -58,9 +63,7 @@ impl<'alloc, T> Vec<'alloc, T> { /// ``` #[inline(always)] pub fn new_in(allocator: &'alloc Allocator) -> Self { - const { - assert!(!needs_drop::(), "Cannot create a Vec where T is a Drop type"); - } + const { Self::ASSERT_T_IS_NOT_DROP }; Self(ManuallyDrop::new(InnerVec::new_in(allocator.bump()))) } @@ -113,9 +116,7 @@ impl<'alloc, T> Vec<'alloc, T> { /// ``` #[inline(always)] pub fn with_capacity_in(capacity: usize, allocator: &'alloc Allocator) -> Self { - const { - assert!(!needs_drop::(), "Cannot create a Vec where T is a Drop type"); - } + const { Self::ASSERT_T_IS_NOT_DROP }; Self(ManuallyDrop::new(InnerVec::with_capacity_in(capacity, allocator.bump()))) } @@ -126,9 +127,7 @@ impl<'alloc, T> Vec<'alloc, T> { /// This is behaviorially identical to [`FromIterator::from_iter`]. #[inline] pub fn from_iter_in>(iter: I, allocator: &'alloc Allocator) -> Self { - const { - assert!(!needs_drop::(), "Cannot create a Vec where T is a Drop type"); - } + const { Self::ASSERT_T_IS_NOT_DROP }; let iter = iter.into_iter(); let hint = iter.size_hint(); @@ -155,9 +154,7 @@ impl<'alloc, T> Vec<'alloc, T> { /// ``` #[inline] pub fn from_array_in(array: [T; N], allocator: &'alloc Allocator) -> Self { - const { - assert!(!needs_drop::(), "Cannot create a Vec where T is a Drop type"); - } + const { Self::ASSERT_T_IS_NOT_DROP }; let boxed = Box::new_in(array, allocator); let ptr = Box::into_non_null(boxed).as_ptr().cast::();