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

feat(array): add to_reversed prototype method #446

Merged
merged 6 commits into from
Nov 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions nova_vm/src/ecmascript/builtins/array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,19 @@ impl Array {
agent[self].elements.is_trivial(agent)
}

// This method creates a "shallow clone" of the elements of a simple array (no descriptors).
// If array is not simple, this cloned array will do some odd things (e.g. getter/setter indexes become holes)
pub(crate) fn to_cloned(self, agent: &mut Agent) -> Array {
let elements = agent[self].elements;
let cloned_elements = agent.heap.elements.shallow_clone(elements.into());
let data = ArrayHeapData {
object_index: None,
elements: cloned_elements,
};
agent.heap.arrays.push(Some(data));
Array(ArrayIndex::last(&agent.heap.arrays))
}

#[inline]
fn internal_get_backing(
self,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ use crate::{
},
builders::ordinary_object_builder::OrdinaryObjectBuilder,
builtins::{
array_species_create, ArgumentsList, ArrayHeapData, Behaviour, Builtin,
array_create, array_species_create, ArgumentsList, ArrayHeapData, Behaviour, Builtin,
BuiltinIntrinsic,
},
execution::{
Expand Down Expand Up @@ -2833,13 +2833,44 @@ impl ArrayPrototype {
}

fn to_reversed(
_agent: &mut Agent,
_gc: GcScope<'_, '_>,

_this_value: Value,
agent: &mut Agent,
mut gc: GcScope<'_, '_>,
this_value: Value,
_: ArgumentsList,
) -> JsResult<Value> {
todo!();
if let Value::Array(array) = this_value {
// Fast path: Array is dense and contains no descriptors. No JS
// functions can thus be called by to_reversed.
if array.is_trivial(agent) && array.is_dense(agent) {
let cloned_array = array.to_cloned(agent);
cloned_array.as_mut_slice(agent).reverse();
return Ok(cloned_array.into_value());
}
}

// 1. Let O be ? ToObject(this value).
let o = to_object(agent, this_value)?;
ivandevp marked this conversation as resolved.
Show resolved Hide resolved
// 2. Let len be ? LengthOfArrayLike(O).
let len = length_of_array_like(agent, gc.reborrow(), o)?;
// 3. Let A be ? ArrayCreate(len).
let a = array_create(agent, len as usize, len as usize, None)?;
// 4. Let k be 0.
let mut k = 0;
// 5. Repeat, while k < len,
while k < len {
// a. Let from be ! ToString(𝔽(len - k - 1)).
let from = PropertyKey::Integer((len - k - 1).try_into().unwrap());
// b. Let Pk be ! ToString(𝔽(k)).
let pk = PropertyKey::try_from(k).unwrap();
// c. Let fromValue be ? Get(O, from).
let from_value = get(agent, gc.reborrow(), o, from)?;
// d. Perform ! CreateDataPropertyOrThrow(A, Pk, fromValue).
create_data_property_or_throw(agent, gc.reborrow(), a, pk, from_value).unwrap();
// e. Set k to k + 1.
k += 1;
}
// 6. Return A.
Ok(a.into_value())
}

fn to_sorted(
Expand Down
66 changes: 66 additions & 0 deletions nova_vm/src/heap/element_array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2304,6 +2304,72 @@ impl ElementArrays {
.contains(&Some(element)),
}
}

/// This method creates a "shallow clone" of the elements of a trivial/dense array.
/// It does not do anything with descriptors and assumes there is a previous validation in place.
pub fn shallow_clone(&mut self, elements_vector: ElementsVector) -> SealableElementsVector {
aapoalas marked this conversation as resolved.
Show resolved Hide resolved
let index = elements_vector.elements_index.into_index();
let ElementArrays {
e2pow4,
e2pow6,
e2pow8,
e2pow10,
e2pow12,
e2pow16,
e2pow24,
e2pow32,
} = self;
let new_index = match elements_vector.cap {
ElementArrayKey::Empty => ElementIndex::from_u32_index(0),
ElementArrayKey::E4 => {
let elements = e2pow4;
elements.values.extend_from_within(index..index + 1);
ElementIndex::last_element_index(&elements.values)
}
ElementArrayKey::E6 => {
let elements = e2pow6;
elements.values.extend_from_within(index..index + 1);
ElementIndex::last_element_index(&elements.values)
}
ElementArrayKey::E8 => {
let elements = e2pow8;
elements.values.extend_from_within(index..index + 1);
ElementIndex::last_element_index(&elements.values)
}
ElementArrayKey::E10 => {
let elements = e2pow10;
elements.values.extend_from_within(index..index + 1);
ElementIndex::last_element_index(&elements.values)
}
ElementArrayKey::E12 => {
let elements = e2pow12;
elements.values.extend_from_within(index..index + 1);
ElementIndex::last_element_index(&elements.values)
}
ElementArrayKey::E16 => {
let elements = e2pow16;
elements.values.extend_from_within(index..index + 1);
ElementIndex::last_element_index(&elements.values)
}
ElementArrayKey::E24 => {
let elements = e2pow24;
elements.values.extend_from_within(index..index + 1);
ElementIndex::last_element_index(&elements.values)
}
ElementArrayKey::E32 => {
let elements = e2pow32;
elements.values.extend_from_within(index..index + 1);
ElementIndex::last_element_index(&elements.values)
}
};

SealableElementsVector {
cap: elements_vector.cap,
elements_index: new_index,
len: elements_vector.len(),
len_writable: true,
}
}
}

impl HeapMarkAndSweep for ElementDescriptor {
Expand Down
13 changes: 0 additions & 13 deletions tests/expectations.json
Original file line number Diff line number Diff line change
Expand Up @@ -488,19 +488,6 @@
"built-ins/Array/prototype/toLocaleString/resizable-buffer.js": "CRASH",
"built-ins/Array/prototype/toLocaleString/user-provided-tolocalestring-grow.js": "CRASH",
"built-ins/Array/prototype/toLocaleString/user-provided-tolocalestring-shrink.js": "CRASH",
"built-ins/Array/prototype/toReversed/frozen-this-value.js": "CRASH",
"built-ins/Array/prototype/toReversed/get-descending-order.js": "CRASH",
"built-ins/Array/prototype/toReversed/holes-not-preserved.js": "CRASH",
"built-ins/Array/prototype/toReversed/ignores-species.js": "CRASH",
"built-ins/Array/prototype/toReversed/immutable.js": "CRASH",
"built-ins/Array/prototype/toReversed/length-casted-to-zero.js": "CRASH",
"built-ins/Array/prototype/toReversed/length-decreased-while-iterating.js": "CRASH",
"built-ins/Array/prototype/toReversed/length-exceeding-array-length-limit.js": "CRASH",
"built-ins/Array/prototype/toReversed/length-increased-while-iterating.js": "CRASH",
"built-ins/Array/prototype/toReversed/length-tolength.js": "CRASH",
"built-ins/Array/prototype/toReversed/this-value-boolean.js": "CRASH",
"built-ins/Array/prototype/toReversed/this-value-nullish.js": "CRASH",
"built-ins/Array/prototype/toReversed/zero-or-one-element.js": "CRASH",
"built-ins/Array/prototype/toSorted/comparefn-called-after-get-elements.js": "CRASH",
"built-ins/Array/prototype/toSorted/comparefn-not-a-function.js": "CRASH",
"built-ins/Array/prototype/toSorted/comparefn-stop-after-error.js": "CRASH",
Expand Down
4 changes: 2 additions & 2 deletions tests/metrics.json
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
{
"results": {
"crash": 16114,
"crash": 16101,
"fail": 8357,
"pass": 20777,
"pass": 20790,
"skip": 40,
"timeout": 3,
"unresolved": 0
Expand Down