Skip to content

Commit

Permalink
refactor(allocator): make Vec non-drop
Browse files Browse the repository at this point in the history
  • Loading branch information
overlookmotel committed Oct 16, 2024
1 parent 2673397 commit 45bdb68
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 12 deletions.
43 changes: 32 additions & 11 deletions crates/oxc_allocator/src/vec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,9 @@
use std::{
self,
fmt::Debug,
fmt::{self, Debug},
hash::{Hash, Hasher},
mem::ManuallyDrop,
ops,
ptr::NonNull,
};
Expand All @@ -17,9 +18,18 @@ use serde::{ser::SerializeSeq, Serialize, Serializer};

use crate::{Allocator, Box};

/// Bumpalo Vec
#[derive(Debug, PartialEq, Eq)]
pub struct Vec<'alloc, T>(vec::Vec<T, &'alloc Bump>);
/// A `Vec` without [`Drop`], which stores its data in the arena allocator.
///
/// Should only be used for storing AST types.
///
/// 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.
///
/// Note: This is not a soundness issue, as Rust does not support relying on `drop`
/// being called to guarantee soundness.
#[derive(PartialEq, Eq)]
pub struct Vec<'alloc, T>(ManuallyDrop<vec::Vec<T, &'alloc Bump>>);

impl<'alloc, T> Vec<'alloc, T> {
/// Constructs a new, empty `Vec<T>`.
Expand All @@ -38,7 +48,7 @@ impl<'alloc, T> Vec<'alloc, T> {
/// ```
#[inline]
pub fn new_in(allocator: &'alloc Allocator) -> Self {
Self(vec::Vec::new_in(allocator))
Self(ManuallyDrop::new(vec::Vec::new_in(allocator)))
}

/// Constructs a new, empty `Vec<T>` with at least the specified capacity
Expand Down Expand Up @@ -90,7 +100,7 @@ impl<'alloc, T> Vec<'alloc, T> {
/// ```
#[inline]
pub fn with_capacity_in(capacity: usize, allocator: &'alloc Allocator) -> Self {
Self(vec::Vec::with_capacity_in(capacity, allocator))
Self(ManuallyDrop::new(vec::Vec::with_capacity_in(capacity, allocator)))
}

/// Create a new [`Vec`] whose elements are taken from an iterator and
Expand All @@ -102,7 +112,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 = vec::Vec::with_capacity_in(capacity, &**allocator);
let mut vec = ManuallyDrop::new(vec::Vec::with_capacity_in(capacity, &**allocator));
vec.extend(iter);
Self(vec)
}
Expand All @@ -128,7 +138,8 @@ impl<'alloc, T> Vec<'alloc, T> {
///
/// [owned slice]: Box
pub fn into_boxed_slice(self) -> Box<'alloc, [T]> {
let slice = self.0.leak();
let inner = ManuallyDrop::into_inner(self.0);
let slice = inner.leak();
let ptr = NonNull::from(slice);
// SAFETY: `ptr` points to a valid slice `[T]`.
// `allocator_api2::vec::Vec::leak` consumes the inner `Vec` without dropping it.
Expand Down Expand Up @@ -160,7 +171,10 @@ impl<'alloc, T> IntoIterator for Vec<'alloc, T> {
type Item = T;

fn into_iter(self) -> Self::IntoIter {
self.0.into_iter()
let inner = ManuallyDrop::into_inner(self.0);
// TODO: `allocator_api2::vec::Vec::IntoIter` is `Drop`.
// Wrap it in `ManuallyDrop` to prevent that.
inner.into_iter()
}
}

Expand Down Expand Up @@ -198,7 +212,7 @@ where
S: Serializer,
{
let mut seq = s.serialize_seq(Some(self.0.len()))?;
for e in &self.0 {
for e in self.0.iter() {
seq.serialize_element(e)?;
}
seq.end()
Expand All @@ -207,12 +221,19 @@ where

impl<'alloc, T: Hash> Hash for Vec<'alloc, T> {
fn hash<H: Hasher>(&self, state: &mut H) {
for e in &self.0 {
for e in self.0.iter() {
e.hash(state);
}
}
}

impl<'alloc, T: Debug> Debug for Vec<'alloc, T> {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
let inner = &*self.0;
f.debug_tuple("Vec").field(inner).finish()
}
}

#[cfg(test)]
mod test {
use super::Vec;
Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_prettier/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ impl<'a> Prettier<'a> {
}

pub fn semi(&self) -> Option<Doc<'a>> {
self.options.semi.then(|| Doc::Str(";"))
self.options.semi.then_some(Doc::Str(";"))
}

pub fn should_print_es5_comma(&self) -> bool {
Expand Down

0 comments on commit 45bdb68

Please sign in to comment.