Remove the 'Delete' trait, use regular 'Drop' instead (#353)

src/array_buffer.rs

@@ -7,7 +7,6 @@ use std::ptr::null_mut;
 use std::slice;
 
 use crate::support::long;
-use crate::support::Delete;
 use crate::support::Opaque;
 use crate::support::Shared;
 use crate::support::SharedRef;
@@ -137,8 +136,8 @@ fn test_default_allocator() {
   new_default_allocator();
 }
 
-impl Delete for Allocator {
-  fn delete(&mut self) {
+impl Drop for Allocator {
+  fn drop(&mut self) {
     unsafe { v8__ArrayBuffer__Allocator__DELETE(self) };
   }
 }
@@ -212,8 +211,8 @@ impl DerefMut for BackingStore {
   }
 }
 
-impl Delete for BackingStore {
-  fn delete(&mut self) {
+impl Drop for BackingStore {
+  fn drop(&mut self) {
     unsafe { v8__BackingStore__DELETE(self) };
   }
 }

src/inspector.rs

@@ -16,7 +16,6 @@
 use crate::scope_traits::InIsolate;
 use crate::support::int;
 use crate::support::CxxVTable;
-use crate::support::Delete;
 use crate::support::FieldOffset;
 use crate::support::Opaque;
 use crate::support::RustVTable;
@@ -583,8 +582,8 @@ impl V8InspectorSession {
   }
 }
 
-impl Delete for V8InspectorSession {
-  fn delete(&mut self) {
+impl Drop for V8InspectorSession {
+  fn drop(&mut self) {
     unsafe { v8_inspector__V8InspectorSession__DELETE(self) };
   }
 }
@@ -615,8 +614,8 @@ impl StringBuffer {
   }
 }
 
-impl Delete for StringBuffer {
-  fn delete(&mut self) {
+impl Drop for StringBuffer {
+  fn drop(&mut self) {
     unsafe { v8_inspector__StringBuffer__DELETE(self) }
   }
 }
@@ -891,8 +890,8 @@ impl V8Inspector {
   }
 }
 
-impl Delete for V8Inspector {
-  fn delete(&mut self) {
+impl Drop for V8Inspector {
+  fn drop(&mut self) {
     unsafe { v8_inspector__V8Inspector__DELETE(self) };
   }
 }

src/isolate.rs

@@ -3,7 +3,6 @@ use crate::array_buffer::Allocator;
 use crate::external_references::ExternalReferences;
 use crate::promise::PromiseRejectMessage;
 use crate::support::intptr_t;
-use crate::support::Delete;
 use crate::support::Opaque;
 use crate::support::SharedRef;
 use crate::support::UniqueRef;
@@ -575,8 +574,8 @@ impl CreateParams {
   }
 }
 
-impl Delete for CreateParams {
-  fn delete(&mut self) {
+impl Drop for CreateParams {
+  fn drop(&mut self) {
     unsafe { v8__Isolate__CreateParams__DELETE(self) }
   }
 }

src/platform/mod.rs

@@ -2,7 +2,6 @@ pub mod task;
 
 pub use task::{Task, TaskBase, TaskImpl};
 
-use crate::support::Delete;
 use crate::support::Opaque;
 use crate::support::UniquePtr;
 use crate::Isolate;
@@ -22,8 +21,8 @@ pub fn new_default_platform() -> UniquePtr<Platform> {
 #[repr(C)]
 pub struct Platform(Opaque);
 
-impl Delete for Platform {
-  fn delete(&mut self) {
+impl Drop for Platform {
+  fn drop(&mut self) {
     unsafe { v8__Platform__DELETE(self) }
   }
 }

src/platform/task.rs

@@ -1,12 +1,12 @@
 use std::mem::drop;
 use std::mem::forget;
+use std::mem::ManuallyDrop;
 
 use crate::support::CxxVTable;
-use crate::support::Delete;
 use crate::support::FieldOffset;
 use crate::support::Opaque;
 use crate::support::RustVTable;
-use crate::support::UniquePtr;
+use crate::support::UniqueRef;
 
 // class Task {
 //  public:
@@ -41,8 +41,8 @@ impl Task {
   }
 }
 
-impl Delete for Task {
-  fn delete(&mut self) {
+impl Drop for Task {
+  fn drop(&mut self) {
     unsafe { v8__Task__DELETE(self) }
   }
 }
@@ -52,13 +52,13 @@ pub trait AsTask {
   fn as_task_mut(&mut self) -> &mut Task;
 
   // TODO: this should be a trait in itself.
-  fn into_unique_ptr(mut self: Box<Self>) -> UniquePtr<Task>
+  fn into_unique_ref(mut self: Box<Self>) -> UniqueRef<Task>
   where
     Self: 'static,
   {
     let task = self.as_task_mut() as *mut Task;
     forget(self);
-    unsafe { UniquePtr::from_raw(task) }
+    unsafe { UniqueRef::from_raw(task) }
   }
 }
 
@@ -90,23 +90,23 @@ pub trait TaskImpl: AsTask {
 }
 
 pub struct TaskBase {
-  cxx_base: Task,
+  cxx_base: ManuallyDrop<Task>,
   offset_within_embedder: FieldOffset<Self>,
   rust_vtable: RustVTable<&'static dyn TaskImpl>,
 }
 
 impl TaskBase {
-  fn construct_cxx_base() -> Task {
+  fn construct_cxx_base() -> ManuallyDrop<Task> {
     unsafe {
       let mut buf = std::mem::MaybeUninit::<Task>::uninit();
       v8__Task__BASE__CONSTRUCT(&mut buf);
-      buf.assume_init()
+      ManuallyDrop::new(buf.assume_init())
     }
   }
 
   fn get_cxx_base_offset() -> FieldOffset<Task> {
     let buf = std::mem::MaybeUninit::<Self>::uninit();
-    FieldOffset::from_ptrs(buf.as_ptr(), unsafe { &(*buf.as_ptr()).cxx_base })
+    FieldOffset::from_ptrs(buf.as_ptr(), unsafe { &*(*buf.as_ptr()).cxx_base })
   }
 
   fn get_offset_within_embedder<T>() -> FieldOffset<Self>
@@ -208,17 +208,21 @@ mod tests {
   }
 
   #[test]
-  fn test_task() {
-    {
-      TestTask::new().run();
-    }
+  fn test_task_1() {
+    let mut task = TestTask::new();
+    task.run();
+    drop(task);
     assert_eq!(RUN_COUNT.swap(0, SeqCst), 1);
     assert_eq!(DROP_COUNT.swap(0, SeqCst), 1);
+  }
 
-    {
-      Box::new(TestTask::new()).into_unique_ptr();
-    }
-    assert_eq!(RUN_COUNT.swap(0, SeqCst), 0);
+  #[test]
+  fn test_task_2() {
+    let mut task = Box::new(TestTask::new()).into_unique_ref();
+    task.run();
+    task.run();
+    drop(task);
+    assert_eq!(RUN_COUNT.swap(0, SeqCst), 2);
     assert_eq!(DROP_COUNT.swap(0, SeqCst), 1);
   }
 }

src/support.rs

@@ -1,10 +1,12 @@
 use std::cell::UnsafeCell;
 use std::marker::PhantomData;
+use std::mem::align_of;
 use std::mem::forget;
+use std::mem::needs_drop;
 use std::mem::size_of;
-use std::mem::transmute;
 use std::ops::Deref;
 use std::ops::DerefMut;
+use std::ptr::drop_in_place;
 use std::ptr::null_mut;
 use std::ptr::NonNull;
 
@@ -19,28 +21,18 @@ pub use std::os::raw::c_long as long;
 
 pub type Opaque = [u8; 0];
 
-pub trait Delete
-where
-  Self: Sized,
-{
-  fn delete(&mut self) -> ();
-}
-
 /// Pointer to object allocated on the C++ heap. The pointer may be null.
 #[repr(transparent)]
-pub struct UniquePtr<T>(Option<UniqueRef<T>>)
-where
-  T: Delete;
+pub struct UniquePtr<T>(Option<UniqueRef<T>>);
 
-impl<T> UniquePtr<T>
-where
-  T: Delete,
-{
+impl<T> UniquePtr<T> {
   pub fn null() -> Self {
+    assert_unique_type_compatible::<Self, T>();
     Self(None)
   }
 
   pub unsafe fn from_raw(ptr: *mut T) -> Self {
+    assert_unique_type_compatible::<Self, T>();
     Self(UniqueRef::try_from_raw(ptr))
   }
 
@@ -54,37 +46,22 @@ where
   pub fn unwrap(self) -> UniqueRef<T> {
     self.0.unwrap()
   }
-
-  unsafe fn _static_assert_has_pointer_repr() {
-    let dummy: fn() -> Self = || unimplemented!();
-    let _ptr: *mut T = transmute(dummy());
-    let _ref: &mut T = transmute(dummy());
-  }
 }
 
-impl<T> From<UniqueRef<T>> for UniquePtr<T>
-where
-  T: Delete,
-{
+impl<T> From<UniqueRef<T>> for UniquePtr<T> {
   fn from(unique_ref: UniqueRef<T>) -> Self {
     Self(Some(unique_ref))
   }
 }
 
-impl<T> Deref for UniquePtr<T>
-where
-  T: Delete,
-{
+impl<T> Deref for UniquePtr<T> {
   type Target = Option<UniqueRef<T>>;
   fn deref(&self) -> &Self::Target {
     &self.0
   }
 }
 
-impl<T> DerefMut for UniquePtr<T>
-where
-  T: Delete,
-{
+impl<T> DerefMut for UniquePtr<T> {
   fn deref_mut(&mut self) -> &mut Self::Target {
     &mut self.0
   }
@@ -92,19 +69,12 @@ where
 
 /// Pointer to object allocated on the C++ heap. The pointer may not be null.
 #[repr(transparent)]
-pub struct UniqueRef<T>(NonNull<T>)
-where
-  T: Delete;
+pub struct UniqueRef<T>(NonNull<T>);
 
-impl<T> UniqueRef<T>
-where
-  T: Delete,
-{
-  pub fn make_shared(self) -> SharedRef<T>
-  where
-    T: Shared,
-  {
-    self.into()
+impl<T> UniqueRef<T> {
+  unsafe fn try_from_raw(ptr: *mut T) -> Option<Self> {
+    assert_unique_type_compatible::<Self, T>();
+    NonNull::new(ptr).map(Self)
   }
 
   pub unsafe fn from_raw(ptr: *mut T) -> Self {
@@ -117,48 +87,47 @@ where
     ptr
   }
 
-  unsafe fn try_from_raw(ptr: *mut T) -> Option<Self> {
-    NonNull::new(ptr).map(Self)
-  }
-
-  unsafe fn _static_assert_has_pointer_repr() {
-    let dummy: fn() -> Self = || unimplemented!();
-    let _ptr: *mut T = transmute(dummy());
-    let _ref: &mut T = transmute(dummy());
+  pub fn make_shared(self) -> SharedRef<T>
+  where
+    T: Shared,
+  {
+    self.into()
   }
 }
 
-impl<T> Deref for UniqueRef<T>
-where
-  T: Delete,
-{
+impl<T> Deref for UniqueRef<T> {
   type Target = T;
   fn deref(&self) -> &Self::Target {
     unsafe { self.0.as_ref() }
   }
 }
 
-impl<T> DerefMut for UniqueRef<T>
-where
-  T: Delete,
-{
+impl<T> DerefMut for UniqueRef<T> {
   fn deref_mut(&mut self) -> &mut Self::Target {
     unsafe { self.0.as_mut() }
   }
 }
 
-impl<T> Drop for UniqueRef<T>
-where
-  T: Delete,
-{
+impl<T> Drop for UniqueRef<T> {
   fn drop(&mut self) {
-    Delete::delete(&mut **self)
+    unsafe { drop_in_place(self.0.as_ptr()) }
   }
 }
 
+fn assert_unique_type_compatible<U, T>() {
+  // Assert that `U` (a `UniqueRef` or `UniquePtr`) has the same memory layout
+  // as a pointer to `T`.
+  assert_eq!(size_of::<U>(), size_of::<*mut T>());
+  assert_eq!(align_of::<U>(), align_of::<*mut T>());
+
+  // Assert that `T` (probably) implements `Drop`. If it doesn't, a regular
+  // reference should be used instead of UniquePtr/UniqueRef.
+  assert!(needs_drop::<T>());
+}
+
 pub trait Shared
 where
-  Self: Delete + 'static,
+  Self: Sized,
 {
   fn clone(shared_ptr: *const SharedRef<Self>) -> SharedRef<Self>;
   fn from_unique(unique: UniqueRef<Self>) -> SharedRef<Self>;
@@ -196,7 +165,7 @@ where
 
 impl<T> From<UniqueRef<T>> for SharedRef<T>
 where
-  T: Delete + Shared,
+  T: Shared,
 {
   fn from(unique: UniqueRef<T>) -> Self {
     <T as Shared>::from_unique(unique)