From b91d7e1b31be193bdeb3a240cd1ae9f09782b290 Mon Sep 17 00:00:00 2001 From: dan sinclair Date: Wed, 10 Aug 2022 18:54:43 +0000 Subject: [PATCH] Spit `expect_variable_ident_decl` into helper and calling methods. This Cl spits the `expect_variable_ident_decl` apart to only accept the `variable_ident_decl` grammar element. A `expect_ident_or_variable_ident_decl` is added for the ident optional version and a helper used by those two methods. This makes it a lot clearer at the caller side how the calls relate to the grammar. Bug: tint:1633 Change-Id: I50aa4852926ff217129fe728e434255ed008da61 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/98661 Reviewed-by: Antonio Maiorano Commit-Queue: Antonio Maiorano Auto-Submit: Dan Sinclair --- src/tint/reader/wgsl/parser_impl.cc | 44 ++++++++++++------- src/tint/reader/wgsl/parser_impl.h | 18 +++++--- .../parser_impl_variable_ident_decl_test.cc | 20 ++++----- 3 files changed, 50 insertions(+), 32 deletions(-) diff --git a/src/tint/reader/wgsl/parser_impl.cc b/src/tint/reader/wgsl/parser_impl.cc index 5b04e7f585..7f4e642086 100644 --- a/src/tint/reader/wgsl/parser_impl.cc +++ b/src/tint/reader/wgsl/parser_impl.cc @@ -615,7 +615,7 @@ Maybe ParserImpl::global_constant_decl(AttributeList& attr return Failure::kNoMatch; } - auto decl = expect_variable_ident_decl(use, /* allow_inferred = */ true); + auto decl = expect_ident_or_variable_ident_decl(use); if (decl.errored) { return Failure::kErrored; } @@ -666,7 +666,7 @@ Maybe ParserImpl::global_constant_decl(AttributeList& attr } // variable_decl -// : VAR variable_qualifier? variable_ident_decl +// : VAR variable_qualifier? (ident | variable_ident_decl) Maybe ParserImpl::variable_decl() { Source source; if (!match(Token::Type::kVar, &source)) { @@ -682,8 +682,7 @@ Maybe ParserImpl::variable_decl() { vq = explicit_vq.value; } - auto decl = expect_variable_ident_decl("variable declaration", - /*allow_inferred = */ true); + auto decl = expect_ident_or_variable_ident_decl("variable declaration"); if (decl.errored) { return Failure::kErrored; } @@ -919,10 +918,9 @@ Expect ParserImpl::expect_texel_format(std::string_view use) { return fmt; } -// variable_ident_decl -// : IDENT COLON type_decl -Expect ParserImpl::expect_variable_ident_decl(std::string_view use, - bool allow_inferred) { +Expect ParserImpl::expect_ident_or_variable_ident_decl_impl( + std::string_view use, + bool allow_inferred) { auto ident = expect_ident(use); if (ident.errored) { return Failure::kErrored; @@ -948,6 +946,18 @@ Expect ParserImpl::expect_variable_ident_decl(std:: return TypedIdentifier{type.value, ident.value, ident.source}; } +// (ident | variable_ident_decl) +Expect ParserImpl::expect_ident_or_variable_ident_decl( + std::string_view use) { + return expect_ident_or_variable_ident_decl_impl(use, true); +} + +// variable_ident_decl +// : IDENT COLON type_decl +Expect ParserImpl::expect_variable_ident_decl(std::string_view use) { + return expect_ident_or_variable_ident_decl_impl(use, false); +} + // access_mode // : 'read' // | 'write' @@ -1353,8 +1363,7 @@ Expect ParserImpl::expect_struct_member() { return Failure::kErrored; } - auto decl = expect_variable_ident_decl("struct member", - /*allow_inferred = */ false); + auto decl = expect_variable_ident_decl("struct member"); if (decl.errored) { return Failure::kErrored; } @@ -1514,8 +1523,7 @@ Expect ParserImpl::expect_param_list() { Expect ParserImpl::expect_param() { auto attrs = attribute_list(); - auto decl = expect_variable_ident_decl("parameter", - /*allow_inferred = */ false); + auto decl = expect_variable_ident_decl("parameter"); if (decl.errored) { return Failure::kErrored; } @@ -1855,11 +1863,11 @@ Maybe ParserImpl::return_statement() { // variable_statement // : variable_decl // | variable_decl EQUAL expression -// | CONST variable_ident_decl EQUAL expression +// | LET (ident | variable_ident_decl) EQUAL expression +// | CONST (ident | variable_ident_decl) EQUAL expression Maybe ParserImpl::variable_statement() { if (match(Token::Type::kConst)) { - auto decl = expect_variable_ident_decl("'const' declaration", - /*allow_inferred = */ true); + auto decl = expect_ident_or_variable_ident_decl("'const' declaration"); if (decl.errored) { return Failure::kErrored; } @@ -1886,8 +1894,7 @@ Maybe ParserImpl::variable_statement() { } if (match(Token::Type::kLet)) { - auto decl = expect_variable_ident_decl("'let' declaration", - /*allow_inferred = */ true); + auto decl = expect_ident_or_variable_ident_decl("'let' declaration"); if (decl.errored) { return Failure::kErrored; } @@ -3172,6 +3179,9 @@ Maybe ParserImpl::assignment_statement() { // const_literal // : INT_LITERAL // | FLOAT_LITERAL +// | bool_literal +// +// bool_literal: // | TRUE // | FALSE Maybe ParserImpl::const_literal() { diff --git a/src/tint/reader/wgsl/parser_impl.h b/src/tint/reader/wgsl/parser_impl.h index ceb0af826c..14f86d5ab6 100644 --- a/src/tint/reader/wgsl/parser_impl.h +++ b/src/tint/reader/wgsl/parser_impl.h @@ -410,13 +410,21 @@ class ParserImpl { /// Parses a `variable_decl` grammar element /// @returns the parsed variable declaration info Maybe variable_decl(); - /// Parses a `variable_ident_decl` grammar element, erroring on parse - /// failure. + /// Helper for parsing ident or variable_ident_decl. Should not be called directly, + /// use the specific version below. + /// @param use a description of what was being parsed if an error was raised. + /// @param allow_inferred allow the identifier to be parsed without a type + /// @returns the parsed identifier, and possibly type, or empty otherwise + Expect expect_ident_or_variable_ident_decl_impl(std::string_view use, + bool allow_inferred); + /// Parses a `ident` or a `variable_ident_decl` grammar element, erroring on parse failure. + /// @param use a description of what was being parsed if an error was raised. + /// @returns the identifier or empty otherwise. + Expect expect_ident_or_variable_ident_decl(std::string_view use); + /// Parses a `variable_ident_decl` grammar element, erroring on parse failure. /// @param use a description of what was being parsed if an error was raised. - /// @param allow_inferred if true, do not fail if variable decl does not - /// specify type /// @returns the identifier and type parsed or empty otherwise - Expect expect_variable_ident_decl(std::string_view use, bool allow_inferred); + Expect expect_variable_ident_decl(std::string_view use); /// Parses a `variable_qualifier` grammar element /// @returns the variable qualifier information Maybe variable_qualifier(); diff --git a/src/tint/reader/wgsl/parser_impl_variable_ident_decl_test.cc b/src/tint/reader/wgsl/parser_impl_variable_ident_decl_test.cc index a811a82054..267998320f 100644 --- a/src/tint/reader/wgsl/parser_impl_variable_ident_decl_test.cc +++ b/src/tint/reader/wgsl/parser_impl_variable_ident_decl_test.cc @@ -19,7 +19,7 @@ namespace { TEST_F(ParserImplTest, VariableIdentDecl_Parses) { auto p = parser("my_var : f32"); - auto decl = p->expect_variable_ident_decl("test", /*allow_inferred = */ false); + auto decl = p->expect_variable_ident_decl("test"); ASSERT_FALSE(p->has_error()) << p->error(); ASSERT_FALSE(decl.errored); ASSERT_EQ(decl->name, "my_var"); @@ -32,7 +32,7 @@ TEST_F(ParserImplTest, VariableIdentDecl_Parses) { TEST_F(ParserImplTest, VariableIdentDecl_Parses_AllowInferredType) { auto p = parser("my_var : f32"); - auto decl = p->expect_variable_ident_decl("test", /*allow_inferred = */ true); + auto decl = p->expect_ident_or_variable_ident_decl("test"); ASSERT_FALSE(p->has_error()) << p->error(); ASSERT_FALSE(decl.errored); ASSERT_EQ(decl->name, "my_var"); @@ -45,14 +45,14 @@ TEST_F(ParserImplTest, VariableIdentDecl_Parses_AllowInferredType) { TEST_F(ParserImplTest, VariableIdentDecl_Inferred_Parse_Failure) { auto p = parser("my_var = 1.0"); - auto decl = p->expect_variable_ident_decl("test", /*allow_inferred = */ false); + auto decl = p->expect_variable_ident_decl("test"); ASSERT_TRUE(p->has_error()); ASSERT_EQ(p->error(), "1:8: expected ':' for test"); } TEST_F(ParserImplTest, VariableIdentDecl_Inferred_Parses_AllowInferredType) { auto p = parser("my_var = 1.0"); - auto decl = p->expect_variable_ident_decl("test", /*allow_inferred = */ true); + auto decl = p->expect_ident_or_variable_ident_decl("test"); ASSERT_FALSE(p->has_error()) << p->error(); ASSERT_FALSE(decl.errored); ASSERT_EQ(decl->name, "my_var"); @@ -63,7 +63,7 @@ TEST_F(ParserImplTest, VariableIdentDecl_Inferred_Parses_AllowInferredType) { TEST_F(ParserImplTest, VariableIdentDecl_MissingIdent) { auto p = parser(": f32"); - auto decl = p->expect_variable_ident_decl("test", /*allow_inferred = */ false); + auto decl = p->expect_variable_ident_decl("test"); ASSERT_TRUE(p->has_error()); ASSERT_TRUE(decl.errored); ASSERT_EQ(p->error(), "1:1: expected identifier for test"); @@ -71,7 +71,7 @@ TEST_F(ParserImplTest, VariableIdentDecl_MissingIdent) { TEST_F(ParserImplTest, VariableIdentDecl_MissingIdent_AllowInferredType) { auto p = parser(": f32"); - auto decl = p->expect_variable_ident_decl("test", /*allow_inferred = */ true); + auto decl = p->expect_ident_or_variable_ident_decl("test"); ASSERT_TRUE(p->has_error()); ASSERT_TRUE(decl.errored); ASSERT_EQ(p->error(), "1:1: expected identifier for test"); @@ -79,7 +79,7 @@ TEST_F(ParserImplTest, VariableIdentDecl_MissingIdent_AllowInferredType) { TEST_F(ParserImplTest, VariableIdentDecl_MissingType) { auto p = parser("my_var :"); - auto decl = p->expect_variable_ident_decl("test", /*allow_inferred = */ false); + auto decl = p->expect_variable_ident_decl("test"); ASSERT_TRUE(p->has_error()); ASSERT_TRUE(decl.errored); ASSERT_EQ(p->error(), "1:9: invalid type for test"); @@ -87,7 +87,7 @@ TEST_F(ParserImplTest, VariableIdentDecl_MissingType) { TEST_F(ParserImplTest, VariableIdentDecl_MissingType_AllowInferredType) { auto p = parser("my_var :"); - auto decl = p->expect_variable_ident_decl("test", /*allow_inferred = */ true); + auto decl = p->expect_ident_or_variable_ident_decl("test"); ASSERT_TRUE(p->has_error()); ASSERT_TRUE(decl.errored); ASSERT_EQ(p->error(), "1:9: invalid type for test"); @@ -95,7 +95,7 @@ TEST_F(ParserImplTest, VariableIdentDecl_MissingType_AllowInferredType) { TEST_F(ParserImplTest, VariableIdentDecl_InvalidIdent) { auto p = parser("123 : f32"); - auto decl = p->expect_variable_ident_decl("test", /*allow_inferred = */ false); + auto decl = p->expect_variable_ident_decl("test"); ASSERT_TRUE(p->has_error()); ASSERT_TRUE(decl.errored); ASSERT_EQ(p->error(), "1:1: expected identifier for test"); @@ -103,7 +103,7 @@ TEST_F(ParserImplTest, VariableIdentDecl_InvalidIdent) { TEST_F(ParserImplTest, VariableIdentDecl_InvalidIdent_AllowInferredType) { auto p = parser("123 : f32"); - auto decl = p->expect_variable_ident_decl("test", /*allow_inferred = */ true); + auto decl = p->expect_ident_or_variable_ident_decl("test"); ASSERT_TRUE(p->has_error()); ASSERT_TRUE(decl.errored); ASSERT_EQ(p->error(), "1:1: expected identifier for test");