[wgsl-in] Fail on repeated attributes

[fornwall]

Fixes #2425.

[ErichDonGubler]

LGTM, minus some nits!

[teoxoy]

There are more attributes that this validation should cover. I think all attributes can't be repeated (the spec mentions to look for exceptions to this rule for each attribute but at a first glance I can't see any such exception).

[fornwall]

There are more attributes that this validation should cover. I think all attributes can't be repeated (the spec mentions to look for exceptions to this rule for each attribute but at a first glance I can't see any such exception).

Thanks @teoxoy ! I've added additional attributes in 1a7c3ab

workgroup_size is missing. I'm planning to do that in a separate PR (to propose changing EntryPoint.workgroup_size from [u32; 3] to Option<[u32; 3]>, which will also fix #2271). Since that is a bigger change touching all backends and frontends, is that ok?

[fornwall]

Rebased to fix merge conflicts in src/front/wgsl/tests.rs. Also squashed the three commits into one, as the second two commits iterations on the first.

[teoxoy]

Sounds good!

Given that all the attributes are an Option<Something>, I feel like a more robust solution would be to introduce a newtype that once you set its inner Option via a set method, you'll keep getting the repeated attributes error on subsequent set calls. Thoughts?

[fornwall]

Given that all the attributes are an Option<Something>, I feel like a more robust solution would be to introduce a newtype that once you set its inner Option via a set method, you'll keep getting the repeated attributes error on subsequent set calls. Thoughts?

Sounds like a good idea, I'll try it out!

[fornwall]

Given that all the attributes are an Option, I feel like a more robust solution would be to introduce a newtype that once you set its inner Option via a set method, you'll keep getting the repeated attributes error on subsequent set calls. Thoughts?

@teoxoy Like this: f09f9f7?

[teoxoy]

LGTM, thanks for making the change!

[teoxoy]

@ErichDonGubler want to give this another look? 👀

[ErichDonGubler]

Feeling silly that my unfamiliarity with WGSL led me to miss other attributes, but otherwise, this LGTM. Could we file an issue for getting enum-exhaustive coverage for these attributes for testing before we merge? I think that that's the only preference I want to call out.

[fornwall]

Could we file an issue for getting enum-exhaustive coverage for these attributes for testing before we merge? I think that that's the only preference I want to call out.

Thanks @ErichDonGubler! Have added gfx-rs/wgpu#4567 as a follow up issue for that, and #2434 for the @workgroup_size remaining attribute (which is a bit bigger, since making workgroup_size an Option will touch more parts).

[teoxoy]

Thanks! Will merge this then.