[wgsl-reader] Allow global variable decorations to have multiple blocks.
This CL updates the WGSL parser to allow global variable decorations
to accept multiple blocks.
Bug: tint:240
Change-Id: Iaf8f794d285d87af6e2ff8c93e34d88331eedccb
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/29840
Commit-Queue: dan sinclair <dsinclair@chromium.org>
Reviewed-by: Sarah Mashayekhi <sarahmashay@google.com>
diff --git a/src/reader/wgsl/parser_impl.cc b/src/reader/wgsl/parser_impl.cc
index 362219c..aa2de91 100644
--- a/src/reader/wgsl/parser_impl.cc
+++ b/src/reader/wgsl/parser_impl.cc
@@ -268,10 +268,19 @@
}
// global_variable_decl
-// : variable_decoration_list variable_decl
-// | variable_decoration_list variable_decl EQUAL const_expr
+// : variable_decoration_list* variable_decl
+// | variable_decoration_list* variable_decl EQUAL const_expr
std::unique_ptr<ast::Variable> ParserImpl::global_variable_decl() {
- auto decos = variable_decoration_list();
+ ast::VariableDecorationList decos;
+ for (;;) {
+ auto s = decos.size();
+ if (!variable_decoration_list(decos)) {
+ return nullptr;
+ }
+ if (s == decos.size()) {
+ break;
+ }
+ }
if (has_error())
return nullptr;
@@ -353,33 +362,33 @@
// variable_decoration_list
// : ATTR_LEFT (variable_decoration COMMA)* variable_decoration ATTR_RIGHT
-ast::VariableDecorationList ParserImpl::variable_decoration_list() {
- ast::VariableDecorationList decos;
-
+bool ParserImpl::variable_decoration_list(ast::VariableDecorationList& decos) {
auto t = peek();
- if (!t.IsAttrLeft())
- return decos;
+ if (!t.IsAttrLeft()) {
+ return true;
+ }
// Check the empty list before verifying the contents
t = peek(1);
if (t.IsAttrRight()) {
set_error(t, "empty variable decoration list");
- return {};
+ return false;
}
// Make sure we're looking at variable decorations not some other kind
if (!IsVariableDecoration(peek(1))) {
- return decos;
+ return true;
}
next(); // consume the peek
auto deco = variable_decoration();
- if (has_error())
- return {};
+ if (has_error()) {
+ return false;
+ }
if (deco == nullptr) {
set_error(peek(), "missing variable decoration for decoration list");
- return {};
+ return false;
}
for (;;) {
decos.push_back(std::move(deco));
@@ -391,11 +400,12 @@
next(); // consume the peek
deco = variable_decoration();
- if (has_error())
- return {};
+ if (has_error()) {
+ return false;
+ }
if (deco == nullptr) {
set_error(peek(), "missing variable decoration after comma");
- return {};
+ return false;
}
}
@@ -404,14 +414,14 @@
deco = variable_decoration();
if (deco != nullptr) {
set_error(t, "missing comma in variable decoration list");
- return {};
+ return false;
}
set_error(t, "missing ]] for variable decoration");
- return {};
+ return false;
}
next(); // consume the peek
- return decos;
+ return true;
}
// variable_decoration
diff --git a/src/reader/wgsl/parser_impl.h b/src/reader/wgsl/parser_impl.h
index b4a1bbc..a170423 100644
--- a/src/reader/wgsl/parser_impl.h
+++ b/src/reader/wgsl/parser_impl.h
@@ -129,9 +129,11 @@
/// Parses a `global_constant_decl` grammar element
/// @returns the const object or nullptr
std::unique_ptr<ast::Variable> global_constant_decl();
- /// Parses a `variable_decoration_list` grammar element
- /// @returns the parsed variable decorations
- ast::VariableDecorationList variable_decoration_list();
+ /// Parses a `variable_decoration_list` 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 variable_decoration_list(ast::VariableDecorationList& decos);
/// Parses a `variable_decoration` grammar element
/// @returns the variable decoration or nullptr if an error is encountered
std::unique_ptr<ast::VariableDecoration> variable_decoration();
diff --git a/src/reader/wgsl/parser_impl_global_variable_decl_test.cc b/src/reader/wgsl/parser_impl_global_variable_decl_test.cc
index a7d4d4e..80238ba 100644
--- a/src/reader/wgsl/parser_impl_global_variable_decl_test.cc
+++ b/src/reader/wgsl/parser_impl_global_variable_decl_test.cc
@@ -77,6 +77,29 @@
ASSERT_TRUE(decos[1]->IsSet());
}
+TEST_F(ParserImplTest, GlobalVariableDecl_WithDecoration_MulitpleGroups) {
+ auto* p = parser("[[binding(2)]] [[set(1)]] var<out> a : f32");
+ auto e = p->global_variable_decl();
+ ASSERT_FALSE(p->has_error()) << p->error();
+ ASSERT_NE(e, nullptr);
+ ASSERT_TRUE(e->IsDecorated());
+
+ EXPECT_EQ(e->name(), "a");
+ ASSERT_NE(e->type(), nullptr);
+ EXPECT_TRUE(e->type()->IsF32());
+ EXPECT_EQ(e->storage_class(), ast::StorageClass::kOutput);
+
+ ASSERT_EQ(e->constructor(), nullptr);
+
+ ASSERT_TRUE(e->IsDecorated());
+ auto* v = e->AsDecorated();
+
+ auto& decos = v->decorations();
+ ASSERT_EQ(decos.size(), 2u);
+ ASSERT_TRUE(decos[0]->IsBinding());
+ ASSERT_TRUE(decos[1]->IsSet());
+}
+
TEST_F(ParserImplTest, GlobalVariableDecl_InvalidDecoration) {
auto* p = parser("[[binding()]] var<out> a : f32");
auto e = p->global_variable_decl();
diff --git a/src/reader/wgsl/parser_impl_variable_decoration_list_test.cc b/src/reader/wgsl/parser_impl_variable_decoration_list_test.cc
index 942abe9..7ab0f83 100644
--- a/src/reader/wgsl/parser_impl_variable_decoration_list_test.cc
+++ b/src/reader/wgsl/parser_impl_variable_decoration_list_test.cc
@@ -25,7 +25,8 @@
TEST_F(ParserImplTest, VariableDecorationList_Parses) {
auto* p = parser(R"([[location(4), builtin(position)]])");
- auto decos = p->variable_decoration_list();
+ ast::VariableDecorationList decos;
+ EXPECT_TRUE(p->variable_decoration_list(decos));
ASSERT_FALSE(p->has_error()) << p->error();
ASSERT_EQ(decos.size(), 2u);
ASSERT_TRUE(decos[0]->IsLocation());
@@ -36,42 +37,48 @@
TEST_F(ParserImplTest, VariableDecorationList_Empty) {
auto* p = parser(R"([[]])");
- auto decos = p->variable_decoration_list();
+ ast::VariableDecorationList decos;
+ EXPECT_FALSE(p->variable_decoration_list(decos));
ASSERT_TRUE(p->has_error());
ASSERT_EQ(p->error(), "1:3: empty variable decoration list");
}
TEST_F(ParserImplTest, VariableDecorationList_Invalid) {
auto* p = parser(R"([[invalid]])");
- auto decos = p->variable_decoration_list();
+ ast::VariableDecorationList decos;
+ EXPECT_TRUE(p->variable_decoration_list(decos));
ASSERT_FALSE(p->has_error());
ASSERT_TRUE(decos.empty());
}
TEST_F(ParserImplTest, VariableDecorationList_ExtraComma) {
auto* p = parser(R"([[builtin(position), ]])");
- auto decos = p->variable_decoration_list();
+ ast::VariableDecorationList decos;
+ EXPECT_FALSE(p->variable_decoration_list(decos));
ASSERT_TRUE(p->has_error());
ASSERT_EQ(p->error(), "1:22: missing variable decoration after comma");
}
TEST_F(ParserImplTest, VariableDecorationList_MissingComma) {
auto* p = parser(R"([[binding(4) location(5)]])");
- auto decos = p->variable_decoration_list();
+ ast::VariableDecorationList decos;
+ EXPECT_FALSE(p->variable_decoration_list(decos));
ASSERT_TRUE(p->has_error());
ASSERT_EQ(p->error(), "1:14: missing comma in variable decoration list");
}
TEST_F(ParserImplTest, VariableDecorationList_BadDecoration) {
auto* p = parser(R"([[location(bad)]])");
- auto decos = p->variable_decoration_list();
+ ast::VariableDecorationList decos;
+ EXPECT_FALSE(p->variable_decoration_list(decos));
ASSERT_TRUE(p->has_error());
ASSERT_EQ(p->error(), "1:12: invalid value for location decoration");
}
TEST_F(ParserImplTest, VariableDecorationList_InvalidBuiltin) {
auto* p = parser("[[builtin(invalid)]]");
- auto decos = p->variable_decoration_list();
+ ast::VariableDecorationList decos;
+ EXPECT_FALSE(p->variable_decoration_list(decos));
ASSERT_TRUE(p->has_error());
ASSERT_EQ(p->error(), "1:11: invalid value for builtin decoration");
}
