Skip to content

Commit

Permalink
Remove some internal deprecated functions
Browse files Browse the repository at this point in the history
- Remove `Array::new_array()`
- Remove `JsValue::set_field()`
- Remove `JsValue::set_property()`
- Remove some uses of JsValue::get_field
  • Loading branch information
HalidOdat committed Feb 26, 2022
1 parent 6093a66 commit 9c28207
Show file tree
Hide file tree
Showing 9 changed files with 74 additions and 140 deletions.
52 changes: 9 additions & 43 deletions boa_engine/src/builtins/array/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -273,32 +273,29 @@ impl Array {
array
}

/// Creates a new `Array` instance.
pub(crate) fn new_array(context: &mut Context) -> JsValue {
Self::array_create(0, None, context)
.expect("creating an empty array with the default prototype must not fail")
.into()
}

/// Utility function for concatenating array objects.
///
/// Returns a Boolean valued property that if `true` indicates that
/// an object should be flattened to its array elements
/// by `Array.prototype.concat`.
fn is_concat_spreadable(this: &JsValue, context: &mut Context) -> JsResult<bool> {
fn is_concat_spreadable(o: &JsValue, context: &mut Context) -> JsResult<bool> {
// 1. If Type(O) is not Object, return false.
if !this.is_object() {
let o = if let Some(o) = o.as_object() {
o
} else {
return Ok(false);
}
};

// 2. Let spreadable be ? Get(O, @@isConcatSpreadable).
let spreadable = this.get_field(WellKnownSymbols::is_concat_spreadable(), context)?;
let spreadable = o.get(WellKnownSymbols::is_concat_spreadable(), context)?;

// 3. If spreadable is not undefined, return ! ToBoolean(spreadable).
if !spreadable.is_undefined() {
return Ok(spreadable.to_boolean());
}

// 4. Return ? IsArray(O).
this.is_array(context)
o.is_array_abstract(context)
}

/// `get Array [ @@species ]`
Expand Down Expand Up @@ -374,37 +371,6 @@ impl Array {
}
}

/// Utility function which takes an existing array object and puts additional
/// values on the end, correctly rewriting the length
pub(crate) fn add_to_array_object(
array_ptr: &JsValue,
add_values: &[JsValue],
context: &mut Context,
) -> JsResult<JsValue> {
let orig_length = array_ptr.get_field("length", context)?.to_length(context)?;

for (n, value) in add_values.iter().enumerate() {
let new_index = orig_length.wrapping_add(n);
array_ptr.set_property(
new_index,
PropertyDescriptor::builder()
.value(value)
.configurable(true)
.enumerable(true)
.writable(true),
);
}

array_ptr.set_field(
"length",
JsValue::new(orig_length.wrapping_add(add_values.len())),
false,
context,
)?;

Ok(array_ptr.clone())
}

/// `Array.isArray( arg )`
///
/// The isArray function takes one argument arg, and returns the Boolean value true
Expand Down
8 changes: 6 additions & 2 deletions boa_engine/src/builtins/array/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1543,8 +1543,12 @@ fn get_relative_end() {
fn array_length_is_not_enumerable() {
let mut context = Context::default();

let array = Array::new_array(&mut context);
let desc = array.get_property("length").unwrap();
let array =
Array::array_create(0, None, &mut context).expect("could not create an empty array");
let desc = array
.__get_own_property__(&"length".into(), &mut context)
.expect("accessing length property on array should not throw")
.expect("there should always be a length property on arrays");
assert!(!desc.expect_enumerable());
}

Expand Down
51 changes: 32 additions & 19 deletions boa_engine/src/builtins/error/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use crate::{
internal_methods::get_prototype_from_constructor, ConstructorBuilder, JsObject, ObjectData,
},
property::Attribute,
Context, JsResult, JsValue,
Context, JsResult, JsString, JsValue,
};
use boa_profiler::Profiler;

