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

It is unclear if [AllowShared] BufferSource is valid #961

Closed
smaug---- opened this issue Mar 3, 2021 · 6 comments · Fixed by #1311
Closed

It is unclear if [AllowShared] BufferSource is valid #961

smaug---- opened this issue Mar 3, 2021 · 6 comments · Fixed by #1311

Comments

@smaug----
Copy link

The spec says
"A type that is not a buffer source type must not be associated with the [AllowShared] extended attribute."
and
"The buffer source types are ArrayBuffer, DataView, and the typed array types."

BufferSource is a union type
typedef (ArrayBufferView or ArrayBuffer) BufferSource;

Yet https://heycam.github.io/webidl/#AllowShared has an example using [AllowShared] BufferSource.

@EdgarChen
Copy link
Member

There is some discussion in #827.

@annevk
Copy link
Member

annevk commented Mar 4, 2021

See also #342 and #865.

My current thinking on this:

  • SharedArrayBuffer should be its own IDL type. We should not overload the ArrayBuffer IDL type to being able to represent ArrayBuffer and SharedArrayBuffer JS types.
  • [AllowShared] only applies to ArrayBufferView types.
  • We should leave BufferSource as-is.
  • We should add SharedBufferSource that also accepts SharedArrayBuffer and that uses [AllowShared] in front of ArrayBufferView.

The outcome of that might well be that [AllowShared] BufferSource is valid as [AllowShared] would end up ignored for ArrayBuffer. I don't have strong feelings about that.

@yuki3
Copy link

yuki3 commented Aug 4, 2021

+1 for Anne's proposal.

Only a nitpick I see is that, considering ArrayBuffer vs SharedArrayBuffer, BufferSource vs SharedBufferSource could be a little confusing. IIUC, the proposal is:

typedef (ArrayBuffer or SharedArrayBuffer or [AllowShared] Int8Array or ...) SharedBufferSource;

Considering ArrayBuffer vs SharedArrayBuffer, the proposed SharedBufferSource does not correspond to SharedArrayBuffer. It corresponds to (ArrayBuffer or SharedArrayBuffer). "Shared" prefix looks confusing to me.

@annevk
Copy link
Member

annevk commented May 7, 2023

That makes sense, I guess for clarity AllowSharedBufferSource would be better, to indicate it's allowed, but not enforced.
Defined as:

typedef (SharedArrayBuffer or [AllowShared] BufferSource) AllowSharedBufferSource;

I think that works because [AllowShared] ArrayBuffer would end up ignored. Though we could flatten it a bit more if that would be preferred:

typedef (ArrayBuffer or SharedArrayBuffer or [AllowShared] ArrayBufferView) AllowSharedBufferSource;

cc @Constellation

@bathos
Copy link
Contributor

bathos commented May 7, 2023

I think that works because [AllowShared] ArrayBuffer would end up ignored.

What makes that ignored? That behavior does make sense to me, but I don’t think it’s what’s currently specified. Type-applicable EAs get distributed to union members unilaterally right now, they aren’t filtered by whether a member does or doesn’t permit that annotation, so I think it would presently imply an error.

@annevk
Copy link
Member

annevk commented May 8, 2023

You're right, #827 still exists.

annevk added a commit that referenced this issue May 30, 2023
Generally IDL objects have been 1:1 with ECMAScript objects, except for
SharedArrayBuffer. That was instead represented as
[AllowShared] ArrayBuffer, although that includes ArrayBuffer as well.
This created a number of challenges, e.g., how to return a
SharedArrayBuffer object.

So we make these changes here that will also require corresponding
downstream fixes:

* SharedArrayBuffer is now its own type.
* [AllowShared] only applies to buffer view types.
* AllowSharedBufferSource takes over from [AllowShared] BufferSource.

Fixes #865 and fixes #961.
annevk added a commit that referenced this issue Jun 15, 2023
Generally IDL objects have been 1:1 with ECMAScript objects, except for
SharedArrayBuffer. That was instead represented as
[AllowShared] ArrayBuffer, although that includes ArrayBuffer as well.
This created a number of challenges, e.g., how to return a
SharedArrayBuffer object.

So we make these changes here that will also require corresponding
downstream fixes:

* SharedArrayBuffer is now its own type.
* [AllowShared] only applies to buffer view types.
* AllowSharedBufferSource takes over from [AllowShared] BufferSource.

Downstream is tracked by #1320.

Fixes #865 and fixes #961.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

5 participants