From 1a63c49b4e864560e27003017b9c5ec0c7894efe Mon Sep 17 00:00:00 2001 From: dan sinclair Date: Thu, 8 Oct 2020 18:33:25 +0000 Subject: [PATCH] Update structs to allow multiple decorations This CL updates to match the spec change allowing multiple struct decorations. Bug: tint:240 Change-Id: Id859c6a331c67c46597fc3c70de06d6cc0f486ec Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/29260 Commit-Queue: dan sinclair Reviewed-by: David Neto --- src/ast/struct.cc | 35 ++++++++--- src/ast/struct.h | 30 ++++++--- src/ast/struct_decoration.h | 3 + src/ast/struct_test.cc | 62 ++++++++++++++++--- src/ast/type/struct_type.h | 4 +- src/reader/spirv/function_memory_test.cc | 3 +- src/reader/spirv/parser_impl.cc | 10 +-- .../spirv/parser_impl_convert_type_test.cc | 3 +- .../spirv/parser_impl_module_var_test.cc | 12 ++-- src/reader/wgsl/parser_impl.cc | 38 +++++++----- src/reader/wgsl/parser_impl.h | 11 ++-- .../wgsl/parser_impl_struct_decl_test.cc | 20 ++++++ ...parser_impl_struct_decoration_decl_test.cc | 13 ++-- src/transform/vertex_pulling_transform.cc | 9 ++- src/type_determiner_test.cc | 12 ++-- src/writer/hlsl/generator_impl_type_test.cc | 8 ++- src/writer/msl/generator_impl_type_test.cc | 7 ++- src/writer/spirv/builder.cc | 7 +-- .../spirv/builder_accessor_expression_test.cc | 44 ++++++------- src/writer/spirv/builder_assign_test.cc | 3 +- .../builder_constructor_expression_test.cc | 6 +- src/writer/spirv/builder_intrinsic_test.cc | 9 +-- src/writer/spirv/builder_type_test.cc | 20 +++--- src/writer/wgsl/generator_impl.cc | 4 +- src/writer/wgsl/generator_impl_type_test.cc | 11 ++-- 25 files changed, 246 insertions(+), 138 deletions(-) diff --git a/src/ast/struct.cc b/src/ast/struct.cc index 01344b6c26..c16632e1be 100644 --- a/src/ast/struct.cc +++ b/src/ast/struct.cc @@ -19,13 +19,19 @@ namespace ast { Struct::Struct() : Node() {} -Struct::Struct(StructDecoration decoration, StructMemberList members) - : Node(), decoration_(decoration), members_(std::move(members)) {} +Struct::Struct(StructMemberList members) + : Node(), members_(std::move(members)) {} + +Struct::Struct(StructDecorationList decorations, StructMemberList members) + : Node(), decorations_(decorations), members_(std::move(members)) {} + +Struct::Struct(const Source& source, StructMemberList members) + : Node(source), members_(std::move(members)) {} Struct::Struct(const Source& source, - StructDecoration decoration, + StructDecorationList decorations, StructMemberList members) - : Node(source), decoration_(decoration), members_(std::move(members)) {} + : Node(source), decorations_(decorations), members_(std::move(members)) {} Struct::Struct(Struct&&) = default; @@ -40,7 +46,21 @@ StructMember* Struct::get_member(const std::string& name) const { return nullptr; } +bool Struct::IsBlockDecorated() const { + for (auto deco : decorations_) { + if (deco == StructDecoration::kBlock) { + return true; + } + } + return false; +} + bool Struct::IsValid() const { + for (auto deco : decorations_) { + if (deco == StructDecoration::kNone) { + return false; + } + } for (const auto& mem : members_) { if (mem == nullptr || !mem->IsValid()) { return false; @@ -51,10 +71,11 @@ bool Struct::IsValid() const { void Struct::to_str(std::ostream& out, size_t indent) const { make_indent(out, indent); - if (decoration_ != StructDecoration::kNone) { - out << "[[" << decoration_ << "]] "; - } out << "Struct{" << std::endl; + for (auto deco : decorations_) { + make_indent(out, indent + 2); + out << "[[" << deco << "]]" << std::endl; + } for (const auto& member : members_) { member->to_str(out, indent + 2); } diff --git a/src/ast/struct.h b/src/ast/struct.h index e12cfd5881..8a55b8d9da 100644 --- a/src/ast/struct.h +++ b/src/ast/struct.h @@ -32,15 +32,22 @@ class Struct : public Node { /// Create a new empty struct statement Struct(); /// Create a new struct statement - /// @param decoration The struct decorations /// @param members The struct members - Struct(StructDecoration decoration, StructMemberList members); + explicit Struct(StructMemberList members); + /// Create a new struct statement + /// @param decorations The struct decorations + /// @param members The struct members + Struct(StructDecorationList decorations, StructMemberList members); /// Create a new struct statement /// @param source The input source for the import statement - /// @param decoration The struct decorations + /// @param members The struct members + Struct(const Source& source, StructMemberList members); + /// Create a new struct statement + /// @param source The input source for the import statement + /// @param decorations The struct decorations /// @param members The struct members Struct(const Source& source, - StructDecoration decoration, + StructDecorationList decorations, StructMemberList members); /// Move constructor Struct(Struct&&); @@ -48,10 +55,12 @@ class Struct : public Node { ~Struct() override; /// Sets the struct decoration - /// @param deco the decoration to set - void set_decoration(StructDecoration deco) { decoration_ = deco; } - /// @returns the struct decoration - StructDecoration decoration() const { return decoration_; } + /// @param decos the list of decorations to set + void set_decorations(StructDecorationList decos) { + decorations_ = std::move(decos); + } + /// @returns the struct decorations + const StructDecorationList& decorations() const { return decorations_; } /// Sets the struct members /// @param members the members to set @@ -64,6 +73,9 @@ class Struct : public Node { /// @returns the struct member or nullptr if not found StructMember* get_member(const std::string& name) const; + /// @returns true if the struct is block decorated + bool IsBlockDecorated() const; + /// @returns true if the node is valid bool IsValid() const override; @@ -75,7 +87,7 @@ class Struct : public Node { private: Struct(const Struct&) = delete; - StructDecoration decoration_ = StructDecoration::kNone; + StructDecorationList decorations_; StructMemberList members_; }; diff --git a/src/ast/struct_decoration.h b/src/ast/struct_decoration.h index fab582c809..fecea8a25f 100644 --- a/src/ast/struct_decoration.h +++ b/src/ast/struct_decoration.h @@ -25,6 +25,9 @@ enum class StructDecoration { kNone = -1, kBlock }; std::ostream& operator<<(std::ostream& out, StructDecoration stage); +/// List of struct decorations +using StructDecorationList = std::vector; + } // namespace ast } // namespace tint diff --git a/src/ast/struct_test.cc b/src/ast/struct_test.cc index 0b416a2ad3..d3bc623975 100644 --- a/src/ast/struct_test.cc +++ b/src/ast/struct_test.cc @@ -35,23 +35,45 @@ TEST_F(StructTest, Creation) { members.push_back( std::make_unique("a", &i32, StructMemberDecorationList())); - Struct s{StructDecoration::kNone, std::move(members)}; + Struct s{std::move(members)}; EXPECT_EQ(s.members().size(), 1u); - EXPECT_EQ(s.decoration(), StructDecoration::kNone); + EXPECT_TRUE(s.decorations().empty()); EXPECT_EQ(s.line(), 0u); EXPECT_EQ(s.column(), 0u); } -TEST_F(StructTest, CreationWithSource) { +TEST_F(StructTest, Creation_WithDecorations) { type::I32Type i32; - Source source{27, 4}; + + StructMemberList members; + members.push_back( + std::make_unique("a", &i32, StructMemberDecorationList())); + + StructDecorationList decos; + decos.push_back(StructDecoration::kBlock); + + Struct s{std::move(decos), std::move(members)}; + EXPECT_EQ(s.members().size(), 1u); + ASSERT_EQ(s.decorations().size(), 1u); + EXPECT_EQ(s.decorations()[0], StructDecoration::kBlock); + EXPECT_EQ(s.line(), 0u); + EXPECT_EQ(s.column(), 0u); +} + +TEST_F(StructTest, CreationWithSourceAndDecorations) { + type::I32Type i32; + StructMemberList members; members.emplace_back( std::make_unique("a", &i32, StructMemberDecorationList())); - Struct s{source, StructDecoration::kNone, std::move(members)}; + StructDecorationList decos; + decos.push_back(StructDecoration::kBlock); + + Struct s{Source{27, 4}, std::move(decos), std::move(members)}; EXPECT_EQ(s.members().size(), 1u); - EXPECT_EQ(s.decoration(), StructDecoration::kNone); + ASSERT_EQ(s.decorations().size(), 1u); + EXPECT_EQ(s.decorations()[0], StructDecoration::kBlock); EXPECT_EQ(s.line(), 27u); EXPECT_EQ(s.column(), 4u); } @@ -63,37 +85,57 @@ TEST_F(StructTest, IsValid) { TEST_F(StructTest, IsValid_Null_StructMember) { type::I32Type i32; + StructMemberList members; members.push_back( std::make_unique("a", &i32, StructMemberDecorationList())); members.push_back(nullptr); - Struct s{StructDecoration::kNone, std::move(members)}; + Struct s{std::move(members)}; EXPECT_FALSE(s.IsValid()); } TEST_F(StructTest, IsValid_Invalid_StructMember) { type::I32Type i32; + StructMemberList members; members.push_back( std::make_unique("", &i32, StructMemberDecorationList())); - Struct s{StructDecoration::kNone, std::move(members)}; + Struct s{std::move(members)}; + EXPECT_FALSE(s.IsValid()); +} + +TEST_F(StructTest, IsValid_NoneDecoration) { + type::I32Type i32; + + StructMemberList members; + members.push_back( + std::make_unique("a", &i32, StructMemberDecorationList())); + + StructDecorationList decos; + decos.push_back(StructDecoration::kNone); + + Struct s{std::move(decos), std::move(members)}; EXPECT_FALSE(s.IsValid()); } TEST_F(StructTest, ToStr) { type::I32Type i32; - Source source{27, 4}; + StructMemberList members; members.emplace_back( std::make_unique("a", &i32, StructMemberDecorationList())); - Struct s{source, StructDecoration::kNone, std::move(members)}; + StructDecorationList decos; + decos.push_back(StructDecoration::kBlock); + + Struct s{std::move(decos), std::move(members)}; std::ostringstream out; s.to_str(out, 2); EXPECT_EQ(out.str(), R"( Struct{ + [[block]] StructMember{a: __i32} } )"); diff --git a/src/ast/type/struct_type.h b/src/ast/type/struct_type.h index 97cb4f2daa..6a41e674e7 100644 --- a/src/ast/type/struct_type.h +++ b/src/ast/type/struct_type.h @@ -42,9 +42,7 @@ class StructType : public Type { const std::string& name() const { return name_; } /// @returns true if the struct has a block decoration - bool IsBlockDecorated() const { - return struct_->decoration() == StructDecoration::kBlock; - } + bool IsBlockDecorated() const { return struct_->IsBlockDecorated(); } /// @returns true if the type is a struct type bool IsStruct() const override; diff --git a/src/reader/spirv/function_memory_test.cc b/src/reader/spirv/function_memory_test.cc index 5d4a4366c6..267ae07600 100644 --- a/src/reader/spirv/function_memory_test.cc +++ b/src/reader/spirv/function_memory_test.cc @@ -806,7 +806,8 @@ TEST_F(SpvParserTest, RemapStorageBuffer_TypesAndVarDeclarations) { } RTArr -> __array__u32_stride_4 S -> __struct_S - [[block]] Struct{ + Struct{ + [[block]] StructMember{[[ offset 0 ]] field0: __u32} StructMember{[[ offset 4 ]] field1: __alias_RTArr__array__u32_stride_4} })")); diff --git a/src/reader/spirv/parser_impl.cc b/src/reader/spirv/parser_impl.cc index a21bf6dab2..92c6a29910 100644 --- a/src/reader/spirv/parser_impl.cc +++ b/src/reader/spirv/parser_impl.cc @@ -789,13 +789,13 @@ ast::type::Type* ParserImpl::ConvertType( const spvtools::opt::analysis::Struct* struct_ty) { // Compute the struct decoration. auto struct_decorations = this->GetDecorationsFor(type_id); - auto ast_struct_decoration = ast::StructDecoration::kNone; + ast::StructDecorationList ast_struct_decorations; if (struct_decorations.size() == 1) { const auto decoration = struct_decorations[0][0]; if (decoration == SpvDecorationBlock) { - ast_struct_decoration = ast::StructDecoration::kBlock; + ast_struct_decorations.push_back(ast::StructDecoration::kBlock); } else if (decoration == SpvDecorationBufferBlock) { - ast_struct_decoration = ast::StructDecoration::kBlock; + ast_struct_decorations.push_back(ast::StructDecoration::kBlock); remap_buffer_block_type_.insert(type_id); } else { Fail() << "struct with ID " << type_id @@ -859,8 +859,8 @@ ast::type::Type* ParserImpl::ConvertType( } // Now make the struct. - auto ast_struct = std::make_unique(ast_struct_decoration, - std::move(ast_members)); + auto ast_struct = std::make_unique( + std::move(ast_struct_decorations), std::move(ast_members)); // The struct type will be assigned a name during EmitAliasTypes. auto ast_struct_type = std::make_unique(std::move(ast_struct)); diff --git a/src/reader/spirv/parser_impl_convert_type_test.cc b/src/reader/spirv/parser_impl_convert_type_test.cc index 8ee2f1d012..3f18da7060 100644 --- a/src/reader/spirv/parser_impl_convert_type_test.cc +++ b/src/reader/spirv/parser_impl_convert_type_test.cc @@ -582,7 +582,8 @@ TEST_F(SpvParserTest, ConvertType_StructWithBlockDecoration) { EXPECT_TRUE(type->IsStruct()); std::stringstream ss; type->AsStruct()->impl()->to_str(ss, 0); - EXPECT_THAT(ss.str(), Eq(R"([[block]] Struct{ + EXPECT_THAT(ss.str(), Eq(R"(Struct{ + [[block]] StructMember{field0: __u32} } )")); diff --git a/src/reader/spirv/parser_impl_module_var_test.cc b/src/reader/spirv/parser_impl_module_var_test.cc index 139358eaa0..06a81c8243 100644 --- a/src/reader/spirv/parser_impl_module_var_test.cc +++ b/src/reader/spirv/parser_impl_module_var_test.cc @@ -1311,7 +1311,8 @@ TEST_F(SpvParserTest, ModuleScopeVar_NonReadableDecoration_DroppedForNow) { __alias_S__struct_S } S -> __struct_S - [[block]] Struct{ + Struct{ + [[block]] StructMember{field0: __u32} StructMember{field1: __f32} StructMember{field2: __array__u32_2} @@ -1338,7 +1339,8 @@ TEST_F(SpvParserTest, ModuleScopeVar_NonWritableDecoration_DroppedForNow) { __alias_S__struct_S } S -> __struct_S - [[block]] Struct{ + Struct{ + [[block]] StructMember{field0: __u32} StructMember{field1: __f32} StructMember{field2: __array__u32_2} @@ -1369,7 +1371,8 @@ TEST_F(SpvParserTest, ModuleScopeVar_ColMajorDecoration_Dropped) { __alias_S__struct_S } S -> __struct_S - [[block]] Struct{ + Struct{ + [[block]] StructMember{field0: __mat_2_3__f32} } })")) << module_str; @@ -1398,7 +1401,8 @@ TEST_F(SpvParserTest, ModuleScopeVar_MatrixStrideDecoration_Dropped) { __alias_S__struct_S } S -> __struct_S - [[block]] Struct{ + Struct{ + [[block]] StructMember{field0: __mat_2_3__f32} } })")) << module_str; diff --git a/src/reader/wgsl/parser_impl.cc b/src/reader/wgsl/parser_impl.cc index 26044d41a3..a91712ffc0 100644 --- a/src/reader/wgsl/parser_impl.cc +++ b/src/reader/wgsl/parser_impl.cc @@ -1445,14 +1445,21 @@ ast::StorageClass ParserImpl::storage_class() { } // struct_decl -// : struct_decoration_decl? STRUCT struct_body_decl +// : struct_decoration_decl* STRUCT struct_body_decl std::unique_ptr ParserImpl::struct_decl() { auto t = peek(); auto source = t.source(); - auto deco = struct_decoration_decl(); - if (has_error()) - return nullptr; + ast::StructDecorationList decos; + for (;;) { + size_t s = decos.size(); + if (!struct_decoration_decl(decos)) { + return nullptr; + } + if (decos.size() == s) { + break; + } + } t = next(); if (!t.IsStruct()) { @@ -1466,22 +1473,25 @@ std::unique_ptr ParserImpl::struct_decl() { } return std::make_unique( - std::make_unique(source, deco, std::move(body))); + std::make_unique(source, std::move(decos), std::move(body))); } // struct_decoration_decl // : ATTR_LEFT struct_decoration ATTR_RIGHT -ast::StructDecoration ParserImpl::struct_decoration_decl() { +bool ParserImpl::struct_decoration_decl(ast::StructDecorationList& decos) { auto t = peek(); - if (!t.IsAttrLeft()) - return ast::StructDecoration::kNone; + if (!t.IsAttrLeft()) { + return true; + } auto deco = struct_decoration(peek(1)); - if (has_error()) - return ast::StructDecoration::kNone; - if (deco == ast::StructDecoration::kNone) { - return deco; + if (has_error()) { + return false; } + if (deco == ast::StructDecoration::kNone) { + return true; + } + decos.push_back(deco); next(); // Consume the peek of [[ next(); // Consume the peek from the struct_decoration @@ -1489,10 +1499,10 @@ ast::StructDecoration ParserImpl::struct_decoration_decl() { t = next(); if (!t.IsAttrRight()) { set_error(t, "missing ]] for struct decoration"); - return ast::StructDecoration::kNone; + return false; } - return deco; + return true; } // struct_decoration diff --git a/src/reader/wgsl/parser_impl.h b/src/reader/wgsl/parser_impl.h index 5cc5248874..bd7ae82f4c 100644 --- a/src/reader/wgsl/parser_impl.h +++ b/src/reader/wgsl/parser_impl.h @@ -155,9 +155,11 @@ class ParserImpl { /// Parses a `struct_decl` grammar element /// @returns the struct type or nullptr on error std::unique_ptr struct_decl(); - /// Parses a `struct_decoration_decl` grammar element - /// @returns the struct decoration or StructDecoraton::kNone - ast::StructDecoration struct_decoration_decl(); + /// Parses a `struct_decoration_decl` grammar element, appending newly + /// parsed decorations to the end of |decos|. + /// @param decos list to store the parsed decorations + /// @returns the true on successful parse; false otherwise + bool struct_decoration_decl(ast::StructDecorationList& decos); /// Parses a `struct_decoration` grammar element /// @param t the current token /// @returns the struct decoration or StructDecoraton::kNone if none matched @@ -177,7 +179,8 @@ class ParserImpl { /// Parses a `function_decl` grammar element /// @returns the parsed function, nullptr otherwise std::unique_ptr function_decl(); - /// Parses a `function_decoration_decl` grammar element + /// Parses a `function_decoration_decl` grammar element, appending newly + /// parsed decorations to the end of |decos|. /// @param decos list to store the parsed decorations /// @returns true on successful parse; false otherwise bool function_decoration_decl(ast::FunctionDecorationList& decos); diff --git a/src/reader/wgsl/parser_impl_struct_decl_test.cc b/src/reader/wgsl/parser_impl_struct_decl_test.cc index b1480e1aae..178b3a5ee2 100644 --- a/src/reader/wgsl/parser_impl_struct_decl_test.cc +++ b/src/reader/wgsl/parser_impl_struct_decl_test.cc @@ -48,6 +48,26 @@ TEST_F(ParserImplTest, StructDecl_ParsesWithDecoration) { ASSERT_EQ(s->impl()->members().size(), 2u); EXPECT_EQ(s->impl()->members()[0]->name(), "a"); EXPECT_EQ(s->impl()->members()[1]->name(), "b"); + ASSERT_EQ(s->impl()->decorations().size(), 1u); + EXPECT_EQ(s->impl()->decorations()[0], ast::StructDecoration::kBlock); +} + +TEST_F(ParserImplTest, StructDecl_ParsesWithMultipleDecoration) { + auto* p = parser(R"( +[[block]] +[[block]] struct { + a : f32; + b : f32; +})"); + auto s = p->struct_decl(); + ASSERT_FALSE(p->has_error()); + ASSERT_NE(s, nullptr); + ASSERT_EQ(s->impl()->members().size(), 2u); + EXPECT_EQ(s->impl()->members()[0]->name(), "a"); + EXPECT_EQ(s->impl()->members()[1]->name(), "b"); + ASSERT_EQ(s->impl()->decorations().size(), 2u); + EXPECT_EQ(s->impl()->decorations()[0], ast::StructDecoration::kBlock); + EXPECT_EQ(s->impl()->decorations()[1], ast::StructDecoration::kBlock); } TEST_F(ParserImplTest, StructDecl_EmptyMembers) { diff --git a/src/reader/wgsl/parser_impl_struct_decoration_decl_test.cc b/src/reader/wgsl/parser_impl_struct_decoration_decl_test.cc index bd64e51ebf..5c0ef8eba3 100644 --- a/src/reader/wgsl/parser_impl_struct_decoration_decl_test.cc +++ b/src/reader/wgsl/parser_impl_struct_decoration_decl_test.cc @@ -23,14 +23,17 @@ namespace { TEST_F(ParserImplTest, StructDecorationDecl_Parses) { auto* p = parser("[[block]]"); - auto d = p->struct_decoration_decl(); + ast::StructDecorationList decos; + ASSERT_TRUE(p->struct_decoration_decl(decos)); ASSERT_FALSE(p->has_error()); - EXPECT_EQ(d, ast::StructDecoration::kBlock); + EXPECT_EQ(decos.size(), 1u); + EXPECT_EQ(decos[0], ast::StructDecoration::kBlock); } TEST_F(ParserImplTest, StructDecorationDecl_MissingAttrRight) { auto* p = parser("[[block"); - p->struct_decoration_decl(); + ast::StructDecorationList decos; + ASSERT_FALSE(p->struct_decoration_decl(decos)); ASSERT_TRUE(p->has_error()); EXPECT_EQ(p->error(), "1:8: missing ]] for struct decoration"); } @@ -38,8 +41,10 @@ TEST_F(ParserImplTest, StructDecorationDecl_MissingAttrRight) { // Note, this isn't an error because it could be an array decoration TEST_F(ParserImplTest, StructDecorationDecl_InvalidDecoration) { auto* p = parser("[[invalid]]"); - p->struct_decoration_decl(); + ast::StructDecorationList decos; + ASSERT_TRUE(p->struct_decoration_decl(decos)); ASSERT_FALSE(p->has_error()); + EXPECT_TRUE(decos.empty()); } } // namespace diff --git a/src/transform/vertex_pulling_transform.cc b/src/transform/vertex_pulling_transform.cc index fe1219298e..5e4d7ce7ca 100644 --- a/src/transform/vertex_pulling_transform.cc +++ b/src/transform/vertex_pulling_transform.cc @@ -226,9 +226,12 @@ void VertexPullingTransform::AddVertexStorageBuffers() { members.push_back(std::make_unique( kStructBufferName, internal_array_type, std::move(member_dec))); - auto* struct_type = ctx_->type_mgr().Get( - std::make_unique(std::make_unique( - ast::StructDecoration::kBlock, std::move(members)))); + ast::StructDecorationList decos; + decos.push_back(ast::StructDecoration::kBlock); + + auto* struct_type = + ctx_->type_mgr().Get(std::make_unique( + std::make_unique(std::move(decos), std::move(members)))); for (uint32_t i = 0; i < vertex_state_->vertex_buffers.size(); ++i) { // The decorated variable with struct type diff --git a/src/type_determiner_test.cc b/src/type_determiner_test.cc index 74adef954b..4b11bf4cdc 100644 --- a/src/type_determiner_test.cc +++ b/src/type_determiner_test.cc @@ -1022,8 +1022,7 @@ TEST_F(TypeDeterminerTest, Expr_MemberAccessor_Struct) { members.push_back(std::make_unique("second_member", &f32, std::move(decos))); - auto strct = std::make_unique(ast::StructDecoration::kNone, - std::move(members)); + auto strct = std::make_unique(std::move(members)); ast::type::StructType st(std::move(strct)); @@ -1058,8 +1057,7 @@ TEST_F(TypeDeterminerTest, Expr_MemberAccessor_Struct_Alias) { members.push_back(std::make_unique("second_member", &f32, std::move(decos))); - auto strct = std::make_unique(ast::StructDecoration::kNone, - std::move(members)); + auto strct = std::make_unique(std::move(members)); auto st = std::make_unique(std::move(strct)); ast::type::AliasType alias("alias", st.get()); @@ -1164,8 +1162,7 @@ TEST_F(TypeDeterminerTest, Expr_Accessor_MultiLevel) { b_members.push_back( std::make_unique("foo", &vec4, std::move(decos))); - auto strctB = std::make_unique(ast::StructDecoration::kNone, - std::move(b_members)); + auto strctB = std::make_unique(std::move(b_members)); ast::type::StructType stB(std::move(strctB)); stB.set_name("B"); @@ -1175,8 +1172,7 @@ TEST_F(TypeDeterminerTest, Expr_Accessor_MultiLevel) { a_members.push_back( std::make_unique("mem", &vecB, std::move(decos))); - auto strctA = std::make_unique(ast::StructDecoration::kNone, - std::move(a_members)); + auto strctA = std::make_unique(std::move(a_members)); ast::type::StructType stA(std::move(strctA)); stA.set_name("A"); diff --git a/src/writer/hlsl/generator_impl_type_test.cc b/src/writer/hlsl/generator_impl_type_test.cc index 4a5b76dbaa..85bbda4320 100644 --- a/src/writer/hlsl/generator_impl_type_test.cc +++ b/src/writer/hlsl/generator_impl_type_test.cc @@ -261,9 +261,11 @@ TEST_F(HlslGeneratorImplTest_Type, DISABLED_EmitType_Struct_WithDecoration) { members.push_back( std::make_unique("b", &f32, std::move(b_deco))); - auto str = std::make_unique(); - str->set_members(std::move(members)); - str->set_decoration(ast::StructDecoration::kBlock); + ast::StructDecorationList decos; + decos.push_back(ast::StructDecoration::kBlock); + + auto str = + std::make_unique(std::move(decos), std::move(members)); ast::type::StructType s(std::move(str)); diff --git a/src/writer/msl/generator_impl_type_test.cc b/src/writer/msl/generator_impl_type_test.cc index 6d4f8d15a4..02a0581661 100644 --- a/src/writer/msl/generator_impl_type_test.cc +++ b/src/writer/msl/generator_impl_type_test.cc @@ -296,9 +296,10 @@ TEST_F(MslGeneratorImplTest, DISABLED_EmitType_Struct_WithDecoration) { members.push_back( std::make_unique("b", &f32, std::move(b_deco))); - auto str = std::make_unique(); - str->set_members(std::move(members)); - str->set_decoration(ast::StructDecoration::kBlock); + ast::StructDecorationList decos; + decos.push_back(ast::StructDecoration::kBlock); + auto str = + std::make_unique(std::move(decos), std::move(members)); ast::type::StructType s(std::move(str)); diff --git a/src/writer/spirv/builder.cc b/src/writer/spirv/builder.cc index 45b8e34ecd..c10f9579b2 100644 --- a/src/writer/spirv/builder.cc +++ b/src/writer/spirv/builder.cc @@ -2384,14 +2384,9 @@ bool Builder::GenerateStructType(ast::type::StructType* struct_type, OperandList ops; ops.push_back(result); - if (impl->decoration() == ast::StructDecoration::kBlock) { + if (impl->IsBlockDecorated()) { push_annot(spv::Op::OpDecorate, {Operand::Int(struct_id), Operand::Int(SpvDecorationBlock)}); - } else { - if (impl->decoration() != ast::StructDecoration::kNone) { - error_ = "unknown struct decoration"; - return false; - } } auto& members = impl->members(); diff --git a/src/writer/spirv/builder_accessor_expression_test.cc b/src/writer/spirv/builder_accessor_expression_test.cc index 44b5038909..7bafbedc4f 100644 --- a/src/writer/spirv/builder_accessor_expression_test.cc +++ b/src/writer/spirv/builder_accessor_expression_test.cc @@ -305,8 +305,7 @@ TEST_F(BuilderTest, MemberAccessor) { members.push_back( std::make_unique("b", &f32, std::move(decos))); - auto s = std::make_unique(ast::StructDecoration::kNone, - std::move(members)); + auto s = std::make_unique(std::move(members)); ast::type::StructType s_type(std::move(s)); s_type.set_name("my_struct"); @@ -363,15 +362,15 @@ TEST_F(BuilderTest, MemberAccessor_Nested) { inner_members.push_back( std::make_unique("b", &f32, std::move(decos))); - ast::type::StructType inner_struct(std::make_unique( - ast::StructDecoration::kNone, std::move(inner_members))); + ast::type::StructType inner_struct( + std::make_unique(std::move(inner_members))); ast::StructMemberList outer_members; outer_members.push_back(std::make_unique( "inner", &inner_struct, std::move(decos))); - ast::type::StructType s_type(std::make_unique( - ast::StructDecoration::kNone, std::move(outer_members))); + ast::type::StructType s_type( + std::make_unique(std::move(outer_members))); s_type.set_name("my_struct"); ast::Variable var("ident", ast::StorageClass::kFunction, &s_type); @@ -431,8 +430,8 @@ TEST_F(BuilderTest, MemberAccessor_Nested_WithAlias) { inner_members.push_back( std::make_unique("b", &f32, std::move(decos))); - ast::type::StructType inner_struct(std::make_unique( - ast::StructDecoration::kNone, std::move(inner_members))); + ast::type::StructType inner_struct( + std::make_unique(std::move(inner_members))); ast::type::AliasType alias("Inner", &inner_struct); @@ -440,8 +439,8 @@ TEST_F(BuilderTest, MemberAccessor_Nested_WithAlias) { outer_members.push_back( std::make_unique("inner", &alias, std::move(decos))); - ast::type::StructType s_type(std::make_unique( - ast::StructDecoration::kNone, std::move(outer_members))); + ast::type::StructType s_type( + std::make_unique(std::move(outer_members))); s_type.set_name("my_struct"); ast::Variable var("ident", ast::StorageClass::kFunction, &s_type); @@ -501,15 +500,15 @@ TEST_F(BuilderTest, MemberAccessor_Nested_Assignment_LHS) { inner_members.push_back( std::make_unique("b", &f32, std::move(decos))); - ast::type::StructType inner_struct(std::make_unique( - ast::StructDecoration::kNone, std::move(inner_members))); + ast::type::StructType inner_struct( + std::make_unique(std::move(inner_members))); ast::StructMemberList outer_members; outer_members.push_back(std::make_unique( "inner", &inner_struct, std::move(decos))); - ast::type::StructType s_type(std::make_unique( - ast::StructDecoration::kNone, std::move(outer_members))); + ast::type::StructType s_type( + std::make_unique(std::move(outer_members))); s_type.set_name("my_struct"); ast::Variable var("ident", ast::StorageClass::kFunction, &s_type); @@ -576,15 +575,15 @@ TEST_F(BuilderTest, MemberAccessor_Nested_Assignment_RHS) { inner_members.push_back( std::make_unique("b", &f32, std::move(decos))); - ast::type::StructType inner_struct(std::make_unique( - ast::StructDecoration::kNone, std::move(inner_members))); + ast::type::StructType inner_struct( + std::make_unique(std::move(inner_members))); ast::StructMemberList outer_members; outer_members.push_back(std::make_unique( "inner", &inner_struct, std::move(decos))); - ast::type::StructType s_type(std::make_unique( - ast::StructDecoration::kNone, std::move(outer_members))); + ast::type::StructType s_type( + std::make_unique(std::move(outer_members))); s_type.set_name("my_struct"); ast::Variable var("ident", ast::StorageClass::kFunction, &s_type); @@ -863,15 +862,13 @@ TEST_F(BuilderTest, Accessor_Mixed_ArrayAndMember) { ast::StructMemberList members; members.push_back( std::make_unique("baz", &vec3, std::move(decos))); - auto s = std::make_unique(ast::StructDecoration::kNone, - std::move(members)); + auto s = std::make_unique(std::move(members)); ast::type::StructType c_type(std::move(s)); c_type.set_name("C"); members.push_back( std::make_unique("bar", &c_type, std::move(decos))); - s = std::make_unique(ast::StructDecoration::kNone, - std::move(members)); + s = std::make_unique(std::move(members)); ast::type::StructType b_type(std::move(s)); b_type.set_name("B"); @@ -879,8 +876,7 @@ TEST_F(BuilderTest, Accessor_Mixed_ArrayAndMember) { members.push_back(std::make_unique("foo", &b_ary_type, std::move(decos))); - s = std::make_unique(ast::StructDecoration::kNone, - std::move(members)); + s = std::make_unique(std::move(members)); ast::type::StructType a_type(std::move(s)); a_type.set_name("A"); diff --git a/src/writer/spirv/builder_assign_test.cc b/src/writer/spirv/builder_assign_test.cc index d81af3b084..ef36625280 100644 --- a/src/writer/spirv/builder_assign_test.cc +++ b/src/writer/spirv/builder_assign_test.cc @@ -269,8 +269,7 @@ TEST_F(BuilderTest, Assign_StructMember) { members.push_back( std::make_unique("b", &f32, std::move(decos))); - auto s = std::make_unique(ast::StructDecoration::kNone, - std::move(members)); + auto s = std::make_unique(std::move(members)); ast::type::StructType s_type(std::move(s)); s_type.set_name("my_struct"); diff --git a/src/writer/spirv/builder_constructor_expression_test.cc b/src/writer/spirv/builder_constructor_expression_test.cc index db910e7fc2..26240864c6 100644 --- a/src/writer/spirv/builder_constructor_expression_test.cc +++ b/src/writer/spirv/builder_constructor_expression_test.cc @@ -1747,8 +1747,7 @@ TEST_F(BuilderTest, Constructor_Type_Struct) { members.push_back( std::make_unique("b", &vec, std::move(decos))); - auto s = std::make_unique(ast::StructDecoration::kNone, - std::move(members)); + auto s = std::make_unique(std::move(members)); ast::type::StructType s_type(std::move(s)); s_type.set_name("my_struct"); @@ -1959,8 +1958,7 @@ TEST_F(BuilderTest, Constructor_Type_ZeroInit_Struct) { members.push_back( std::make_unique("a", &f32, std::move(decos))); - auto s = std::make_unique(ast::StructDecoration::kNone, - std::move(members)); + auto s = std::make_unique(std::move(members)); ast::type::StructType s_type(std::move(s)); s_type.set_name("my_struct"); diff --git a/src/writer/spirv/builder_intrinsic_test.cc b/src/writer/spirv/builder_intrinsic_test.cc index a16c21ea89..f89c0e34d6 100644 --- a/src/writer/spirv/builder_intrinsic_test.cc +++ b/src/writer/spirv/builder_intrinsic_test.cc @@ -2732,8 +2732,7 @@ TEST_F(BuilderTest, Call_ArrayLength) { members.push_back( std::make_unique("a", &ary, std::move(decos))); - auto s = std::make_unique(ast::StructDecoration::kNone, - std::move(members)); + auto s = std::make_unique(std::move(members)); ast::type::StructType s_type(std::move(s)); s_type.set_name("my_struct"); @@ -2792,8 +2791,7 @@ TEST_F(BuilderTest, Call_ArrayLength_OtherMembersInStruct) { members.push_back( std::make_unique("a", &ary, std::move(decos))); - auto s = std::make_unique(ast::StructDecoration::kNone, - std::move(members)); + auto s = std::make_unique(std::move(members)); ast::type::StructType s_type(std::move(s)); s_type.set_name("my_struct"); @@ -2854,8 +2852,7 @@ TEST_F(BuilderTest, DISABLED_Call_ArrayLength_Ptr) { members.push_back( std::make_unique("a", &ary, std::move(decos))); - auto s = std::make_unique(ast::StructDecoration::kNone, - std::move(members)); + auto s = std::make_unique(std::move(members)); ast::type::StructType s_type(std::move(s)); s_type.set_name("my_struct"); diff --git a/src/writer/spirv/builder_type_test.cc b/src/writer/spirv/builder_type_test.cc index 4e2ae08157..3a7e84d909 100644 --- a/src/writer/spirv/builder_type_test.cc +++ b/src/writer/spirv/builder_type_test.cc @@ -328,8 +328,7 @@ TEST_F(BuilderTest_Type, GenerateStruct) { members.push_back( std::make_unique("a", &f32, std::move(decos))); - auto s = std::make_unique(ast::StructDecoration::kNone, - std::move(members)); + auto s = std::make_unique(std::move(members)); ast::type::StructType s_type(std::move(s)); s_type.set_name("my_struct"); @@ -355,7 +354,10 @@ TEST_F(BuilderTest_Type, GenerateStruct_Decorated) { members.push_back( std::make_unique("a", &f32, std::move(decos))); - auto s = std::make_unique(ast::StructDecoration::kBlock, + ast::StructDecorationList struct_decos; + struct_decos.push_back(ast::StructDecoration::kBlock); + + auto s = std::make_unique(std::move(struct_decos), std::move(members)); ast::type::StructType s_type(std::move(s)); s_type.set_name("my_struct"); @@ -390,8 +392,7 @@ TEST_F(BuilderTest_Type, GenerateStruct_DecoratedMembers) { members.push_back( std::make_unique("b", &f32, std::move(b_decos))); - auto s = std::make_unique(ast::StructDecoration::kNone, - std::move(members)); + auto s = std::make_unique(std::move(members)); ast::type::StructType s_type(std::move(s)); ast::Module mod; @@ -429,8 +430,7 @@ TEST_F(BuilderTest_Type, GenerateStruct_NonLayout_Matrix) { members.push_back(std::make_unique("c", &glsl_mat4x4, std::move(empty_c))); - auto s = std::make_unique(ast::StructDecoration::kNone, - std::move(members)); + auto s = std::make_unique(std::move(members)); ast::type::StructType s_type(std::move(s)); ast::Module mod; @@ -477,8 +477,7 @@ TEST_F(BuilderTest_Type, GenerateStruct_DecoratedMembers_LayoutMatrix) { members.push_back(std::make_unique("c", &glsl_mat4x4, std::move(c_decos))); - auto s = std::make_unique(ast::StructDecoration::kNone, - std::move(members)); + auto s = std::make_unique(std::move(members)); ast::type::StructType s_type(std::move(s)); ast::Module mod; @@ -543,8 +542,7 @@ TEST_F(BuilderTest_Type, GenerateStruct_DecoratedMembers_LayoutArraysOfMatrix) { members.push_back(std::make_unique("c", &glsl_mat4x4, std::move(c_decos))); - auto s = std::make_unique(ast::StructDecoration::kNone, - std::move(members)); + auto s = std::make_unique(std::move(members)); ast::type::StructType s_type(std::move(s)); ast::Module mod; diff --git a/src/writer/wgsl/generator_impl.cc b/src/writer/wgsl/generator_impl.cc index a8a4a88145..446c92bc9d 100644 --- a/src/writer/wgsl/generator_impl.cc +++ b/src/writer/wgsl/generator_impl.cc @@ -437,8 +437,8 @@ bool GeneratorImpl::EmitType(ast::type::Type* type) { } } else if (type->IsStruct()) { auto* str = type->AsStruct()->impl(); - if (str->decoration() != ast::StructDecoration::kNone) { - out_ << "[[" << str->decoration() << "]] "; + for (auto deco : str->decorations()) { + out_ << "[[" << deco << "]]" << std::endl; } out_ << "struct {" << std::endl; diff --git a/src/writer/wgsl/generator_impl_type_test.cc b/src/writer/wgsl/generator_impl_type_test.cc index c6e1352384..495e1e4cd0 100644 --- a/src/writer/wgsl/generator_impl_type_test.cc +++ b/src/writer/wgsl/generator_impl_type_test.cc @@ -160,15 +160,18 @@ TEST_F(WgslGeneratorImplTest, EmitType_Struct_WithDecoration) { members.push_back( std::make_unique("b", &f32, std::move(b_deco))); - auto str = std::make_unique(); - str->set_members(std::move(members)); - str->set_decoration(ast::StructDecoration::kBlock); + ast::StructDecorationList decos; + decos.push_back(ast::StructDecoration::kBlock); + + auto str = + std::make_unique(std::move(decos), std::move(members)); ast::type::StructType s(std::move(str)); GeneratorImpl g; ASSERT_TRUE(g.EmitType(&s)) << g.error(); - EXPECT_EQ(g.result(), R"([[block]] struct { + EXPECT_EQ(g.result(), R"([[block]] +struct { a : i32; [[offset(4)]] b : f32; })");