Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(allocator): reduce repeat code to prevent Drop types in arena #8655

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 10 additions & 10 deletions crates/oxc_allocator/src/boxed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ use std::{
fmt::{self, Debug, Formatter},
hash::{Hash, Hasher},
marker::PhantomData,
mem::needs_drop,
ops::{self, Deref},
ptr::{self, NonNull},
};
Expand All @@ -31,6 +30,13 @@ use crate::Allocator;
/// with a [`Drop`] type.
pub struct Box<'alloc, T: ?Sized>(NonNull<T>, PhantomData<(&'alloc (), T)>);

impl<T: ?Sized> 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::<T>(), "Cannot create a Box<T> where T is a Drop type");
}

impl<T> Box<'_, T> {
/// Put a `value` into a memory arena and get back a [`Box`] with ownership
/// to the allocation.
Expand All @@ -48,9 +54,7 @@ impl<T> Box<'_, T> {
#[expect(clippy::inline_always)]
#[inline(always)]
pub fn new_in(value: T, allocator: &Allocator) -> Self {
const {
assert!(!needs_drop::<T>(), "Cannot create a Box<T> where T is a Drop type");
}
const { Self::ASSERT_T_IS_NOT_DROP };

Self(NonNull::from(allocator.alloc(value)), PhantomData)
}
Expand All @@ -62,9 +66,7 @@ impl<T> 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::<T>(), "Cannot create a Box<T> where T is a Drop type");
}
const { Self::ASSERT_T_IS_NOT_DROP };

Self(NonNull::dangling(), PhantomData)
}
Expand Down Expand Up @@ -115,9 +117,7 @@ impl<T: ?Sized> Box<'_, T> {
#[expect(clippy::inline_always)]
#[inline(always)]
pub(crate) const unsafe fn from_non_null(ptr: NonNull<T>) -> Self {
const {
assert!(!needs_drop::<T>(), "Cannot create a Box<T> where T is a Drop type");
}
const { Self::ASSERT_T_IS_NOT_DROP };

Self(ptr, PhantomData)
}
Expand Down
25 changes: 16 additions & 9 deletions crates/oxc_allocator/src/hash_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

use std::{
hash::Hash,
mem::{needs_drop, ManuallyDrop},
mem::ManuallyDrop,
ops::{Deref, DerefMut},
};

Expand Down Expand Up @@ -59,16 +59,26 @@ unsafe impl<K, V> 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::<K>(),
"Cannot create a HashMap<K, V> where K is a Drop type"
);
assert!(
!std::mem::needs_drop::<V>(),
"Cannot create a HashMap<K, V> 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::<K>(), "Cannot create a HashMap<K, V> where K is a Drop type");
assert!(!needs_drop::<V>(), "Cannot create a HashMap<K, V> 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))
Expand All @@ -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::<K>(), "Cannot create a HashMap<K, V> where K is a Drop type");
assert!(!needs_drop::<V>(), "Cannot create a HashMap<K, V> 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());
Expand Down
6 changes: 1 addition & 5 deletions crates/oxc_allocator/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@

#![warn(missing_docs)]

use std::mem::needs_drop;

use bumpalo::Bump;

mod address;
Expand Down Expand Up @@ -291,9 +289,7 @@ impl Allocator {
#[expect(clippy::inline_always)]
#[inline(always)]
pub fn alloc<T>(&self, val: T) -> &mut T {
const {
assert!(!needs_drop::<T>(), "Cannot allocate Drop type in arena");
}
const { assert!(!std::mem::needs_drop::<T>(), "Cannot allocate Drop type in arena") };

self.bump.alloc(val)
}
Expand Down
23 changes: 10 additions & 13 deletions crates/oxc_allocator/src/vec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use std::{
self,
fmt::{self, Debug},
hash::{Hash, Hasher},
mem::{needs_drop, ManuallyDrop},
mem::ManuallyDrop,
ops,
ptr::NonNull,
slice::SliceIndex,
Expand Down Expand Up @@ -43,6 +43,11 @@ unsafe impl<T> Send for Vec<'_, T> {}
unsafe impl<T> 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::<T>(), "Cannot create a Vec<T> where T is a Drop type");

/// Constructs a new, empty `Vec<T>`.
///
/// The vector will not allocate until elements are pushed onto it.
Expand All @@ -58,9 +63,7 @@ impl<'alloc, T> Vec<'alloc, T> {
/// ```
#[inline(always)]
pub fn new_in(allocator: &'alloc Allocator) -> Self {
const {
assert!(!needs_drop::<T>(), "Cannot create a Vec<T> where T is a Drop type");
}
const { Self::ASSERT_T_IS_NOT_DROP };

Self(ManuallyDrop::new(InnerVec::new_in(allocator.bump())))
}
Expand Down Expand Up @@ -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::<T>(), "Cannot create a Vec<T> where T is a Drop type");
}
const { Self::ASSERT_T_IS_NOT_DROP };

Self(ManuallyDrop::new(InnerVec::with_capacity_in(capacity, allocator.bump())))
}
Expand All @@ -126,9 +127,7 @@ impl<'alloc, T> Vec<'alloc, T> {
/// This is behaviorially identical to [`FromIterator::from_iter`].
#[inline]
pub fn from_iter_in<I: IntoIterator<Item = T>>(iter: I, allocator: &'alloc Allocator) -> Self {
const {
assert!(!needs_drop::<T>(), "Cannot create a Vec<T> where T is a Drop type");
}
const { Self::ASSERT_T_IS_NOT_DROP };

let iter = iter.into_iter();
let hint = iter.size_hint();
Expand All @@ -155,9 +154,7 @@ impl<'alloc, T> Vec<'alloc, T> {
/// ```
#[inline]
pub fn from_array_in<const N: usize>(array: [T; N], allocator: &'alloc Allocator) -> Self {
const {
assert!(!needs_drop::<T>(), "Cannot create a Vec<T> 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::<T>();
Expand Down
Loading