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.

Sign up for GitHub

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

Jump to bottom

Refactor Array.prototype.find* and TypedArray variants to use FindViaPredicate #3134

Merged
merged 3 commits into from
Jul 13, 2023
Merged

Refactor Array.prototype.find* and TypedArray variants to use FindViaPredicate #3134

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
21e0aa9
Refactor `Array.prototype.{find,findIndex}` and TypedArray variants t…
dirkdev98 Jul 11, 2023
110abce
Fix cargo fmt
dirkdev98 Jul 11, 2023
fd1af53
Remove duplicate callable check, simplify returns
dirkdev98 Jul 12, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
187 changes: 97 additions & 90 deletions boa_engine/src/builtins/array/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,13 @@ pub(crate) use array_iterator::ArrayIterator;
#[cfg(test)]
mod tests;

/// Direction for `find_via_predicate`
#[derive(Clone, Copy, Eq, PartialEq)]
pub(crate) enum Direction {
Ascending,
Descending,
}

/// JavaScript `Array` built-in implementation.
#[derive(Debug, Clone, Copy)]
pub(crate) struct Array;
Expand Down Expand Up @@ -1519,38 +1526,20 @@ impl Array {
// 2. Let len be ? LengthOfArrayLike(O).
let len = o.length_of_array_like(context)?;

// 3. If IsCallable(predicate) is false, throw a TypeError exception.
let predicate = args.get_or_undefined(0).as_callable().ok_or_else(|| {
HalidOdat marked this conversation as resolved.
Show resolved Hide resolved
JsNativeError::typ().with_message("Array.prototype.find: predicate is not callable")
})?;

let this_arg = args.get_or_undefined(1);

// 4. Let k be 0.
let mut k = 0;
// 5. Repeat, while k < len,
while k < len {
// a. Let Pk be ! ToString(𝔽(k)).
let pk = k;
// b. Let kValue be ? Get(O, Pk).
let k_value = o.get(pk, context)?;
// c. Let testResult be ! ToBoolean(? Call(predicate, thisArg, « kValue, 𝔽(k), O »)).
let test_result = predicate
.call(
this_arg,
&[k_value.clone(), k.into(), o.clone().into()],
context,
)?
.to_boolean();
// d. If testResult is true, return kValue.
if test_result {
return Ok(k_value);
}
// e. Set k to k + 1.
k += 1;
// 3. Let findRec be ? FindViaPredicate(O, len, ascending, predicate, thisArg).
let find_rec =
find_via_predicate(&o, len, Direction::Ascending, predicate, this_arg, context);

// 4. Return findRec.[[Value]].
match find_rec {
Ok((_, value)) => Ok(value),
Err(err) => Err(err),
}
// 6. Return undefined.
Ok(JsValue::undefined())
}

/// `Array.prototype.findIndex( predicate [ , thisArg ] )`
Expand All @@ -1576,35 +1565,21 @@ impl Array {
// 2. Let len be ? LengthOfArrayLike(O).
let len = o.length_of_array_like(context)?;

// 3. If IsCallable(predicate) is false, throw a TypeError exception.
let predicate = args.get_or_undefined(0).as_callable().ok_or_else(|| {
JsNativeError::typ()
.with_message("Array.prototype.findIndex: predicate is not callable")
})?;

let this_arg = args.get_or_undefined(1);

// 4. Let k be 0.
let mut k = 0;
// 5. Repeat, while k < len,
while k < len {
// a. Let Pk be ! ToString(𝔽(k)).
let pk = k;
// b. Let kValue be ? Get(O, Pk).
let k_value = o.get(pk, context)?;
// c. Let testResult be ! ToBoolean(? Call(predicate, thisArg, « kValue, 𝔽(k), O »)).
let test_result = predicate
.call(this_arg, &[k_value, k.into(), o.clone().into()], context)?
.to_boolean();
// d. If testResult is true, return 𝔽(k).
if test_result {
return Ok(JsValue::new(k));
}
// e. Set k to k + 1.
k += 1;
// 3. Let findRec be ? FindViaPredicate(O, len, ascending, predicate, thisArg).
let find_rec =
find_via_predicate(&o, len, Direction::Ascending, predicate, this_arg, context);

// 4. Return findRec.[[Index]].
match find_rec {
Ok((index, _)) => Ok(index),
Err(err) => Err(err),
}
// 6. Return -1𝔽.
Ok(JsValue::new(-1))
}

/// `Array.prototype.findLast( predicate, [thisArg] )`
Expand All @@ -1628,35 +1603,20 @@ impl Array {
// 2. Let len be ? LengthOfArrayLike(O).
let len = o.length_of_array_like(context)?;

// 3. If IsCallable(predicate) is false, throw a TypeError exception.
let predicate = args.get_or_undefined(0).as_callable().ok_or_else(|| {
JsNativeError::typ().with_message("Array.prototype.findLast: predicate is not callable")
})?;

let this_arg = args.get_or_undefined(1);

// 4. Let k be len - 1. (implementation differs slightly from spec because k is unsigned)
// 5. Repeat, while k >= 0, (implementation differs slightly from spec because k is unsigned)
for k in (0..len).rev() {
// a. Let Pk be ! ToString(𝔽(k)).
// b. Let kValue be ? Get(O, Pk).
let k_value = o.get(k, context)?;
// c. Let testResult be ! ToBoolean(? Call(predicate, thisArg, « kValue, 𝔽(k), O »)).
let test_result = predicate
.call(
this_arg,
&[k_value.clone(), k.into(), this.clone()],
context,
)?
.to_boolean();
// d. If testResult is true, return kValue.
if test_result {
return Ok(k_value);
}
// e. Set k to k - 1.
// 3. Let findRec be ? FindViaPredicate(O, len, descending, predicate, thisArg).
let find_rec =
find_via_predicate(&o, len, Direction::Descending, predicate, this_arg, context);

// 4. Return findRec.[[Value]].
match find_rec {
Ok((_, value)) => Ok(value),
Err(err) => Err(err),
jedel1043 marked this conversation as resolved.
Show resolved Hide resolved
}
// 6. Return undefined.
Ok(JsValue::undefined())
}

/// `Array.prototype.findLastIndex( predicate [ , thisArg ] )`
Expand All @@ -1680,32 +1640,21 @@ impl Array {
// 2. Let len be ? LengthOfArrayLike(O).
let len = o.length_of_array_like(context)?;

// 3. If IsCallable(predicate) is false, throw a TypeError exception.
let predicate = args.get_or_undefined(0).as_callable().ok_or_else(|| {
JsNativeError::typ()
.with_message("Array.prototype.findLastIndex: predicate is not callable")
})?;

let this_arg = args.get_or_undefined(1);

// 4. Let k be len - 1. (implementation differs slightly from spec because k is unsigned)
// 5. Repeat, while k >= 0, (implementation differs slightly from spec because k is unsigned)
for k in (0..len).rev() {
// a. Let Pk be ! ToString(𝔽(k)).
// b. Let kValue be ? Get(O, Pk).
let k_value = o.get(k, context)?;
// c. Let testResult be ! ToBoolean(? Call(predicate, thisArg, « kValue, 𝔽(k), O »)).
let test_result = predicate
.call(this_arg, &[k_value, k.into(), this.clone()], context)?
.to_boolean();
// d. If testResult is true, return 𝔽(k).
if test_result {
return Ok(JsValue::new(k));
}
// e. Set k to k - 1.
// 3. Let findRec be ? FindViaPredicate(O, len, descending, predicate, thisArg).
let find_rec =
find_via_predicate(&o, len, Direction::Descending, predicate, this_arg, context);

// 4. Return findRec.[[Index]].
match find_rec {
Ok((index, _)) => Ok(index),
Err(err) => Err(err),
}
// 6. Return -1𝔽.
Ok(JsValue::new(-1))
}

/// `Array.prototype.flat( [depth] )`
Expand Down Expand Up @@ -3076,3 +3025,61 @@ impl Array {
unscopable_list
}
}

/// `FindViaPredicate ( O, len, direction, predicate, thisArg )`
///
/// More information:
/// - [ECMAScript reference][spec]
///
/// [spec]: https://tc39.es/ecma262/#sec-findviapredicate
pub(crate) fn find_via_predicate(
o: &JsObject,
len: u64,
direction: Direction,
predicate: &JsObject,
this_arg: &JsValue,
context: &mut Context<'_>,
) -> JsResult<(JsValue, JsValue)> {
// 1. If IsCallable(predicate) is false, throw a TypeError exception.
if !predicate.is_callable() {
return Err(JsNativeError::typ()
.with_message("predicate is not callable")
.into());
}

let indices = match direction {
// 2. If direction is ascending, then
// a. Let indices be a List of the integers in the interval from 0 (inclusive) to len (exclusive), in ascending order.
Direction::Ascending => itertools::Either::Left(0..len),
// 3. Else,
// a. Let indices be a List of the integers in the interval from 0 (inclusive) to len (exclusive), in descending order.
Direction::Descending => itertools::Either::Right((0..len).rev()),
};

// 4. For each integer k of indices, do
for k in indices {
// a. Let Pk be ! ToString(𝔽(k)).
let pk = k;

// b. NOTE: If O is a TypedArray, the following invocation of Get will return a normal completion.
// c. Let kValue be ? Get(O, Pk).
let k_value = o.get(pk, context)?;

// d. Let testResult be ? Call(predicate, thisArg, « kValue, 𝔽(k), O »).
let test_result = predicate
.call(
this_arg,
&[k_value.clone(), k.into(), o.clone().into()],
context,
)?
.to_boolean();

if test_result {
// e. If ToBoolean(testResult) is true, return the Record { [[Index]]: 𝔽(k), [[Value]]: kValue }.
return Ok((JsValue::new(k), k_value));
}
}

// 5. Return the Record { [[Index]]: -1𝔽, [[Value]]: undefined }
Ok((JsValue::new(-1), JsValue::undefined()))
}
92 changes: 27 additions & 65 deletions boa_engine/src/builtins/typed_array/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

use crate::{
builtins::{
array::ArrayIterator,
array::{find_via_predicate, ArrayIterator, Direction},
array_buffer::{ArrayBuffer, SharedMemoryOrder},
iterable::iterable_to_list,
typed_array::integer_indexed_object::{ContentType, IntegerIndexed},
Expand Down Expand Up @@ -320,7 +320,7 @@ impl IntrinsicObject for TypedArray {
.method(Self::fill, "fill", 1)
.method(Self::filter, "filter", 1)
.method(Self::find, "find", 1)
.method(Self::findindex, "findIndex", 1)
.method(Self::find_index, "findIndex", 1)
.method(Self::foreach, "forEach", 1)
.method(Self::includes, "includes", 1)
.method(Self::index_of, "indexOf", 1)
Expand Down Expand Up @@ -1160,41 +1160,21 @@ impl TypedArray {
// 3. Let len be O.[[ArrayLength]].
let len = o.array_length();

// 4. If IsCallable(predicate) is false, throw a TypeError exception.
let predicate = match args.get_or_undefined(0).as_object() {
Some(obj) if obj.is_callable() => obj,
_ => {
return Err(JsNativeError::typ()
.with_message(
"TypedArray.prototype.find called with non-callable predicate function",
)
.into())
}
};
let predicate = args.get_or_undefined(0).as_callable().ok_or_else(|| {
JsNativeError::typ()
.with_message("TypedArray.prototype.find: predicate is not callable")
})?;
let this_arg = args.get_or_undefined(1);

// 5. Let k be 0.
// 6. Repeat, while k < len,
for k in 0..len {
// a. Let Pk be ! ToString(𝔽(k)).
// b. Let kValue be ! Get(O, Pk).
let k_value = obj.get(k, context).expect("Get cannot fail here");
// 4. Let findRec be ? FindViaPredicate(O, len, ascending, predicate, thisArg).
let find_rec =
find_via_predicate(obj, len, Direction::Ascending, predicate, this_arg, context);

// c. Let testResult be ! ToBoolean(? Call(predicate, thisArg, « kValue, 𝔽(k), O »)).
// d. If testResult is true, return kValue.
if predicate
.call(
args.get_or_undefined(1),
&[k_value.clone(), k.into(), this.clone()],
context,
)?
.to_boolean()
{
return Ok(k_value);
}
// 5. Return findRec.[[Value]].
match find_rec {
Ok((_, value)) => Ok(value),
Err(err) => Err(err),
}

// 7. Return undefined.
Ok(JsValue::undefined())
}

/// `23.2.3.12 %TypedArray%.prototype.findIndex ( predicate [ , thisArg ] )`
Expand All @@ -1203,7 +1183,7 @@ impl TypedArray {
/// - [ECMAScript reference][spec]
///
/// [spec]: https://tc39.es/ecma262/#sec-%typedarray%.prototype.findindex
pub(crate) fn findindex(
pub(crate) fn find_index(
this: &JsValue,
args: &[JsValue],
context: &mut Context<'_>,
Expand All @@ -1226,39 +1206,21 @@ impl TypedArray {
// 3. Let len be O.[[ArrayLength]].
let len = o.array_length();

// 4. If IsCallable(predicate) is false, throw a TypeError exception.
let predicate = match args.get_or_undefined(0).as_object() {
Some(obj) if obj.is_callable() => obj,
_ => return Err(JsNativeError::typ()
.with_message(
"TypedArray.prototype.findindex called with non-callable predicate function",
)
.into()),
};
let predicate = args.get_or_undefined(0).as_callable().ok_or_else(|| {
JsNativeError::typ()
.with_message("TypedArray.prototype.findIndex: predicate is not callable")
})?;
let this_arg = args.get_or_undefined(1);

// 5. Let k be 0.
// 6. Repeat, while k < len,
for k in 0..len {
// a. Let Pk be ! ToString(𝔽(k)).
// b. Let kValue be ! Get(O, Pk).
let k_value = obj.get(k, context).expect("Get cannot fail here");
// 4. Let findRec be ? FindViaPredicate(O, len, ascending, predicate, thisArg).
let find_rec =
find_via_predicate(obj, len, Direction::Ascending, predicate, this_arg, context);

// c. Let testResult be ! ToBoolean(? Call(predicate, thisArg, « kValue, 𝔽(k), O »)).
// d. If testResult is true, return 𝔽(k).
if predicate
.call(
args.get_or_undefined(1),
&[k_value.clone(), k.into(), this.clone()],
context,
)?
.to_boolean()
{
return Ok(k.into());
}
// 5. Return findRec.[[Index]].
match find_rec {
Ok((index, _)) => Ok(index),
Err(err) => Err(err),
}

// 7. Return -1𝔽.
Ok((-1).into())
}

/// `23.2.3.13 %TypedArray%.prototype.forEach ( callbackfn [ , thisArg ] )`
Expand Down