From 252b58ac2ffae84c6e1962750c6dd55dafd57104 Mon Sep 17 00:00:00 2001
From: Bert Belder <bertbelder@gmail.com>
Date: Mon, 31 Aug 2020 07:35:09 +0200
Subject: [PATCH] Fix minor SharedPtr/SharedRef issues, and add unit tests

* The `Default` trait did not actually get derived for `SharedPtr<T>`.
  This is solved by implementing `Default` manually.
* Trait function `Shared::get()` used to return a mutable raw pointer
  (`*mut Self`), but it would be inconceivable to ever mutate the
  referenced value. It was changed to return a const pointer instead.
* Added some basic unit tests for types `SharedPtr` and `SharedRef`.
---
 src/array_buffer.rs |  4 +--
 src/support.rs      | 85 +++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 85 insertions(+), 4 deletions(-)

diff --git a/src/array_buffer.rs b/src/array_buffer.rs
index 1c106f1734..2d196930da 100644
--- a/src/array_buffer.rs
+++ b/src/array_buffer.rs
@@ -113,7 +113,7 @@ impl Shared for Allocator {
       )
     }
   }
-  fn get(ptr: &SharedPtrBase<Self>) -> *mut Self {
+  fn get(ptr: &SharedPtrBase<Self>) -> *const Self {
     unsafe { std__shared_ptr__v8__ArrayBuffer__Allocator__get(ptr) }
   }
   fn reset(ptr: &mut SharedPtrBase<Self>) {
@@ -219,7 +219,7 @@ impl Shared for BackingStore {
       std__shared_ptr__v8__BackingStore__CONVERT__std__unique_ptr(unique_ptr)
     }
   }
-  fn get(ptr: &SharedPtrBase<Self>) -> *mut Self {
+  fn get(ptr: &SharedPtrBase<Self>) -> *const Self {
     unsafe { std__shared_ptr__v8__BackingStore__get(ptr) }
   }
   fn reset(ptr: &mut SharedPtrBase<Self>) {
diff --git a/src/support.rs b/src/support.rs
index 856fef539f..1941516238 100644
--- a/src/support.rs
+++ b/src/support.rs
@@ -178,7 +178,7 @@ where
 {
   fn clone(shared_ptr: &SharedPtrBase<Self>) -> SharedPtrBase<Self>;
   fn from_unique_ptr(unique_ptr: UniquePtr<Self>) -> SharedPtrBase<Self>;
-  fn get(shared_ptr: &SharedPtrBase<Self>) -> *mut Self;
+  fn get(shared_ptr: &SharedPtrBase<Self>) -> *const Self;
   fn reset(shared_ptr: &mut SharedPtrBase<Self>);
   fn use_count(shared_ptr: &SharedPtrBase<Self>) -> long;
 }
@@ -186,6 +186,7 @@ where
 /// Private base type which is shared by the `SharedPtr` and `SharedRef`
 /// implementations.
 #[repr(C)]
+#[derive(Eq, PartialEq)]
 pub struct SharedPtrBase<T: Shared>([usize; 2], PhantomData<T>);
 
 unsafe impl<T: Shared + Sync> Send for SharedPtrBase<T> {}
@@ -205,7 +206,6 @@ impl<T: Shared> Drop for SharedPtrBase<T> {
 
 /// Wrapper around a C++ shared_ptr. A shared_ptr may be be null.
 #[repr(C)]
-#[derive(Default)]
 pub struct SharedPtr<T: Shared>(SharedPtrBase<T>);
 
 impl<T: Shared> SharedPtr<T> {
@@ -238,6 +238,12 @@ impl<T: Shared> Clone for SharedPtr<T> {
   }
 }
 
+impl<T: Shared> Default for SharedPtr<T> {
+  fn default() -> Self {
+    Self(Default::default())
+  }
+}
+
 impl<T, U> From<U> for SharedPtr<T>
 where
   T: Shared,
@@ -625,13 +631,85 @@ where
 #[cfg(test)]
 mod tests {
   use super::*;
+  use std::ptr::null;
   use std::sync::atomic::AtomicBool;
   use std::sync::atomic::Ordering;
 
+  #[derive(Eq, PartialEq)]
+  struct MockSharedObj {
+    pub inner: u32,
+  }
+
+  impl MockSharedObj {
+    const INSTANCE_A: Self = Self { inner: 11111 };
+    const INSTANCE_B: Self = Self { inner: 22222 };
+
+    const SHARED_PTR_BASE_A: SharedPtrBase<Self> =
+      SharedPtrBase([1, 1], PhantomData);
+    const SHARED_PTR_BASE_B: SharedPtrBase<Self> =
+      SharedPtrBase([2, 2], PhantomData);
+  }
+
+  impl Shared for MockSharedObj {
+    fn clone(_: &SharedPtrBase<Self>) -> SharedPtrBase<Self> {
+      unimplemented!()
+    }
+
+    fn from_unique_ptr(_: UniquePtr<Self>) -> SharedPtrBase<Self> {
+      unimplemented!()
+    }
+
+    fn get(p: &SharedPtrBase<Self>) -> *const Self {
+      match p {
+        &Self::SHARED_PTR_BASE_A => &Self::INSTANCE_A,
+        &Self::SHARED_PTR_BASE_B => &Self::INSTANCE_B,
+        p if p == &Default::default() => null(),
+        _ => unreachable!(),
+      }
+    }
+
+    fn reset(p: &mut SharedPtrBase<Self>) {
+      forget(take(p));
+    }
+
+    fn use_count(_: &SharedPtrBase<Self>) -> long {
+      unimplemented!()
+    }
+  }
+
+  #[test]
+  fn shared_ptr_and_shared_ref() {
+    let mut shared_ptr_a1 = SharedPtr(MockSharedObj::SHARED_PTR_BASE_A);
+    assert!(!shared_ptr_a1.is_null());
+
+    let shared_ref_a: SharedRef<_> = shared_ptr_a1.take().unwrap();
+    assert_eq!(shared_ref_a.inner, 11111);
+
+    assert!(shared_ptr_a1.is_null());
+
+    let shared_ptr_a2: SharedPtr<_> = shared_ref_a.into();
+    assert!(!shared_ptr_a2.is_null());
+    assert_eq!(shared_ptr_a2.unwrap().inner, 11111);
+
+    let mut shared_ptr_b1 = SharedPtr(MockSharedObj::SHARED_PTR_BASE_B);
+    assert!(!shared_ptr_b1.is_null());
+
+    let shared_ref_b: SharedRef<_> = shared_ptr_b1.take().unwrap();
+    assert_eq!(shared_ref_b.inner, 22222);
+
+    assert!(shared_ptr_b1.is_null());
+
+    let shared_ptr_b2: SharedPtr<_> = shared_ref_b.into();
+    assert!(!shared_ptr_b2.is_null());
+    assert_eq!(shared_ptr_b2.unwrap().inner, 22222);
+  }
+
   static TEST_OBJ_DROPPED: AtomicBool = AtomicBool::new(false);
+
   struct TestObj {
     pub id: u32,
   }
+
   impl Drop for TestObj {
     fn drop(&mut self) {
       assert!(!TEST_OBJ_DROPPED.swap(true, Ordering::SeqCst));
@@ -639,12 +717,15 @@ mod tests {
   }
 
   struct TestObjRef(TestObj);
+
   impl Deref for TestObjRef {
     type Target = TestObj;
+
     fn deref(&self) -> &TestObj {
       &self.0
     }
   }
+
   impl Borrow<TestObj> for TestObjRef {
     fn borrow(&self) -> &TestObj {
       &**self