Update struct member decorations to allow multiple blocks. This CL updates the struct member decoration parsing to allow multiple blocks of decorations. Bug: tint:240 Change-Id: I97293ef30333f63c33bbc6e728dba11abc020c7c Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/29280 Commit-Queue: dan sinclair <dsinclair@chromium.org> Reviewed-by: David Neto <dneto@google.com>
diff --git a/src/reader/wgsl/parser_impl.cc b/src/reader/wgsl/parser_impl.cc index a91712f..032c14c 100644 --- a/src/reader/wgsl/parser_impl.cc +++ b/src/reader/wgsl/parser_impl.cc
@@ -1557,12 +1557,21 @@ } // struct_member -// : struct_member_decoration_decl variable_ident_decl SEMICOLON +// : struct_member_decoration_decl+ variable_ident_decl SEMICOLON std::unique_ptr<ast::StructMember> ParserImpl::struct_member() { auto t = peek(); auto source = t.source(); - auto decos = struct_member_decoration_decl(); + ast::StructMemberDecorationList decos; + for (;;) { + size_t s = decos.size(); + if (!struct_member_decoration_decl(decos)) { + return nullptr; + } + if (decos.size() == s) { + break; + } + } if (has_error()) return nullptr; @@ -1590,21 +1599,21 @@ // : // | ATTR_LEFT (struct_member_decoration COMMA)* // struct_member_decoration ATTR_RIGHT -ast::StructMemberDecorationList ParserImpl::struct_member_decoration_decl() { +bool ParserImpl::struct_member_decoration_decl( + ast::StructMemberDecorationList& decos) { auto t = peek(); - if (!t.IsAttrLeft()) - return {}; + if (!t.IsAttrLeft()) { + return true; + } next(); // Consume the peek t = peek(); if (t.IsAttrRight()) { set_error(t, "empty struct member decoration found"); - return {}; + return false; } - ast::StructMemberDecorationList decos; - bool found_offset = false; for (;;) { auto deco = struct_member_decoration(); if (has_error()) @@ -1612,13 +1621,6 @@ if (deco == nullptr) break; - if (deco->IsOffset()) { - if (found_offset) { - set_error(peek(), "duplicate offset decoration found"); - return {}; - } - found_offset = true; - } decos.push_back(std::move(deco)); t = next(); @@ -1628,9 +1630,9 @@ if (!t.IsAttrRight()) { set_error(t, "missing ]] for struct member decoration"); - return {}; + return false; } - return decos; + return true; } // struct_member_decoration
diff --git a/src/reader/wgsl/parser_impl.h b/src/reader/wgsl/parser_impl.h index bd7ae82..2d53423 100644 --- a/src/reader/wgsl/parser_impl.h +++ b/src/reader/wgsl/parser_impl.h
@@ -170,9 +170,11 @@ /// Parses a `struct_member` grammar element /// @returns the struct member or nullptr std::unique_ptr<ast::StructMember> struct_member(); - /// Parses a `struct_member_decoration_decl` grammar element + /// 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 - ast::StructMemberDecorationList struct_member_decoration_decl(); + bool struct_member_decoration_decl(ast::StructMemberDecorationList& decos); /// Parses a `struct_member_decoration` grammar element /// @returns the decoration or nullptr if none found std::unique_ptr<ast::StructMemberDecoration> struct_member_decoration();
diff --git a/src/reader/wgsl/parser_impl_struct_member_decoration_decl_test.cc b/src/reader/wgsl/parser_impl_struct_member_decoration_decl_test.cc index 9f57e0e..af0f721 100644 --- a/src/reader/wgsl/parser_impl_struct_member_decoration_decl_test.cc +++ b/src/reader/wgsl/parser_impl_struct_member_decoration_decl_test.cc
@@ -24,43 +24,41 @@ TEST_F(ParserImplTest, StructMemberDecorationDecl_EmptyStr) { auto* p = parser(""); - auto deco = p->struct_member_decoration_decl(); + ast::StructMemberDecorationList decos; + ASSERT_TRUE(p->struct_member_decoration_decl(decos)); ASSERT_FALSE(p->has_error()); - EXPECT_EQ(deco.size(), 0u); + EXPECT_EQ(decos.size(), 0u); } TEST_F(ParserImplTest, StructMemberDecorationDecl_EmptyBlock) { auto* p = parser("[[]]"); - auto deco = p->struct_member_decoration_decl(); + ast::StructMemberDecorationList decos; + ASSERT_FALSE(p->struct_member_decoration_decl(decos)); ASSERT_TRUE(p->has_error()); EXPECT_EQ(p->error(), "1:3: empty struct member decoration found"); } TEST_F(ParserImplTest, StructMemberDecorationDecl_Single) { auto* p = parser("[[offset(4)]]"); - auto deco = p->struct_member_decoration_decl(); + ast::StructMemberDecorationList decos; + ASSERT_TRUE(p->struct_member_decoration_decl(decos)); ASSERT_FALSE(p->has_error()); - ASSERT_EQ(deco.size(), 1u); - EXPECT_TRUE(deco[0]->IsOffset()); -} - -TEST_F(ParserImplTest, StructMemberDecorationDecl_HandlesDuplicate) { - auto* p = parser("[[offset(2), offset(4)]]"); - auto deco = p->struct_member_decoration_decl(); - ASSERT_TRUE(p->has_error()) << p->error(); - EXPECT_EQ(p->error(), "1:23: duplicate offset decoration found"); + ASSERT_EQ(decos.size(), 1u); + EXPECT_TRUE(decos[0]->IsOffset()); } TEST_F(ParserImplTest, StructMemberDecorationDecl_InvalidDecoration) { auto* p = parser("[[offset(nan)]]"); - auto deco = p->struct_member_decoration_decl(); + ast::StructMemberDecorationList decos; + ASSERT_FALSE(p->struct_member_decoration_decl(decos)); ASSERT_TRUE(p->has_error()) << p->error(); EXPECT_EQ(p->error(), "1:10: invalid value for offset decoration"); } TEST_F(ParserImplTest, StructMemberDecorationDecl_MissingClose) { auto* p = parser("[[offset(4)"); - auto deco = p->struct_member_decoration_decl(); + ast::StructMemberDecorationList decos; + ASSERT_FALSE(p->struct_member_decoration_decl(decos)); ASSERT_TRUE(p->has_error()) << p->error(); EXPECT_EQ(p->error(), "1:12: missing ]] for struct member decoration"); }
diff --git a/src/reader/wgsl/parser_impl_struct_member_test.cc b/src/reader/wgsl/parser_impl_struct_member_test.cc index 7efe135..3c1ca11 100644 --- a/src/reader/wgsl/parser_impl_struct_member_test.cc +++ b/src/reader/wgsl/parser_impl_struct_member_test.cc
@@ -52,6 +52,24 @@ EXPECT_EQ(m->decorations()[0]->AsOffset()->offset(), 2u); } +TEST_F(ParserImplTest, StructMember_ParsesWithMultipleDecorations) { + auto* i32 = tm()->Get(std::make_unique<ast::type::I32Type>()); + + auto* p = parser(R"([[offset(2)]] +[[offset(4)]] a : i32;)"); + auto m = p->struct_member(); + ASSERT_FALSE(p->has_error()); + ASSERT_NE(m, nullptr); + + EXPECT_EQ(m->name(), "a"); + EXPECT_EQ(m->type(), i32); + EXPECT_EQ(m->decorations().size(), 2u); + EXPECT_TRUE(m->decorations()[0]->IsOffset()); + EXPECT_EQ(m->decorations()[0]->AsOffset()->offset(), 2u); + EXPECT_TRUE(m->decorations()[1]->IsOffset()); + EXPECT_EQ(m->decorations()[1]->AsOffset()->offset(), 4u); +} + TEST_F(ParserImplTest, StructMember_InvalidDecoration) { auto* p = parser("[[offset(nan)]] a : i32;"); auto m = p->struct_member();
diff --git a/src/writer/wgsl/generator_impl.cc b/src/writer/wgsl/generator_impl.cc index 446c92b..187d06e 100644 --- a/src/writer/wgsl/generator_impl.cc +++ b/src/writer/wgsl/generator_impl.cc
@@ -444,24 +444,14 @@ increment_indent(); for (const auto& mem : str->members()) { - make_indent(); - if (!mem->decorations().empty()) { - out_ << "[["; - bool first = true; - for (const auto& deco : mem->decorations()) { - if (!first) { - out_ << ", "; - } + for (const auto& deco : mem->decorations()) { + make_indent(); - first = false; - // TODO(dsinclair): Split this out when we have more then one - assert(deco->IsOffset()); - - out_ << "offset(" << deco->AsOffset()->offset() << ")"; - } - out_ << "]] "; + // TODO(dsinclair): Split this out when we have more then one + assert(deco->IsOffset()); + out_ << "[[offset(" << deco->AsOffset()->offset() << ")]]" << std::endl; } - + make_indent(); out_ << mem->name() << " : "; if (!EmitType(mem->type())) { return false;
diff --git a/src/writer/wgsl/generator_impl_alias_type_test.cc b/src/writer/wgsl/generator_impl_alias_type_test.cc index e0b04ff..009b236 100644 --- a/src/writer/wgsl/generator_impl_alias_type_test.cc +++ b/src/writer/wgsl/generator_impl_alias_type_test.cc
@@ -62,7 +62,8 @@ ASSERT_TRUE(g.EmitAliasType(&alias)) << g.error(); EXPECT_EQ(g.result(), R"(type a = struct { a : f32; - [[offset(4)]] b : i32; + [[offset(4)]] + b : i32; }; )"); }
diff --git a/src/writer/wgsl/generator_impl_type_test.cc b/src/writer/wgsl/generator_impl_type_test.cc index 495e1e4..d7360b3 100644 --- a/src/writer/wgsl/generator_impl_type_test.cc +++ b/src/writer/wgsl/generator_impl_type_test.cc
@@ -143,7 +143,8 @@ ASSERT_TRUE(g.EmitType(&s)) << g.error(); EXPECT_EQ(g.result(), R"(struct { a : i32; - [[offset(4)]] b : f32; + [[offset(4)]] + b : f32; })"); } @@ -173,7 +174,8 @@ EXPECT_EQ(g.result(), R"([[block]] struct { a : i32; - [[offset(4)]] b : f32; + [[offset(4)]] + b : f32; })"); }