From 98c4d6f42e1920cd11c3f7d1016d976a6bbb0d22 Mon Sep 17 00:00:00 2001
From: Jim Blandy <jimb@red-bean.com>
Date: Fri, 27 Sep 2024 17:00:21 -0700
Subject: [PATCH] [naga] Permit only structs as binding array elements. (#6333)

Require `T` to be a struct in `binding_array<T, ...>`; do not permit
arrays.

In #5428, the validator was changed to accept binding array types that
the SPIR-V backend couldn't properly emit. Specifically, the validator
was changed to accept `binding_array<array<T>>`, but the SPIR-V
backend wasn't changed to wrap the binding array elements in a SPIR-V
struct type, as Vulkan requires. So the type would be accepted by the
validator, and then rejected by the backend.
---
 naga/src/valid/type.rs   |   1 -
 naga/tests/validation.rs | 110 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 110 insertions(+), 1 deletion(-)

diff --git a/naga/src/valid/type.rs b/naga/src/valid/type.rs
index 0c4466077..c0c25dab7 100644
--- a/naga/src/valid/type.rs
+++ b/naga/src/valid/type.rs
@@ -677,7 +677,6 @@ impl super::Validator {
                     // Currently Naga only supports binding arrays of structs for non-handle types.
                     match gctx.types[base].inner {
                         crate::TypeInner::Struct { .. } => {}
-                        crate::TypeInner::Array { .. } => {}
                         _ => return Err(TypeError::BindingArrayBaseTypeNotStruct(base)),
                     };
                 }
diff --git a/naga/tests/validation.rs b/naga/tests/validation.rs
index f20ae688b..b8a30c49b 100644
--- a/naga/tests/validation.rs
+++ b/naga/tests/validation.rs
@@ -496,3 +496,113 @@ fn main(input: VertexOutput) {{
         }
     }
 }
+
+#[allow(dead_code)]
+struct BindingArrayFixture {
+    module: naga::Module,
+    span: naga::Span,
+    ty_u32: naga::Handle<naga::Type>,
+    ty_array: naga::Handle<naga::Type>,
+    ty_struct: naga::Handle<naga::Type>,
+    validator: naga::valid::Validator,
+}
+
+impl BindingArrayFixture {
+    fn new() -> Self {
+        let mut module = naga::Module::default();
+        let span = naga::Span::default();
+        let ty_u32 = module.types.insert(
+            naga::Type {
+                name: Some("u32".into()),
+                inner: naga::TypeInner::Scalar(naga::Scalar::U32),
+            },
+            span,
+        );
+        let ty_array = module.types.insert(
+            naga::Type {
+                name: Some("array<u32, 10>".into()),
+                inner: naga::TypeInner::Array {
+                    base: ty_u32,
+                    size: naga::ArraySize::Constant(std::num::NonZeroU32::new(10).unwrap()),
+                    stride: 4,
+                },
+            },
+            span,
+        );
+        let ty_struct = module.types.insert(
+            naga::Type {
+                name: Some("S".into()),
+                inner: naga::TypeInner::Struct {
+                    members: vec![naga::StructMember {
+                        name: Some("m".into()),
+                        ty: ty_u32,
+                        binding: None,
+                        offset: 0,
+                    }],
+                    span: 4,
+                },
+            },
+            span,
+        );
+        let validator = naga::valid::Validator::new(Default::default(), Default::default());
+        BindingArrayFixture {
+            module,
+            span,
+            ty_u32,
+            ty_array,
+            ty_struct,
+            validator,
+        }
+    }
+}
+
+#[test]
+fn binding_arrays_hold_structs() {
+    let mut t = BindingArrayFixture::new();
+    let _binding_array = t.module.types.insert(
+        naga::Type {
+            name: Some("binding_array_of_struct".into()),
+            inner: naga::TypeInner::BindingArray {
+                base: t.ty_struct,
+                size: naga::ArraySize::Dynamic,
+            },
+        },
+        t.span,
+    );
+
+    assert!(t.validator.validate(&t.module).is_ok());
+}
+
+#[test]
+fn binding_arrays_cannot_hold_arrays() {
+    let mut t = BindingArrayFixture::new();
+    let _binding_array = t.module.types.insert(
+        naga::Type {
+            name: Some("binding_array_of_array".into()),
+            inner: naga::TypeInner::BindingArray {
+                base: t.ty_array,
+                size: naga::ArraySize::Dynamic,
+            },
+        },
+        t.span,
+    );
+
+    assert!(t.validator.validate(&t.module).is_err());
+}
+
+#[test]
+fn binding_arrays_cannot_hold_scalars() {
+    let mut t = BindingArrayFixture::new();
+    let _binding_array = t.module.types.insert(
+        naga::Type {
+            name: Some("binding_array_of_scalar".into()),
+            inner: naga::TypeInner::BindingArray {
+                base: t.ty_u32,
+                size: naga::ArraySize::Dynamic,
+            },
+        },
+        t.span,
+    );
+
+    assert!(t.validator.validate(&t.module).is_err());
+}