`unreachable!()` reached in naga during wgsl -> spv translation

[zakarumych]

thread 'main' panicked at 'internal error: entered unreachable code: Expression [5] is not cached!', ~/.cargo/registry/src/github.com-1ecc6299db9ec823/naga-0.8.5/src/back/spv/block.rs:329:31

repro here

[zakarumych]

This happens on translation to spv step.

[zakarumych]

It doesn't happen if I replace var uniforms: Uniforms; with var<uniform> uniforms: Uniforms;

[kvark]

We discussed this about in RuRust gamedev Telegram room. Basically their shader wasn't validating because validate is not a default feature. So really it behaves as designed! We could consider 2 smaller sub-improvements here:

1. enable "validate" by default to avoid confusion
2. try to avoid unreachable!() in the backends for the reasons of using an unvalidated source module. Instead, return an error like some backends do.

I think (1) is acceptable, but I'm not sure that (2) is. Debugging any unreachable! case is easier when it's just a panic, and in normal operation you know whether your modules are validated properly or not. I.e. this is a one-time confusion.

[zakarumych]

If the conclusion would be to left this as panic, I would suggest custom panicking method. Ideally only truly unreachable code should use unreachable!() macro. Any code that is reachable only for invalid module should clearly communicate so. For example:

#[cold]
#[inline(never)]
fn invalid_module(message: &str) {
    panic!("Module is invalid. {}", message)
}

Otherwise the next user who see this unreachable panic would immediately think this is a bug.

[Gordon-F]

enable "validate" by default to avoid confusion

Validation was disabled mainly for web target (#1213). Faster translation, less lib size. I don't think that should be enabled by default.

[jimblandy]

I agree that unreachable is inappropriate here, given that unvalidated translation is a feature we're trying to support. The code is quite reachable under supported use cases

What might help is to ensure that any panic reachable only without validation mentioned that fact in its panic message. Then people who encounter these panics would know that turning on validation should be their next step.