mozjs
mozjs copied to clipboard
servo
Use of `Pin` for `Heap<T>` to indicate Immovability
Currently, Heap<T> effectively duplicates pinning requirements with its own set of rules. This issue proposes a change to use Pin.
Although Pin<Box<Heap<T>>> is more nested than the current Box<Heap<T>>, it clearly signifies the aim of being immovable.
Not that much would change with regards to the public API. Perhaps Heap::boxed could be deprecated in favour of something like Heap::pinned, but this can be discussed later.
From my understanding, none of the methods of Heap<T> would need to change, but I may be mistaken.
I do not believe ptr needs to be public with the addition of Heap::new, but I may be mistaken about servo's use cases.
API Changes
The inline comments show which fields/methods are old, new or remain the same.
pub struct Heap<T: GCMethods + Copy> {
ptr: UnsafeCell<T>,
_phantom: PhantomPinned, // New ZST Field
}
impl<T: GCMethods + Copy> Heap<T> {
pub unsafe fn new(v: T) -> Heap<T>; // New
pub fn boxed(v: T) -> Box<Heap<T>>; // Old
pub fn boxed(v: T) -> Pin<Box<Heap<T>>>; // New
pub fn set(&self, v: T); // Remains Same
pub fn get(&self) -> T; // Remains Same
pub unsafe fn get_unsafe(&self) -> *mut T; // Remains Same
pub unsafe fn handle(&self) -> Handle<T>; // Remains Same
}
Code Samples are difficult since most of will be identical with just a few functions changed. The best way would be to see what changes there are with servo itself.
@sagudev has made a branch of servo, servo:mozjs-pinned-heap. Some of the Heap API isn't quite as stated here, but its based on my branch mozjs:pin-heap.
In general, pinning can be quite annoying, but I believe it would be beneficial to express this constraint in the type system as opposed to having the possibility of breaking the invariant.
The API, as implemented, is as follows:
impl<T: GCMethods + Copy> Heap<T> {
pub unsafe fn new(v: T) -> Heap<T>; // New, replaces `Heap::ptr` being a pub field.
pub fn pinned(v: T) -> Pin<Box<Heap<T>>> // New
where Heap<T>: Default;
#[deprecated(note = "Use Heap::pinned instead")]
pub fn boxed(v: T) -> Box<Heap<T>> // Old, but deprecated
where Heap<T>: Default;
pub fn set(self: Pin<&Self>, v: T); // Old, but breaking change with signature changed to `self: Pin<&Self>` instead of `&self`
pub unsafe fn set_unsafe(&self, v: T); // New, doesn't require `Pin<&Self>`, but unsafe.
pub fn get(&self) -> T;
pub fn get_unsafe(&self) -> *mut T;
pub unsafe fn handle(&self) -> Handle<T>;
}
NOTE-TO-SELF: In servo dom objects are usually already boxed (except if new_uninherited).