Bug 218697 – [JSC] Support @@species in ArrayBuffer / SharedArrayBuffer slice

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Yusuke Suzuki]

[JSC] Support @@species in ArrayBuffer / SharedArrayBuffer slice

[Yusuke Suzuki]

Created attachment 413550 [details]
Patch

[Yusuke Suzuki]

Created attachment 413551 [details]
Patch

[Yusuke Suzuki]

Created attachment 413552 [details]
Patch

[Yusuke Suzuki]

Created attachment 413553 [details]
Patch

[Ross Kirsling]

Comment on attachment 413553 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=413553&action=review

LGTM, based on my read of the spec & existing ArrayPrototype code and pending EWS.

> Source/JavaScriptCore/runtime/ArrayBufferSharingMode.h:33
>  enum class ArrayBufferSharingMode {

Should we mark this `: bool`?

> Source/JavaScriptCore/runtime/JSArrayBufferPrototype.cpp:59
> +enum class SpeciesConstructResult {

Should we mark this `: uint8_t`?

> Source/JavaScriptCore/runtime/JSArrayBufferPrototype.cpp:101
> +    // 16. Let new be ? Construct(ctor, « ð½(newLen) »).

It's kind of surprising that these `slice` checks have are all moved into speciesConstructArrayBuffer itself, but that may be fine as long as we're sure to link to the relevant spec sections beforehand:

https://tc39.es/ecma262/#sec-arraybuffer.prototype.slice
https://tc39.es/ecma262/#sec-sharedarraybuffer.prototype.slice

[Yusuke Suzuki]

Comment on attachment 413553 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=413553&action=review

Thanks!

>> Source/JavaScriptCore/runtime/ArrayBufferSharingMode.h:33
>>  enum class ArrayBufferSharingMode {
> 
> Should we mark this `: bool`?

Nice! I'll use `uint8_t` for now since we use this as an index in some places :)

>> Source/JavaScriptCore/runtime/JSArrayBufferPrototype.cpp:59
>> +enum class SpeciesConstructResult {
> 
> Should we mark this `: uint8_t`?

Nice, done.

>> Source/JavaScriptCore/runtime/JSArrayBufferPrototype.cpp:101
>> +    // 16. Let new be ? Construct(ctor, « ð½(newLen) »).
> 
> It's kind of surprising that these `slice` checks have are all moved into speciesConstructArrayBuffer itself, but that may be fine as long as we're sure to link to the relevant spec sections beforehand:
> 
> https://tc39.es/ecma262/#sec-arraybuffer.prototype.slice
> https://tc39.es/ecma262/#sec-sharedarraybuffer.prototype.slice

Nice! I'll put these links in this function :)

[Yusuke Suzuki]

Committed r269574: <https://trac.webkit.org/changeset/269574>

[Radar WebKit Bug Importer]

<rdar://problem/71175759>