Privatize JsValue's internals and expose it through a JsVariant (wi…

…th immutable reference) (#4080)

* WIP

* WIP

* WIP

* WIP

* WIP

* WIP

* WIP

* WIP

* boa_engine done

* all crates done

* clippies and fixes

* Remove Ref<> for now

* Address comments

* Fix new merges from main

* Remove as_defined in favor of map/map_or

* Fix lints

* Fix error introduced when moving to map

* Fix lints

* Add more tests and fix them

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

@@ -38,7 +38,9 @@ use std::cmp::{min, Ordering};
 use super::{BuiltInBuilder, BuiltInConstructor, IntrinsicObject};
 
 mod array_iterator;
+use crate::value::JsVariant;
 pub(crate) use array_iterator::ArrayIterator;
+
 #[cfg(test)]
 mod tests;
 
@@ -537,9 +539,9 @@ impl Array {
         // 3. Else,
         //     a. If IsCallable(mapfn) is false, throw a TypeError exception.
         //     b. Let mapping be true.
-        let mapping = match mapfn {
-            JsValue::Undefined => None,
-            JsValue::Object(o) if o.is_callable() => Some(o),
+        let mapping = match mapfn.variant() {
+            JsVariant::Undefined => None,
+            JsVariant::Object(o) if o.is_callable() => Some(o),
             _ => {
                 return Err(JsNativeError::typ()
                     .with_message(format!("`{}` is not callable", mapfn.type_of()))
@@ -2665,9 +2667,9 @@ impl Array {
         context: &mut Context,
     ) -> JsResult<JsValue> {
         // 1. If comparefn is not undefined and IsCallable(comparefn) is false, throw a TypeError exception.
-        let comparefn = match args.get_or_undefined(0) {
-            JsValue::Object(ref obj) if obj.is_callable() => Some(obj),
-            JsValue::Undefined => None,
+        let comparefn = match args.get_or_undefined(0).variant() {
+            JsVariant::Object(obj) if obj.is_callable() => Some(obj),
+            JsVariant::Undefined => None,
             _ => {
                 return Err(JsNativeError::typ()
                     .with_message("The comparison function must be either a function or undefined")
@@ -2728,9 +2730,9 @@ impl Array {
         context: &mut Context,
     ) -> JsResult<JsValue> {
         // 1. If comparefn is not undefined and IsCallable(comparefn) is false, throw a TypeError exception.
-        let comparefn = match args.get_or_undefined(0) {
-            JsValue::Object(ref obj) if obj.is_callable() => Some(obj),
-            JsValue::Undefined => None,
+        let comparefn = match args.get_or_undefined(0).variant() {
+            JsVariant::Object(obj) if obj.is_callable() => Some(obj),
+            JsVariant::Undefined => None,
             _ => {
                 return Err(JsNativeError::typ()
                     .with_message("The comparison function must be either a function or undefined")
@@ -3319,7 +3321,7 @@ fn compare_array_elements(
         let args = [x.clone(), y.clone()];
         //     a. Let v be ? ToNumber(? Call(comparefn, undefined, « x, y »)).
         let v = cmp
-            .call(&JsValue::Undefined, &args, context)?
+            .call(&JsValue::undefined(), &args, context)?
             .to_number(context)?;
         //     b. If v is NaN, return +0𝔽.
         //     c. Return v.

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

@@ -875,7 +875,8 @@ fn array_spread_non_iterable() {
 fn get_relative_start() {
     #[track_caller]
     fn assert(context: &mut Context, arg: Option<&JsValue>, len: u64, expected: u64) {
-        let arg = arg.unwrap_or(&JsValue::Undefined);
+        const UNDEFINED: &JsValue = &JsValue::undefined();
+        let arg = arg.unwrap_or(UNDEFINED);
         assert_eq!(
             Array::get_relative_start(context, arg, len).unwrap(),
             expected
@@ -902,7 +903,8 @@ fn get_relative_start() {
 fn get_relative_end() {
     #[track_caller]
     fn assert(context: &mut Context, arg: Option<&JsValue>, len: u64, expected: u64) {
-        let arg = arg.unwrap_or(&JsValue::Undefined);
+        const UNDEFINED: &JsValue = &JsValue::undefined();
+        let arg = arg.unwrap_or(UNDEFINED);
         assert_eq!(
             Array::get_relative_end(context, arg, len).unwrap(),
             expected

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

@@ -880,7 +880,7 @@ impl ArrayBuffer {
                 // 8. If allocatingResizableBuffer is true, then
                 //    c. Set obj.[[ArrayBufferMaxByteLength]] to maxByteLength.
                 max_byte_len,
-                detach_key: JsValue::Undefined,
+                detach_key: JsValue::undefined(),
             },
         );
 

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

@@ -1,4 +1,4 @@
-use crate::Context;
+use crate::{run_test_actions, Context, TestAction};
 
 #[test]
 fn create_byte_data_block() {
@@ -19,3 +19,122 @@ fn create_shared_byte_data_block() {
     // Rainy day
     assert!(super::shared::create_shared_byte_data_block(u64::MAX, context).is_err());
 }
+
+#[test]
+fn get_values() {
+    run_test_actions([
+        TestAction::run(
+            r#"
+            var buffer = new ArrayBuffer(12);
+            var sample = new DataView(buffer, 0);
+
+            sample.setUint8(0, 127);
+            sample.setUint8(1, 255);
+            sample.setUint8(2, 255);
+            sample.setUint8(3, 255);
+            sample.setUint8(4, 128);
+            sample.setUint8(5, 0);
+            sample.setUint8(6, 0);
+            sample.setUint8(7, 0);
+            sample.setUint8(8, 1);
+            sample.setUint8(9, 0);
+            sample.setUint8(10, 0);
+            sample.setUint8(11, 0);
+        "#,
+        ),
+        TestAction::assert("sample.getUint32(0, false) == 2147483647"),
+        TestAction::assert("sample.getUint32(1, false) == 4294967168"),
+        TestAction::assert("sample.getUint32(2, false) == 4294934528"),
+        TestAction::assert("sample.getUint32(3, false) == 4286578688"),
+        TestAction::assert("sample.getUint32(4, false) == 2147483648"),
+        TestAction::assert("sample.getUint32(5, false) == 1"),
+        TestAction::assert("sample.getUint32(6, false) == 256"),
+        TestAction::assert("sample.getUint32(7, false) == 65536"),
+        TestAction::assert("sample.getUint32(8, false) == 16777216"),
+        TestAction::assert("sample.getUint32(0, true) == 4294967167"),
+        TestAction::assert("sample.getUint32(1, true) == 2164260863"),
+        TestAction::assert("sample.getUint32(2, true) == 8454143"),
+        TestAction::assert("sample.getUint32(3, true) == 33023"),
+        TestAction::assert("sample.getUint32(4, true) == 128"),
+        TestAction::assert("sample.getUint32(5, true) == 16777216"),
+        TestAction::assert("sample.getUint32(6, true) == 65536"),
+        TestAction::assert("sample.getUint32(7, true) == 256"),
+        TestAction::assert("sample.getUint32(8, true) == 1"),
+    ]);
+}
+
+#[test]
+fn sort() {
+    run_test_actions([
+        TestAction::run(
+            r#"
+            function cmp(a, b) {
+                return a.length === b.length && a.every((v, i) => v === b[i]);
+            }
+
+            var TypedArrayCtor = [
+                Int8Array,
+                Uint8Array,
+                Int16Array,
+                Uint16Array,
+                Int32Array,
+                Uint32Array,
+                Float32Array,
+                Float64Array,
+            ];
+
+            var descending = TypedArrayCtor.map((ctor) => new ctor([4, 3, 2, 1]).sort());
+            var mixed = TypedArrayCtor.map((ctor) => new ctor([3, 4, 1, 2]).sort());
+            var repeating = TypedArrayCtor.map((ctor) => new ctor([0, 1, 1, 2, 3, 3, 4]).sort());
+        "#,
+        ),
+        // Descending
+        TestAction::assert("cmp(descending[0], [1, 2, 3, 4])"),
+        TestAction::assert("cmp(descending[1], [1, 2, 3, 4])"),
+        TestAction::assert("cmp(descending[2], [1, 2, 3, 4])"),
+        TestAction::assert("cmp(descending[3], [1, 2, 3, 4])"),
+        TestAction::assert("cmp(descending[4], [1, 2, 3, 4])"),
+        TestAction::assert("cmp(descending[5], [1, 2, 3, 4])"),
+        TestAction::assert("cmp(descending[6], [1, 2, 3, 4])"),
+        TestAction::assert("cmp(descending[7], [1, 2, 3, 4])"),
+        // Mixed
+        TestAction::assert("cmp(mixed[0], [1, 2, 3, 4])"),
+        TestAction::assert("cmp(mixed[1], [1, 2, 3, 4])"),
+        TestAction::assert("cmp(mixed[2], [1, 2, 3, 4])"),
+        TestAction::assert("cmp(mixed[3], [1, 2, 3, 4])"),
+        TestAction::assert("cmp(mixed[4], [1, 2, 3, 4])"),
+        TestAction::assert("cmp(mixed[5], [1, 2, 3, 4])"),
+        TestAction::assert("cmp(mixed[6], [1, 2, 3, 4])"),
+        TestAction::assert("cmp(mixed[7], [1, 2, 3, 4])"),
+        // Repeating
+        TestAction::assert("cmp(repeating[0], [0, 1, 1, 2, 3, 3, 4])"),
+        TestAction::assert("cmp(repeating[1], [0, 1, 1, 2, 3, 3, 4])"),
+        TestAction::assert("cmp(repeating[2], [0, 1, 1, 2, 3, 3, 4])"),
+        TestAction::assert("cmp(repeating[3], [0, 1, 1, 2, 3, 3, 4])"),
+        TestAction::assert("cmp(repeating[4], [0, 1, 1, 2, 3, 3, 4])"),
+        TestAction::assert("cmp(repeating[5], [0, 1, 1, 2, 3, 3, 4])"),
+        TestAction::assert("cmp(repeating[6], [0, 1, 1, 2, 3, 3, 4])"),
+        TestAction::assert("cmp(repeating[7], [0, 1, 1, 2, 3, 3, 4])"),
+    ]);
+}
+
+#[test]
+fn sort_negative_zero() {
+    run_test_actions([
+        TestAction::run(
+            r#"
+            function cmp(a, b) {
+                return a.length === b.length && a.every((v, i) => v === b[i]);
+            }
+
+            var TypedArrayCtor = [Float32Array, Float64Array];
+            var negativeZero = TypedArrayCtor.map((ctor) => new ctor([1, 0, -0, 2]).sort());
+            var infinities = TypedArrayCtor.map((ctor) => new ctor([3, 4, Infinity, -Infinity, 1, 2]).sort());
+        "#,
+        ),
+        TestAction::assert("cmp(negativeZero[0], [-0, 0, 1, 2])"),
+        TestAction::assert("cmp(negativeZero[1], [-0, 0, 1, 2])"),
+        TestAction::assert("cmp(infinities[0], [-Infinity, 1, 2, 3, 4, Infinity])"),
+        TestAction::assert("cmp(infinities[1], [-Infinity, 1, 2, 3, 4, Infinity])"),
+    ]);
+}

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

@@ -423,7 +423,7 @@ impl Date {
 
         // 4. If t is NaN, return NaN.
         if t.is_nan() {
-            return Ok(JsValue::from(f64::NAN));
+            return Ok(JsValue::new(f64::NAN));
         };
 
         if LOCAL {

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

@@ -511,12 +511,12 @@ impl BuiltInFunctionObject {
             // It is a Syntax Error if FormalParameters Contains YieldExpression is true.
             if generator && contains(&parameters, ContainsSymbol::YieldExpression) {
                 return Err(JsNativeError::syntax().with_message(
-                        if r#async {
-                            "yield expression is not allowed in formal parameter list of async generator"
-                        } else {
-                            "yield expression is not allowed in formal parameter list of generator"
-                        }
-                    ).into());
+                    if r#async {
+                        "yield expression is not allowed in formal parameter list of async generator"
+                    } else {
+                        "yield expression is not allowed in formal parameter list of generator"
+                    }
+                ).into());
             }
 
             // It is a Syntax Error if FormalParameters Contains AwaitExpression is true.

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

@@ -649,7 +649,7 @@ impl Locale {
             .keywords
             .get(&key!("kn"))
             .map(Value::as_tinystr_slice);
-        Ok(JsValue::Boolean(match kn {
+        Ok(JsValue::new(match kn {
             Some([]) => true,
             Some([kn]) if kn == "true" => true,
             _ => false,

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

@@ -174,7 +174,7 @@ impl Intl {
         let ll = locale::canonicalize_locale_list(locales, context)?;
 
         // 2. Return CreateArrayFromList(ll).
-        Ok(JsValue::Object(Array::create_array_from_list(
+        Ok(JsValue::new(Array::create_array_from_list(
             ll.into_iter().map(|loc| js_string!(loc.to_string()).into()),
             context,
         )))

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

@@ -19,6 +19,12 @@ use num_bigint::BigInt;
 use num_traits::Num;
 pub(crate) use options::*;
 
+use super::{
+    locale::{canonicalize_locale_list, filter_locales, resolve_locale, validate_extension},
+    options::{coerce_options_to_object, IntlOptions},
+    Service,
+};
+use crate::value::JsVariant;
 use crate::{
     builtins::{
         builder::BuiltInBuilder, options::get_option, string::is_trimmable_whitespace,
@@ -41,12 +47,6 @@ use crate::{
     NativeFunction,
 };
 
-use super::{
-    locale::{canonicalize_locale_list, filter_locales, resolve_locale, validate_extension},
-    options::{coerce_options_to_object, IntlOptions},
-    Service,
-};
-
 #[cfg(test)]
 mod tests;
 
@@ -338,7 +338,7 @@ impl BuiltInConstructor for NumberFormat {
                 break 'block default_use_grouping;
             }
             // 3. If value is true, return true.
-            if let &JsValue::Boolean(true) = &value {
+            if let Some(true) = value.as_boolean() {
                 break 'block GroupingStrategy::Always;
             }
 
@@ -750,8 +750,8 @@ fn unwrap_number_format(nf: &JsValue, context: &mut Context) -> JsResult<JsObjec
 
         //    a. Return ? Get(nf, %Intl%.[[FallbackSymbol]]).
         let nf = nf_o.get(fallback_symbol, context)?;
-        if let JsValue::Object(nf) = nf {
-            if let Ok(nf) = nf.downcast::<NumberFormat>() {
+        if let Some(nf) = nf.as_object() {
+            if let Ok(nf) = nf.clone().downcast::<NumberFormat>() {
                 return Ok(nf);
             }
         }
@@ -771,16 +771,16 @@ fn to_intl_mathematical_value(value: &JsValue, context: &mut Context) -> JsResul
 
     // TODO: Add support in `FixedDecimal` for infinity and NaN, which
     // should remove the returned errors.
-    match prim_value {
+    match prim_value.variant() {
         // 2. If Type(primValue) is BigInt, return ℝ(primValue).
-        JsValue::BigInt(bi) => {
+        JsVariant::BigInt(bi) => {
             let bi = bi.to_string();
             FixedDecimal::try_from(bi.as_bytes())
                 .map_err(|err| JsNativeError::range().with_message(err.to_string()).into())
         }
         // 3. If Type(primValue) is String, then
         //     a. Let str be primValue.
-        JsValue::String(s) => {
+        JsVariant::String(s) => {
             // 5. Let text be StringToCodePoints(str).
             // 6. Let literal be ParseText(text, StringNumericLiteral).
             // 7. If literal is a List of errors, return not-a-number.
@@ -791,18 +791,18 @@ 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()
             })
         }
         // 4. Else,
-        other => {
+        _ => {
             // a. Let x be ? ToNumber(primValue).
             // b. If x is -0𝔽, return negative-zero.
             // c. Let str be Number::toString(x, 10).
-            let x = other.to_number(context)?;
+            let x = prim_value.to_number(context)?;
 
             FixedDecimal::try_from_f64(x, FloatPrecision::Floating)
                 .map_err(|err| JsNativeError::range().with_message(err.to_string()).into())

core/engine/src/builtins/iterable/async_from_sync_iterator.rs

@@ -172,7 +172,7 @@ impl AsyncFromSyncIterator {
                 // b. Perform ! Call(promiseCapability.[[Resolve]], undefined, « iterResult »).
                 promise_capability
                     .resolve()
-                    .call(&JsValue::Undefined, &[iter_result], context)
+                    .call(&JsValue::undefined(), &[iter_result], context)
                     .expect("cannot fail according to spec");
 
                 // c. Return promiseCapability.[[Promise]].

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

@@ -314,8 +314,8 @@ impl IteratorResult {
     /// Gets a new `IteratorResult` from a value. Returns `Err` if
     /// the value is not a [`JsObject`]
     pub(crate) fn from_value(value: JsValue) -> JsResult<Self> {
-        if let JsValue::Object(o) = value {
-            Ok(Self { object: o })
+        if let Some(object) = value.into_object() {
+            Ok(Self { object })
         } else {
             Err(JsNativeError::typ()
                 .with_message("next value should be an object")