Skip to content

Commit 32f4fdd

Browse files
committed
refactor(data_structures): add PhantomData field to Stack and NonEmptyStack (#15794)
Data structures which own `T`s should contain a `PhantomData<T>` field to signal to compiler the ownership semantics. For example `std::vec::Vec` does this. Add `PhantomData` field to `Stack` and `NonEmptyStack`. It's unclear to me if this is actually required for soundness or not. In the case of `std::vec::Vec`, it definitely is, but `Vec` uses `#[may_dangle]` in its `Drop` impl. Ideally we would use `#[may_dangle]` in these `Stack` types too, to loosen the borrow-checker's restrictions, but we're not able to as it's not available in stable Rust. Given that we're not using `#[may_dangle]`, maybe `PhantomData` is not required either, but it costs nothing, so it's better to be on safe side and add it.
1 parent c81a331 commit 32f4fdd

File tree

2 files changed

+9
-3
lines changed

2 files changed

+9
-3
lines changed

crates/oxc_data_structures/src/stack/non_empty.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
use std::{
2+
marker::PhantomData,
23
ops::{Deref, DerefMut},
34
ptr::NonNull,
45
};
@@ -76,6 +77,8 @@ pub struct NonEmptyStack<T> {
7677
start: NonNull<T>,
7778
/// Pointer to end of allocation
7879
end: NonNull<T>,
80+
/// Inform compiler that `NonEmptyStack<T>` owns `T`s
81+
_marker: PhantomData<T>,
7982
}
8083

8184
impl<T> StackCapacity<T> for NonEmptyStack<T> {}
@@ -211,7 +214,7 @@ impl<T> NonEmptyStack<T> {
211214
unsafe { start.as_ptr().write(initial_value) };
212215

213216
// `cursor` is positioned at start i.e. pointing at initial value
214-
Self { cursor: start, start, end }
217+
Self { cursor: start, start, end, _marker: PhantomData }
215218
}
216219

217220
/// Get reference to first value on stack.

crates/oxc_data_structures/src/stack/standard.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
use std::{
2+
marker::PhantomData,
23
ops::{Deref, DerefMut},
34
ptr::NonNull,
45
};
@@ -49,6 +50,8 @@ pub struct Stack<T> {
4950
start: NonNull<T>,
5051
/// Pointer to end of allocation containing stack
5152
end: NonNull<T>,
53+
/// Inform compiler that `Stack<T>` owns `T`s
54+
_marker: PhantomData<T>,
5255
}
5356

5457
impl<T> Default for Stack<T> {
@@ -115,7 +118,7 @@ impl<T> Stack<T> {
115118

116119
// Create stack with equal `start` and `end`
117120
let dangling = NonNull::dangling();
118-
Self { cursor: dangling, start: dangling, end: dangling }
121+
Self { cursor: dangling, start: dangling, end: dangling, _marker: PhantomData }
119122
}
120123

121124
/// Create new `Stack` with pre-allocated capacity for `capacity` entries.
@@ -180,7 +183,7 @@ impl<T> Stack<T> {
180183
let (start, end) = unsafe { Self::allocate(capacity_bytes) };
181184

182185
// `cursor` is positioned at start
183-
Self { cursor: start, start, end }
186+
Self { cursor: start, start, end, _marker: PhantomData }
184187
}
185188

186189
// Note: There is no need to implement `first` and `first_mut` methods.

0 commit comments

Comments
 (0)