Repository: gfx-rs/wgpu (language: rust)

Issue:
Negative indexing does not fail shader module creation but it should
<!-- Thank you for filing this! Please read the [debugging tips](https://github.com/gfx-rs/wgpu/wiki/Debugging-wgpu-Applications).
That may let you investigate on your own, or provide additional information that helps us to assist.-->

**Description**
While doing wgpu bump in servo I noticed that:

    OK /_webgpu/webgpu/cts.https.html?q=webgpu:shader,validation,expression,access,array:early_eval_errors:*

        FAIL [expected PASS] subtest: :case="runtime_array_const_oob_neg"

**Repro steps**
```wgsl
@group(0) @binding(0) var<storage> x : array<u32>;
        fn y() -> u32 {
          let tmp = x[-1];
          return 0;
        }
```
should make error but it does not. Per spec https://www.w3.org/TR/WGSL/#array-access-expr we should raise error if idx expr is const and is oob.

**Expected vs observed behavior**
Test should still pass, before it passed because we failed with some other error:
```
Shader validation error: Function [0] 'y' is invalid
  ┌─ :2:9
  │  
2 │ ╭         fn y() -> u32 {
3 │ │           let tmp = x[-1];
4 │ │           return 0;
  │ │                  ^ naga::Expression [4]
  │ ╰───────────────────^ naga::Function [0]
  │  
  = The `return` value Some([4]) does not match the function return value
```
so this is technically not a regression.

**Platform**
Wgpu bump was from d8e7ab1ad13f2bf2f9702401d1bc625e26b1c2e6 to d8833d079833c62b4fd00325d0ba08ec0c8bc309



Discussion / hints:

Fix patch:
diff --git a/naga/src/valid/expression.rs b/naga/src/valid/expression.rs
index 9ef3a9edfb..b0c54a3df4 100644
--- a/naga/src/valid/expression.rs
+++ b/naga/src/valid/expression.rs
@@ -280,25 +280,26 @@ impl super::Validator {
                     }
                 }
 
-                // If we know both the length and the index, we can do the
-                // bounds check now.
-                if let crate::proc::IndexableLength::Known(known_length) =
-                    base_type.indexable_length(module)?
+                // If index is const we can do check for non-negative index
+                match module
+                    .to_ctx()
+                    .eval_expr_to_u32_from(index, &function.expressions)
                 {
-                    match module
-                        .to_ctx()
-                        .eval_expr_to_u32_from(index, &function.expressions)
-                    {
-                        Ok(value) => {
+                    Ok(value) => {
+                        // If we know both the length and the index, we can do the
+                        // bounds check now.
+                        if let crate::proc::IndexableLength::Known(known_length) =
+                            base_type.indexable_length(module)?
+                        {
                             if value >= known_length {
                                 return Err(ExpressionError::IndexOutOfBounds(base, value));
                             }
                         }
-                        Err(crate::proc::U32EvalError::Negative) => {
-                            return Err(ExpressionError::NegativeIndex(base))
-                        }
-                        Err(crate::proc::U32EvalError::NonConst) => {}
                     }
+                    Err(crate::proc::U32EvalError::Negative) => {
+                        return Err(ExpressionError::NegativeIndex(base))
+                    }
+                    Err(crate::proc::U32EvalError::NonConst) => {}
                 }
 
                 ShaderStages::all()


Test patch: