-
Notifications
You must be signed in to change notification settings - Fork 858
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
Implement ES2023 copy methods on Array and TypedArray #1569
Conversation
Is |
Yeah, I meant to do that, but I had a long meeting while I was updating the description and I missed it 😅 Here it is: #1572 |
This looks pretty complete to me -- anyone else have an opinion? Otherwise I will merge it in the next few days. |
This looks good, but I want to keep the code clean and fix one warning: I added "ErrorProne" a while back and I'm trying to clear its warnings (and may consider making them errors). Would you mind fixing this one minor one that shows up in the build with these new changes? /home/runner/work/rhino/rhino/rhino/src/main/java/org/mozilla/javascript/NativeArray.java:2239: warning: [UnnecessaryParentheses] These grouping parentheses are unnecessary; it is unlikely the code will be misinterpreted without them |
Sure, done. |
This PR adds:
Array.prototype.toReversed
Array.prototype.toSorted
Array.prototype.toSpliced
Array.prototype.with
TypedArray.prototype.toReversed
TypedArray.prototype.toSorted
TypedArray.prototype.with
(There is no
TypedArray.prototype.toSpliced
in the standard).I tried to follow the spec very closely, even matching (most) variable names.
For arrays, it passes all test262 cases except those marked with
Reflect.construct
. Note that, to do so, I couldn't rely on some optimization, for example thedenseOnly
flag.For typed arrays, in addition to the cases marked with
Reflect.construct
, there are some other test262 cases that don't pass:prototype/xxx/metadata
- caused by TypedArray implementations have a null prototype instead of the TypedArray intrinsic #1565 as far as I can tell. There's already Investigate failures on TypedArray test262 cases #1528 which tracks similar problems.prototype/xxx/length-property-ignored.js
- do not pass because rhino does not allow to configureTypedArray.prototype.length
- tracked by Cannot modify length property of TypedArray #1572prototype/toReversed/this-value-invalid.js
andprototype/toSorted/this-value-invalid.js
- these try to call$DETACHBUFFER
, which we do not support - see Implement$262.detachArrayBuffer
for typed arrays #1527prototype/toString/BigInt/detached-buffer.js
and `prototype/with/BigInt/early-type-coercion-bigint.js - tracked ES2020 support BigInt64Array & BigUint64Array Typed Arrays #1410prototype/with/index-validated-against-current-length.js
- we do not supportArrayBuffer.prototype.resize
; ES2024 resizable ArrayBuffer #1397 tracks thisShould fix #1307
I ended up with a single PR that does both
Array
andTypedArray
because I imagined they would share part of the implementation... but in the end, they are completely separate. Let me know if you would prefer two separate PRs.Please squash this if accepted! 🙂