Resolver: Validate array el is not [[block]] struct

Fixed: tint:90
Change-Id: I9500a8c7fad9acf5f797dd2903217821c953acb6
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48421
Commit-Queue: Ben Clayton <bclayton@google.com>
Auto-Submit: Ben Clayton <bclayton@google.com>
Reviewed-by: James Price <jrprice@google.com>

src/resolver/decoration_validation_test.cc

@@ -438,5 +438,25 @@ TEST_F(ArrayStrideTest, MultipleDecorations) {
 
 }  // namespace
 }  // namespace ArrayStrideTests
+
+namespace StructBlockTests {
+namespace {
+
+using StructBlockTest = ResolverTest;
+TEST_F(StructBlockTest, StructUsedAsArrayElement) {
+  auto* s = Structure("S", {Member("x", ty.i32())},
+                      {create<ast::StructBlockDecoration>()});
+  auto* a = ty.array(s, 4);
+  Global("G", a, ast::StorageClass::kPrivate);
+
+  EXPECT_FALSE(r()->Resolve());
+  EXPECT_EQ(r()->error(),
+            "error: A structure type with a [[block]] decoration cannot be "
+            "used as an element of an array");
+}
+
+}  // namespace
+}  // namespace StructBlockTests
+
 }  // namespace resolver
 }  // namespace tint

src/resolver/resolver.cc

@@ -2110,16 +2110,12 @@ const sem::Array* Resolver::Array(sem::ArrayType* arr, const Source& source) {
     return sem;
   }
 
-  // First check the element type is legal
+  if (!ValidateArray(arr, source)) {
-  auto* el_ty = arr->type();
-  if (!IsStorable(el_ty)) {
-    builder_->Diagnostics().add_error(
-        el_ty->FriendlyName(builder_->Symbols()) +
-            " cannot be used as an element type of an array",
-        source);
     return nullptr;
   }
 
+  auto* el_ty = arr->type();
+
   uint32_t el_align = 0;
   uint32_t el_size = 0;
   if (!DefaultAlignAndSize(el_ty, el_align, el_size, source)) {
@@ -2147,19 +2143,7 @@ const sem::Array* Resolver::Array(sem::ArrayType* arr, const Source& source) {
         return nullptr;
       }
       explicit_stride = stride->stride();
-      bool is_valid_stride = (explicit_stride >= el_size) &&
+      if (!ValidateArrayStrideDecoration(stride, el_size, el_align, source)) {
-                             (explicit_stride >= el_align) &&
-                             (explicit_stride % el_align == 0);
-      if (!is_valid_stride) {
-        // https://gpuweb.github.io/gpuweb/wgsl/#array-layout-rules
-        // Arrays decorated with the stride attribute must have a stride that is
-        // at least the size of the element type, and be a multiple of the
-        // element type's alignment value.
-        diagnostics_.add_error(
-            "arrays decorated with the stride attribute must have a stride "
-            "that is at least the size of the element type, and be a multiple "
-            "of the element type's alignment value.",
-            source);
         return nullptr;
       }
     }
@@ -2173,6 +2157,55 @@ const sem::Array* Resolver::Array(sem::ArrayType* arr, const Source& source) {
   return create_semantic(implicit_stride);
 }
 
+bool Resolver::ValidateArray(const sem::ArrayType* arr, const Source& source) {
+  auto* el_ty = arr->type();
+
+  if (!IsStorable(el_ty)) {
+    builder_->Diagnostics().add_error(
+        el_ty->FriendlyName(builder_->Symbols()) +
+            " cannot be used as an element type of an array",
+        source);
+    return false;
+  }
+
+  if (auto* el_str = el_ty->As<sem::StructType>()) {
+    if (el_str->impl()->IsBlockDecorated()) {
+      // https://gpuweb.github.io/gpuweb/wgsl/#attributes
+      // A structure type with the block attribute must not be:
+      // * the element type of an array type
+      // * the member type in another structure
+      diagnostics_.add_error(
+          "A structure type with a [[block]] decoration cannot be used as an "
+          "element of an array",
+          source);
+      return false;
+    }
+  }
+  return true;
+}
+
+bool Resolver::ValidateArrayStrideDecoration(const ast::StrideDecoration* deco,
+                                             uint32_t el_size,
+                                             uint32_t el_align,
+                                             const Source& source) {
+  auto stride = deco->stride();
+  bool is_valid_stride =
+      (stride >= el_size) && (stride >= el_align) && (stride % el_align == 0);
+  if (!is_valid_stride) {
+    // https://gpuweb.github.io/gpuweb/wgsl/#array-layout-rules
+    // Arrays decorated with the stride attribute must have a stride that is
+    // at least the size of the element type, and be a multiple of the
+    // element type's alignment value.
+    diagnostics_.add_error(
+        "arrays decorated with the stride attribute must have a stride "
+        "that is at least the size of the element type, and be a multiple "
+        "of the element type's alignment value.",
+        source);
+    return false;
+  }
+  return true;
+}
+
 bool Resolver::ValidateStructure(const sem::StructType* st) {
   for (auto* member : st->impl()->members()) {
     if (auto* r = member->type()->UnwrapAll()->As<sem::ArrayType>()) {

src/resolver/resolver.h

@@ -236,6 +236,11 @@ class Resolver {
 
   // AST and Type validation methods
   // Each return true on success, false on failure.
+  bool ValidateArray(const sem::ArrayType* arr, const Source& source);
+  bool ValidateArrayStrideDecoration(const ast::StrideDecoration* deco,
+                                     uint32_t el_size,
+                                     uint32_t el_align,
+                                     const Source& source);
   bool ValidateAssignment(const ast::AssignmentStatement* a);
   bool ValidateBinary(ast::BinaryExpression* expr);
   bool ValidateEntryPoint(const ast::Function* func, const FunctionInfo* info);

test/compute_boids.wgsl

@@ -34,7 +34,7 @@ fn frag_main() -> [[location(0)]] vec4<f32> {
 }
 
 // compute shader
-[[block]] struct Particle {
+struct Particle {
   pos : vec2<f32>;
   vel : vec2<f32>;
 };