Add INIT macros with default values

[eliemichel]

Motivated by #158, and inspired by how Dawn does it, this PR introduces a WGPU_FOO_INIT macro to initialize a struct WGPUFoo with default values.

• Generate INIT macro for all structs
• Collect default values for all struct from WebGPU JS spec
• Enable the use of constants as default values
• State default values in generated docstring (point to INIT macro in struct docstring, give default value in member docstring)
• Initialize SType for WGPUChainedStruct and WGPUChainedStructOut

[eliemichel]

I spotted one subtlety that the proposed approach does not handle properly:

Fields of GPUBindGroupLayoutEntry like buffer or texture are optional in JS, but in C their nullability is instead expressed through a special value of buffer.type or texture.sampleType, namely BindingNotUsed (I'm not questionong this choice, that I find indeed more confortable).

Let's take a simplified example:

struct WGPUBindGroupLayoutEntry {
    // [...]
    WGPUBufferBindingLayout buffer;
    WGPUTextureBindingLayout texture;
}

struct WGPUBufferBindingLayout {
    // [...]
    WGPUBufferBindingType type;
}

struct WGPUBufferBindingLayout {
    // [...]
    WGPUTextureSampleType sampleType;
}

This means that when initializing the buffer and texture fields of WGPUBindGroupLayoutEntry, we want to set buffer.type and texture.sampleType to BindingNotUsed. But per the JS spec when initializing a GPUBufferBindingLayout the default value of type is expected to by Uniform.

I see 2 options:

• Option A We create two different WGPU_BUFFER_BINDING_LAYOUT_INIT macros, one with this name and that sets type to Uniform and one internal that is used to initialize WGPUBindGroupLayoutEntry. Unclear how to unambiguously define this behavior.
• Option B We set the default value for type to BindingNotUsed, which diverges slightly from the JS spec but there is probably only little case where one initializes a WGPUBufferBindingLayout object without it being part of a WGPUBindGroupLayoutEntry.

[eliemichel]

Another limitation spotted: .chain fields are not automatically initialized with their proper SType. This is related to the previous issue in that we should initialize the WGPUChainedStruct type differently depending on its parent struct.

[kainino0x]

Wow, thank you so much!! I have been working through all of the resolved changes and many tentative changes but I had been procrastinating on this one because it would be a lot of work :) Will start reviewing soon.

[kainino0x]

• State default values in generated docstring: Shall we?

I think this would be great.

I see 2 options:

• Option A We create two different WGPU_BUFFER_BINDING_LAYOUT_INIT macros, one with this name and that sets type to Uniform and one internal that is used to initialize WGPUBindGroupLayoutEntry. Unclear how to unambiguously define this behavior.
• Option B We set the default value for type to BindingNotUsed, which diverges slightly from the JS spec but there is probably only little case where one initializes a WGPUBufferBindingLayout object without it being part of a WGPUBindGroupLayoutEntry.

I have been planning to use Option B. We actually started doing this already in Dawn: https://dawn-review.googlesource.com/c/dawn/+/214854, here for VertexBufferStepMode which has the same problem.
We still apply the defaulting from Undefined -> (default) but that only happens if the user explicitly writes Undefined.

