refactor(allocator)!: Allocator do not deref to bumpalo::Bump (#8569

)

Do not expose that our arena allocator is based on Bumpalo, by removing `Deref` and `DerefMut` impls. Instead implement the methods we use on `Allocator` itself.

That our allocator is based on Bumpalo is now an internal implementation detail. This will allow us to replace the allocator in future, and enables statically preventing `Drop` types from being stored in the arena (next PR in this stack).

I've intentionally only implemented a small subset of `Bump`'s methods - only `alloc`, `alloc_str`, and `reset`. This will make it simpler to implement a new allocator in future, without having to cover all of bumpalo's large API surface. In the meantime, if it turns out we need additional methods, it will be simple to add them, by just delegating to that method on `Bump`.

crates/oxc_allocator/src/allocator_api2.rs

@@ -1,47 +1,50 @@
+// All methods just delegate to `bumpalo`, so all marked `#[inline(always)]`
+#![expect(clippy::inline_always)]
+
 use std::{alloc::Layout, ptr::NonNull};
 
 use allocator_api2::alloc::{AllocError, Allocator};
 
 /// SAFETY:
 /// <https://github.com/fitzgen/bumpalo/blob/4eeab8847c85d5cde135ca21ae14a54e56b05224/src/lib.rs#L1938>
 unsafe impl Allocator for &crate::Allocator {
-    #[inline]
+    #[inline(always)]
     fn allocate(&self, layout: Layout) -> Result<NonNull<[u8]>, AllocError> {
-        (&self.bump).allocate(layout)
+        self.bump().allocate(layout)
     }
 
-    #[inline]
+    #[inline(always)]
     unsafe fn deallocate(&self, ptr: NonNull<u8>, layout: Layout) {
-        (&self.bump).deallocate(ptr, layout);
+        self.bump().deallocate(ptr, layout);
     }
 
-    #[inline]
+    #[inline(always)]
     unsafe fn shrink(
         &self,
         ptr: NonNull<u8>,
         old_layout: Layout,
         new_layout: Layout,
     ) -> Result<NonNull<[u8]>, AllocError> {
-        (&self.bump).shrink(ptr, old_layout, new_layout)
+        self.bump().shrink(ptr, old_layout, new_layout)
     }
 
-    #[inline]
+    #[inline(always)]
     unsafe fn grow(
         &self,
         ptr: NonNull<u8>,
         old_layout: Layout,
         new_layout: Layout,
     ) -> Result<NonNull<[u8]>, AllocError> {
-        (&self.bump).grow(ptr, old_layout, new_layout)
+        self.bump().grow(ptr, old_layout, new_layout)
     }
 
-    #[inline]
+    #[inline(always)]
     unsafe fn grow_zeroed(
         &self,
         ptr: NonNull<u8>,
         old_layout: Layout,
         new_layout: Layout,
     ) -> Result<NonNull<[u8]>, AllocError> {
-        (&self.bump).grow_zeroed(ptr, old_layout, new_layout)
+        self.bump().grow_zeroed(ptr, old_layout, new_layout)
     }
 }

crates/oxc_allocator/src/hash_map.rs

