feat: cppgc::Ptr (#1523)

examples/cppgc.rs

@@ -21,7 +21,11 @@ impl std::fmt::Display for Rope {
 }
 
 impl Rope {
-  pub fn new(part: String, next: v8::cppgc::Member<Rope>) -> Rope {
+  pub fn new(part: String, next: Option<v8::cppgc::Ptr<Rope>>) -> Rope {
+    let next = match next {
+      Some(p) => v8::cppgc::Member::new(&p),
+      None => v8::cppgc::Member::empty(),
+    };
     Self { part, next }
   }
 }
@@ -55,15 +59,15 @@ fn main() {
         &heap,
         Rope::new(
           String::from("Hello "),
-          v8::cppgc::make_garbage_collected(
+          Some(v8::cppgc::make_garbage_collected(
             &heap,
-            Rope::new(String::from("World!"), v8::cppgc::Member::empty()),
+            Rope::new(String::from("World!"), None),
-          ),
+          )),
         ),
       )
     };
 
-    println!("{}", rope.borrow().unwrap());
+    println!("{}", rope);
 
     // Manually trigger garbage collection.
     heap.enable_detached_garbage_collections_for_testing();
@@ -76,7 +80,7 @@ fn main() {
     }
 
     // Should still be live here:
-    println!("{}", rope.borrow().unwrap());
+    println!("{}", rope);
 
     println!("Collect: NoHeapPointers");
     unsafe {

src/binding.cc

@@ -3936,8 +3936,8 @@ void cppgc__WeakMember__Assign(cppgc::WeakMember<RustObj>* member,
   member->operator=(other);
 }
 
-cppgc::Persistent<RustObj>* cppgc__Persistent__CONSTRUCT() {
+cppgc::Persistent<RustObj>* cppgc__Persistent__CONSTRUCT(RustObj* obj) {
-  return new cppgc::Persistent<RustObj>(nullptr);
+  return new cppgc::Persistent<RustObj>(obj);
 }
 
 void cppgc__Persistent__DESTRUCT(cppgc::Persistent<RustObj>* self) {
@@ -3952,8 +3952,8 @@ RustObj* cppgc__Persistent__Get(cppgc::Persistent<RustObj>* self) {
   return self->Get();
 }
 
-cppgc::WeakPersistent<RustObj>* cppgc__WeakPersistent__CONSTRUCT() {
+cppgc::WeakPersistent<RustObj>* cppgc__WeakPersistent__CONSTRUCT(RustObj* obj) {
-  return new cppgc::WeakPersistent<RustObj>(nullptr);
+  return new cppgc::WeakPersistent<RustObj>(obj);
 }
 
 void cppgc__WeakPersistent__DESTRUCT(cppgc::WeakPersistent<RustObj>* self) {

src/cppgc.rs

@@ -8,6 +8,7 @@ use crate::support::UniqueRef;
 use crate::Data;
 use crate::TracedReference;
 use std::marker::PhantomData;
+use std::ptr::NonNull;
 
 extern "C" {
   fn cppgc__initialize_process(platform: *mut Platform);
@@ -63,12 +64,14 @@ extern "C" {
     other: *mut RustObj,
   );
 
-  fn cppgc__Persistent__CONSTRUCT() -> *mut PersistentInner;
+  fn cppgc__Persistent__CONSTRUCT(obj: *mut RustObj) -> *mut PersistentInner;
   fn cppgc__Persistent__DESTRUCT(this: *mut PersistentInner);
   fn cppgc__Persistent__Assign(this: *mut PersistentInner, ptr: *mut RustObj);
   fn cppgc__Persistent__Get(this: *const PersistentInner) -> *mut RustObj;
 
-  fn cppgc__WeakPersistent__CONSTRUCT() -> *mut WeakPersistentInner;
+  fn cppgc__WeakPersistent__CONSTRUCT(
+    obj: *mut RustObj,
+  ) -> *mut WeakPersistentInner;
   fn cppgc__WeakPersistent__DESTRUCT(this: *mut WeakPersistentInner);
   fn cppgc__WeakPersistent__Assign(
     this: *mut WeakPersistentInner,
@@ -154,18 +157,6 @@ pub trait Traced {
   fn trace(&self, visitor: &Visitor);
 }
 
-impl<T: GarbageCollected> Traced for Member<T> {
-  fn trace(&self, visitor: &Visitor) {
-    unsafe { cppgc__Visitor__Trace__Member(visitor, &self.inner) }
-  }
-}
-
-impl<T: GarbageCollected> Traced for WeakMember<T> {
-  fn trace(&self, visitor: &Visitor) {
-    unsafe { cppgc__Visitor__Trace__WeakMember(visitor, &self.inner) }
-  }
-}
-
 impl<T> Traced for TracedReference<T> {
   fn trace(&self, visitor: &Visitor) {
     unsafe {
@@ -290,11 +281,11 @@ pub trait GarbageCollected {
 /// # Safety
 ///
 /// The caller must ensure that the returned pointer is always stored on
-/// the stack, or moved into one of the Persistent types.
+/// the stack, or is safely moved into one of the other cppgc pointer types.
 pub unsafe fn make_garbage_collected<T: GarbageCollected>(
   heap: &Heap,
   obj: T,
-) -> Member<T> {
+) -> Ptr<T> {
   unsafe extern "C" fn trace<T: GarbageCollected>(
     obj: *const RustObj,
     visitor: *mut Visitor,
@@ -312,7 +303,7 @@ pub unsafe fn make_garbage_collected<T: GarbageCollected>(
     - std::mem::size_of::<RustObj>())
     + std::mem::size_of::<T>();
 
-  let handle = unsafe {
+  let pointer = unsafe {
     cppgc__make_garbage_collectable(
       heap as *const Heap as *mut _,
       additional_bytes,
@@ -322,10 +313,13 @@ pub unsafe fn make_garbage_collected<T: GarbageCollected>(
   };
 
   unsafe {
-    get_object_from_rust_obj::<T>(handle).write(obj);
+    get_object_from_rust_obj::<T>(pointer).write(obj);
   }
 
-  Member::new(handle)
+  Ptr {
+    pointer: NonNull::new_unchecked(pointer),
+    _phantom: PhantomData,
+  }
 }
 
 #[doc(hidden)]
@@ -333,6 +327,12 @@ pub trait GetRustObj<T: GarbageCollected> {
   fn get_rust_obj(&self) -> *mut RustObj;
 }
 
+impl<T: GarbageCollected> GetRustObj<T> for *mut RustObj {
+  fn get_rust_obj(&self) -> *mut RustObj {
+    *self
+  }
+}
+
 macro_rules! member {
   ($( # $attr:tt )* $name:ident) => {
     paste::paste! {
@@ -379,18 +379,24 @@ macro_rules! member {
       }
 
       impl<T: GarbageCollected> $name<T> {
-        pub(crate) fn new(obj: *mut RustObj) -> Self {
-          Self {
-            inner: [< $name Inner >]::new(obj),
-            _phantom: PhantomData,
-          }
-        }
-
         #[doc = "Create a new empty "]
         #[doc = stringify!($name)]
         #[doc = " which may be set later."]
         pub fn empty() -> Self {
-          Self::new(std::ptr::null_mut())
+          Self {
+            inner: [< $name Inner >]::new(std::ptr::null_mut()),
+            _phantom: PhantomData,
+          }
+        }
+
+        #[doc = "Create a new "]
+        #[doc = stringify!(name)]
+        #[doc = " and initialize it with an object."]
+        pub fn new(other: &impl GetRustObj<T>) -> Self {
+          Self {
+            inner: [< $name Inner >]::new(other.get_rust_obj()),
+            _phantom: PhantomData,
+          }
         }
 
         #[doc = "Set the object pointed to by this "]
@@ -422,6 +428,12 @@ macro_rules! member {
         }
       }
 
+      impl<T: GarbageCollected> Traced for $name<T> {
+        fn trace(&self, visitor: &Visitor) {
+          unsafe { [< cppgc__Visitor__Trace__ $name >](visitor, &self.inner) }
+        }
+      }
+
       impl<T: GarbageCollected> std::fmt::Debug for $name<T> {
         fn fmt(&self, fmt: &mut std::fmt::Formatter) -> std::fmt::Result {
           fmt.debug_struct(stringify!($name)).finish()
@@ -467,7 +479,18 @@ macro_rules! persistent {
         #[doc = stringify!($name)]
         #[doc = " which may be set later."]
         pub fn empty() -> Self {
-          let this = unsafe { [< cppgc__ $name __CONSTRUCT >]() };
+          let this = unsafe { [< cppgc__ $name __CONSTRUCT >](std::ptr::null_mut()) };
+          Self {
+            inner: this,
+            _phantom: PhantomData,
+          }
+        }
+
+        #[doc = "Create a new "]
+        #[doc = stringify!(name)]
+        #[doc = " and initialize it with an object."]
+        pub fn new(other: &impl GetRustObj<T>) -> Self {
+          let this = unsafe { [< cppgc__ $name __CONSTRUCT >](other.get_rust_obj()) };
           Self {
             inner: this,
             _phantom: PhantomData,
@@ -551,3 +574,51 @@ persistent! {
   /// thread.
   WeakPersistent
 }
+
+/// Ptr is used to refer to an on-heap object from the stack.
+#[derive(Clone, Copy)]
+pub struct Ptr<T: GarbageCollected> {
+  pointer: NonNull<RustObj>,
+  _phantom: PhantomData<T>,
+}
+
+impl<T: GarbageCollected> Ptr<T> {
+  /// Create a new Ptr.
+  ///
+  /// # Safety
+  ///
+  /// The caller must ensure that the returned pointer is always stored on
+  /// the stack, or is safely moved into one of the other cppgc pointer types.
+  pub unsafe fn new(other: &impl GetRustObj<T>) -> Option<Self> {
+    NonNull::new(other.get_rust_obj()).map(|pointer| Self {
+      pointer,
+      _phantom: PhantomData,
+    })
+  }
+}
+
+impl<T: GarbageCollected> std::ops::Deref for Ptr<T> {
+  type Target = T;
+
+  fn deref(&self) -> &T {
+    unsafe { &*get_object_from_rust_obj(self.pointer.as_ptr()) }
+  }
+}
+
+impl<T: GarbageCollected> GetRustObj<T> for Ptr<T> {
+  fn get_rust_obj(&self) -> *mut RustObj {
+    self.pointer.as_ptr()
+  }
+}
+
+impl<T: GarbageCollected + std::fmt::Debug> std::fmt::Debug for Ptr<T> {
+  fn fmt(&self, fmt: &mut std::fmt::Formatter) -> std::fmt::Result {
+    std::fmt::Debug::fmt(&**self, fmt)
+  }
+}
+
+impl<T: GarbageCollected + std::fmt::Display> std::fmt::Display for Ptr<T> {
+  fn fmt(&self, fmt: &mut std::fmt::Formatter) -> std::fmt::Result {
+    std::fmt::Display::fmt(&**self, fmt)
+  }
+}

src/object.rs

@@ -1,6 +1,6 @@
 use crate::cppgc::GarbageCollected;
 use crate::cppgc::GetRustObj;
-use crate::cppgc::Member;
+use crate::cppgc::Ptr;
 use crate::cppgc::RustObj;
 use crate::isolate::Isolate;
 use crate::support::int;
@@ -721,16 +721,16 @@ impl Object {
   /// # Safety
   ///
   /// The caller must ensure that the returned pointer is always stored on
-  /// the stack, or moved into one of the Persistent types.
+  /// the stack, or is safely moved into one of the other cppgc pointer types.
   #[inline(always)]
   pub unsafe fn unwrap<const TAG: u16, T: GarbageCollected>(
     isolate: &mut Isolate,
     wrapper: Local<Object>,
-  ) -> Member<T> {
+  ) -> Option<Ptr<T>> {
     // TODO: use a const assert once const expressions are stable
     assert!(TAG < LAST_TAG);
     let ptr = unsafe { v8__Object__Unwrap(isolate as *mut _, &*wrapper, TAG) };
-    Member::new(ptr)
+    Ptr::new(&ptr)
   }
 
   /// Returns true if this object can be generally used to wrap object objects.

tests/test_cppgc.rs

@@ -89,7 +89,7 @@ fn cppgc_object_wrap() {
   ) {
     let obj = args.get(0).try_into().unwrap();
     let member = unsafe { v8::Object::unwrap::<TAG, Wrap>(scope, obj) };
-    rv.set(member.borrow().unwrap().value.get(scope).unwrap());
+    rv.set(member.unwrap().value.get(scope).unwrap());
   }
 
   {