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

fix(allocator)!: make Vec non-drop #6623

Merged
merged 1 commit into from
Oct 19, 2024
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
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)]
overlookmotel marked this conversation as resolved.
Show resolved Hide resolved
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
Loading