Skip to content

Invalid special_id calling Array prototype methods on TAs #91

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

Closed
littledivy opened this issue Nov 19, 2023 · 4 comments
Closed

Invalid special_id calling Array prototype methods on TAs #91

littledivy opened this issue Nov 19, 2023 · 4 comments

Comments

@littledivy
Copy link
Contributor

const u8 = new Uint8Array(1);

Array.prototype.forEach.call(u8, () => {}); // special_id != TA
u8.forEach(() => {});  // special_id == TA (expected)

This causes https://github.com/tc39/test262/blob/c1281dba453b4d97f2bc04b8dfe722c5d5f90b89/test/built-ins/Array/prototype/forEach/callbackfn-resize-arraybuffer.js#L26 to fail in #85

@bnoordhuis
Copy link
Contributor

What specifically needs to change and what's special_id? The Array methods work properly on arraylikes, AFAICT.

@littledivy
Copy link
Contributor Author

littledivy commented Nov 20, 2023

Currently these methods have predefined IDs that "hint" the this_val type to the method:

static JSValue js_array_every(JSContext *ctx, JSValueConst this_val,
                              int argc, JSValueConst *argv, int special)
{
    // ...
    if (special & special_TA) {
        obj = JS_DupValue(ctx, this_val);
        len = js_typed_array_get_length_internal(ctx, obj);
        if (len < 0)
            goto exception;

Array prototype method def:

static const JSCFunctionListEntry js_array_proto_funcs[] = {
    // ...
    JS_CFUNC_MAGIC_DEF("every", 1, js_array_every, special_every ), // <-- special_TA path not taken.

We need to change the Array prototype methods to typecheck argument and get the length using js_typed_array_get_length_internal (for variable-length TA) when special_id is special_every.

I opened this issue because it's probably a big change.

@bnoordhuis
Copy link
Contributor

Ah okay, I follow you now. I believe it should Just Work(TM), just slower than otherwise because it takes the general array-like path instead of the typed array fast path.

For something like Array.prototype.map.call(new Uint8Array(1), v => v) you'll get back an Array, not a TypedArray, but that's per spec.

Can you be specific about the issues you hit?

@littledivy
Copy link
Contributor Author

My bad, got confused about resizable arraybuffer behaviour with Array methods. #85 (comment)

@littledivy littledivy closed this as not planned Won't fix, can't repro, duplicate, stale Nov 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants