diff --git a/crates/oxc_allocator/src/boxed.rs b/crates/oxc_allocator/src/boxed.rs index c5e1e05aa79b8..68a4a133485c1 100644 --- a/crates/oxc_allocator/src/boxed.rs +++ b/crates/oxc_allocator/src/boxed.rs @@ -7,6 +7,7 @@ use std::{ fmt::{self, Debug, Formatter}, hash::{Hash, Hasher}, marker::PhantomData, + mem::needs_drop, ops::{self, Deref}, ptr::{self, NonNull}, }; @@ -18,14 +19,16 @@ use crate::Allocator; /// A `Box` without [`Drop`], which stores its data in the arena allocator. /// -/// Should only be used for storing AST types. +/// ## No `Drop`s /// -/// Must NOT be used to store types which have a [`Drop`] implementation. -/// `T::drop` will NOT be called on the `Box`'s contents when the `Box` is dropped. -/// If `T` owns memory outside of the arena, this will be a memory leak. +/// Objects allocated into Oxc memory arenas are never [`Dropped`](Drop). Memory is released in bulk +/// when the allocator is dropped, without dropping the individual objects in the arena. /// -/// Note: This is not a soundness issue, as Rust does not support relying on `drop` -/// being called to guarantee soundness. +/// Therefore, it would produce a memory leak if you allocated [`Drop`] types into the arena +/// which own memory allocations outside the arena. +/// +/// Static checks make this impossible to do. [`Box::new_in`] will refuse to compile if called +/// with a [`Drop`] type. pub struct Box<'alloc, T: ?Sized>(NonNull, PhantomData<(&'alloc (), T)>); impl Box<'_, T> { @@ -68,6 +71,10 @@ impl Box<'_, T> { /// let in_arena: Box = Box::new_in(5, &arena); /// ``` pub fn new_in(value: T, allocator: &Allocator) -> Self { + const { + assert!(!needs_drop::(), "Cannot create a Box where T is a Drop type"); + } + Self(NonNull::from(allocator.alloc(value)), PhantomData) } @@ -78,6 +85,10 @@ 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"); + } + Self(NonNull::dangling(), PhantomData) } } @@ -97,6 +108,10 @@ impl Box<'_, T> { /// `ptr` must have been created from a `*mut T` or `&mut T` (not a `*const T` / `&T`). #[inline] 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"); + } + Self(ptr, PhantomData) } diff --git a/crates/oxc_allocator/src/hash_map.rs b/crates/oxc_allocator/src/hash_map.rs index cded3efcd9ada..acc5ce8bfe7d9 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::ManuallyDrop, + mem::{needs_drop, ManuallyDrop}, ops::{Deref, DerefMut}, }; @@ -36,12 +36,16 @@ type FxHashMap<'alloc, K, V> = hashbrown::HashMap(ManuallyDrop>); @@ -56,6 +60,11 @@ impl<'alloc, K, V> HashMap<'alloc, K, V> { /// 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"); + } + let inner = FxHashMap::with_hasher_in(FxBuildHasher, allocator.bump()); Self(ManuallyDrop::new(inner)) } @@ -66,6 +75,11 @@ 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"); + } + let inner = FxHashMap::with_capacity_and_hasher_in(capacity, FxBuildHasher, allocator.bump()); Self(ManuallyDrop::new(inner)) diff --git a/crates/oxc_allocator/src/lib.rs b/crates/oxc_allocator/src/lib.rs index f446c3f4c6ac6..0e3396cb41eb8 100644 --- a/crates/oxc_allocator/src/lib.rs +++ b/crates/oxc_allocator/src/lib.rs @@ -5,27 +5,41 @@ //! memory management data types from `std` adapted to use this arena. //! //! ## No `Drop`s -//! Objects allocated into oxc memory arenas are never [`Dropped`](Drop), making -//! it relatively easy to leak memory if you're not careful. Memory is released -//! in bulk when the allocator is dropped. +//! +//! Objects allocated into Oxc memory arenas are never [`Dropped`](Drop). +//! Memory is released in bulk when the allocator is dropped, without dropping the individual +//! objects in the arena. +//! +//! Therefore, it would produce a memory leak if you allocated [`Drop`] types into the arena +//! which own memory allocations outside the arena. +//! +//! Static checks make this impossible to do. [`Allocator::alloc`], [`Box::new_in`], [`Vec::new_in`], +//! and all other methods which store data in the arena will refuse to compile if called with +//! a [`Drop`] type. //! //! ## Examples -//! ``` +//! +//! ```ignore //! use oxc_allocator::{Allocator, Box}; //! //! struct Foo { //! pub a: i32 //! } +//! //! impl std::ops::Drop for Foo { -//! fn drop(&mut self) { -//! // Arena boxes are never dropped. -//! unreachable!(); -//! } +//! fn drop(&mut self) {} +//! } +//! +//! struct Bar { +//! v: std::vec::Vec, //! } //! //! let allocator = Allocator::default(); +//! +//! // This will fail to compile because `Foo` implements `Drop` //! let foo = Box::new_in(Foo { a: 0 }, &allocator); -//! drop(foo); +//! // This will fail to compile because `Bar` contains a `std::vec::Vec`, and it implements `Drop` +//! let bar = Box::new_in(Bar { v: vec![1, 2, 3] }, &allocator); //! ``` //! //! Consumers of the [`oxc` umbrella crate](https://crates.io/crates/oxc) pass @@ -41,6 +55,8 @@ #![warn(missing_docs)] +use std::mem::needs_drop; + use bumpalo::Bump; mod address; @@ -92,6 +108,10 @@ 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"); + } + self.bump.alloc(val) } diff --git a/crates/oxc_allocator/src/vec.rs b/crates/oxc_allocator/src/vec.rs index 3589a4b32c46d..08c4168b1eb3c 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::ManuallyDrop, + mem::{needs_drop, ManuallyDrop}, ops, ptr::NonNull, slice::SliceIndex, @@ -25,12 +25,16 @@ use crate::{Allocator, Box, String}; /// A `Vec` without [`Drop`], which stores its data in the arena allocator. /// -/// Must NOT be used to store types which have a [`Drop`] implementation. -/// `T::drop` will NOT be called on the `Vec`'s contents when the `Vec` is dropped. -/// If `T` owns memory outside of the arena, this will be a memory leak. +/// ## No `Drop`s /// -/// Note: This is not a soundness issue, as Rust does not support relying on `drop` -/// being called to guarantee soundness. +/// Objects allocated into Oxc memory arenas are never [`Dropped`](Drop). Memory is released in bulk +/// when the allocator is dropped, without dropping the individual objects in the arena. +/// +/// Therefore, it would produce a memory leak if you allocated [`Drop`] types into the arena +/// which own memory allocations outside the arena. +/// +/// Static checks make this impossible to do. [`Vec::new_in`] and all other methods which create +/// a [`Vec`] will refuse to compile if called with a [`Drop`] type. #[derive(PartialEq, Eq)] pub struct Vec<'alloc, T>(pub(crate) ManuallyDrop>); @@ -56,6 +60,10 @@ 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"); + } + Self(ManuallyDrop::new(InnerVec::new_in(allocator.bump()))) } @@ -108,6 +116,10 @@ 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"); + } + Self(ManuallyDrop::new(InnerVec::with_capacity_in(capacity, allocator.bump()))) } @@ -117,6 +129,10 @@ 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"); + } + let iter = iter.into_iter(); let hint = iter.size_hint(); let capacity = hint.1.unwrap_or(hint.0); @@ -143,6 +159,10 @@ 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"); + } + let boxed = Box::new_in(array, allocator); let ptr = Box::into_non_null(boxed).as_ptr().cast::(); // SAFETY: `ptr` has correct alignment - it was just allocated as `[T; N]`.