Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Privatize JsValue's internals and expose it through a JsVariant (with immutable reference) #4080

Merged
merged 21 commits into from
Dec 19, 2024
Merged
22 changes: 12 additions & 10 deletions core/engine/src/builtins/array/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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()))
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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.
Expand Down
6 changes: 4 additions & 2 deletions core/engine/src/builtins/array/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion core/engine/src/builtins/array_buffer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
},
);

Expand Down
121 changes: 120 additions & 1 deletion core/engine/src/builtins/array_buffer/tests.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::Context;
use crate::{run_test_actions, Context, TestAction};

#[test]
fn create_byte_data_block() {
Expand All @@ -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])"),
]);
}
2 changes: 1 addition & 1 deletion core/engine/src/builtins/date/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
12 changes: 6 additions & 6 deletions core/engine/src/builtins/function/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion core/engine/src/builtins/intl/locale/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion core/engine/src/builtins/intl/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)))
Expand Down
30 changes: 15 additions & 15 deletions core/engine/src/builtins/intl/number_format/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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;

Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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);
}
}
Expand All @@ -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.
Expand All @@ -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())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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]].
Expand Down
4 changes: 2 additions & 2 deletions core/engine/src/builtins/iterable/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
Loading
Loading