Remove transmutes from UniquePtr/UniqueRef implementation (#352)

src/V8.rs

@@ -7,7 +7,7 @@ use std::sync::Mutex;
 use std::vec::Vec;
 
 use crate::platform::Platform;
-use crate::support::UniquePtr;
+use crate::support::UniqueRef;
 
 extern "C" {
   fn v8__V8__SetFlagsFromCommandLine(argc: *mut c_int, argv: *mut *mut c_char);
@@ -91,10 +91,10 @@ pub fn get_version() -> &'static str {
 // V8::ShutdownPlatform is called.
 /// Sets the v8::Platform to use. This should be invoked before V8 is
 /// initialized.
-pub fn initialize_platform(platform: UniquePtr<Platform>) {
+pub fn initialize_platform(platform: UniqueRef<Platform>) {
   let mut global_state_guard = GLOBAL_STATE.lock().unwrap();
   assert_eq!(*global_state_guard, Uninitialized);
-  unsafe { v8__V8__InitializePlatform(&mut *platform.into_raw()) };
+  unsafe { v8__V8__InitializePlatform(platform.into_raw()) };
   *global_state_guard = PlatformInitialized;
 }
 

src/array_buffer.rs

@@ -138,7 +138,7 @@ fn test_default_allocator() {
 }
 
 impl Delete for Allocator {
-  fn delete(&'static mut self) {
+  fn delete(&mut self) {
     unsafe { v8__ArrayBuffer__Allocator__DELETE(self) };
   }
 }

src/inspector.rs

@@ -70,9 +70,7 @@ extern "C" {
     stack_trace: &mut V8StackTrace,
   ) -> ();
 
-  fn v8_inspector__V8InspectorSession__DELETE(
+  fn v8_inspector__V8InspectorSession__DELETE(this: &mut V8InspectorSession);
-    this: &'static mut V8InspectorSession,
-  );
   fn v8_inspector__V8InspectorSession__dispatchProtocolMessage(
     session: *mut V8InspectorSession,
     message: &StringView,
@@ -83,14 +81,14 @@ extern "C" {
     break_details: &StringView,
   );
 
-  fn v8_inspector__StringBuffer__DELETE(this: &'static mut StringBuffer) -> ();
+  fn v8_inspector__StringBuffer__DELETE(this: &mut StringBuffer) -> ();
   fn v8_inspector__StringBuffer__string(this: &mut StringBuffer)
     -> &StringView;
   fn v8_inspector__StringBuffer__create(
     source: &StringView,
   ) -> UniquePtr<StringBuffer>;
 
-  fn v8_inspector__V8Inspector__DELETE(this: &'static mut V8Inspector);
+  fn v8_inspector__V8Inspector__DELETE(this: &mut V8Inspector);
   fn v8_inspector__V8Inspector__create(
     isolate: *mut Isolate,
     client: *mut V8InspectorClient,
@@ -586,7 +584,7 @@ impl V8InspectorSession {
 }
 
 impl Delete for V8InspectorSession {
-  fn delete(&'static mut self) {
+  fn delete(&mut self) {
     unsafe { v8_inspector__V8InspectorSession__DELETE(self) };
   }
 }
@@ -618,7 +616,7 @@ impl StringBuffer {
 }
 
 impl Delete for StringBuffer {
-  fn delete(&'static mut self) {
+  fn delete(&mut self) {
     unsafe { v8_inspector__StringBuffer__DELETE(self) }
   }
 }
@@ -894,7 +892,7 @@ impl V8Inspector {
 }
 
 impl Delete for V8Inspector {
-  fn delete(&'static mut self) {
+  fn delete(&mut self) {
     unsafe { v8_inspector__V8Inspector__DELETE(self) };
   }
 }

src/isolate.rs

@@ -576,7 +576,7 @@ impl CreateParams {
 }
 
 impl Delete for CreateParams {
-  fn delete(&'static mut self) {
+  fn delete(&mut self) {
     unsafe { v8__Isolate__CreateParams__DELETE(self) }
   }
 }

src/lib.rs

@@ -5,7 +5,7 @@
 //! ```rust
 //! use rusty_v8 as v8;
 //!
-//! let platform = v8::new_default_platform();
+//! let platform = v8::new_default_platform().unwrap();
 //! v8::V8::initialize_platform(platform);
 //! v8::V8::initialize();
 //!

src/platform/mod.rs

@@ -23,7 +23,7 @@ pub fn new_default_platform() -> UniquePtr<Platform> {
 pub struct Platform(Opaque);
 
 impl Delete for Platform {
-  fn delete(&'static mut self) {
+  fn delete(&mut self) {
     unsafe { v8__Platform__DELETE(self) }
   }
 }

src/platform/task.rs

@@ -42,7 +42,7 @@ impl Task {
 }
 
 impl Delete for Task {
-  fn delete(&'static mut self) {
+  fn delete(&mut self) {
     unsafe { v8__Task__DELETE(self) }
   }
 }

src/support.rs

@@ -1,10 +1,11 @@
 use std::cell::UnsafeCell;
 use std::marker::PhantomData;
-use std::mem::replace;
+use std::mem::forget;
 use std::mem::size_of;
 use std::mem::transmute;
 use std::ops::Deref;
 use std::ops::DerefMut;
+use std::ptr::null_mut;
 use std::ptr::NonNull;
 
 // TODO use libc::intptr_t when stable.
@@ -20,14 +21,14 @@ pub type Opaque = [u8; 0];
 
 pub trait Delete
 where
-  Self: Sized + 'static,
+  Self: Sized,
 {
-  fn delete(&'static mut self) -> ();
+  fn delete(&mut self) -> ();
 }
 
 /// Pointer to object allocated on the C++ heap. The pointer may be null.
 #[repr(transparent)]
-pub struct UniquePtr<T>(Option<&'static mut T>)
+pub struct UniquePtr<T>(Option<UniqueRef<T>>)
 where
   T: Delete;
 
@@ -39,22 +40,25 @@ where
     Self(None)
   }
 
-  pub fn new(r: &'static mut T) -> Self {
+  pub unsafe fn from_raw(ptr: *mut T) -> Self {
-    Self(Some(r))
+    Self(UniqueRef::try_from_raw(ptr))
-  }
-
-  pub unsafe fn from_raw(p: *mut T) -> Self {
-    transmute(p)
   }
 
   pub fn into_raw(self) -> *mut T {
-    unsafe { transmute(self) }
+    self
+      .0
+      .map(|unique_ref| unique_ref.into_raw())
+      .unwrap_or_else(null_mut)
   }
 
   pub fn unwrap(self) -> UniqueRef<T> {
-    let p = self.into_raw();
+    self.0.unwrap()
-    assert!(!p.is_null());
+  }
-    unsafe { UniqueRef::from_raw(p) }
+
+  unsafe fn _static_assert_has_pointer_repr() {
+    let dummy: fn() -> Self = || unimplemented!();
+    let _ptr: *mut T = transmute(dummy());
+    let _ref: &mut T = transmute(dummy());
   }
 }
 
@@ -63,7 +67,7 @@ where
   T: Delete,
 {
   fn from(unique_ref: UniqueRef<T>) -> Self {
-    unsafe { Self::from_raw(unique_ref.into_raw()) }
+    Self(Some(unique_ref))
   }
 }
 
@@ -71,7 +75,7 @@ impl<T> Deref for UniquePtr<T>
 where
   T: Delete,
 {
-  type Target = Option<&'static mut T>;
+  type Target = Option<UniqueRef<T>>;
   fn deref(&self) -> &Self::Target {
     &self.0
   }
@@ -86,20 +90,9 @@ where
   }
 }
 
-impl<T> Drop for UniquePtr<T>
-where
-  T: Delete,
-{
-  fn drop(&mut self) {
-    if let Some(v) = self.0.take() {
-      Delete::delete(v)
-    }
-  }
-}
-
 /// Pointer to object allocated on the C++ heap. The pointer may not be null.
 #[repr(transparent)]
-pub struct UniqueRef<T>(&'static mut T)
+pub struct UniqueRef<T>(NonNull<T>)
 where
   T: Delete;
 
@@ -107,10 +100,6 @@ impl<T> UniqueRef<T>
 where
   T: Delete,
 {
-  pub fn new(r: &'static mut T) -> Self {
-    Self(r)
-  }
-
   pub fn make_shared(self) -> SharedRef<T>
   where
     T: Shared,
@@ -118,12 +107,24 @@ where
     self.into()
   }
 
-  pub unsafe fn from_raw(p: *mut T) -> Self {
+  pub unsafe fn from_raw(ptr: *mut T) -> Self {
-    transmute(NonNull::new(p))
+    Self::try_from_raw(ptr).unwrap()
   }
 
   pub fn into_raw(self) -> *mut T {
-    unsafe { transmute(self) }
+    let ptr = self.0.as_ptr();
+    forget(self);
+    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());
   }
 }
 
@@ -133,7 +134,7 @@ where
 {
   type Target = T;
   fn deref(&self) -> &Self::Target {
-    self.0
+    unsafe { self.0.as_ref() }
   }
 }
 
@@ -142,7 +143,7 @@ where
   T: Delete,
 {
   fn deref_mut(&mut self) -> &mut Self::Target {
-    self.0
+    unsafe { self.0.as_mut() }
   }
 }
 
@@ -151,10 +152,7 @@ where
   T: Delete,
 {
   fn drop(&mut self) {
-    let inner = replace(&mut self.0, unsafe {
+    Delete::delete(&mut **self)
-      transmute(NonNull::<&'static mut T>::dangling())
-    });
-    Delete::delete(inner)
   }
 }
 

tests/test_api.rs

@@ -29,7 +29,7 @@ fn setup() -> SetupGuard {
   let mut g = INIT_LOCK.lock().unwrap();
   *g += 1;
   if *g == 1 {
-    v8::V8::initialize_platform(v8::new_default_platform());
+    v8::V8::initialize_platform(v8::new_default_platform().unwrap());
     v8::V8::initialize();
   }
   SetupGuard {}