Aligning CreateShaderModule with the WebGPU spec

[nical]

Is your feature request related to a problem? Please describe.

I'm looking into aligning Gecko's WebGPU implementation of CreateShaderModule with the spec. This issue is to discuss where the pieces fit in term of the rust API vs the web's.

Describe the solution you'd like and alternatives

I'm very new to this so not strongly opinionated. My gut feeling is that perhaps wgpu_core should expose compilation messages in a format closer to the specification, perhaps decorated with its extra internal details. Questions below:

Multiple errors and warnings

Currently it looks like Device::create_shader_module returns a single error. It's not incompatible with the specification (which expects an array of compilation messages, since we can always return an array of size 1), but I suspect we'll want to be able to get multiple errors as well as warnings in the long run.

Error representation

The error enum CreateShaderModuleError is very different from the CompilationMessage of the specification. This is not necessarily a big issue as long as one can be converted into the other, however I'm going to make a subjective statement: I think that CreateShaderModuleError is a bit too low level and not nearly as nice as the spec from the point of view of the API's consumer (Consumer has to match the enum which differentiate between parsing, validation, etc.). On the other hand the specification exposes more generic members (line/column/message/etc.) that are immediately useful to the api consumer (more so than knowing which part of wgpu or naga's internals found the issue.

Gecko can take care of converting from the wgpu_core representation to the spec's, but I suspect it would be valuable to wgpu users in general to have an easer error type to consume, either by replacing CreateShaderModuleError or by providing a helper to convert it to something closter to the spec's representation.

Should the offset be computed by wgpu_core

I suspect not but confirmation can't hurt.
The spec exposes an offset that I didn't see in wgpu's error types.
The offset is relative to the utf16 representation, so I guess the right thing to do is for the browser to build that information from the line/column pair and the utf16 string.

Summary

• Is the general rule of thumb for dealing with difference between wgpu and the webgpu spec?
• Should create_shader_module return an array of error/warning/info messages instead of a single one?
• Should the message representation look more like the spec's.

[jimblandy]

My opinions:

The single-error / array-of-errors distinction should be addressed on the Firefox side for now. Naga is simply not set up to produce multiple error messages. Doing that effectively entails not just fancier plumbing for the results themselves, but also changes to prevent meaningless error cascades, parser recovery tricks, etc. etc. Once we undertake all that, then sure, let's change wgpu's error type. But for now it'd just look dumb. Let's put our Potemkin Village right where it's required, at the WebIDL boundary.

Representation-wise, I think the Naga front::wgsl::ParseError type is actually pretty nice:

#[derive(Clone, Debug)]
pub struct ParseError {
    message: String,
    labels: Vec<(Span, Cow<'static, str>)>,
    notes: Vec<String>,
}

That seems pretty good for Rust consumers. I'd say we should convert from that to the WebGPU GPUCompilationMessage in Firefox.

But those points aside, as far as error-handling is concerned, aligning with the spec is non-controversial - just go ahead and change whatever is helpful. I agree that the fine-grained error enums from Naga are probably not that interesting to users. They're valuable for testing, though.

[nical]

I think that having common methods like fn location(&self, source: &str) -> Option<(u32, u32)> implemented at the CreateShaderModuleError level without changing the underlying data structure would go a long way.

[jimblandy]

That sounds good to me too.

(But, I'd say, go ahead and make a struct - I have no idea what those two u32's are.)

[nical]

Got started in gfx-rs/naga#1937

[stefnotch]

I think this is fixed, now that #5410 has been merged. Let me know if there's anything left to do, or anything that could be improved.

[teoxoy]

Yeah, I think we are done on this front.