aside: In C++ it occurs to me we could have the default values in wgpu::BindGroupLayoutEntry be BindingNotUsed but have the default values in e.g. wgpu::BufferBindingLayout be Vertex so that if you do entry.buffer = {}; it does what it does in JS. But I think I prefer to simply have it required to be explicit in C/C++, unlike in JS. (Similarly I've determined I want the default in wgpu::VertexBufferLayout to be VertexBufferNotUsed so users can default-initialize or zero-initialize an array of wgpu::VertexBufferLayout and have it be "empty".)

Another limitation spotted: .chain fields are not automatically initialized with their proper SType. This is related to the previous issue in that we should initialize the WGPUChainedStruct type differently depending on its parent struct.

Hmm, I hope we don't run into problems with the INIT design.

When would we need to initialize WGPUChainedStruct differently? I believe sType values should always be 1:1 with the struct types.

[kainino0x]

I did not review thoroughly yet but it is looking quite good! Just a few small notes for things I saw so far.

BTW I filed #423 as a possible (noncritical) followup.

[eliemichel]

I have been planning to use Option B.

Ok! I've applied it for all occurences of "NotUsed" in enum values, assuming it encompasses all such cases (namely vertex step mode and the 3 binding layouts).

Hmm, I hope we don't run into problems with the INIT design.

I think that's fine actually, I could use _wgpu_MAKE_INIT_STRUCT inline

State default values in generated docstring: Shall we?

I think this would be great.

Added them! NB: The default value is given for all member, including when it is not explicitly specified in webgpu.yml due to the zero-value being the expected default. I think it is good to make things explicit but we could also leave them unspecified and globally say that unspecified means 0.

[kainino0x]

Hmm, I hope we don't run into problems with the INIT design.

I think that's fine actually, I could use _wgpu_MAKE_INIT_STRUCT inline

Ah, that's true!

State default values in generated docstring: Shall we?

I think this would be great.

Added them! NB: The default value is given for all member, including when it is not explicitly specified in webgpu.yml due to the zero-value being the expected default. I think it is good to make things explicit but we could also leave them unspecified and globally say that unspecified means 0.

Will review but that seems good to me.

[eliemichel]

Adding test revealed a lot of small things (which I should have tested indeed).

I notably took the questionable liberty of adding (back?) an Undefined entry to the VertexFormat enum.

Another note: I realize the generation of default values in g.DefaultValue is not robost to non-empty value of $.ExtSuffix: is there a way to access this value on the gen.go side?

[kainino0x]

I notably took the questionable liberty of adding (back?) an Undefined entry to the VertexFormat enum.

Hm, I don't feel strongly either way. Alternative would be to zero-init those fields with {} which is what Dawn does, but that's also less self-documenting. There has been some back and forth on whether we should name the mostly-unused 0 values.

Another note: I realize the generation of default values in g.DefaultValue is not robost to non-empty value of $.ExtSuffix: is there a way to access this value on the gen.go side?

Honestly I have no idea. If it doesn't cause problems for generation right now, it's fine and we'll just deal with it later.

[eliemichel]

Hm, I don't feel strongly either way. Alternative would be to zero-init those fields with {} which is what Dawn does, but that's also less self-documenting. There has been some back and forth on whether we should name the mostly-unused 0 values.

The issue occurs in some other cases as well actually, because we generate INIT macros for structs that are only used to return info (e.g., about the adapter) for which it really doesn't make sense to add an Undefined.

However, as you can see above CI complains about using {} to zero-initialize things in C (it is considered a GNU extension). IIRC it is an annoying case where what C++ writes {} is actually what C write {0} (and notably {0} in C initializes all fields to 0 while in C++ it only initializes the first member to 0...). Anyways, I thus generate initializers of the form (WGPUFoo)0 when a member has type enum WGPUFoo and no default value is specified in webgpu.yml.

Since I had to add this, I'll roll back the (re)introduction of Undefined in VertexFormat.

Honestly I have no idea. If it doesn't cause problems for generation right now, it's fine and we'll just deal with it later.

It does not cause any issue for now

[eliemichel]

Mmh now C++ complains about C-style cast... I'll try with a _wgpu_ZERO_INIT macro then

[eliemichel]

Now It is smart enough to realize that 0 is not a valid value, better to default to the first non-null entry of the enum in the end.

[kainino0x]

Somehow I submitted some of the comments before others. Here's the rest

[kainino0x]

I went through all the differences between this and Dawn and added comments for things that should change in this PR. Here are all the other differences that don't need to change in this PR:

• Places where Dawn is behind or the definition order is different
• Spelling of default values differs (but value is the same so it doesn't matter except for the C vs C++ differences you were already investigating):
  • 0.0f vs 0.f
  • In many (but not all) places it uses {} instead of a named value
  • In some places it uses a named value instead of {}
  • It doesn't use _wgpu_MAKE_INIT_STRUCT when initializing .chain, it just uses a bare {NULL _wgpu_COMMA WGPUSType_RenderPassMaxDrawCount}
• (I was wrong about this before) the new CallbackInfo.mode fields init to {} instead of WaitAnyOnly (some outdated duplicates are WaitAnyOnly)
• It incorrectly comments /*.next=*/ as /*.nextInChain*/
• It incorrectly uses false instead of 0 (since it doesn't include stdbool.h)
• It incorrectly uses nullptr instead of NULL (in some places where it doesn't use {})

[kainino0x]

Oh and 0xFFFFFFFF vs 4294967295

[eliemichel]

Thanks for going through all this!

I think the key remaining difference now is that we cannot use {}, as far as I can tell (as mentionned above, it is not standard C) so the generator now just goes and figure the very first enum entry and use it as default. This also means we need to decide on a default value for CallbackInfo.mode: WaitAnyOnly was fine with me but if we rather want to force the user to choose one, we need to reintroduce an Undefined value to initialize to (otherwise C++ complains)

edit I realize { 0 } is actually doing the same thing in C and C++, and is probably what we should use in lieu of {} (rather than looking for the first enum entry).

[kainino0x]

I pulled your latest code and I'm experimenting. I see that 0 or {0} hits -Wassign-enum in clang (from -Weverything). Trying to determine if that is just a "maybe you made a typo" warning or a "you're doing UB" warning (I've researched this before but didn't remember). This answer https://stackoverflow.com/a/33814842 says basically that the underlying type will be a signed or unsigned integer type of arbitrary bit length large enough to hold all of the values, which means this is safe because 0 is always representable.

(WGPUCompilationMessageType)0 works in C but of course hits -Wold-style-cast in C++. We could conditionally choose between that for C and WGPUCompilationMessageType(0) for C++ (assuming MSVC is OK with these).

#if defined(__cplusplus)
#  define _wgpu_ENUM_ZERO_INIT(type) type(0)
#else
#  define _wgpu_ENUM_ZERO_INIT(type) (type)0
#endif

Or we could just give names to all the enums' zero values, which IMO seems more reasonable. :)

[kainino0x]

Tried it out, that seems to work fine: https://github.com/kainino0x/webgpu-headers/actions/runs/11945415786
So perhaps we can land the PR with that and we'll follow up on it in #427.

[kainino0x]

I addressed things using _wgpu_ENUM_ZERO_INIT, finally reviewed the Go code, and did some minor nits to the documentation. Everything looks great now, if there was anything else you wanted to change feel free to open more PR(s) - I'm going to land this now!