diff --git a/src/reader/wgsl/parser_impl.cc b/src/reader/wgsl/parser_impl.cc index 362219c77f..aa2de91da0 100644 --- a/src/reader/wgsl/parser_impl.cc +++ b/src/reader/wgsl/parser_impl.cc @@ -268,10 +268,19 @@ void ParserImpl::global_decl() { } // 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 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 @@ std::unique_ptr ParserImpl::global_constant_decl() { // 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 @@ ast::VariableDecorationList ParserImpl::variable_decoration_list() { 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 @@ ast::VariableDecorationList ParserImpl::variable_decoration_list() { 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 b4a1bbca29..a17042354a 100644 --- a/src/reader/wgsl/parser_impl.h +++ b/src/reader/wgsl/parser_impl.h @@ -129,9 +129,11 @@ class ParserImpl { /// Parses a `global_constant_decl` grammar element /// @returns the const object or nullptr std::unique_ptr 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 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 a7d4d4e363..80238ba539 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 @@ TEST_F(ParserImplTest, GlobalVariableDecl_WithDecoration) { ASSERT_TRUE(decos[1]->IsSet()); } +TEST_F(ParserImplTest, GlobalVariableDecl_WithDecoration_MulitpleGroups) { + auto* p = parser("[[binding(2)]] [[set(1)]] var 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 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 942abe9f3e..7ab0f830f9 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 @@ namespace { 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_Parses) { 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"); }