Expand Down Expand Up @@ -108,33 +108,46 @@ impl Error {
_: &[JsValue],
context: &mut Context,
) -> JsResult<JsValue> {
if !this.is_object() {
// 1. Let O be the this value.
let o = if let Some(o) = this.as_object() {
o
// 2. If Type(O) is not Object, throw a TypeError exception.
} else {
return context.throw_type_error("'this' is not an Object");
}
let name = this.get_field("name", context)?;
let name_to_string;
};

// 3. Let name be ? Get(O, "name").
let name = o.get("name", context)?;

// 4. If name is undefined, set name to "Error"; otherwise set name to ? ToString(name).
let name = if name.is_undefined() {
"Error"
JsString::new("Error")
} else {
name_to_string = name.to_string(context)?;
name_to_string.as_str()
name.to_string(context)?
};

let message = this.get_field("message", context)?;
let message_to_string;
let message = if message.is_undefined() {
""
// 5. Let msg be ? Get(O, "message").
let msg = o.get("message", context)?;

// 6. If msg is undefined, set msg to the empty String; otherwise set msg to ? ToString(msg).
let msg = if msg.is_undefined() {
JsString::empty()
} else {
message_to_string = message.to_string(context)?;
message_to_string.as_str()
msg.to_string(context)?
};

// 7. If name is the empty String, return msg.
if name.is_empty() {
Ok(message.into())
} else if message.is_empty() {
Ok(name.into())
} else {
Ok(format!("{name}: {message}").into())
return Ok(msg.into());
}

// 8. If msg is the empty String, return name.
if msg.is_empty() {
return Ok(name.into());
}

// 9. Return the string-concatenation of name, the code unit 0x003A (COLON),
// the code unit 0x0020 (SPACE), and msg.
Ok(format!("{name}: {msg}").into())
}
}
2 changes: 1 addition & 1 deletion boa_engine/src/builtins/json/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,7 @@ impl Json {
// 2. If Type(value) is Object or BigInt, then
if value.is_object() || value.is_bigint() {
// a. Let toJSON be ? GetV(value, "toJSON").
let to_json = value.get_field("toJSON", context)?;
let to_json = value.get_v("toJSON", context)?;

// b. If IsCallable(toJSON) is true, then
if let Some(obj) = to_json.as_object() {
Expand Down
8 changes: 4 additions & 4 deletions boa_engine/src/builtins/json/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -343,31 +343,31 @@ fn json_parse_array_with_reviver() {
.unwrap();
assert_eq!(
result
.get_field("0", &mut context)
.get_v("0", &mut context)
.unwrap()
.to_number(&mut context)
.unwrap() as u8,
2u8
);
assert_eq!(
result
.get_field("1", &mut context)
.get_v("1", &mut context)
.unwrap()
.to_number(&mut context)
.unwrap() as u8,
4u8
);
assert_eq!(
result
.get_field("2", &mut context)
.get_v("2", &mut context)
.unwrap()
.to_number(&mut context)
.unwrap() as u8,
6u8
);
assert_eq!(
result
.get_field("3", &mut context)
.get_v("3", &mut context)
.unwrap()
.to_number(&mut context)
.unwrap() as u8,
Expand Down
5 changes: 4 additions & 1 deletion boa_engine/src/builtins/string/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2065,6 +2065,9 @@ pub(crate) fn get_substitution(
result.push_str("$<");
} else {
// a. Assert: Type(namedCaptures) is Object.
let named_captures = named_captures
.as_object()
.expect("should be an object according to spec");

// b. Scan until the next > U+003E (GREATER-THAN SIGN).
let mut group_name = StdString::new();
Expand All @@ -2089,7 +2092,7 @@ pub(crate) fn get_substitution(
} else {
// i. Let groupName be the enclosed substring.
// ii. Let capture be ? Get(namedCaptures, groupName).
let capture = named_captures.get_field(group_name, context)?;
let capture = named_captures.get(group_name, context)?;

// iii. If capture is undefined, replace the text through > with the empty String.
// iv. Otherwise, replace the text through > with ? ToString(capture).
Expand Down
8 changes: 5 additions & 3 deletions boa_engine/src/object/operations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -449,12 +449,14 @@ impl JsObject {
}

// 4. If Type(C) is not Object, throw a TypeError exception.
if !c.is_object() {
let c = if let Some(c) = c.as_object() {
c
} else {
return context.throw_type_error("property 'constructor' is not an object");
}
};

// 5. Let S be ? Get(C, @@species).
let s = c.get_field(WellKnownSymbols::species(), context)?;
let s = c.get(WellKnownSymbols::species(), context)?;

// 6. If S is either undefined or null, return defaultConstructor.
if s.is_null_or_undefined() {
Expand Down
57 changes: 0 additions & 57 deletions boa_engine/src/value/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -339,51 +339,6 @@ impl JsValue {
}
}

/// Set the field in the value
///
/// Similar to `7.3.4 Set ( O, P, V, Throw )`, but returns the value instead of a boolean.
///
/// More information:
/// - [ECMAScript][spec]
///
/// [spec]: https://tc39.es/ecma262/#sec-set-o-p-v-throw
#[inline]
pub(crate) fn set_field<K, V>(
&self,
key: K,
value: V,
throw: bool,
context: &mut Context,
) -> JsResult<Self>
where
K: Into<PropertyKey>,
V: Into<Self>,
{
// 1. Assert: Type(O) is Object.
// TODO: Currently the value may not be an object.
// In that case this function does nothing.
// 2. Assert: IsPropertyKey(P) is true.
// 3. Assert: Type(Throw) is Boolean.

let key = key.into();
let value = value.into();
let _timer = Profiler::global().start_event("Value::set_field", "value");
if let Self::Object(ref obj) = *self {
// 4. Let success be ? O.[[Set]](P, V, O).
let success = obj
.clone()
.__set__(key, value.clone(), obj.clone().into(), context)?;

// 5. If success is false and Throw is true, throw a TypeError exception.
// 6. Return success.
if !success && throw {
return context.throw_type_error("Cannot assign value to property");
}
return Ok(value);
}
Ok(value)
}

/// Set the kind of an object.
#[inline]
pub fn set_data(&self, data: ObjectData) {
Expand All @@ -392,18 +347,6 @@ impl JsValue {
}
}

/// Set the property in the value.
#[inline]
pub(crate) fn set_property<K, P>(&self, key: K, property: P)
where
K: Into<PropertyKey>,
P: Into<PropertyDescriptor>,
{
if let Some(object) = self.as_object() {
object.insert(key.into(), property.into());
}
}

/// The abstract operation `ToPrimitive` takes an input argument and an optional argumen`PreferredType`pe.
///
/// <https://tc39.es/ecma262/#sec-toprimitive>
Expand Down
23 changes: 13 additions & 10 deletions boa_engine/src/vm/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,12 @@ impl Context {
Opcode::PushValueToArray => {
let value = self.vm.pop();
let array = self.vm.pop();
let array = Array::add_to_array_object(&array, &[value], self)?;
let o = array.as_object().expect("should be an object");
let len = o
.length_of_array_like(self)
.expect("should have 'length' property");
o.create_data_property_or_throw(len, value, self)
.expect("should be able to create new data property");
self.vm.push(array);
}
Opcode::PushIteratorToArray => {
Expand All @@ -191,7 +196,7 @@ impl Context {
if next.done {
break;
}
Array::add_to_array_object(&array, &[next.value], self)?;
Array::push(&array, &[next.value], self)?;
}

self.vm.push(array);
Expand Down Expand Up @@ -1169,10 +1174,7 @@ impl Context {
values.push(next.value);
}

let array = Array::array_create(0, None, self)
.expect("Array creation with 0 length should never fail");

Array::add_to_array_object(&array.clone().into(), &values, self)?;
let array = Array::create_array_from_list(values, self);

self.vm.push(iterator);
self.vm.push(next_function);
Expand Down Expand Up @@ -1235,13 +1237,14 @@ impl Context {
for _ in 0..rest_count {
args.push(self.vm.pop());
}
let array = Array::new_array(self);
Array::add_to_array_object(&array, &args, self)
.expect("could not add to array object");
let array: _ = Array::create_array_from_list(args, self);

self.vm.push(array);
} else {
self.vm.pop();
let array = Array::new_array(self);

let array = Array::array_create(0, None, self)
.expect("could not create an empty array");
self.vm.push(array);
}
}
Expand Down

0 comments on commit 9c28207

Please sign in to comment.