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

IDL sequence should be Iterable, not Array #6

Open
Tracked by #99
kainino0x opened this issue Apr 3, 2021 · 7 comments
Open
Tracked by #99

IDL sequence should be Iterable, not Array #6

kainino0x opened this issue Apr 3, 2021 · 7 comments

Comments

@kainino0x
Copy link

WebIDL sequences are (I think) equivalent to TypeScript Iterables - the WebIDL conversion rules from JS will iterate the value to produce an array. For example:

dictionary GPUBindGroupLayoutDescriptor : GPUObjectDescriptorBase {
    required sequence<GPUBindGroupLayoutEntry> entries;
};

generates:

  interface GPUBindGroupLayoutDescriptor extends GPUObjectDescriptorBase {
    entries: Array<GPUBindGroupLayoutEntry>;
  }

but should generate:

  interface GPUBindGroupLayoutDescriptor extends GPUObjectDescriptorBase {
    entries: Iterable<GPUBindGroupLayoutEntry>;
  }
@darionco
Copy link
Owner

The WebIDL spec defines a sequence as an ECMAScript Array.

Unfortunately Array and Iterable are not a 1 to 1 mapping because Array always allows index access to its members through square brackets notation while an Iterable is only expected to implement the Symbol.iterator property as seen in the TypeScript declaration

An alternative would be declaring an iterable object as defined in the WebIDL spec.

@kainino0x
Copy link
Author

Ah, there's an effective difference between input sequences and output sequences.
IDL allows any JS iterable to be passed in to a sequence-typed WebIDL binding, but when it passes a WebIDL sequence out to a JS value it produces an array.

WebGPU doesn't actually use any output sequences (IIRC), instead using FrozenArray (which as of today it only uses in one place).

@kainino0x
Copy link
Author

Unfortunately I can't think of an easy way to be 100% correct about input vs output types. Many types can be used in both input and output (any that are sequence or dictionary, at least). IIRC these are only recommended to be used in input situations but they're allowed to be used in output situations as well.

@darionco
Copy link
Owner

yeah, this one is a difficult one.

for function arguments or return types a simple input/output heuristic could be used:

undefined setSequence(sequence<any> value);
sequence<any> getSequence();

would translate to:

setSequence(Iterable<any> value): void;
getSequence(): Array<any>;

Properties are more difficult since they can either be readonly or read/write which makes it so they must always be Array.

The closest solution that I can think of is to follow the heuristic above and create setters and getters to be verbose:

dictionary GPUBindGroupLayoutDescriptor : GPUObjectDescriptorBase {
    required sequence<GPUBindGroupLayoutEntry> entries;
};

would translate to:

interface GPUBindGroupLayoutDescriptor extends GPUObjectDescriptorBase {
    set entries(value: Iterable<GPUBindGroupLayoutEntry>);
    get entries(): Array<GPUBindGroupLayoutEntry>;
}

but that feels off

@kainino0x
Copy link
Author

Yeah, that wouldn't work if the type were used as a local variable:

const x: GPUBindGroupLayoutDescriptor = { entries: new Set(...) };
const y = x.entries[0];

Unfortunately the only thing I can think of is to generate both input and output versions of every type (perhaps differentiated by a type parameter??), then optionally prune any input or output variants that go unused...

@kainino0x
Copy link
Author

We can not worry about this for now for WebGPU though. The diff that we have to deal with manually is very small.

@kainino0x
Copy link
Author

In principle all the descriptors need to be readonly on input too, but fortunately (I guess) typescript is not strict about this. microsoft/TypeScript#18770

I did a little experimentation with the idea I had above and it could work:
gpuweb/types@main...kainino0x:idlsequence-proof-of-concept

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants