Use of `Pin` for `Heap<T>` to indicate Immovability

[Redfire75369]

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
}

[jdm]

Can you provide any code samples of what using the pinned version looks like?

[Redfire75369]

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.

[sagudev]

For reference mozilla has MOZ_NON_MEMMOVABLE.

[Redfire75369]

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>;
}

[sagudev]

This might help to solve: https://github.com/servo/servo/issues/25726

[sagudev]

NOTE-TO-SELF: In servo dom objects are usually already boxed (except if new_uninherited).