@@ -56,7 +56,7 @@ impl<'alloc, K, V> HashMap<'alloc, K, V> {
     /// until it is first inserted into.
     #[inline(always)]
     pub fn new_in(allocator: &'alloc Allocator) -> Self {
-        let inner = FxHashMap::with_hasher_in(FxBuildHasher, allocator);
+        let inner = FxHashMap::with_hasher_in(FxBuildHasher, allocator.bump());
         Self(ManuallyDrop::new(inner))
     }
 
@@ -66,7 +66,8 @@ 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 {
-        let inner = FxHashMap::with_capacity_and_hasher_in(capacity, FxBuildHasher, allocator);
+        let inner =
+            FxHashMap::with_capacity_and_hasher_in(capacity, FxBuildHasher, allocator.bump());
         Self(ManuallyDrop::new(inner))
     }
 

crates/oxc_allocator/src/lib.rs

@@ -31,18 +31,15 @@
 //! Consumers of the [`oxc` umbrella crate](https://crates.io/crates/oxc) pass
 //! [`Allocator`] references to other tools.
 //!
-//! ```
+//! ```ignore
 //! use oxc::{allocator::Allocator, parser::Parser, span::SourceType};
 //!
-//! let allocator = Allocator::default()
+//! let allocator = Allocator::default();
 //! let parsed = Parser::new(&allocator, "let x = 1;", SourceType::default());
 //! assert!(parsed.errors.is_empty());
 //! ```
+
 #![warn(missing_docs)]
-use std::{
-    convert::From,
-    ops::{Deref, DerefMut},
-};
 
 use bumpalo::Bump;
 
@@ -75,46 +72,121 @@ pub struct Allocator {
     bump: Bump,
 }
 
-/// SAFETY: Not actually safe, but for enabling `Send` for downstream crates.
-unsafe impl Send for Allocator {}
-/// SAFETY: Not actually safe, but for enabling `Sync` for downstream crates.
-unsafe impl Sync for Allocator {}
+impl Allocator {
+    /// Allocate an object in this [`Allocator`] and return an exclusive reference to it.
+    ///
+    /// # Panics
+    /// Panics if reserving space for `T` fails.
+    ///
+    /// # Example
+    /// ```
+    /// use oxc_allocator::Allocator;
+    ///
+    /// let allocator = Allocator::default();
+    /// let x = allocator.alloc([1u8; 20]);
+    /// assert_eq!(x, &[1u8; 20]);
+    /// ```
+    //
+    // `#[inline(always)]` because this is a very hot path and `Bump::alloc` is a very small function.
+    // We always want it to be inlined.
+    #[expect(clippy::inline_always)]
+    #[inline(always)]
+    pub fn alloc<T>(&self, val: T) -> &mut T {
+        self.bump.alloc(val)
+    }
 
-impl From<Bump> for Allocator {
-    fn from(bump: Bump) -> Self {
-        Self { bump }
+    /// Copy a string slice into this [`Allocator`] and return a reference to it.
+    ///
+    /// # Panics
+    /// Panics if reserving space for the string fails.
+    ///
+    /// # Example
+    /// ```
+    /// use oxc_allocator::Allocator;
+    /// let allocator = Allocator::default();
+    /// let hello = allocator.alloc_str("hello world");
+    /// assert_eq!(hello, "hello world");
+    /// ```
+    //
+    // `#[inline(always)]` because this is a hot path and `Bump::alloc_str` is a very small function.
+    // We always want it to be inlined.
+    #[expect(clippy::inline_always)]
+    #[inline(always)]
+    pub fn alloc_str<'alloc>(&'alloc self, src: &str) -> &'alloc mut str {
+        self.bump.alloc_str(src)
     }
-}
 
-impl Deref for Allocator {
-    type Target = Bump;
+    /// Reset this allocator.
+    ///
+    /// Performs mass deallocation on everything allocated in this arena by resetting the pointer
+    /// into the underlying chunk of memory to the start of the chunk.
+    /// Does not run any `Drop` implementations on deallocated objects.
+    ///
+    /// If this arena has allocated multiple chunks to bump allocate into, then the excess chunks
+    /// are returned to the global allocator.
+    ///
+    /// ## Example
+    ///
+    /// ```
+    /// use oxc_allocator::Allocator;
+    ///
+    /// let mut allocator = Allocator::default();
+    ///
+    /// // Allocate a bunch of things.
+    /// {
+    ///     for i in 0..100 {
+    ///         allocator.alloc(i);
+    ///     }
+    /// }
+    ///
+    /// // Reset the arena.
+    /// allocator.reset();
+    ///
+    /// // Allocate some new things in the space previously occupied by the
+    /// // original things.
+    /// for j in 200..400 {
+    ///     allocator.alloc(j);
+    /// }
+    /// ```
+    //
+    // `#[inline(always)]` because it just delegates to `bumpalo`
+    #[expect(clippy::inline_always)]
+    #[inline(always)]
+    pub fn reset(&mut self) {
+        self.bump.reset();
+    }
 
-    fn deref(&self) -> &Self::Target {
+    /// Get inner [`bumpalo::Bump`].
+    ///
+    /// This method is not public. We don't want to expose `bumpalo::Allocator` to user.
+    /// The fact that we're using `bumpalo` is an internal implementation detail.
+    //
+    // `#[inline(always)]` because it's a no-op
+    #[expect(clippy::inline_always)]
+    #[inline(always)]
+    pub(crate) fn bump(&self) -> &Bump {
         &self.bump
     }
 }
 
-impl DerefMut for Allocator {
-    fn deref_mut(&mut self) -> &mut Self::Target {
-        &mut self.bump
-    }
-}
+/// SAFETY: Not actually safe, but for enabling `Send` for downstream crates.
+unsafe impl Send for Allocator {}
+/// SAFETY: Not actually safe, but for enabling `Sync` for downstream crates.
+unsafe impl Sync for Allocator {}
 
 #[cfg(test)]
 mod test {
-    use std::ops::Deref;
-
-    use bumpalo::Bump;
-
     use crate::Allocator;
 
     #[test]
     fn test_api() {
-        let bump = Bump::new();
-        let allocator: Allocator = bump.into();
-        #[allow(clippy::explicit_deref_methods)]
+        let mut allocator = Allocator::default();
         {
-            _ = allocator.deref();
+            let array = allocator.alloc([123; 10]);
+            assert_eq!(array, &[123; 10]);
+            let str = allocator.alloc_str("hello");
+            assert_eq!(str, "hello");
         }
+        allocator.reset();
     }
 }

crates/oxc_allocator/src/string.rs

@@ -38,7 +38,7 @@ impl<'alloc> String<'alloc> {
     /// [`with_capacity_in`]: String::with_capacity_in
     #[inline(always)]
     pub fn new_in(allocator: &'alloc Allocator) -> String<'alloc> {
-        Self(BumpaloString::new_in(allocator))
+        Self(BumpaloString::new_in(allocator.bump()))
     }
 
     /// Creates a new empty [`String`] with specified capacity.
@@ -57,7 +57,7 @@ impl<'alloc> String<'alloc> {
     /// [`new_in`]: String::new_in
     #[inline(always)]
     pub fn with_capacity_in(capacity: usize, allocator: &'alloc Allocator) -> String<'alloc> {
-        Self(BumpaloString::with_capacity_in(capacity, allocator))
+        Self(BumpaloString::with_capacity_in(capacity, allocator.bump()))
     }
 
     /// Construct a new [`String`] from a string slice.
@@ -74,7 +74,7 @@ impl<'alloc> String<'alloc> {
     /// ```
     #[inline(always)]
     pub fn from_str_in(s: &str, allocator: &'alloc Allocator) -> String<'alloc> {
-        Self(BumpaloString::from_str_in(s, allocator))
+        Self(BumpaloString::from_str_in(s, allocator.bump()))
     }
 
     /// Convert `Vec<u8>` into [`String`].
@@ -157,7 +157,7 @@ impl<'alloc> String<'alloc> {
         allocator: &'alloc Allocator,
     ) -> String<'alloc> {
         // SAFETY: Safety conditions of this method are the same as `BumpaloString`'s method
-        Self(BumpaloString::from_raw_parts_in(buf, length, capacity, allocator))
+        Self(BumpaloString::from_raw_parts_in(buf, length, capacity, allocator.bump()))
     }
 
     /// Convert this `String<'alloc>` into an `&'alloc str`. This is analogous to

crates/oxc_allocator/src/vec.rs

@@ -56,7 +56,7 @@ impl<'alloc, T> Vec<'alloc, T> {
     /// ```
     #[inline(always)]
     pub fn new_in(allocator: &'alloc Allocator) -> Self {
-        Self(ManuallyDrop::new(InnerVec::new_in(allocator)))
+        Self(ManuallyDrop::new(InnerVec::new_in(allocator.bump())))
     }
 
     /// Constructs a new, empty `Vec<T>` with at least the specified capacity
@@ -108,7 +108,7 @@ impl<'alloc, T> Vec<'alloc, T> {
     /// ```
     #[inline(always)]
     pub fn with_capacity_in(capacity: usize, allocator: &'alloc Allocator) -> Self {
-        Self(ManuallyDrop::new(InnerVec::with_capacity_in(capacity, allocator)))
+        Self(ManuallyDrop::new(InnerVec::with_capacity_in(capacity, allocator.bump())))
     }
 
     /// Create a new [`Vec`] whose elements are taken from an iterator and
@@ -120,7 +120,7 @@ impl<'alloc, T> Vec<'alloc, T> {
         let iter = iter.into_iter();
         let hint = iter.size_hint();
         let capacity = hint.1.unwrap_or(hint.0);
-        let mut vec = ManuallyDrop::new(InnerVec::with_capacity_in(capacity, &**allocator));
+        let mut vec = ManuallyDrop::new(InnerVec::with_capacity_in(capacity, allocator.bump()));
         vec.extend(iter);
         Self(vec)
     }
@@ -149,7 +149,7 @@ impl<'alloc, T> Vec<'alloc, T> {
         // `ptr` was allocated with correct size for `[T; N]`.
         // `len` and `capacity` are both `N`.
         // Allocated size cannot be larger than `isize::MAX`, or `Box::new_in` would have failed.
-        let vec = unsafe { InnerVec::from_raw_parts_in(ptr, N, N, &**allocator) };
+        let vec = unsafe { InnerVec::from_raw_parts_in(ptr, N, N, allocator.bump()) };
         Self(ManuallyDrop::new(vec))
     }