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

Support multiple diagnostic items in shader compilation #5295

Open
Runi-c opened this issue Feb 23, 2024 · 3 comments
Open

Support multiple diagnostic items in shader compilation #5295

Runi-c opened this issue Feb 23, 2024 · 3 comments
Labels
area: validation Issues related to validation, diagnostics, and error handling kind: diagnostics Error message should be better naga Shader Translator type: enhancement New feature or request

Comments

@Runi-c
Copy link

Runi-c commented Feb 23, 2024

Is your feature request related to a problem? Please describe.
Right now naga-based tooling can only display one validation error at a time, it would be nice if such tooling could get access to all the errors naga can detect in order to ease working with wgsl files especially.

Describe the solution you'd like
It seems like Validator::validate could be feasibly modified to produce an iterator over validation errors (or more likely such behavior moved into something like Validator::validate_all or Validator::iter which validate calls and returns the first element).

Describe alternatives you've considered
wgsl_analyzer works around this for wgsl files by implementing its own validation layer, but this results in occasional mismatches with naga and lack of support for naga extensions like naga_oil.

@teoxoy teoxoy added type: enhancement New feature or request naga Shader Translator labels Feb 27, 2024
@ErichDonGubler
Copy link
Member

AFAIK, most WGPU maintainership (myself included) is aware of this and wants it, but we haven't prioritized it yet (and we're not sure when we will). I might add as an alternative that we could also accept an error receiver of some kind from client code, like &dyn FnMut(<some-error-type>), instead of forcing collection of errors. The design space still needs to be explored, and consensus still needs to be driven, but we're happy to have it. I'd likely be the one to field such API discussions, so HMU in Matrix or keep replying here if you'd like to get the ball rolling!

CC @udoprog.

@Runi-c
Copy link
Author

Runi-c commented Mar 7, 2024

@ErichDonGubler I feel I'm not experienced enough in Rust to be able to accurately assess design considerations for different API suggestions, but I definitely have an interest in pushing for this, for the sake of being able to better work with WGSL for my current project.

An error receiver is an interesting idea, but at least for the tooling I'm working on, I'd likely just use it to produce a collection anyway. Are there use-cases that would be underserved by a simple Iterator<Item = WithSpan<ValidationError>>? Perhaps both methods could be supported to avoid the possibility of forcing collection boilerplate?

@ErichDonGubler ErichDonGubler added area: validation Issues related to validation, diagnostics, and error handling kind: diagnostics Error message should be better labels Mar 13, 2024
@ErichDonGubler ErichDonGubler changed the title naga::valid::Validator - Allow the validator to return all validation errors instead of just the first one Support multiple diagnostic items in shader compilation Oct 23, 2024
@ErichDonGubler
Copy link
Member

FTR: On Firefox's side, this is estimated at XL ("multiple weeks of work"). I think most of the work is going to be:

  1. Design a new diagnostic emission API to be used by naga.
  2. Design the API for consuming these errors. We need not finalize an API before making changes. We can expose new APIs behind a Cargo feature, i.e. unstable-naga-multiple-errors, on which current stable APIs will be re-implemented.
  3. Consume the above API.
    1. To start, only use the "fatal" subset of the above API: assume every error means shader processing can't proceed. This and (1) should be a single PR.
    2. Do not solve every case where Naga might benefit from this, but demonstrate that the above is workable by fixing a few interesting ones in another PR. For example, we could allow Naga to proceed past type mismatches in arguments for functions whose return type does not depend on input.

I don't think we need to consult downstream before finishing (3), because having "unstable" gating of this functionality means we can safely ship and evolve with downstream feedback.

@gfx-rs/naga: I suspect we might be able to reduce the estimate to L, because the above feels like a smaller scope than I was previously imagining (i.e., needing to consult downstream before landing anything).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: validation Issues related to validation, diagnostics, and error handling kind: diagnostics Error message should be better naga Shader Translator type: enhancement New feature or request
Projects
Status: No status
Development

No branches or pull requests

3 participants