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

Spec Review #27

Open
ljharb opened this issue Jan 10, 2025 · 3 comments
Open

Spec Review #27

ljharb opened this issue Jan 10, 2025 · 3 comments

Comments

@ljharb
Copy link
Member

ljharb commented Jan 10, 2025

@erights erights mentioned this issue Jan 12, 2025
31 tasks
@gibson042
Copy link
Collaborator

gibson042 commented Jan 12, 2025

Thanks for the review!

I don't think I understand; we cannot make assertions about input that comes from ECMAScript code. Step 1.b.iv returns false (observably indicating lack of the requested effects) for attempts to change the configurability of a TypedArray index property—setting it to false on a mutable receiver [preexisting behavior] or to true on an immutable ArrayBuffer [new behavior]—and step 1.b.vii does likewise for attempts to change the writability. In both cases, we need to generalize from "can't set to false" to "can't change". But we could combine steps 1.b.ii and 1.b.iii into something like «If IsImmutableBuffer(O.[[ViewedArrayBuffer]]) is true, let mutable be false; else let mutable be true.».

  • It kind of seems like it might be simpler in [[DefineOwnProperty]] to have two branches - the mutable one would be unchanged, and the immutable one would only have to check whether the descriptors differed or not in any way.

I prefer instead to show that the same checks are being made (both for mutable vs. immutable here and for similarity with other [[DefineOwnProperty]]s, not to mention the preceding [[GetOwnProperty]]), just with different polarity. But this can be worked out as the proposal matures.

  • why would https://tc39.es/proposal-immutable-arraybuffer/#sec-typedarraysetelement ever be called with an immutable buffer, with the current spec? it seems like maybe it'd be better to add an assertion in step 1 that isImmutableBuffer is true? (i'm looking at [[Set]] right above it, which seems to skip calling TypedArraySetElement when the buffer is immutable, but please clarify if i'm misunderstanding)

SetTypedArrayFromArrayLike via %TypedArray%.prototype.set for one (e.g., new Uint8Array(arrayBuffer. transferToImmutable()).set([99])), and potentially more operations in the future. We're planning to make TypedArraySetElement throw an exception when invoked on an immutable instance, which should be up by the end of this week.

  • am i correct that an arraybuffer's mutability can never change, it's set at creation time?

Correct.

  • it seems strange that immutable and mutable arraybuffers have different slots - rather than [[ArrayBufferIsImmutable]], why not a single slot with a boolean that's on every arraybuffer?

It's following the precedent set by [[ArrayBufferMaxByteLength]], which could have been specified as present for all ArrayBuffer instances and set to undefined/EMPTY/etc. for fixed-length ones, but instead was specified as present only for resizable instances.

#28

Removed in #28 as well, it was referring to the IsDetachedBuffer check but is shared with slice anyway so doesn't need confirmation.

The first is now gone; the second relates to strictness of nonsensical calls like ab.sliceToImmutable(4, 2) and is intentionally open for now.

@ljharb
Copy link
Member Author

ljharb commented Jan 12, 2025

To clarify on the first point, what i mean is, i don't like seeing "set [[Writable]] to mutable", i want to see hardcoded true or false in the descriptors so i don't have to think too hard about what values are being set there.

@gibson042
Copy link
Collaborator

We're planning to make TypedArraySetElement throw an exception when invoked on an immutable instance, which should be up by the end of this week.

#29. As with most other failing assignments, the thrown exceptions are limited to strict mode.

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