feat(ecmascript): %TypedArray%.prototype.set #744

yossydev · 2025-06-01T08:07:42Z

I think there are still a few things we can do.

ref: #145
doc: https://tc39.es/ecma262/multipage/indexed-collections.html#sec-%typedarray%.prototype.set

…m_slice

yossydev · 2025-06-01T08:10:21Z

tests/expectations.json

-  "built-ins/TypedArray/prototype/set/BigInt/boolean-tobigint.js": "FAIL",
-  "built-ins/TypedArray/prototype/set/BigInt/null-tobigint.js": "FAIL",
-  "built-ins/TypedArray/prototype/set/BigInt/number-tobigint.js": "FAIL",
-  "built-ins/TypedArray/prototype/set/BigInt/src-typedarray-big.js": "FAIL",
  "built-ins/TypedArray/prototype/set/BigInt/src-typedarray-not-big-throws.js": "FAIL",


Here are the reasons for the test262 failures.

internal error: entered unreachable code

thread 'main' panicked at nova_vm/src/ecmascript/types/spec/data_block.rs:683:13: internal error: entered unreachable code note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace fn from_le_value(agent: &mut Agent, value: Numeric) -> Self { let Ok(value) = BigInt::try_from(value) else { unreachable!() }; to_big_int64_big_int(agent, value).to_le() }

Uncaught exception: Error: SharedArrayBuffer not implemented (Testing with Int16Array.)

Uncaught exception: Error: SharedArrayBuffer not implemented (Testing with Int16Array.) Test result: Fail

.../ecmascript/builtins/indexed_collections/typed_array_objects/typed_array_intrinsic_object.rs

…copy_from_slice" This reverts commit 7877d47.

aapoalas

A bit more work to do around the GC handling here. You could also use the existing optimised TypedArray copy functions here instead of using the generic GetValueFromBuffer and SetValueFromBuffer methods.

aapoalas · 2025-06-06T09:28:13Z

nova_vm/src/ecmascript/builtins/indexed_collections/typed_array_objects/abstract_operations.rs

+/// targetOffset, reading the values from source.
+pub(crate) fn set_typed_array_from_typed_array<
+    'a,
+    TargetType: Viewable + std::fmt::Debug,


nitpick: Preferablytake out the std::fmt::Debug bounds once you don't need them for debugging anymore.

aapoalas · 2025-06-06T09:33:56Z

nova_vm/src/ecmascript/builtins/indexed_collections/typed_array_objects/abstract_operations.rs

+        ));
+    };
+    // 17. If target.[[ContentType]] is not source.[[ContentType]], throw a TypeError exception.
+    let is_type_match = has_matching_content_type::<TargetType>(target.get(agent));


issue: You should be checking source against TargetType; this is probably the reason for one of the failing tests.

Suggested change

let is_type_match = has_matching_content_type::<TargetType>(target.get(agent));

let is_type_match = has_matching_content_type::<TargetType>(source.get(agent));

aapoalas · 2025-06-06T09:35:09Z

nova_vm/src/ecmascript/builtins/indexed_collections/typed_array_objects/abstract_operations.rs

+    // 18. If IsSharedArrayBuffer(srcBuffer) is true, IsSharedArrayBuffer(targetBuffer) is true, and srcBuffer.[[ArrayBufferData]] is targetBuffer.[[ArrayBufferData]], let sameSharedArrayBuffer be true; otherwise, let sameSharedArrayBuffer be false.
+    // TODO: SharedArrayBuffer that we can even take here.
+    // 19. If SameValue(srcBuffer, targetBuffer) is true or sameSharedArrayBuffer is true, then
+    let mut src_byte_index = if same_value(agent, src_buffer, target_buffer) {


nitpick: I think you can just compare src_buffer == target_buffer directly; SameValue(objA, objB) is equal to objA == objB.

aapoalas · 2025-06-06T09:36:57Z

nova_vm/src/ecmascript/builtins/indexed_collections/typed_array_objects/abstract_operations.rs

+            src_byte_length,
+            gc.nogc(),
+        )
+        .unbind()?;


issue: You need to rebind here since the resulting cloned src_buffer needs to be tracked by the GC:

Suggested change

.unbind()?;

.unbind()?.bind(gc.nogc());

aapoalas · 2025-06-06T09:41:12Z

nova_vm/src/ecmascript/builtins/indexed_collections/typed_array_objects/abstract_operations.rs

+    if core::any::TypeId::of::<SrcType>() == core::any::TypeId::of::<TargetType>() {
+        // a. NOTE: The transfer must be performed in a manner that preserves the bit-level encoding of the source data.
+        // Repeat, while targetByteIndex < limit,
+        while target_byte_index < limit {


issue: Here we want to use the methods you've already added to dealing with copying data from one TypedArray to another, that just perform copy_from_slice from &[SrcType: Viewable] to &mut [TargetType: Viewable].

aapoalas · 2025-06-06T10:03:13Z

.../ecmascript/builtins/indexed_collections/typed_array_objects/typed_array_intrinsic_object.rs

+                    scoped_o.get(agent),
+                    target_offset,
+                    scoped_source.get(agent),
+                    gc.reborrow()


nitpick: You could just pass in gc directly to all the SetTypedArrayFrom... calls, which would mean you don't need to .unbind()? the result but can just use ? directly.

aapoalas · 2025-06-06T10:03:48Z

.../ecmascript/builtins/indexed_collections/typed_array_objects/typed_array_intrinsic_object.rs

+                    scoped_o.get(agent),
+                    target_offset,
+                    source,
+                    gc.reborrow()


Suggested change

gc.reborrow()

gc

aapoalas · 2025-06-06T10:04:02Z

.../ecmascript/builtins/indexed_collections/typed_array_objects/typed_array_intrinsic_object.rs

+                ),
+                V
+            )
+            .unbind()?;


Suggested change

.unbind()?;

?;

aapoalas · 2025-06-06T10:05:25Z

.../ecmascript/builtins/indexed_collections/typed_array_objects/typed_array_intrinsic_object.rs

+                scoped_o.get(agent),
+                set_typed_array_from_array_like::<T>(
+                    agent,
+                    scoped_o.get(agent),


issue: Since you already have scoped_o as Scoped, it doesn't make sense to get its result and then immediately scope it again inside the SetTypedArrayFromX calls: Just pass the Scoped<TypedArray> directly instead, and that'll save you one unnecessary scoping.

aapoalas · 2025-06-06T10:06:44Z

.../ecmascript/builtins/indexed_collections/typed_array_objects/typed_array_intrinsic_object.rs

+                    agent,
+                    scoped_o.get(agent),
+                    target_offset,
+                    scoped_source.get(agent),


issue: Here too, you can just pass in the scoped_source: Scoped<Value> directly; inside the call you'll need to do let src = source.replace_self(agent, src); after you've done let src = to_object(agent, source.get(agent), gc.nogc()); but otherwise it should be all good.

yossydev added 4 commits June 1, 2025 16:28

feat(ecmascript): %TypedArray%.prototype.set

5dd773f

chore(ecmascript): update expectations

86f02e2

chore(ecmascript): update metrics

6a686d2

chore(ecmascript): target and source are sameType then, used copy_fro…

7877d47

…m_slice

yossydev commented Jun 1, 2025

View reviewed changes

Revert "chore(ecmascript): target and source are sameType then, used …

62a04ef

…copy_from_slice" This reverts commit 7877d47.

yossydev changed the title ~~feat: %TypedArray%.prototype.set~~ feat(ecmascript): %TypedArray%.prototype.set Jun 1, 2025

chore(ecmascript): update expectations and metrics

1b88e70

yossydev marked this pull request as ready for review June 1, 2025 09:12

aapoalas requested changes Jun 6, 2025

View reviewed changes

yossydev added 4 commits June 12, 2025 09:01

chore(ecmascript): remove std::fmt::Debug

8d042e2

chore(ecmascript): source against TargetType

2f57750

chore(ecmascript): compare src_buffer == target_buffer

426cca4

chore(ecmascript) fix review

cae79d9

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(ecmascript): %TypedArray%.prototype.set #744

feat(ecmascript): %TypedArray%.prototype.set #744

Uh oh!

yossydev commented Jun 1, 2025

Uh oh!

yossydev Jun 1, 2025

Uh oh!

Uh oh!

aapoalas left a comment

Uh oh!

aapoalas Jun 6, 2025

Uh oh!

aapoalas Jun 6, 2025

Uh oh!

aapoalas Jun 6, 2025

Uh oh!

aapoalas Jun 6, 2025

Uh oh!

aapoalas Jun 6, 2025

Uh oh!

aapoalas Jun 6, 2025

Uh oh!

aapoalas Jun 6, 2025

Uh oh!

aapoalas Jun 6, 2025

Uh oh!

aapoalas Jun 6, 2025

Uh oh!

aapoalas Jun 6, 2025

Uh oh!

Uh oh!

	let is_type_match = has_matching_content_type::<TargetType>(target.get(agent));
	let is_type_match = has_matching_content_type::<TargetType>(source.get(agent));

feat(ecmascript): %TypedArray%.prototype.set #744

Are you sure you want to change the base?

feat(ecmascript): %TypedArray%.prototype.set #744

Uh oh!

Conversation

yossydev commented Jun 1, 2025

Uh oh!

Choose a reason for hiding this comment

internal error: entered unreachable code

Uncaught exception: Error: SharedArrayBuffer not implemented (Testing with Int16Array.)

Uh oh!

Uh oh!

aapoalas left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!