[wgsl-reader] Allow array decorations to have multiple blocks.

This CL updates the WGSL parser to allow array decorations to accept
multiple blocks. The stride decoration on arrays was turned into a
proper decoration object instead of just storing the stride directly.

Bug: tint:240
Change-Id: I6cdc7400d8847e3e043b846ea5c9f86cb795cf86
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/29780
Commit-Queue: dan sinclair <dsinclair@chromium.org>
Reviewed-by: Sarah Mashayekhi <sarahmashay@google.com>
Reviewed-by: David Neto <dneto@google.com>

src/reader/spirv/parser_impl.cc

@@ -45,6 +45,7 @@
 #include "src/ast/scalar_constructor_expression.h"
 #include "src/ast/set_decoration.h"
 #include "src/ast/sint_literal.h"
+#include "src/ast/stride_decoration.h"
 #include "src/ast/struct.h"
 #include "src/ast/struct_decoration.h"
 #include "src/ast/struct_member.h"
@@ -772,7 +773,9 @@ bool ParserImpl::ApplyArrayDecorations(
         return Fail() << "invalid array type ID " << type_id
                       << ": multiple ArrayStride decorations";
       }
-      ast_type->set_array_stride(stride);
+      ast::ArrayDecorationList decos;
+      decos.push_back(std::make_unique<ast::StrideDecoration>(stride));
+      ast_type->set_decorations(std::move(decos));
     } else {
       return Fail() << "invalid array type ID " << type_id
                     << ": unknown decoration "

src/reader/wgsl/parser_impl.cc

@@ -41,6 +41,7 @@
 #include "src/ast/set_decoration.h"
 #include "src/ast/sint_literal.h"
 #include "src/ast/stage_decoration.h"
+#include "src/ast/stride_decoration.h"
 #include "src/ast/struct_member_offset_decoration.h"
 #include "src/ast/switch_statement.h"
 #include "src/ast/type/alias_type.h"
@@ -1105,9 +1106,9 @@ ast::type::AliasType* ParserImpl::type_alias() {
 //   | VEC3 LESS_THAN type_decl GREATER_THAN
 //   | VEC4 LESS_THAN type_decl GREATER_THAN
 //   | PTR LESS_THAN storage_class, type_decl GREATER_THAN
-//   | array_decoration_list? ARRAY LESS_THAN type_decl COMMA
+//   | array_decoration_list* ARRAY LESS_THAN type_decl COMMA
 //          INT_LITERAL GREATER_THAN
-//   | array_decoration_list? ARRAY LESS_THAN type_decl
+//   | array_decoration_list* ARRAY LESS_THAN type_decl
 //          GREATER_THAN
 //   | MAT2x2 LESS_THAN type_decl GREATER_THAN
 //   | MAT2x3 LESS_THAN type_decl GREATER_THAN
@@ -1153,19 +1154,28 @@ ast::type::Type* ParserImpl::type_decl() {
     return type_decl_pointer(t);
   }
 
-  auto deco = array_decoration_list();
+  ast::ArrayDecorationList decos;
+  for (;;) {
+    size_t s = decos.size();
+    if (!array_decoration_list(decos)) {
+      return nullptr;
+    }
+    if (decos.size() == s) {
+      break;
+    }
+  }
   if (has_error()) {
     return nullptr;
   }
-  if (deco != 0) {
+  if (!decos.empty()) {
     t = peek();
   }
-  if (deco != 0 && !t.IsArray()) {
+  if (!decos.empty() && !t.IsArray()) {
     set_error(t, "found array decoration but no array");
     return nullptr;
   }
   if (t.IsArray()) {
-    return type_decl_array(t, deco);
+    return type_decl_array(t, std::move(decos));
   }
   if (t.IsMat2x2() || t.IsMat2x3() || t.IsMat2x4() || t.IsMat3x2() ||
       t.IsMat3x3() || t.IsMat3x4() || t.IsMat4x2() || t.IsMat4x3() ||
@@ -1258,7 +1268,8 @@ ast::type::Type* ParserImpl::type_decl_vector(Token t) {
       std::make_unique<ast::type::VectorType>(subtype, count));
 }
 
-ast::type::Type* ParserImpl::type_decl_array(Token t, uint32_t stride) {
+ast::type::Type* ParserImpl::type_decl_array(Token t,
+                                             ast::ArrayDecorationList decos) {
   next();  // Consume the peek
 
   t = next();
@@ -1296,9 +1307,7 @@ ast::type::Type* ParserImpl::type_decl_array(Token t, uint32_t stride) {
   }
 
   auto ty = std::make_unique<ast::type::ArrayType>(subtype, size);
-  if (stride != 0) {
-    ty->set_array_stride(stride);
-  }
+  ty->set_decorations(std::move(decos));
   return ctx_.type_mgr().Get(std::move(ty));
 }
 
@@ -1309,48 +1318,62 @@ ast::type::Type* ParserImpl::type_decl_array(Token t, uint32_t stride) {
 //
 // As there is currently only one decoration I'm combining these for now.
 // we can split apart later if needed.
-uint32_t ParserImpl::array_decoration_list() {
+bool ParserImpl::array_decoration_list(ast::ArrayDecorationList& decos) {
   auto t = peek();
   if (!t.IsAttrLeft()) {
-    return 0;
+    return true;
   }
   t = peek(1);
   if (!t.IsStride()) {
-    return 0;
+    return true;
   }
 
   next();  // consume the peek of [[
-  next();  // consume the peek of stride
 
-  t = next();
-  if (!t.IsParenLeft()) {
-    set_error(t, "missing ( for stride attribute");
-    return 0;
-  }
+  for (;;) {
+    t = next();
+    if (!t.IsStride()) {
+      set_error(t, "unknown array decoration");
+      return false;
+    }
 
-  t = next();
-  if (!t.IsSintLiteral()) {
-    set_error(t, "missing value for stride decoration");
-    return 0;
-  }
-  if (t.to_i32() < 0) {
-    set_error(t, "invalid stride value: " + t.to_str());
-    return 0;
-  }
-  uint32_t stride = static_cast<uint32_t>(t.to_i32());
+    t = next();
+    if (!t.IsParenLeft()) {
+      set_error(t, "missing ( for stride attribute");
+      return false;
+    }
 
-  t = next();
-  if (!t.IsParenRight()) {
-    set_error(t, "missing ) for stride attribute");
-    return 0;
+    t = next();
+    if (!t.IsSintLiteral()) {
+      set_error(t, "missing value for stride decoration");
+      return false;
+    }
+    if (t.to_i32() < 0) {
+      set_error(t, "invalid stride value: " + t.to_str());
+      return false;
+    }
+    uint32_t stride = static_cast<uint32_t>(t.to_i32());
+    decos.push_back(std::make_unique<ast::StrideDecoration>(stride));
+
+    t = next();
+    if (!t.IsParenRight()) {
+      set_error(t, "missing ) for stride attribute");
+      return false;
+    }
+
+    t = peek();
+    if (!t.IsComma()) {
+      break;
+    }
+    next();  // Consume the peek
   }
 
   t = next();
   if (!t.IsAttrRight()) {
     set_error(t, "missing ]] for array decoration");
-    return 0;
+    return false;
   }
-  return stride;
+  return true;
 }
 
 ast::type::Type* ParserImpl::type_decl_matrix(Token t) {

src/reader/wgsl/parser_impl.h

@@ -21,6 +21,7 @@
 #include <unordered_map>
 #include <utility>
 
+#include "src/ast/array_decoration.h"
 #include "src/ast/assignment_statement.h"
 #include "src/ast/builtin.h"
 #include "src/ast/call_statement.h"
@@ -172,8 +173,8 @@ class ParserImpl {
   std::unique_ptr<ast::StructMember> struct_member();
   /// Parses a `struct_member_decoration_decl` grammar element, appending newly
   /// parsed decorations to the end of |decos|.
-  /// @params decos the decoration list
-  /// @returns the list of decorations
+  /// @param decos the decoration list
+  /// @returns true if parsing was successful.
   bool struct_member_decoration_decl(ast::StructMemberDecorationList& decos);
   /// Parses a `struct_member_decoration` grammar element
   /// @returns the decoration or nullptr if none found
@@ -395,8 +396,8 @@ class ParserImpl {
  private:
   ast::type::Type* type_decl_pointer(Token t);
   ast::type::Type* type_decl_vector(Token t);
-  ast::type::Type* type_decl_array(Token t, uint32_t stride);
-  uint32_t array_decoration_list();
+  ast::type::Type* type_decl_array(Token t, ast::ArrayDecorationList decos);
+  bool array_decoration_list(ast::ArrayDecorationList& decos);
   ast::type::Type* type_decl_matrix(Token t);
 
   std::unique_ptr<ast::ConstructorExpression> const_expr_internal(

src/reader/wgsl/parser_impl_type_decl_test.cc

@@ -13,6 +13,7 @@
 // limitations under the License.
 
 #include "gtest/gtest.h"
+#include "src/ast/stride_decoration.h"
 #include "src/ast/type/alias_type.h"
 #include "src/ast/type/array_type.h"
 #include "src/ast/type/bool_type.h"
@@ -423,6 +424,44 @@ TEST_F(ParserImplTest, TypeDecl_Array_Runtime_Stride) {
   EXPECT_EQ(a->array_stride(), 16u);
 }
 
+TEST_F(ParserImplTest, TypeDecl_Array_MultipleDecorations_OneBlock) {
+  auto* p = parser("[[stride(16), stride(32)]] array<f32>");
+  auto* t = p->type_decl();
+  ASSERT_NE(t, nullptr) << p->error();
+  ASSERT_FALSE(p->has_error());
+  ASSERT_TRUE(t->IsArray());
+
+  auto* a = t->AsArray();
+  ASSERT_TRUE(a->IsRuntimeArray());
+  ASSERT_TRUE(a->type()->IsF32());
+
+  auto& decos = a->decorations();
+  ASSERT_EQ(decos.size(), 2u);
+  EXPECT_TRUE(decos[0]->IsStride());
+  EXPECT_EQ(decos[0]->AsStride()->stride(), 16u);
+  EXPECT_TRUE(decos[1]->IsStride());
+  EXPECT_EQ(decos[1]->AsStride()->stride(), 32u);
+}
+
+TEST_F(ParserImplTest, TypeDecl_Array_MultipleDecorations_MultipleBlocks) {
+  auto* p = parser("[[stride(16)]] [[stride(32)]] array<f32>");
+  auto* t = p->type_decl();
+  ASSERT_NE(t, nullptr) << p->error();
+  ASSERT_FALSE(p->has_error());
+  ASSERT_TRUE(t->IsArray());
+
+  auto* a = t->AsArray();
+  ASSERT_TRUE(a->IsRuntimeArray());
+  ASSERT_TRUE(a->type()->IsF32());
+
+  auto& decos = a->decorations();
+  ASSERT_EQ(decos.size(), 2u);
+  EXPECT_TRUE(decos[0]->IsStride());
+  EXPECT_EQ(decos[0]->AsStride()->stride(), 16u);
+  EXPECT_TRUE(decos[1]->IsStride());
+  EXPECT_EQ(decos[1]->AsStride()->stride(), 32u);
+}
+
 TEST_F(ParserImplTest, TypeDecl_Array_Decoration_MissingArray) {
   auto* p = parser("[[stride(16)]] f32");
   auto* t = p->type_decl();