wgsl: remove ignore, add phony-assignment

[dneto0]

Fixes: #2115

[github-actions]

Previews, as seen when this build job started (18dad74):
WebGPU | IDL
WGSL
Explainer

[kvark]

LGTM.
Any reason you didn't just extend the assignment_statement grammar by allowing "_"?

[dneto0]

LGTM.
Any reason you didn't just extend the assignment_statement grammar by allowing "_"?

Yes. The _ term has no type. But the assignment statement says the LHS has a reference type, etc.

If/when we get destructuring tuple assignment we can revisit, but then the explanation gets a lot more complicated.

[dneto0]

Also, there are other parts of the spec where it says that an assignment statement induces a write to memory. And that doesn't happen with a phony assignment.

[kdashg]

WGSL meeting minutes 2021-09-21

• AB: Concerned about saying “evaluates e”. Happier to say side effects occur but otherwise not necessarily fully evaluated.
• JG: I think all this is “as-if”, so we’re not necessarily signing up to actually evaluate the expression, but rather that it’s as-if evaluated.
• MM: Can add note saying expressions that have no side-effects are expected to take zero time at runtime.
• AB: Ignore had the same issue; trying to avoid sweeping away a binding reference. Not clear it’s guaranteed in the underlying API.
• BC: use case is keeping bindings alive, so reflection can “see” all the intended bindings.
  • Want to reconsider “must use the value-return”. Add magic _ identifier (and remove ignore()) #2115 That motivates this thing.
  • Don’t think there was as much put into considering that original rule than already spent on arguing over ignore and this phony-assignment.
• JG: I still want this phony assignment even without that must-use-return-value rule.
• JG: It’s modifying how you spell something we already have.
• MM: Can we have all three: ignore, phony-assignment, and turn make must-use-return-value a warning instead of error.
• JG: Most people would not want ignore if you had phony-assignment. And follows a ton of other languages, and it opens flexibility for future.
• JG: Let’s talk about must-use-return-value separately.
• JG: would have preferred to see _ as a special purpose identifier. For someone reading the spec, say “there is also this magic identifier”.
• MM: If we pick one of ignore and _, let’s pick _.
• BC: Agree, let’s pick _.
• DN: I’ll go off and rework this PR to be framed with _ as special purpose identifier.
• BC to raise an issue to reconsider the must-use-return-value rule.
• AB: Concern that static use at WGSL may not translate to a static use in underlying API. At what level do you shader reflection: at WGSL or at lower level.
• JG: Reflection must be sure to work at the WGSL level. What that means for underlying implementation is up to the underlying implementation.
• MM: We already have this problem. E.g. if (false) {foobar..} can hide foobar
• MM: JG is right about portability. Around 3 years ago we asked DirectX team at Microsoft and they said any reflection API that doesn’t consider optimizations would be useless. Applications would often not know what inputs/outputs are in their shader config, then compile the shader, then ask later via reflection.
• JG: Definitely tension here between portability vs. optimization.
• BC: This is tested between Dawn and Tint. Not sure it’s described correctly in the spec.
• AB: My understanding of engines is what Myles described. Engines want the slimmest interface.
• MM: If we do add a reflection API, the first version has to be portable. Maybe there can be a second version that is intentionally non-portable.

[ben-clayton]

BC to raise an issue to reconsider the must-use-return-value rule.

Done: #2138

[kdashg]

WGSL meeting minutes 2021-09-28

• DN to reword PR as per previous meeting
• MM: It does feel weird to me to do _ = texture to mark static use instead of ignore(texture)
• JG: IIRC there’s no special sauce in our builtin ignore, so it could be a user function? (though it would have to be e.g. ignore_texture())
• BC: Go has _ =. It’s a sink for values.
• DM: Can’t we do let foo = texture?
• ??: I think we require let-able types to be constructable
• MM: If I want my buffer to be static use, you’d want to say _ = my_buffer, which we can’t do today
• …
• JG: Could we not just also wave the magic wand and waive requirement for underscore assignment to be e.g. constructable, which would allow this.
• DN: We
• JG: Maybe acceptable if not very satisfying? Least bad here?
• MM: underscore has a problem because it’s unnatural to write _ = texturething.
• DM: But ignore would need the same amount of magic (e.g. ignore(buffer)). So may as well have the magic in one place: put it all in the _ rule.
• JG: Alright, previous resolution stands then: Replace ignore with _ and make _ sufficiently magical.

[dneto0]

Updates:

• rebased against main
• as requested, unify phony-assignment more closely with regular assignment
• regular assignment is now called "updating assignment"
• Also, for a phony assignment, the type of the right-hand-side is not restricted. So you can do nice things like _ = buffer_with_runtime_sized_array because that RHS is a reference, and the Load Rule does not kick in.

[github-actions]

Previews, as seen when this build job started (ee48da7):
WebGPU | IDL
WGSL
Explainer

[kdashg]

WGSL meeting minutes 2021-10-05

• Since last time:
  • rebased against main
  • as requested, unify phony-assignment more closely with regular assignment
  • regular assignment is now called "updating assignment"
  • Also, for a phony assignment, the type of the right-hand-side is not restricted. So you can do nice things like _ = buffer_with_runtime_sized_array because that RHS is a reference, and the Load Rule does not kick in.
  • Still removes the ignore builtin. (So speak up if you want to keep it)
• MM: I wonder how the wording is going to work with e.g. let a,_ = foo(), like would it be required to be constructable?
• DN: Well we don’t have multiple assignment yet.
• DM: Why not just require &tex, instead of having this extra complication in the specification we write.
• DN: I can put that back.
• MM: Either sounds fine.
• DN to fix and merge after DM approval

[dneto0]

Discussed in the 2021-10-05 meeting.

Agreed to the content, except for the change to allow any type for the expression.

[dneto0]

I've rebased the change, and reverted back to the original constraints on the expression type.

[github-actions]

Previews, as seen when this build job started (a1603b5):
WebGPU | IDL
WGSL
Explainer

[kvark]

Looks good, thank you!
Just one question/note

[kvark]

any reason this isn't assigning by value?

[dneto0]

textures and samplers are not constructible.

The constraint on the RHS of phony-assignment is constructible or pointer, to make it as close as possible to regular assignment, which only allows constructible.

There's a special exception for passing textures and samplers as function call parameters.

[jrprice]

But we don't support the address-of operator on textures/samplers, so this example code is not valid. If we want to use this phony-assignment to retain sampler/texture bindings we'd need it to support assigning textures and samplers by value too.

[dneto0]

ok, so I guess we'll have to add those cases too. :-)

[dneto0]

Please take a look at the latest commit.

[github-actions]

Previews, as seen when this build job started (34dfcd2):
WebGPU | IDL
WGSL
Explainer

[kdashg]

kvark on Matrix: "Great, please merge"