Skip to content

Add [AllowShared] to allow accepting SharedArrayBuffers#353

Merged
domenic merged 1 commit into
masterfrom
allowshared
May 3, 2017
Merged

Add [AllowShared] to allow accepting SharedArrayBuffers#353
domenic merged 1 commit into
masterfrom
allowshared

Conversation

@domenic

@domenic domenic commented Apr 28, 2017

Copy link
Copy Markdown
Member

Previous to this commit, any API that accepted any of the buffer source types would also accept a SharedArrayBuffer. This was not the approach we want; instead, we want APIs to explicitly opt-in. Enabled by previous work creating annotated types, this adds the new [AllowShared] extended attribute to allow APIs to opt in to accepting SharedArrayBuffers.

While here, this removes an unimplemented check for detached array buffers when converting from ES values to IDL types, fixing #352.


Preview | Diff

Previous to this commit, any API that accepted any of the buffer source types would also accept a SharedArrayBuffer. This was not the approach we want; instead, we want APIs to explicitly opt-in. Enabled by previous work creating annotated types, this adds the new [AllowShared] extended attribute to allow APIs to opt in to accepting SharedArrayBuffers.

While here, this removes an unimplemented check for detached array buffers when converting from ES values to IDL types, fixing #352.
@annevk

annevk commented Apr 30, 2017

Copy link
Copy Markdown
Member

So this is primarily for WebGL at this point right? Do we have some follow-up filed there?

@domenic

domenic commented May 1, 2017

Copy link
Copy Markdown
Member Author

Yes, although I am told it might expand over time, e.g. for web audio or similar.

We don't have a follow-up filed on Web GL, because to my knowledge their specs are developed in private with no public issue tracker or "editor's draft". I am not sure what the procedure will be. Maybe we'll need to maintain a moneypatch somewhere.

I'll try to poke tc39/proposal-ecmascript-sharedmem#168 about that issue.

@bzbarsky

bzbarsky commented May 1, 2017

Copy link
Copy Markdown
Contributor

@domenic

domenic commented May 1, 2017

Copy link
Copy Markdown
Member Author

Ah OK yeah, I just found that myself; clearly my info was out of date.

@bzbarsky

bzbarsky commented May 1, 2017

Copy link
Copy Markdown
Contributor

@tobie tobie left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Comment thread index.bs
A type that is not a [=buffer source type=] must not be
[=extended attributes associated with|associated with=] the [{{AllowShared}}] extended attribute.

See the rules for converting ECMAScript values to IDL [=buffer source types=] in

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I admit I had to look "entails" up. Maybe instead write:

[…] for the specific requirements that using [{{AllowShared}}] creates.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is already used for every other extended attribute, so I'd rather be consistent.

@domenic

domenic commented May 2, 2017

Copy link
Copy Markdown
Member Author

Does anyone else want to review before we merge? Ideally speak up within the next day-ish.

@domenic
domenic merged commit c59bdcb into master May 3, 2017
@domenic
domenic deleted the allowshared branch May 3, 2017 19:29
kenrussell pushed a commit to KhronosGroup/WebGL that referenced this pull request May 10, 2017
* Support SharedArrayBuffers

This annotates all ArrayBuffer or ArrayBufferView types with [AllowShared], per the proto-specification at https://tc39.github.io/ecmascript_sharedmem/dom_shmem.html#webgl, extended to also modify the Web GL 2.0 entrypoints. This allows the backing data to be a SharedArrayBuffer, instead of only non-shared ArrayBuffers, per whatwg/webidl#353.

This also makes some related editorial changes, removing the redundant BufferDataSource typedef in favor of the defined-in-Web IDL BufferSource type, and consolidating two overloads of bufferData (one accepting ArrayBuffer? and the other accepting ArrayBufferView) into a single overload accepting BufferSource?.

* Also allow SharedArrayBuffer in the -List typdefs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants