Refactor Rooted<T> (#576)

* Factor out MaybeUninit from Rooted

Signed-off-by: Greg Morenz <greg-morenz@droid.cafe>

* Rename ptr to data

Signed-off-by: Greg Morenz <greg-morenz@droid.cafe>

---------

Signed-off-by: Greg Morenz <greg-morenz@droid.cafe>

Author: gmorenz

mozjs-sys/src/jsgc.rs

@@ -104,7 +104,7 @@ pub trait Rootable: crate::trace::Traceable + Sized {
     unsafe extern "C" fn trace(this: *mut c_void, trc: *mut JSTracer, _name: *const c_char) {
         let rooted = this as *mut Rooted<Self>;
         let rooted = rooted.as_mut().unwrap();
-        <Self as crate::trace::Traceable>::trace(rooted.ptr.assume_init_mut(), trc);
+        <Self as crate::trace::Traceable>::trace(&mut rooted.data, trc);
     }
 }
 
@@ -132,11 +132,7 @@ pub struct RootedBase {
 pub struct Rooted<T: RootKind> {
     pub vtable: T::Vtable,
     pub base: RootedBase,
-
-    /// The rooted value
-    ///
-    /// This will be initialied iff there is a `RootedGuard` for this `Rooted`
-    pub ptr: mem::MaybeUninit<T>,
+    pub data: T,
 }
 
 /// Trait that provides a GC-safe default value for the given type, if one exists.
@@ -292,7 +288,7 @@ impl<const N: usize> ValueArray<N> {
         Self { elements }
     }
 
-    pub unsafe fn get_ptr(&self) -> *const JS::Value {
+    pub fn get_ptr(&self) -> *const JS::Value {
         self.elements.as_ptr()
     }
 

mozjs-sys/src/jsimpls.rs

@@ -25,7 +25,6 @@ use crate::jsid::VoidId;
 use crate::jsval::{JSVal, UndefinedValue};
 
 use std::marker::PhantomData;
-use std::mem;
 use std::ops::Deref;
 use std::ptr;
 
@@ -144,7 +143,7 @@ impl<const N: usize> From<&Rooted<ValueArray<N>>> for JS::HandleValueArray {
     fn from(array: &Rooted<ValueArray<N>>) -> JS::HandleValueArray {
         JS::HandleValueArray {
             length_: N,
-            elements_: unsafe { array.ptr.assume_init_ref().get_ptr() },
+            elements_: array.data.get_ptr(),
         }
     }
 }
@@ -429,14 +428,14 @@ impl RootedBase {
 }
 
 impl<T: RootKind> JS::Rooted<T> {
-    pub fn new_unrooted() -> JS::Rooted<T> {
+    pub fn new_unrooted(initial: T) -> JS::Rooted<T> {
         JS::Rooted {
             vtable: T::VTABLE,
             base: RootedBase {
                 stack: ptr::null_mut(),
                 prev: ptr::null_mut(),
             },
-            ptr: mem::MaybeUninit::zeroed(),
+            data: initial,
         }
     }
 

mozjs/src/conversions.rs

@@ -722,8 +722,8 @@ impl<C: Clone, T: FromJSValConvertible<Config = C>> FromJSValConvertible for Vec
         let zero = mem::zeroed();
         let mut iterator = ForOfIterator {
             cx_: cx,
-            iterator: RootedObject::new_unrooted(),
-            nextMethod: RootedValue::new_unrooted(),
+            iterator: RootedObject::new_unrooted(ptr::null_mut()),
+            nextMethod: RootedValue::new_unrooted(JSVal { asBits_: 0 }),
             index: ::std::u32::MAX, // NOT_ARRAY
             ..zero
         };
@@ -737,7 +737,7 @@ impl<C: Clone, T: FromJSValConvertible<Config = C>> FromJSValConvertible for Vec
             return Err(());
         }
 
-        if iterator.iterator.ptr.assume_init_ref().is_null() {
+        if iterator.iterator.data.is_null() {
             return Ok(ConversionResult::Failure("Value is not iterable".into()));
         }
 

mozjs/src/gc/macros.rs

@@ -1,15 +1,15 @@
 #[macro_export]
 macro_rules! rooted {
 	(in($cx:expr) let $($var:ident)+ = $init:expr) => {
-        let mut __root = $crate::jsapi::Rooted::new_unrooted();
+        let mut __root = ::std::mem::MaybeUninit::uninit();
         let $($var)+ = $crate::gc::RootedGuard::new($cx, &mut __root, $init);
     };
 	(in($cx:expr) let $($var:ident)+: $type:ty = $init:expr) => {
-        let mut __root = $crate::jsapi::Rooted::new_unrooted();
+        let mut __root = ::std::mem::MaybeUninit::uninit();
         let $($var)+: $crate::gc::RootedGuard<$type> = $crate::gc::RootedGuard::new($cx, &mut __root, $init);
     };
 	(in($cx:expr) let $($var:ident)+: $type:ty) => {
-        let mut __root = $crate::jsapi::Rooted::new_unrooted();
+        let mut __root = ::std::mem::MaybeUninit::uninit();
         // SAFETY:
         // We're immediately storing the initial value in a rooted location.
         let $($var)+: $crate::gc::RootedGuard<$type> = $crate::gc::RootedGuard::new(

mozjs/src/gc/root.rs

@@ -26,10 +26,10 @@ pub struct RootedGuard<'a, T: 'a + RootKind> {
 }
 
 impl<'a, T: 'a + RootKind> RootedGuard<'a, T> {
-    pub fn new(cx: *mut JSContext, root: &'a mut Rooted<T>, initial: T) -> Self {
-        root.ptr.write(initial);
+    pub fn new(cx: *mut JSContext, root: &'a mut MaybeUninit<Rooted<T>>, initial: T) -> Self {
+        let root: *mut Rooted<T> = root.write(Rooted::new_unrooted(initial));
+
         unsafe {
-            let root: *mut Rooted<T> = root;
             Rooted::add_to_root_stack(root, cx);
             RootedGuard {
                 root,
@@ -48,7 +48,7 @@ impl<'a, T: 'a + RootKind> RootedGuard<'a, T> {
 
     pub fn as_ptr(&self) -> *mut T {
         // SAFETY: self.root points to an inbounds allocation
-        unsafe { (&raw mut (*self.root).ptr).cast() }
+        unsafe { (&raw mut (*self.root).data) }
     }
 
     /// Safety: GC must not run during the lifetime of the returned reference.
@@ -63,17 +63,12 @@ impl<'a, T: 'a + RootKind> RootedGuard<'a, T> {
     where
         T: Copy,
     {
-        // SAFETY: The rooted value is initialized as long as we exist
-        unsafe { (*self.root).ptr.assume_init() }
+        *self.deref()
     }
 
     pub fn set(&mut self, v: T) {
-        // SAFETY: The rooted value is initialized as long as we exist
-        unsafe {
-            // Make sure the drop impl for T is called
-            (*self.root).ptr.assume_init_drop();
-            (*self.root).ptr.write(v);
-        }
+        // SAFETY: GC does not run during this block
+        unsafe { *self.as_mut() = v };
     }
 }
 
@@ -90,18 +85,18 @@ where
 impl<'a, T: 'a + RootKind> Deref for RootedGuard<'a, T> {
     type Target = T;
     fn deref(&self) -> &T {
-        // SAFETY: The rooted value is initialized as long as we exist
-        unsafe { (*self.root).ptr.assume_init_ref() }
+        unsafe { &(*self.root).data }
     }
 }
 
 impl<'a, T: 'a + RootKind> Drop for RootedGuard<'a, T> {
     fn drop(&mut self) {
-        // SAFETY: The rooted value is initialized as long as we exist
+        // SAFETY: The `drop_in_place` invariants are upheld:
+        // https://doc.rust-lang.org/std/ptr/fn.drop_in_place.html#safety
         unsafe {
-            // Make sure the drop impl for T is called
-            (*self.root).ptr.assume_init_drop();
-            (*self.root).ptr = MaybeUninit::zeroed();
+            let ptr = self.as_ptr();
+            ptr::drop_in_place(ptr);
+            ptr.write_bytes(0, 1);
         }
 
         unsafe {