Remove Box<T> from JsValue for JsString

core/engine/src/builtins/date/mod.rs

@@ -237,7 +237,7 @@ impl BuiltInConstructor for Date {
                         if let Some(v) = v.as_string() {
                             // 1. Assert: The next step never returns an abrupt completion because v is a String.
                             // 2. Let tv be the result of parsing v as a date, in exactly the same manner as for the parse method (21.4.3.2).
-                            let tv = parse_date(v, context.host_hooks().as_ref());
+                            let tv = parse_date(&v, context.host_hooks().as_ref());
                             if let Some(tv) = tv {
                                 tv as f64
                             } else {

core/engine/src/builtins/eval/mod.rs

@@ -115,7 +115,7 @@ impl Eval {
         // 5. Perform ? HostEnsureCanCompileStrings(evalRealm, « », x, direct).
         context
             .host_hooks()
-            .ensure_can_compile_strings(eval_realm, &[], x, direct, context)?;
+            .ensure_can_compile_strings(eval_realm, &[], &x, direct, context)?;
 
         // 11. Perform the following substeps in an implementation-defined order, possibly interleaving parsing and error detection:
         //     a. Let script be ParseText(StringToCodePoints(x), Script).

core/engine/src/builtins/function/mod.rs

@@ -791,9 +791,7 @@ impl BuiltInFunctionObject {
         let target_name = target.get(js_string!("name"), context)?;
 
         // 9. If Type(targetName) is not String, set targetName to the empty String.
-        let target_name = target_name
-            .as_string()
-            .map_or_else(JsString::default, Clone::clone);
+        let target_name = target_name.as_string().unwrap_or_default();
 
         // 10. Perform SetFunctionName(F, targetName, "bound").
         set_function_name(&f, &target_name.into(), Some(js_str!("bound")), context);

core/engine/src/builtins/function/tests.rs

@@ -159,8 +159,7 @@ fn closure_capture_clone() {
                                     &js_string!("key").into(),
                                     &mut context.into()
                                 )?
-                                .and_then(|prop| prop.value().cloned())
-                                .and_then(|val| val.as_string().cloned())
+                                .and_then(|prop| prop.value().and_then(JsValue::as_string))
                                 .ok_or_else(
                                     || JsNativeError::typ().with_message("invalid `key` property")
                                 )?

core/engine/src/builtins/intl/list_format/mod.rs

@@ -496,7 +496,7 @@ fn string_list_from_iterable(iterable: &JsValue, context: &mut Context) -> JsRes
     //     a. Let next be ? IteratorStepValue(iteratorRecord).
     while let Some(next) = iterator.step_value(context)? {
         // c. If next is not a String, then
-        let Some(s) = next.as_string().cloned() else {
+        let Some(s) = next.as_string() else {
             // i. Let error be ThrowCompletion(a newly created TypeError object).
             // ii. Return ? IteratorClose(iteratorRecord, error).
             return Err(iterator

core/engine/src/builtins/intl/number_format/mod.rs

@@ -797,7 +797,7 @@ fn to_intl_mathematical_value(value: &JsValue, context: &mut Context) -> JsResul
             //     c. If rounded is +∞𝔽, return positive-infinity.
             //     d. If rounded is +0𝔽 and intlMV < 0, return negative-zero.
             //     e. If rounded is +0𝔽, return 0.
-            js_string_to_fixed_decimal(s).ok_or_else(|| {
+            js_string_to_fixed_decimal(&s).ok_or_else(|| {
                 JsNativeError::syntax()
                     .with_message("could not parse the provided string")
                     .into()

core/engine/src/builtins/json/mod.rs

@@ -224,8 +224,7 @@ impl Json {
                     // This is safe, because EnumerableOwnPropertyNames with 'key' type only returns strings.
                     let p = p
                         .as_string()
-                        .expect("EnumerableOwnPropertyNames only returns strings")
-                        .clone();
+                        .expect("EnumerableOwnPropertyNames only returns strings");
 
                     // 1. Let newElement be ? InternalizeJSONProperty(val, P, reviver).
                     let new_element =
@@ -315,7 +314,7 @@ impl Json {
                         // g. If item is not undefined and item is not currently an element of PropertyList, then
                         // i. Append item to the end of PropertyList.
                         if let Some(s) = v.as_string() {
-                            property_set.insert(s.clone());
+                            property_set.insert(s);
                         } else if v.is_number() {
                             property_set.insert(
                                 v.to_string(context)
@@ -482,7 +481,7 @@ impl Json {
 
         // 8. If Type(value) is String, return QuoteJSONString(value).
         if let Some(s) = value.as_string() {
-            return Ok(Some(Self::quote_json_string(s)));
+            return Ok(Some(Self::quote_json_string(&s)));
         }
 
         // 9. If Type(value) is Number, then

core/engine/src/builtins/object/mod.rs

@@ -872,10 +872,11 @@ impl OrdinaryObject {
         let tag = o.get(JsSymbol::to_string_tag(), context)?;
 
         // 16. If Type(tag) is not String, set tag to builtinTag.
-        let tag_str = tag.as_string().map_or(builtin_tag, JsString::as_str);
+        let tag = tag.as_string();
+        let tag = tag.as_ref().map_or(builtin_tag, JsString::as_str);
 
         // 17. Return the string-concatenation of "[object ", tag, and "]".
-        Ok(js_string!(js_str!("[object "), tag_str, js_str!("]")).into())
+        Ok(js_string!(js_str!("[object "), tag, js_str!("]")).into())
     }
 
     /// `Object.prototype.toLocaleString( [ reserved1 [ , reserved2 ] ] )`

core/engine/src/builtins/string/mod.rs

@@ -299,7 +299,6 @@ impl String {
     fn this_string_value(this: &JsValue) -> JsResult<JsString> {
         // 1. If Type(value) is String, return value.
         this.as_string()
-            .cloned()
             // 2. If Type(value) is Object and value has a [[StringData]] internal slot, then
             //     a. Let s be value.[[StringData]].
             //     b. Assert: Type(s) is String.

core/engine/src/builtins/temporal/plain_date/mod.rs

@@ -286,6 +286,7 @@ impl BuiltInConstructor for PlainDate {
             .get_or_undefined(3)
             .map(|s| {
                 s.as_string()
+                    .as_ref()
                     .map(JsString::to_std_string_lossy)
                     .ok_or_else(|| JsNativeError::typ().with_message("calendar must be a string."))
             })

core/engine/src/builtins/temporal/plain_date_time/mod.rs

@@ -400,6 +400,7 @@ impl BuiltInConstructor for PlainDateTime {
             .get_or_undefined(9)
             .map(|s| {
                 s.as_string()
+                    .as_ref()
                     .map(JsString::to_std_string_lossy)
                     .ok_or_else(|| JsNativeError::typ().with_message("calendar must be a string."))
             })

core/engine/src/builtins/temporal/plain_month_day/mod.rs

@@ -143,6 +143,7 @@ impl BuiltInConstructor for PlainMonthDay {
             .get_or_undefined(2)
             .map(|s| {
                 s.as_string()
+                    .as_ref()
                     .map(JsString::to_std_string_lossy)
                     .ok_or_else(|| JsNativeError::typ().with_message("calendar must be a string."))
             })

core/engine/src/builtins/temporal/plain_year_month/mod.rs

@@ -196,6 +196,7 @@ impl BuiltInConstructor for PlainYearMonth {
             .get_or_undefined(2)
             .map(|s| {
                 s.as_string()
+                    .as_ref()
                     .map(JsString::to_std_string_lossy)
                     .ok_or_else(|| JsNativeError::typ().with_message("calendar must be a string."))
             })

core/engine/src/builtins/temporal/zoneddatetime/mod.rs

@@ -432,6 +432,7 @@ impl BuiltInConstructor for ZonedDateTime {
             .get_or_undefined(2)
             .map(|s| {
                 s.as_string()
+                    .as_ref()
                     .map(JsString::to_std_string_lossy)
                     .ok_or_else(|| JsNativeError::typ().with_message("calendar must be a string."))
             })

core/engine/src/class.rs

@@ -96,7 +96,7 @@
 //!
 //!     assert_eq!(
 //!         result.as_string().unwrap(),
-//!         &js_str!("My pet is 3 years old. Right, buddy? - woof!")
+//!         js_str!("My pet is 3 years old. Right, buddy? - woof!")
 //!     );
 //! }
 //! ```

core/engine/src/error.rs

@@ -428,6 +428,7 @@ impl JsError {
                 {
                     Cow::Owned(
                         msg.as_string()
+                            .as_ref()
                             .map(JsString::to_std_string)
                             .transpose()
                             .map_err(|_| TryNativeError::InvalidMessageEncoding)?

core/engine/src/object/builtins/jsarray.rs

@@ -130,7 +130,6 @@ impl JsArray {
         )
         .map(|x| {
             x.as_string()
-                .cloned()
                 .expect("Array.prototype.join always returns string")
         })
     }

core/engine/src/object/builtins/jstypedarray.rs

@@ -806,7 +806,6 @@ impl JsTypedArray {
         )
         .map(|x| {
             x.as_string()
-                .cloned()
                 .expect("TypedArray.prototype.join always returns string")
         })
     }

core/engine/src/object/jsobject.rs

@@ -1024,7 +1024,6 @@ impl<T: NativeObject> Debug for JsObject<T> {
                     Some(prop) => prop
                         .value()
                         .and_then(JsValue::as_string)
-                        .cloned()
                         .unwrap_or_default(),
                 };
 

core/engine/src/value/display.rs

@@ -202,6 +202,7 @@ pub(crate) fn log_string_from(x: &JsValue, print_internals: bool, print_children
                         || "<error>".into(),
                         |v| {
                             v.as_string()
+                                .as_ref()
                                 .map_or_else(
                                     || v.display().to_string(),
                                     JsString::to_std_string_escaped,
@@ -214,7 +215,7 @@ pub(crate) fn log_string_from(x: &JsValue, print_internals: bool, print_children
                     .as_ref()
                     .and_then(PropertyDescriptor::value)
                     .map(|v| {
-                        v.as_string().map_or_else(
+                        v.as_string().as_ref().map_or_else(
                             || v.display().to_string(),
                             JsString::to_std_string_escaped,
                         )

core/engine/src/value/equality.rs

@@ -94,12 +94,12 @@ impl JsValue {
             //    b. If n is NaN, return false.
             //    c. Return the result of the comparison x == n.
             (JsVariant::BigInt(a), JsVariant::String(b)) => {
-                JsBigInt::from_js_string(b).as_ref() == Some(a)
+                JsBigInt::from_js_string(&b).as_ref() == Some(a)
             }
 
             // 7. If Type(x) is String and Type(y) is BigInt, return the result of the comparison y == x.
             (JsVariant::String(a), JsVariant::BigInt(b)) => {
-                JsBigInt::from_js_string(a).as_ref() == Some(b)
+                JsBigInt::from_js_string(&a).as_ref() == Some(b)
             }
 
             // 8. If Type(x) is Boolean, return the result of the comparison ! ToNumber(x) == y.

core/engine/src/value/inner/legacy.rs

@@ -228,9 +228,9 @@ impl EnumBasedValue {
     /// Returns the value as a boxed `[JsString]`.
     #[must_use]
     #[inline]
-    pub(crate) const fn as_string(&self) -> Option<&JsString> {
+    pub(crate) fn as_string(&self) -> Option<JsString> {
         match self {
-            Self::String(value) => Some(value),
+            Self::String(value) => Some(value.clone()),
             _ => None,
         }
     }
@@ -248,7 +248,7 @@ impl EnumBasedValue {
             Self::BigInt(v) => JsVariant::BigInt(v),
             Self::Object(v) => JsVariant::Object(v.clone()),
             Self::Symbol(v) => JsVariant::Symbol(v),
-            Self::String(v) => JsVariant::String(v),
+            Self::String(v) => JsVariant::String(v.clone()),
         }
     }
 }

core/engine/src/value/inner/nan_boxed.rs

@@ -125,7 +125,7 @@ mod bits {
     use crate::object::ErasedVTableObject;
     use boa_engine::{JsBigInt, JsObject, JsSymbol};
     use boa_gc::GcBox;
-    use boa_string::JsString;
+    use boa_string::{JsString, RawJsString};
     use std::ptr::NonNull;
 
     /// The mask for the bits that indicate if the value is a NaN-value.
@@ -360,8 +360,8 @@ mod bits {
     /// The box is forgotten after this operation. It must be dropped separately,
     /// by calling `[Self::drop_pointer]`.
     #[inline(always)]
-    pub(super) unsafe fn tag_string(value: Box<JsString>) -> u64 {
-        let value = Box::into_raw(value) as u64;
+    pub(super) unsafe fn tag_string(value: JsString) -> u64 {
+        let value = JsString::into_raw(value).addr().get() as u64;
         let value_masked: u64 = value & MASK_POINTER_VALUE;
 
         // Assert alignment and location of the pointer.
@@ -387,16 +387,57 @@ mod bits {
         unsafe { NonNull::new_unchecked((value & MASK_POINTER_VALUE) as *mut T).as_ref() }
     }
 
+    /// Returns a clone of a [`JsString`] from a tagged value.
+    ///
+    /// # Safety
+    ///
+    /// The pointer must be a valid pointer to a [`JsString`], otherwise this
+    /// will result in undefined behavior.
+    #[inline(always)]
+    pub(super) unsafe fn untag_string_pointer(value: u64) -> JsString {
+        let value = (value & MASK_POINTER_VALUE) as *mut RawJsString;
+
+        // SAFETY: JsValue always holds a valid, non-null JsString, so this is safe.
+        let ptr = unsafe { NonNull::new_unchecked(value) };
+
+        // SAFETY: The caller must guarantee that the JsValue is of type JsString, which is always valid.
+        let this = unsafe { JsString::from_raw(ptr) };
+
+        let result = this.clone();
+
+        // SAFETY: Dropping the `this` would result in a use-after-free if all reference are dropped.
+        std::mem::forget(this);
+
+        result
+    }
+
     /// Returns a boxed T from a tagged value.
     ///
     /// # Safety
+    ///
     /// The pointer must be a valid pointer to a T on the heap, otherwise this
     /// will result in undefined behavior.
     #[allow(clippy::unnecessary_box_returns)]
     pub(super) unsafe fn untag_pointer_owned<T>(value: u64) -> Box<T> {
         // This is safe since we already checked the pointer is not null as this point.
         unsafe { Box::from_raw((value & MASK_POINTER_VALUE) as *mut T) }
     }
+
+    /// Returns the inner [`JsString`] from a tagged value.
+    ///
+    /// # Safety
+    ///
+    /// The pointer must be a valid pointer to a [`JsString`], otherwise this
+    /// will result in undefined behavior.
+    pub(super) unsafe fn untag_string_owned(value: u64) -> JsString {
+        let value = (value & MASK_POINTER_VALUE) as *mut RawJsString;
+
+        // SAFETY: JsValue always holds a valid, non-null JsString, so this is safe.
+        let ptr = unsafe { NonNull::new_unchecked(value) };
+
+        // SAFETY: The caller must guarantee that the JsValue is of type JsString, which is always valid.
+        unsafe { JsString::from_raw(ptr) }
+    }
 }
 
 // Verify that all const values and masks are nan.
@@ -453,12 +494,12 @@ impl Clone for NanBoxedValue {
     fn clone(&self) -> Self {
         if let Some(o) = self.as_object() {
             Self::object(o.clone())
+        } else if let Some(s) = self.as_string() {
+            Self::string(s.clone())
         } else if let Some(b) = self.as_bigint() {
             Self::bigint(b.clone())
         } else if let Some(s) = self.as_symbol() {
             Self::symbol(s.clone())
-        } else if let Some(s) = self.as_string() {
-            Self::string(s.clone())
         } else {
             Self(self.0)
         }
@@ -535,7 +576,7 @@ impl NanBoxedValue {
     #[must_use]
     #[inline(always)]
     pub(crate) fn string(value: JsString) -> Self {
-        Self::from_inner_unchecked(unsafe { bits::tag_string(Box::new(value)) })
+        Self::from_inner_unchecked(unsafe { bits::tag_string(value) })
     }
 
     /// Returns true if a value is undefined.
@@ -673,9 +714,9 @@ impl NanBoxedValue {
     /// Returns the value as a boxed `[JsString]`.
     #[must_use]
     #[inline(always)]
-    pub(crate) const fn as_string(&self) -> Option<&JsString> {
+    pub(crate) fn as_string(&self) -> Option<JsString> {
         if self.is_string() {
-            Some(unsafe { bits::untag_pointer::<'_, JsString>(self.0) })
+            Some(unsafe { bits::untag_string_pointer(self.0) })
         } else {
             None
         }
@@ -692,7 +733,7 @@ impl NanBoxedValue {
                 core::mem::forget(obj); // Prevent double drop.
                 JsVariant::Object(o)
             }
-            bits::MASK_STRING => JsVariant::String(unsafe { bits::untag_pointer(self.0) }),
+            bits::MASK_STRING => JsVariant::String(unsafe { bits::untag_string_pointer(self.0) }),
             bits::MASK_SYMBOL => JsVariant::Symbol(unsafe { bits::untag_pointer(self.0) }),
             bits::MASK_BIGINT => JsVariant::BigInt(unsafe { bits::untag_pointer(self.0) }),
             bits::MASK_INT32 => JsVariant::Integer32(bits::untag_i32(self.0)),
@@ -711,7 +752,7 @@ impl Drop for NanBoxedValue {
     fn drop(&mut self) {
         match self.0 & bits::MASK_KIND {
             bits::MASK_OBJECT => drop(unsafe { bits::untag_object_owned(self.0) }),
-            bits::MASK_STRING => drop(unsafe { bits::untag_pointer_owned::<JsString>(self.0) }),
+            bits::MASK_STRING => drop(unsafe { bits::untag_string_owned(self.0) }),
             bits::MASK_SYMBOL => drop(unsafe { bits::untag_pointer_owned::<JsSymbol>(self.0) }),
             bits::MASK_BIGINT => drop(unsafe { bits::untag_pointer_owned::<JsBigInt>(self.0) }),
             _ => {}
@@ -814,8 +855,8 @@ macro_rules! assert_type {
     ($value: ident is string($scalar: ident)) => {
         assert_type!(@@is $value, 0, 0, 0, 0, 0, 0, 1, 0, 0);
         assert_type!(@@as $value, 0, 0, 0, 0, 0, 0, 1, 0, 0);
-        assert_eq!(Some(&$scalar), $value.as_string());
-        assert_eq!($value.as_variant(), JsVariant::String(&$scalar));
+        assert_eq!(Some(&$scalar), $value.as_string().as_ref());
+        assert_eq!($value.as_variant(), JsVariant::String($scalar));
     };
 }