Bug 218697 - [JSC] Support @@species in ArrayBuffer / SharedArrayBuffer slice
Summary: [JSC] Support @@species in ArrayBuffer / SharedArrayBuffer slice
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Yusuke Suzuki
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-11-08 18:13 PST by Yusuke Suzuki
Modified: 2020-11-08 21:03 PST (History)
11 users (show)

See Also:

Attachments
Patch (41.86 KB, patch)
2020-11-08 18:16 PST, Yusuke Suzuki 		ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (42.00 KB, patch)
2020-11-08 18:26 PST, Yusuke Suzuki 		ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (44.65 KB, patch)
2020-11-08 18:37 PST, Yusuke Suzuki 		ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (44.80 KB, patch)
2020-11-08 19:01 PST, Yusuke Suzuki 		ross.kirsling: review+
Details | Formatted Diff | Diff
Show Obsolete (3) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Yusuke Suzuki 2020-11-08 18:13:41 PST 
[JSC] Support @@species in ArrayBuffer / SharedArrayBuffer slice
Comment 1 Yusuke Suzuki 2020-11-08 18:16:48 PST 
Created attachment 413550 [details]
Patch
Comment 2 Yusuke Suzuki 2020-11-08 18:26:44 PST 
Created attachment 413551 [details]
Patch
Comment 3 Yusuke Suzuki 2020-11-08 18:37:40 PST 
Created attachment 413552 [details]
Patch
Comment 4 Yusuke Suzuki 2020-11-08 19:01:20 PST 
Created attachment 413553 [details]
Patch
Comment 5 Ross Kirsling 2020-11-08 19:53:16 PST 
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
Comment 6 Yusuke Suzuki 2020-11-08 20:18:09 PST 
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 :)
Comment 7 Yusuke Suzuki 2020-11-08 21:02:49 PST 
Committed r269574: <https://trac.webkit.org/changeset/269574>
Comment 8 Radar WebKit Bug Importer 2020-11-08 21:03:19 PST 
<rdar://problem/71175759>