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 <amaiorano@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
Auto-Submit: Dan Sinclair <dsinclair@chromium.org>
This commit is contained in:
dan sinclair 2022-08-10 18:54:43 +00:00 committed by Dawn LUCI CQ
parent 7058a17bda
commit b91d7e1b31
3 changed files with 50 additions and 32 deletions

View File

@ -615,7 +615,7 @@ Maybe<const ast::Variable*> 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<const ast::Variable*> ParserImpl::global_constant_decl(AttributeList& attr
}
// variable_decl
// : VAR variable_qualifier? variable_ident_decl
// : VAR variable_qualifier? (ident | variable_ident_decl)
Maybe<ParserImpl::VarDeclInfo> ParserImpl::variable_decl() {
Source source;
if (!match(Token::Type::kVar, &source)) {
@ -682,8 +682,7 @@ Maybe<ParserImpl::VarDeclInfo> 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<ast::TexelFormat> ParserImpl::expect_texel_format(std::string_view use) {
return fmt;
}
// variable_ident_decl
// : IDENT COLON type_decl
Expect<ParserImpl::TypedIdentifier> ParserImpl::expect_variable_ident_decl(std::string_view use,
bool allow_inferred) {
Expect<ParserImpl::TypedIdentifier> 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::TypedIdentifier> ParserImpl::expect_variable_ident_decl(std::
return TypedIdentifier{type.value, ident.value, ident.source};
}
// (ident | variable_ident_decl)
Expect<ParserImpl::TypedIdentifier> 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::TypedIdentifier> 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<ast::StructMember*> 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::ParameterList> ParserImpl::expect_param_list() {
Expect<ast::Parameter*> 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<const ast::ReturnStatement*> 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<const ast::VariableDeclStatement*> 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<const ast::VariableDeclStatement*> 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<const ast::Statement*> ParserImpl::assignment_statement() {
// const_literal
// : INT_LITERAL
// | FLOAT_LITERAL
// | bool_literal
//
// bool_literal:
// | TRUE
// | FALSE
Maybe<const ast::LiteralExpression*> ParserImpl::const_literal() {

View File

@ -410,13 +410,21 @@ class ParserImpl {
/// Parses a `variable_decl` grammar element
/// @returns the parsed variable declaration info
Maybe<VarDeclInfo> 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<TypedIdentifier> 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<TypedIdentifier> 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<TypedIdentifier> expect_variable_ident_decl(std::string_view use, bool allow_inferred);
Expect<TypedIdentifier> expect_variable_ident_decl(std::string_view use);
/// Parses a `variable_qualifier` grammar element
/// @returns the variable qualifier information
Maybe<VariableQualifier> variable_qualifier();

View File

@ -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");