Remove type alias struct variant.
This CL removes the `type IDENT = struct` format in favour of the
`struct IDENT` variant.
Bug: tint:175
Change-Id: I4fde8012fd07f811cd0bd80445198f6bbc92b720
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/30661
Commit-Queue: dan sinclair <dsinclair@chromium.org>
Commit-Queue: Ryan Harrison <rharrison@chromium.org>
Reviewed-by: Ryan Harrison <rharrison@chromium.org>
diff --git a/src/reader/wgsl/parser_impl.cc b/src/reader/wgsl/parser_impl.cc
index f365cfb..33a487f 100644
--- a/src/reader/wgsl/parser_impl.cc
+++ b/src/reader/wgsl/parser_impl.cc
@@ -261,7 +261,7 @@
return;
}
- auto str = struct_decl("");
+ auto str = struct_decl();
if (has_error()) {
return;
}
@@ -1080,7 +1080,6 @@
// type_alias
// : TYPE IDENT EQUAL type_decl
-// | TYPE IDENT EQUAL struct_decl
ast::type::Type* ParserImpl::type_alias() {
auto t = peek();
if (!t.IsType())
@@ -1105,17 +1104,8 @@
if (has_error())
return nullptr;
if (type == nullptr) {
- auto str = struct_decl(name);
- if (has_error())
- return nullptr;
- if (str == nullptr) {
- set_error(peek(), "invalid type alias");
- return nullptr;
- }
-
- type = ctx_.type_mgr().Get(std::move(str));
- register_constructed(name, type);
- return type;
+ set_error(peek(), "invalid type alias");
+ return nullptr;
}
if (type == nullptr) {
set_error(peek(), "invalid type for alias");
@@ -1501,9 +1491,8 @@
}
// struct_decl
-// : struct_decoration_decl* STRUCT struct_body_decl
-std::unique_ptr<ast::type::StructType> ParserImpl::struct_decl(
- const std::string& name) {
+// : struct_decoration_decl* STRUCT IDENT struct_body_decl
+std::unique_ptr<ast::type::StructType> ParserImpl::struct_decl() {
auto t = peek();
auto source = t.source();
@@ -1527,17 +1516,12 @@
}
next(); // Consume the peek
- // If there is no name this is a global struct call. This check will go
- // away when the type_alias struct entry is removed.
- std::string str_name = name;
- if (name.empty()) {
- t = next();
- if (!t.IsIdentifier()) {
- set_error(t, "missing identifier for struct declaration");
- return nullptr;
- }
- str_name = t.to_str();
+ t = next();
+ if (!t.IsIdentifier()) {
+ set_error(t, "missing identifier for struct declaration");
+ return nullptr;
}
+ auto name = t.to_str();
auto body = struct_body_decl();
if (has_error()) {
@@ -1545,7 +1529,7 @@
}
return std::make_unique<ast::type::StructType>(
- str_name,
+ name,
std::make_unique<ast::Struct>(source, std::move(decos), std::move(body)));
}
diff --git a/src/reader/wgsl/parser_impl.h b/src/reader/wgsl/parser_impl.h
index fd8661a..d50147e 100644
--- a/src/reader/wgsl/parser_impl.h
+++ b/src/reader/wgsl/parser_impl.h
@@ -156,9 +156,8 @@
/// @returns the storage class or StorageClass::kNone if none matched
ast::StorageClass storage_class();
/// Parses a `struct_decl` grammar element
- /// @param name the name of the struct
/// @returns the struct type or nullptr on error
- std::unique_ptr<ast::type::StructType> struct_decl(const std::string& name);
+ std::unique_ptr<ast::type::StructType> struct_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
diff --git a/src/reader/wgsl/parser_impl_global_decl_test.cc b/src/reader/wgsl/parser_impl_global_decl_test.cc
index a030ed4..cc93b06 100644
--- a/src/reader/wgsl/parser_impl_global_decl_test.cc
+++ b/src/reader/wgsl/parser_impl_global_decl_test.cc
@@ -92,17 +92,6 @@
EXPECT_EQ(m.constructed_types()[0]->AsAlias()->name(), "A");
}
-TEST_F(ParserImplTest, GlobalDecl_TypeAlias_Struct) {
- auto* p = parser("type A = struct { a : f32; };");
- p->global_decl();
- ASSERT_FALSE(p->has_error()) << p->error();
-
- auto m = p->module();
- ASSERT_EQ(m.constructed_types().size(), 1u);
- ASSERT_TRUE(m.constructed_types()[0]->IsStruct());
- EXPECT_EQ(m.constructed_types()[0]->AsStruct()->name(), "A");
-}
-
TEST_F(ParserImplTest, GlobalDecl_TypeAlias_StructIdent) {
auto* p = parser(R"(struct A {
a : f32;
@@ -239,21 +228,6 @@
EXPECT_EQ(p->error(), "1:22: missing ';' for struct declaration");
}
-// This was failing due to not finding the missing ;. https://crbug.com/tint/218
-TEST_F(ParserImplTest, TypeDecl_Struct_Empty) {
- auto* p = parser("type str = struct {};");
- p->global_decl();
- ASSERT_FALSE(p->has_error()) << p->error();
-
- auto module = p->module();
- ASSERT_EQ(module.constructed_types().size(), 1u);
-
- ASSERT_TRUE(module.constructed_types()[0]->IsStruct());
- auto* str = module.constructed_types()[0]->AsStruct();
- EXPECT_EQ(str->name(), "str");
- EXPECT_EQ(str->impl()->members().size(), 0u);
-}
-
} // namespace
} // namespace wgsl
} // namespace reader
diff --git a/src/reader/wgsl/parser_impl_struct_decl_test.cc b/src/reader/wgsl/parser_impl_struct_decl_test.cc
index 8a80237..3222117 100644
--- a/src/reader/wgsl/parser_impl_struct_decl_test.cc
+++ b/src/reader/wgsl/parser_impl_struct_decl_test.cc
@@ -28,22 +28,7 @@
a : i32;
[[offset(4)]] b : f32;
})");
- auto s = p->struct_decl("");
- ASSERT_FALSE(p->has_error());
- ASSERT_NE(s, nullptr);
- ASSERT_EQ(s->name(), "S");
- ASSERT_EQ(s->impl()->members().size(), 2u);
- EXPECT_EQ(s->impl()->members()[0]->name(), "a");
- EXPECT_EQ(s->impl()->members()[1]->name(), "b");
-}
-
-TEST_F(ParserImplTest, StructDecl_Parses_WithoutName) {
- auto* p = parser(R"(
-struct {
- a : i32;
- [[offset(4)]] b : f32;
-})");
- auto s = p->struct_decl("S");
+ auto s = p->struct_decl();
ASSERT_FALSE(p->has_error());
ASSERT_NE(s, nullptr);
ASSERT_EQ(s->name(), "S");
@@ -58,7 +43,7 @@
a : f32;
b : f32;
})");
- auto s = p->struct_decl("");
+ auto s = p->struct_decl();
ASSERT_FALSE(p->has_error());
ASSERT_NE(s, nullptr);
ASSERT_EQ(s->name(), "B");
@@ -76,7 +61,7 @@
a : f32;
b : f32;
})");
- auto s = p->struct_decl("");
+ auto s = p->struct_decl();
ASSERT_FALSE(p->has_error());
ASSERT_NE(s, nullptr);
ASSERT_EQ(s->name(), "S");
@@ -90,7 +75,7 @@
TEST_F(ParserImplTest, StructDecl_EmptyMembers) {
auto* p = parser("struct S {}");
- auto s = p->struct_decl("");
+ auto s = p->struct_decl();
ASSERT_FALSE(p->has_error());
ASSERT_NE(s, nullptr);
ASSERT_EQ(s->impl()->members().size(), 0u);
@@ -98,7 +83,7 @@
TEST_F(ParserImplTest, StructDecl_MissingIdent) {
auto* p = parser("struct {}");
- auto s = p->struct_decl("");
+ auto s = p->struct_decl();
ASSERT_TRUE(p->has_error());
ASSERT_EQ(s, nullptr);
EXPECT_EQ(p->error(), "1:8: missing identifier for struct declaration");
@@ -106,7 +91,7 @@
TEST_F(ParserImplTest, StructDecl_MissingBracketLeft) {
auto* p = parser("struct S }");
- auto s = p->struct_decl("");
+ auto s = p->struct_decl();
ASSERT_TRUE(p->has_error());
ASSERT_EQ(s, nullptr);
EXPECT_EQ(p->error(), "1:10: missing { for struct declaration");
@@ -114,7 +99,7 @@
TEST_F(ParserImplTest, StructDecl_InvalidStructBody) {
auto* p = parser("struct S { a : B; }");
- auto s = p->struct_decl("");
+ auto s = p->struct_decl();
ASSERT_TRUE(p->has_error());
ASSERT_EQ(s, nullptr);
EXPECT_EQ(p->error(), "1:16: unknown constructed type 'B'");
@@ -122,7 +107,7 @@
TEST_F(ParserImplTest, StructDecl_InvalidStructDecorationDecl) {
auto* p = parser("[[block struct S { a : i32; }");
- auto s = p->struct_decl("");
+ auto s = p->struct_decl();
ASSERT_TRUE(p->has_error());
ASSERT_EQ(s, nullptr);
EXPECT_EQ(p->error(), "1:9: missing ]] for struct decoration");
@@ -130,7 +115,7 @@
TEST_F(ParserImplTest, StructDecl_MissingStruct) {
auto* p = parser("[[block]] S {}");
- auto s = p->struct_decl("");
+ auto s = p->struct_decl();
ASSERT_TRUE(p->has_error());
ASSERT_EQ(s, nullptr);
EXPECT_EQ(p->error(), "1:11: missing struct declaration");
diff --git a/src/reader/wgsl/parser_impl_type_alias_test.cc b/src/reader/wgsl/parser_impl_type_alias_test.cc
index 4977b0b..72d5012 100644
--- a/src/reader/wgsl/parser_impl_type_alias_test.cc
+++ b/src/reader/wgsl/parser_impl_type_alias_test.cc
@@ -89,59 +89,6 @@
EXPECT_EQ(p->error(), "1:10: unknown constructed type 'B'");
}
-TEST_F(ParserImplTest, TypeDecl_ParsesStruct) {
- auto* p = parser("type a = struct { b: i32; c: f32;}");
- auto* t = p->type_alias();
- ASSERT_FALSE(p->has_error());
- ASSERT_NE(t, nullptr);
- ASSERT_TRUE(t->IsStruct());
- auto* str = t->AsStruct();
- EXPECT_EQ(str->name(), "a");
- EXPECT_EQ(str->impl()->members().size(), 2u);
-}
-
-TEST_F(ParserImplTest, TypeDecl_InvalidStruct) {
- auto* p = parser("type a = [[block]] {}");
- auto* t = p->type_alias();
- ASSERT_TRUE(p->has_error());
- ASSERT_EQ(t, nullptr);
- EXPECT_EQ(p->error(), "1:20: missing struct declaration");
-}
-
-TEST_F(ParserImplTest, TypeDecl_Struct_WithStride) {
- auto* p = parser(
- "type a = [[block]] struct { [[offset(0)]] data: [[stride(4)]] "
- "array<f32>; "
- "}");
- auto* t = p->type_alias();
- ASSERT_FALSE(p->has_error());
- ASSERT_NE(t, nullptr);
- ASSERT_TRUE(t->IsStruct());
- auto* str = t->AsStruct();
- EXPECT_EQ(str->name(), "a");
- EXPECT_EQ(str->impl()->members().size(), 1u);
-
- const auto* ty = str->impl()->members()[0]->type();
- ASSERT_TRUE(ty->IsArray());
- const auto* arr = ty->AsArray();
- EXPECT_TRUE(arr->has_array_stride());
- EXPECT_EQ(arr->array_stride(), 4u);
-}
-
-TEST_F(ParserImplTest, TypeDecl_Struct_Empty) {
- auto* p = parser("type str = struct {};");
- p->global_decl();
- ASSERT_FALSE(p->has_error()) << p->error();
-
- auto module = p->module();
- ASSERT_EQ(module.constructed_types().size(), 1u);
-
- ASSERT_TRUE(module.constructed_types()[0]->IsStruct());
- auto* str = module.constructed_types()[0]->AsStruct();
- EXPECT_EQ(str->name(), "str");
- EXPECT_EQ(str->impl()->members().size(), 0u);
-}
-
} // namespace
} // namespace wgsl
} // namespace reader
diff --git a/src/writer/hlsl/generator_impl.cc b/src/writer/hlsl/generator_impl.cc
index 166f2f7..70a1cb4 100644
--- a/src/writer/hlsl/generator_impl.cc
+++ b/src/writer/hlsl/generator_impl.cc
@@ -202,22 +202,22 @@
if (ty->IsAlias()) {
auto* alias = ty->AsAlias();
- // This will go away once type_alias doesn't accept anonymous
- // structs anymore
- if (alias->type()->IsStruct() &&
- alias->type()->AsStruct()->name() == alias->name()) {
- if (!EmitStructType(out, alias->type()->AsStruct())) {
+ // HLSL typedef is for intrinsic types only. For an alias'd struct,
+ // generate a secondary struct with the new name.
+ if (alias->type()->IsStruct()) {
+ if (!EmitStructType(out, alias->type()->AsStruct(), alias->name())) {
return false;
}
- } else {
- out << "typedef ";
- if (!EmitType(out, alias->type(), "")) {
- return false;
- }
- out << " " << namer_.NameFor(alias->name()) << ";" << std::endl;
+ return true;
}
+ out << "typedef ";
+ if (!EmitType(out, alias->type(), "")) {
+ return false;
+ }
+ out << " " << namer_.NameFor(alias->name()) << ";" << std::endl;
} else if (ty->IsStruct()) {
- if (!EmitStructType(out, ty->AsStruct())) {
+ auto* str = ty->AsStruct();
+ if (!EmitStructType(out, str, str->name())) {
return false;
}
} else {
@@ -1975,11 +1975,12 @@
}
bool GeneratorImpl::EmitStructType(std::ostream& out,
- const ast::type::StructType* str) {
+ const ast::type::StructType* str,
+ const std::string& name) {
// TODO(dsinclair): Block decoration?
// if (str->impl()->decoration() != ast::StructDecoration::kNone) {
// }
- out << "struct " << str->name() << " {" << std::endl;
+ out << "struct " << name << " {" << std::endl;
increment_indent();
for (const auto& mem : str->impl()->members()) {
diff --git a/src/writer/hlsl/generator_impl.h b/src/writer/hlsl/generator_impl.h
index 219a613..9f3529a 100644
--- a/src/writer/hlsl/generator_impl.h
+++ b/src/writer/hlsl/generator_impl.h
@@ -264,8 +264,11 @@
/// Handles generating a structure declaration
/// @param out the output stream
/// @param ty the struct to generate
+ /// @param name the struct name
/// @returns true if the struct is emitted
- bool EmitStructType(std::ostream& out, const ast::type::StructType* ty);
+ bool EmitStructType(std::ostream& out,
+ const ast::type::StructType* ty,
+ const std::string& name);
/// Handles a unary op expression
/// @param pre the preamble for the expression stream
/// @param out the output of the expression stream
diff --git a/src/writer/hlsl/generator_impl_alias_type_test.cc b/src/writer/hlsl/generator_impl_alias_type_test.cc
index aa4c4ad..e878b44 100644
--- a/src/writer/hlsl/generator_impl_alias_type_test.cc
+++ b/src/writer/hlsl/generator_impl_alias_type_test.cc
@@ -63,13 +63,13 @@
auto str = std::make_unique<ast::Struct>();
str->set_members(std::move(members));
- ast::type::StructType s("a", std::move(str));
- ast::type::AliasType alias("a", &s);
+ ast::type::StructType s("A", std::move(str));
+ ast::type::AliasType alias("B", &s);
ast::Module m;
GeneratorImpl g(&m);
ASSERT_TRUE(gen().EmitConstructedType(out(), &alias)) << gen().error();
- EXPECT_EQ(result(), R"(struct a {
+ EXPECT_EQ(result(), R"(struct B {
float a;
int b;
};
diff --git a/src/writer/hlsl/generator_impl_function_test.cc b/src/writer/hlsl/generator_impl_function_test.cc
index 2558917..a5cae3e 100644
--- a/src/writer/hlsl/generator_impl_function_test.cc
+++ b/src/writer/hlsl/generator_impl_function_test.cc
@@ -32,7 +32,6 @@
#include "src/ast/stage_decoration.h"
#include "src/ast/struct.h"
#include "src/ast/struct_member_offset_decoration.h"
-#include "src/ast/type/alias_type.h"
#include "src/ast/type/array_type.h"
#include "src/ast/type/f32_type.h"
#include "src/ast/type/i32_type.h"
@@ -310,13 +309,12 @@
str->set_members(std::move(members));
ast::type::StructType s("Uniforms", std::move(str));
- auto alias = std::make_unique<ast::type::AliasType>("Uniforms", &s);
auto coord_var =
std::make_unique<ast::DecoratedVariable>(std::make_unique<ast::Variable>(
- "uniforms", ast::StorageClass::kUniform, alias.get()));
+ "uniforms", ast::StorageClass::kUniform, &s));
- mod()->AddConstructedType(alias.get());
+ mod()->AddConstructedType(&s);
ast::VariableDecorationList decos;
decos.push_back(std::make_unique<ast::BindingDecoration>(0));
diff --git a/src/writer/hlsl/generator_impl_type_test.cc b/src/writer/hlsl/generator_impl_type_test.cc
index 716ed7c..62c7bae 100644
--- a/src/writer/hlsl/generator_impl_type_test.cc
+++ b/src/writer/hlsl/generator_impl_type_test.cc
@@ -181,7 +181,7 @@
ast::type::StructType s("S", std::move(str));
- ASSERT_TRUE(gen().EmitStructType(out(), &s)) << gen().error();
+ ASSERT_TRUE(gen().EmitStructType(out(), &s, "S")) << gen().error();
EXPECT_EQ(result(), R"(struct S {
int a;
float b;
@@ -263,7 +263,7 @@
ast::type::StructType s("S", std::move(str));
- ASSERT_TRUE(gen().EmitStructType(out(), &s)) << gen().error();
+ ASSERT_TRUE(gen().EmitStructType(out(), &s, "S")) << gen().error();
EXPECT_EQ(result(), R"(struct S {
int double_tint_0;
float float_tint_0;
@@ -293,8 +293,8 @@
ast::type::StructType s("S", std::move(str));
- ASSERT_TRUE(gen().EmitStructType(out(), &s)) << gen().error();
- EXPECT_EQ(result(), R"(struct S {
+ ASSERT_TRUE(gen().EmitStructType(out(), &s, "B")) << gen().error();
+ EXPECT_EQ(result(), R"(struct B {
int a;
float b;
})");
diff --git a/src/writer/msl/generator_impl.cc b/src/writer/msl/generator_impl.cc
index 0771d41..fd710de 100644
--- a/src/writer/msl/generator_impl.cc
+++ b/src/writer/msl/generator_impl.cc
@@ -245,19 +245,11 @@
if (ty->IsAlias()) {
auto* alias = ty->AsAlias();
- // This will go away once type_alias does not accept anonymous structs
- if (alias->type()->IsStruct() &&
- alias->type()->AsStruct()->name() == alias->name()) {
- if (!EmitStructType(alias->type()->AsStruct())) {
- return false;
- }
- } else {
- out_ << "typedef ";
- if (!EmitType(alias->type(), "")) {
- return false;
- }
- out_ << " " << namer_.NameFor(alias->name()) << ";" << std::endl;
+ out_ << "typedef ";
+ if (!EmitType(alias->type(), "")) {
+ return false;
}
+ out_ << " " << namer_.NameFor(alias->name()) << ";" << std::endl;
} else if (ty->IsStruct()) {
if (!EmitStructType(ty->AsStruct())) {
return false;
diff --git a/src/writer/msl/generator_impl_alias_type_test.cc b/src/writer/msl/generator_impl_alias_type_test.cc
index 0eec82f2..1003694 100644
--- a/src/writer/msl/generator_impl_alias_type_test.cc
+++ b/src/writer/msl/generator_impl_alias_type_test.cc
@@ -80,35 +80,6 @@
)");
}
-TEST_F(MslGeneratorImplTest, EmitConstructedType_AliasMatchStruct) {
- ast::type::I32Type i32;
- ast::type::F32Type f32;
-
- ast::StructMemberList members;
- members.push_back(std::make_unique<ast::StructMember>(
- "a", &f32, ast::StructMemberDecorationList{}));
-
- ast::StructMemberDecorationList b_deco;
- b_deco.push_back(std::make_unique<ast::StructMemberOffsetDecoration>(4));
- members.push_back(
- std::make_unique<ast::StructMember>("b", &i32, std::move(b_deco)));
-
- auto str = std::make_unique<ast::Struct>();
- str->set_members(std::move(members));
-
- ast::type::StructType s("a", std::move(str));
- ast::type::AliasType alias("a", &s);
-
- ast::Module m;
- GeneratorImpl g(&m);
- ASSERT_TRUE(g.EmitConstructedType(&alias)) << g.error();
- EXPECT_EQ(g.result(), R"(struct a {
- float a;
- int b;
-};
-)");
-}
-
TEST_F(MslGeneratorImplTest, EmitConstructedType_AliasStructIdent) {
ast::type::I32Type i32;
ast::type::F32Type f32;
diff --git a/src/writer/wgsl/generator_impl.cc b/src/writer/wgsl/generator_impl.cc
index f820017..a732b00 100644
--- a/src/writer/wgsl/generator_impl.cc
+++ b/src/writer/wgsl/generator_impl.cc
@@ -168,21 +168,11 @@
make_indent();
if (ty->IsAlias()) {
auto* alias = ty->AsAlias();
- // Emitting an alias to a struct where the names are the same means we can
- // skip emitting the alias and just emit the struct. This will go away once
- // the anonymous structs are removed from the type alias
- if (alias->type()->IsStruct() &&
- alias->type()->AsStruct()->name() == alias->name()) {
- if (!EmitStructType(alias->type()->AsStruct())) {
- return false;
- }
- } else {
- out_ << "type " << alias->name() << " = ";
- if (!EmitType(alias->type())) {
- return false;
- }
- out_ << ";" << std::endl;
+ out_ << "type " << alias->name() << " = ";
+ if (!EmitType(alias->type())) {
+ return false;
}
+ out_ << ";" << std::endl;
} else if (ty->IsStruct()) {
if (!EmitStructType(ty->AsStruct())) {
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 4b0d69a..c16d043 100644
--- a/src/writer/wgsl/generator_impl_alias_type_test.cc
+++ b/src/writer/wgsl/generator_impl_alias_type_test.cc
@@ -39,7 +39,7 @@
)");
}
-TEST_F(WgslGeneratorImplTest, EmitAliasType_Struct_Ident) {
+TEST_F(WgslGeneratorImplTest, EmitConstructedType_Struct) {
ast::type::I32Type i32;
ast::type::F32Type f32;
@@ -70,8 +70,7 @@
)");
}
-// This should go away once type_alias'd anonymous structs are removed.
-TEST_F(WgslGeneratorImplTest, EmitAliasType_Struct) {
+TEST_F(WgslGeneratorImplTest, EmitAliasType_ToStruct) {
ast::type::I32Type i32;
ast::type::F32Type f32;
@@ -88,15 +87,11 @@
str->set_members(std::move(members));
ast::type::StructType s("A", std::move(str));
- ast::type::AliasType alias("A", &s);
+ ast::type::AliasType alias("B", &s);
GeneratorImpl g;
ASSERT_TRUE(g.EmitConstructedType(&alias)) << g.error();
- EXPECT_EQ(g.result(), R"(struct A {
- a : f32;
- [[offset(4)]]
- b : i32;
-};
+ EXPECT_EQ(g.result(), R"(type B = A;
)");
}
