From 683444e7a5c1110d08c24f825fca0f1803dab4c1 Mon Sep 17 00:00:00 2001
From: Hans Larsen <hans@larsen.online>
Date: Fri, 20 Dec 2024 15:21:26 -0800
Subject: [PATCH] Fix Drop::drop and clippies

---
 core/engine/src/value/inner.rs | 43 +++++++++++++---------------------
 core/engine/src/value/mod.rs   | 14 ++++-------
 2 files changed, 21 insertions(+), 36 deletions(-)

diff --git a/core/engine/src/value/inner.rs b/core/engine/src/value/inner.rs
index f28672b7037..69a2d8393aa 100644
--- a/core/engine/src/value/inner.rs
+++ b/core/engine/src/value/inner.rs
@@ -94,7 +94,7 @@ impl NanBitTag {
         // Either it is a constant float value,
         if value == f64::INFINITY.to_bits()
             || value == f64::NEG_INFINITY.to_bits()
-            // or it is exactly a NaN value, which is the same as the BigInt tag.
+            // or it is exactly a NaN value, which is the same as the `BigInt` tag.
             // Reminder that pointers cannot be null, so this is safe.
             || value == NanBitTag::Pointer as u64
         {
@@ -137,7 +137,7 @@ impl NanBitTag {
         value & NanBitTag::Integer32 as u64 == NanBitTag::Integer32 as u64
     }
 
-    /// Checks that a value is a valid BigInt.
+    /// Checks that a value is a valid `BigInt`.
     #[inline]
     const fn is_bigint(value: u64) -> bool {
         (value & NanBitTag::TaggedMask as u64 == NanBitTag::Pointer as u64)
@@ -296,27 +296,6 @@ impl NanBitTag {
         // Simply cast for bits.
         Self::Pointer as u64 | Self::String as u64 | value
     }
-
-    /// Drops a value if it is a pointer, otherwise do nothing.
-    #[inline]
-    unsafe fn drop_pointer(value: u64) {
-        let value_ptr = value & Self::PointerMask as u64;
-
-        if value_ptr == 0
-            || value & NanBitTag::Pointer as u64 != 0
-            || value == NanBitTag::Pointer as u64
-        {
-            return;
-        }
-
-        match value & 0x3 {
-            0 => drop(unsafe { Box::from_raw(value_ptr as *mut JsBigInt) }),
-            1 => drop(unsafe { Box::from_raw(value_ptr as *mut JsObject) }),
-            2 => drop(unsafe { Box::from_raw(value_ptr as *mut JsSymbol) }),
-            3 => drop(unsafe { Box::from_raw(value_ptr as *mut JsString) }),
-            _ => unreachable!(),
-        }
-    }
 }
 
 /// A NaN-boxed `[super::JsValue]`'s inner.
@@ -341,14 +320,16 @@ impl fmt::Debug for InnerValue {
 
 impl Finalize for InnerValue {
     fn finalize(&self) {
-        eprintln!("Finalizing InnerValue: {:?}", self.as_variant());
+        if let Some(o) = self.as_object() {
+            o.finalize();
+        }
     }
 }
 
 #[allow(unsafe_op_in_unsafe_fn)]
 unsafe impl Trace for InnerValue {
     custom_trace! {this, mark, {
-        if let JsVariant::Object(o) = this.as_variant() {
+        if let Some(o) = this.as_object() {
             mark(o);
         }
     }}
@@ -615,9 +596,17 @@ impl InnerValue {
 
 impl Drop for InnerValue {
     fn drop(&mut self) {
+        let maybe_ptr = self.0 & NanBitTag::PointerMask as u64;
+
         // Drop the pointer if it is a pointer.
-        unsafe {
-            NanBitTag::drop_pointer(self.0);
+        if self.is_object() {
+            drop(unsafe { Box::from_raw(maybe_ptr as *mut JsObject) });
+        } else if self.is_bigint() {
+            drop(unsafe { Box::from_raw(maybe_ptr as *mut JsBigInt) });
+        } else if self.is_symbol() {
+            drop(unsafe { Box::from_raw(maybe_ptr as *mut JsSymbol) });
+        } else if self.is_string() {
+            drop(unsafe { Box::from_raw(maybe_ptr as *mut JsString) });
         }
     }
 }
diff --git a/core/engine/src/value/mod.rs b/core/engine/src/value/mod.rs
index 8363ff0fdf5..9e976b168ff 100644
--- a/core/engine/src/value/mod.rs
+++ b/core/engine/src/value/mod.rs
@@ -173,7 +173,7 @@ impl JsValue {
     #[inline]
     #[must_use]
     pub fn is_callable(&self) -> bool {
-        self.as_object().map_or(false, |obj| obj.is_callable())
+        self.as_object().map_or(false, JsObject::is_callable)
     }
 
     /// Returns the callable value if the value is callable, otherwise `None`.
@@ -197,7 +197,7 @@ impl JsValue {
     #[inline]
     #[must_use]
     pub fn is_constructor(&self) -> bool {
-        self.as_object().map_or(false, |obj| obj.is_constructor())
+        self.as_object().map_or(false, JsObject::is_constructor)
     }
 
     /// Returns the constructor if the value is a constructor, otherwise `None`.
@@ -258,11 +258,7 @@ impl JsValue {
     #[inline]
     #[must_use]
     pub fn as_symbol(&self) -> Option<JsSymbol> {
-        if let Some(symbol) = self.0.as_symbol() {
-            Some(symbol.clone())
-        } else {
-            None
-        }
+        self.0.as_symbol().cloned()
     }
 
     /// Returns true if the value is undefined.
@@ -322,7 +318,7 @@ impl JsValue {
     #[must_use]
     pub fn as_number(&self) -> Option<f64> {
         if let Some(i) = self.as_i32() {
-            Some(i as f64)
+            Some(f64::from(i))
         } else {
             self.0.as_float64()
         }
@@ -459,7 +455,7 @@ impl JsValue {
             JsVariant::Undefined => Err(JsNativeError::typ()
                 .with_message("cannot convert undefined to a BigInt")
                 .into()),
-            JsVariant::String(ref string) => JsBigInt::from_js_string(string).map_or_else(
+            JsVariant::String(string) => JsBigInt::from_js_string(string).map_or_else(
                 || {
                     Err(JsNativeError::syntax()
                         .with_message(format!(