gfx-rs / wgpu

A cross-platform, safe, pure-Rust graphics API.
https://wgpu.rs
Apache License 2.0
12.67k stars 928 forks source link

[naga wgsl-in] Does WGSL really allow duplicate diagnostic filtering directives? #6556

Open jimblandy opened 6 hours ago

jimblandy commented 6 hours ago

I'm skeptical that it was the language designers' intention to allow duplicate global diagnostic filters, although I can see why the standard supports that interpretation.

In naga::diagnostic_filter, we have ShouldConflictOnFullDuplicate, whose comments say:

/// Determines whether [`DiagnosticFilterMap::add`] should consider full duplicates a conflict.
///
/// In WGSL, directive position does not consider this case a conflict, while attribute position
/// does.

In the WGSL spec, I can see that:

Maybe I'm just not seeing the rationale, or I haven't found the discussion, but it seems weird to me that duplicates would be tolerated in one place but forbidden in the other. Because of the way the spec is structured, I think there's some chance this was just an oversight.

And my coder's heart deeply and fiercely resents being required to add an enum like ShouldConflictOnFullDuplicate if there isn't some justification for it. That kind of vermin will take over if you let it.

CC @ErichDonGubler

jimblandy commented 6 hours ago

Filed: https://github.com/gpuweb/gpuweb/issues/4976

ErichDonGubler commented 3 hours ago

Its definitely intentional (and has CTS coverage), but even a non-normative rationale isn't in the spec. It's unclear why this is desirable. I'm interested to see what upstream says.