From a31c536cd5e2b9522ed7a571fa729d1a7ade6ddb Mon Sep 17 00:00:00 2001
From: GrayJack <gr41.j4ck@gmail.com>
Date: Wed, 27 Nov 2024 01:14:05 -0300
Subject: [PATCH] refactor: Make `IsJanetAbstract` trait an unsafe trait

BREAKING CHANGE: This changes how the trait is implemented, requiring the implementer to use the unsafe keyword
---
 CHANGELOG.md          |  1 +
 src/types/abstract.rs | 23 ++++++++++++++++-------
 src/types/io.rs       |  2 +-
 src/types/random.rs   |  2 +-
 4 files changed, 19 insertions(+), 9 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 0aaef34bcb..19a0cba344 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -9,6 +9,7 @@ All notable changes to the library should be put here
 - **Breaking** Refactor: Turn `JanetConversionError` into a enum
 - **Breaking** Refactor: Refactor `CFunOptions` to use `CStr` instead of `str`
 - **Breaking** Refactor: Use `usize` for length/index/capacity in collections
+- **Breaking** Refactor: Make `IsJanetAbstract` an unsafe trait
 - **Breaking** Feat: Make `amalgation` feature enabled by default
 - Feat: Add `Janet::dynamic_from_cstr` constructor
 - Feat: Add `JanetArgs::get_value` and `JanetArgs::get_tagged` trait methods
diff --git a/src/types/abstract.rs b/src/types/abstract.rs
index 8269c8ec37..d184b5a580 100644
--- a/src/types/abstract.rs
+++ b/src/types/abstract.rs
@@ -341,8 +341,17 @@ impl Ord for JanetAbstract {
 }
 
 /// The trait that encodes the information required to instantiate the implementer as
-/// [`JanetAbstract`]
-pub trait IsJanetAbstract {
+/// [`JanetAbstract`].
+///
+/// # Safety
+/// Implementing this trait is not trivial if the type implements [`Drop`] and can cause
+/// undefined behavior if not implemented carefully.
+///
+/// Usually that can be solved by defining [`Get`](IsJanetAbstract::Get) as a
+/// [`ManuallyDrop<Self>`]. But in that case, you also become responsible to define
+/// [`JanetAbstractType::gc`] function to properly handle the dropping of the type for the
+/// Janet-side.
+pub unsafe trait IsJanetAbstract {
     /// The type that you get when you call [`JanetAbstract::get`] family of functions.
     ///
     /// This is usually set to `Self` when the type does not implement [`Drop`], or
@@ -375,7 +384,7 @@ pub fn register<T: IsJanetAbstract>() {
     }
 }
 
-impl IsJanetAbstract for i64 {
+unsafe impl IsJanetAbstract for i64 {
     type Get = i64;
 
     const SIZE: usize = core::mem::size_of::<Self>();
@@ -386,7 +395,7 @@ impl IsJanetAbstract for i64 {
     }
 }
 
-impl IsJanetAbstract for u64 {
+unsafe impl IsJanetAbstract for u64 {
     type Get = u64;
 
     const SIZE: usize = core::mem::size_of::<Self>();
@@ -397,7 +406,7 @@ impl IsJanetAbstract for u64 {
     }
 }
 
-impl<A> IsJanetAbstract for ManuallyDrop<A>
+unsafe impl<A> IsJanetAbstract for ManuallyDrop<A>
 where
     A: IsJanetAbstract,
 {
@@ -490,7 +499,7 @@ mod tests {
         bytes: None,
     };
 
-    impl IsJanetAbstract for TestDrop {
+    unsafe impl IsJanetAbstract for TestDrop {
         type Get = ManuallyDrop<Self>;
 
         const SIZE: usize = core::mem::size_of::<Self>();
@@ -538,7 +547,7 @@ mod tests {
         bytes: None,
     };
 
-    impl IsJanetAbstract for TestDrop2 {
+    unsafe impl IsJanetAbstract for TestDrop2 {
         type Get = Self;
 
         const SIZE: usize = core::mem::size_of::<Self>();
diff --git a/src/types/io.rs b/src/types/io.rs
index cb75cb4d4c..2db7746093 100644
--- a/src/types/io.rs
+++ b/src/types/io.rs
@@ -100,7 +100,7 @@ impl JanetFile {
     }
 }
 
-impl IsJanetAbstract for JanetFile {
+unsafe impl IsJanetAbstract for JanetFile {
     type Get = Self;
 
     const SIZE: usize = mem::size_of::<evil_janet::JanetFile>();
diff --git a/src/types/random.rs b/src/types/random.rs
index 84b87d450c..aa0cdbd41f 100644
--- a/src/types/random.rs
+++ b/src/types/random.rs
@@ -25,7 +25,7 @@ impl fmt::Debug for JanetRng {
     }
 }
 
-impl IsJanetAbstract for JanetRng {
+unsafe impl IsJanetAbstract for JanetRng {
     type Get = Self;
 
     const SIZE: usize = mem::size_of::<Self>();