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

WGSL template arg. parsing should permit a single trailing comma #6394

Open
ErichDonGubler opened this issue Oct 10, 2024 · 0 comments
Open
Labels
area: correctness We're behaving incorrectly area: cts Issues stemming from the WebGPU Conformance Test Suite area: naga front-end area: validation Issues related to validation, diagnostics, and error handling good first issue Good for newcomers lang: WGSL WebGPU Shading Language naga Shader Translator type: bug Something isn't working

Comments

@ErichDonGubler
Copy link
Member

Description

Until recently, Dawn didn't allow a trailing comma in template argument lists, and WebGPU CTS also tested that parse cases with a trailing comma failed to parse. Naga also implemented this behavior. However, it turns out that this was all incorrect! 😱

The WGSL. spec. states that the template_arg_comma_list must accept a single optional trailing comma:

Screenshot 2024-10-10 at 6 06 51 PM

As of gpuweb/cts#3959, the CTS fixed this to follow the spec., with Dawn following suit in https://crbug.com/366000875. Naga should also fix this, so we don't diverge.

Repro steps

Try to compile this shader:

// ../in.wgsl

@group(0) @binding(0)
var<storage, read,> x: i32;

@group(0) @binding(1)
var<storage,> x: i32;

…which, at time of writing, yields the following in naga-cli:

$ cargo run --package naga-cli -- ../in.wgsl

    Finished dev [unoptimized + debuginfo] target(s) in 0.08s
     Running `target/debug/naga ../in.wgsl`
Could not parse WGSL:
error: expected identifier, found '>'
  ┌─ ../in.wgsl:7:13
  │
7 │ var<storage,> x: i32;
  │             ^ expected identifier


Error: nu::shell::non_zero_exit_code

  × External command had a non-zero exit code
   ╭─[entry #3:1:1]
 1 │ cargo run -p naga-cli -- ../in.wgsl
   · ──┬──
   ·   ╰── exited with code 1
   ╰────

Alternatively, if lines 3-4 are commented out:

    Finished dev [unoptimized + debuginfo] target(s) in 0.08s
     Running `target/debug/naga ../in.wgsl`
Could not parse WGSL:
error: expected identifier, found '>'
  ┌─ ../in.wgsl:7:13
  │
7 │ var<storage,> x: i32;
  │             ^ expected identifier


Error: nu::shell::non_zero_exit_code

  × External command had a non-zero exit code
   ╭─[entry #4:1:1]
 1 │ cargo run -p naga-cli -- ../in.wgsl | clip
   · ──┬──
   ·   ╰── exited with code 1
   ╰────

Expected vs observed behavior

The repro steps should produce no errors.

Extra materials

-

Platform

-

@ErichDonGubler ErichDonGubler added type: bug Something isn't working good first issue Good for newcomers area: validation Issues related to validation, diagnostics, and error handling area: correctness We're behaving incorrectly area: cts Issues stemming from the WebGPU Conformance Test Suite naga Shader Translator area: naga front-end lang: WGSL WebGPU Shading Language labels Oct 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: correctness We're behaving incorrectly area: cts Issues stemming from the WebGPU Conformance Test Suite area: naga front-end area: validation Issues related to validation, diagnostics, and error handling good first issue Good for newcomers lang: WGSL WebGPU Shading Language naga Shader Translator type: bug Something isn't working
Projects
Status: Todo
Development

No branches or pull requests

1 participant