reader/wgsl: match spec rules for function calls
Function calls should be parsed in `primary_expression`. Renames the
old `postfix_expression` to `singular_expression`, with the recursive
part now becoming `postfix_expression`.
Fixed: tint:170
Change-Id: I1afad794880916db38a25d73447cdaf84abd6584
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/49464
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Auto-Submit: James Price <jrprice@google.com>
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index 658f7dd..dbb4681 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -675,12 +675,12 @@
reader/wgsl/parser_impl_param_list_test.cc
reader/wgsl/parser_impl_paren_rhs_stmt_test.cc
reader/wgsl/parser_impl_pipeline_stage_test.cc
- reader/wgsl/parser_impl_postfix_expression_test.cc
reader/wgsl/parser_impl_primary_expression_test.cc
reader/wgsl/parser_impl_relational_expression_test.cc
reader/wgsl/parser_impl_sampled_texture_type_test.cc
reader/wgsl/parser_impl_sampler_type_test.cc
reader/wgsl/parser_impl_shift_expression_test.cc
+ reader/wgsl/parser_impl_singular_expression_test.cc
reader/wgsl/parser_impl_statement_test.cc
reader/wgsl/parser_impl_statements_test.cc
reader/wgsl/parser_impl_storage_class_test.cc
diff --git a/src/reader/wgsl/parser_impl.cc b/src/reader/wgsl/parser_impl.cc
index f643a21..7bbad45 100644
--- a/src/reader/wgsl/parser_impl.cc
+++ b/src/reader/wgsl/parser_impl.cc
@@ -2196,7 +2196,7 @@
}
// primary_expression
-// : IDENT
+// : IDENT argument_expression_list?
// | type_decl PAREN_LEFT argument_expression_list* PAREN_RIGHT
// | const_literal
// | paren_rhs_stmt
@@ -2235,8 +2235,30 @@
if (t.IsIdentifier() && !get_constructed(t.to_str())) {
next();
- return create<ast::IdentifierExpression>(
+
+ auto* ident = create<ast::IdentifierExpression>(
t.source(), builder_.Symbols().Register(t.to_str()));
+
+ if (match(Token::Type::kParenLeft, &source)) {
+ return sync(Token::Type::kParenRight, [&]() -> Maybe<ast::Expression*> {
+ ast::ExpressionList params;
+
+ auto t2 = peek();
+ if (!t2.IsParenRight() && !t2.IsEof()) {
+ auto list = expect_argument_expression_list();
+ if (list.errored)
+ return Failure::kErrored;
+ params = list.value;
+ }
+
+ if (!expect("call expression", Token::Type::kParenRight))
+ return Failure::kErrored;
+
+ return create<ast::CallExpression>(source, ident, params);
+ });
+ }
+
+ return ident;
}
auto type = type_decl();
@@ -2267,12 +2289,12 @@
return Failure::kNoMatch;
}
-// postfix_expr
+// postfix_expression
// :
// | BRACE_LEFT logical_or_expression BRACE_RIGHT postfix_expr
-// | PAREN_LEFT argument_expression_list* PAREN_RIGHT postfix_expr
// | PERIOD IDENTIFIER postfix_expr
-Maybe<ast::Expression*> ParserImpl::postfix_expr(ast::Expression* prefix) {
+Maybe<ast::Expression*> ParserImpl::postfix_expression(
+ ast::Expression* prefix) {
Source source;
if (match(Token::Type::kBracketLeft, &source)) {
return sync(Token::Type::kBracketRight, [&]() -> Maybe<ast::Expression*> {
@@ -2285,36 +2307,17 @@
if (!expect("array accessor", Token::Type::kBracketRight))
return Failure::kErrored;
- return postfix_expr(
+ return postfix_expression(
create<ast::ArrayAccessorExpression>(source, prefix, param.value));
});
}
- if (match(Token::Type::kParenLeft, &source)) {
- return sync(Token::Type::kParenRight, [&]() -> Maybe<ast::Expression*> {
- ast::ExpressionList params;
-
- auto t = peek();
- if (!t.IsParenRight() && !t.IsEof()) {
- auto list = expect_argument_expression_list();
- if (list.errored)
- return Failure::kErrored;
- params = list.value;
- }
-
- if (!expect("call expression", Token::Type::kParenRight))
- return Failure::kErrored;
-
- return postfix_expr(create<ast::CallExpression>(source, prefix, params));
- });
- }
-
if (match(Token::Type::kPeriod)) {
auto ident = expect_ident("member accessor");
if (ident.errored)
return Failure::kErrored;
- return postfix_expr(create<ast::MemberAccessorExpression>(
+ return postfix_expression(create<ast::MemberAccessorExpression>(
ident.source, prefix,
create<ast::IdentifierExpression>(
ident.source, builder_.Symbols().Register(ident.value))));
@@ -2323,16 +2326,16 @@
return prefix;
}
-// postfix_expression
+// singular_expression
// : primary_expression postfix_expr
-Maybe<ast::Expression*> ParserImpl::postfix_expression() {
+Maybe<ast::Expression*> ParserImpl::singular_expression() {
auto prefix = primary_expression();
if (prefix.errored)
return Failure::kErrored;
if (!prefix.matched)
return Failure::kNoMatch;
- return postfix_expr(prefix.value);
+ return postfix_expression(prefix.value);
}
// argument_expression_list
@@ -2361,7 +2364,7 @@
}
// unary_expression
-// : postfix_expression
+// : singular_expression
// | MINUS unary_expression
// | BANG unary_expression
Maybe<ast::Expression*> ParserImpl::unary_expression() {
@@ -2385,7 +2388,7 @@
return create<ast::UnaryOpExpression>(source, op, expr.value);
}
- return postfix_expression();
+ return singular_expression();
}
// multiplicative_expr
diff --git a/src/reader/wgsl/parser_impl.h b/src/reader/wgsl/parser_impl.h
index e01cf25..49a2d26 100644
--- a/src/reader/wgsl/parser_impl.h
+++ b/src/reader/wgsl/parser_impl.h
@@ -557,10 +557,10 @@
/// Parses the recursive portion of the postfix_expression
/// @param prefix the left side of the expression
/// @returns the parsed expression or nullptr
- Maybe<ast::Expression*> postfix_expr(ast::Expression* prefix);
- /// Parses a `postfix_expression` grammar elment
+ Maybe<ast::Expression*> postfix_expression(ast::Expression* prefix);
+ /// Parses a `singular_expression` grammar elment
/// @returns the parsed expression or nullptr
- Maybe<ast::Expression*> postfix_expression();
+ Maybe<ast::Expression*> singular_expression();
/// Parses a `unary_expression` grammar element
/// @returns the parsed expression or nullptr
Maybe<ast::Expression*> unary_expression();
diff --git a/src/reader/wgsl/parser_impl_postfix_expression_test.cc b/src/reader/wgsl/parser_impl_singular_expression_test.cc
similarity index 81%
rename from src/reader/wgsl/parser_impl_postfix_expression_test.cc
rename to src/reader/wgsl/parser_impl_singular_expression_test.cc
index 0b6eafa..30e2067 100644
--- a/src/reader/wgsl/parser_impl_postfix_expression_test.cc
+++ b/src/reader/wgsl/parser_impl_singular_expression_test.cc
@@ -19,9 +19,9 @@
namespace wgsl {
namespace {
-TEST_F(ParserImplTest, PostfixExpression_Array_ConstantIndex) {
+TEST_F(ParserImplTest, SingularExpression_Array_ConstantIndex) {
auto p = parser("a[1]");
- auto e = p->postfix_expression();
+ auto e = p->singular_expression();
EXPECT_TRUE(e.matched);
EXPECT_FALSE(e.errored);
EXPECT_FALSE(p->has_error()) << p->error();
@@ -41,9 +41,9 @@
EXPECT_EQ(c->literal()->As<ast::SintLiteral>()->value(), 1);
}
-TEST_F(ParserImplTest, PostfixExpression_Array_ExpressionIndex) {
+TEST_F(ParserImplTest, SingularExpression_Array_ExpressionIndex) {
auto p = parser("a[1 + b / 4]");
- auto e = p->postfix_expression();
+ auto e = p->singular_expression();
EXPECT_TRUE(e.matched);
EXPECT_FALSE(e.errored);
EXPECT_FALSE(p->has_error()) << p->error();
@@ -59,9 +59,9 @@
ASSERT_TRUE(ary->idx_expr()->Is<ast::BinaryExpression>());
}
-TEST_F(ParserImplTest, PostfixExpression_Array_MissingIndex) {
+TEST_F(ParserImplTest, SingularExpression_Array_MissingIndex) {
auto p = parser("a[]");
- auto e = p->postfix_expression();
+ auto e = p->singular_expression();
EXPECT_FALSE(e.matched);
EXPECT_TRUE(e.errored);
EXPECT_EQ(e.value, nullptr);
@@ -69,9 +69,9 @@
EXPECT_EQ(p->error(), "1:3: unable to parse expression inside []");
}
-TEST_F(ParserImplTest, PostfixExpression_Array_MissingRightBrace) {
+TEST_F(ParserImplTest, SingularExpression_Array_MissingRightBrace) {
auto p = parser("a[1");
- auto e = p->postfix_expression();
+ auto e = p->singular_expression();
EXPECT_FALSE(e.matched);
EXPECT_TRUE(e.errored);
EXPECT_EQ(e.value, nullptr);
@@ -79,9 +79,9 @@
EXPECT_EQ(p->error(), "1:4: expected ']' for array accessor");
}
-TEST_F(ParserImplTest, PostfixExpression_Array_InvalidIndex) {
+TEST_F(ParserImplTest, SingularExpression_Array_InvalidIndex) {
auto p = parser("a[if(a() {})]");
- auto e = p->postfix_expression();
+ auto e = p->singular_expression();
EXPECT_FALSE(e.matched);
EXPECT_TRUE(e.errored);
EXPECT_EQ(e.value, nullptr);
@@ -89,9 +89,9 @@
EXPECT_EQ(p->error(), "1:3: unable to parse expression inside []");
}
-TEST_F(ParserImplTest, PostfixExpression_Call_Empty) {
+TEST_F(ParserImplTest, SingularExpression_Call_Empty) {
auto p = parser("a()");
- auto e = p->postfix_expression();
+ auto e = p->singular_expression();
EXPECT_TRUE(e.matched);
EXPECT_FALSE(e.errored);
EXPECT_FALSE(p->has_error()) << p->error();
@@ -107,9 +107,9 @@
EXPECT_EQ(c->params().size(), 0u);
}
-TEST_F(ParserImplTest, PostfixExpression_Call_WithArgs) {
+TEST_F(ParserImplTest, SingularExpression_Call_WithArgs) {
auto p = parser("test(1, b, 2 + 3 / b)");
- auto e = p->postfix_expression();
+ auto e = p->singular_expression();
EXPECT_TRUE(e.matched);
EXPECT_FALSE(e.errored);
EXPECT_FALSE(p->has_error()) << p->error();
@@ -128,9 +128,9 @@
EXPECT_TRUE(c->params()[2]->Is<ast::BinaryExpression>());
}
-TEST_F(ParserImplTest, PostfixExpression_Call_InvalidArg) {
+TEST_F(ParserImplTest, SingularExpression_Call_InvalidArg) {
auto p = parser("a(if(a) {})");
- auto e = p->postfix_expression();
+ auto e = p->singular_expression();
EXPECT_FALSE(e.matched);
EXPECT_TRUE(e.errored);
EXPECT_EQ(e.value, nullptr);
@@ -138,9 +138,9 @@
EXPECT_EQ(p->error(), "1:3: unable to parse argument expression");
}
-TEST_F(ParserImplTest, PostfixExpression_Call_HangingComma) {
+TEST_F(ParserImplTest, SingularExpression_Call_HangingComma) {
auto p = parser("a(b, )");
- auto e = p->postfix_expression();
+ auto e = p->singular_expression();
EXPECT_FALSE(e.matched);
EXPECT_TRUE(e.errored);
EXPECT_EQ(e.value, nullptr);
@@ -148,9 +148,9 @@
EXPECT_EQ(p->error(), "1:6: unable to parse argument expression after comma");
}
-TEST_F(ParserImplTest, PostfixExpression_Call_MissingRightParen) {
+TEST_F(ParserImplTest, SingularExpression_Call_MissingRightParen) {
auto p = parser("a(");
- auto e = p->postfix_expression();
+ auto e = p->singular_expression();
EXPECT_FALSE(e.matched);
EXPECT_TRUE(e.errored);
EXPECT_EQ(e.value, nullptr);
@@ -158,9 +158,9 @@
EXPECT_EQ(p->error(), "1:3: expected ')' for call expression");
}
-TEST_F(ParserImplTest, PostfixExpression_MemberAccessor) {
+TEST_F(ParserImplTest, SingularExpression_MemberAccessor) {
auto p = parser("a.b");
- auto e = p->postfix_expression();
+ auto e = p->singular_expression();
EXPECT_TRUE(e.matched);
EXPECT_FALSE(e.errored);
EXPECT_FALSE(p->has_error()) << p->error();
@@ -177,9 +177,9 @@
p->builder().Symbols().Get("b"));
}
-TEST_F(ParserImplTest, PostfixExpression_MemberAccesssor_InvalidIdent) {
+TEST_F(ParserImplTest, SingularExpression_MemberAccesssor_InvalidIdent) {
auto p = parser("a.if");
- auto e = p->postfix_expression();
+ auto e = p->singular_expression();
EXPECT_FALSE(e.matched);
EXPECT_TRUE(e.errored);
EXPECT_EQ(e.value, nullptr);
@@ -187,9 +187,9 @@
EXPECT_EQ(p->error(), "1:3: expected identifier for member accessor");
}
-TEST_F(ParserImplTest, PostfixExpression_MemberAccessor_MissingIdent) {
+TEST_F(ParserImplTest, SingularExpression_MemberAccessor_MissingIdent) {
auto p = parser("a.");
- auto e = p->postfix_expression();
+ auto e = p->singular_expression();
EXPECT_FALSE(e.matched);
EXPECT_TRUE(e.errored);
EXPECT_EQ(e.value, nullptr);
@@ -197,9 +197,9 @@
EXPECT_EQ(p->error(), "1:3: expected identifier for member accessor");
}
-TEST_F(ParserImplTest, PostfixExpression_NonMatch_returnLHS) {
+TEST_F(ParserImplTest, SingularExpression_NonMatch_returnLHS) {
auto p = parser("a b");
- auto e = p->postfix_expression();
+ auto e = p->singular_expression();
EXPECT_TRUE(e.matched);
EXPECT_FALSE(e.errored);
EXPECT_FALSE(p->has_error()) << p->error();
@@ -207,9 +207,9 @@
ASSERT_TRUE(e->Is<ast::IdentifierExpression>());
}
-TEST_F(ParserImplTest, PostfixExpression_Array_NestedArrayAccessor) {
+TEST_F(ParserImplTest, SingularExpression_Array_NestedArrayAccessor) {
auto p = parser("a[b[c]]");
- auto e = p->postfix_expression();
+ auto e = p->singular_expression();
EXPECT_TRUE(e.matched);
EXPECT_FALSE(e.errored);
EXPECT_FALSE(p->has_error()) << p->error();
diff --git a/test/BUILD.gn b/test/BUILD.gn
index 30ace0e..cd3e013 100644
--- a/test/BUILD.gn
+++ b/test/BUILD.gn
@@ -438,12 +438,12 @@
"../src/reader/wgsl/parser_impl_param_list_test.cc",
"../src/reader/wgsl/parser_impl_paren_rhs_stmt_test.cc",
"../src/reader/wgsl/parser_impl_pipeline_stage_test.cc",
- "../src/reader/wgsl/parser_impl_postfix_expression_test.cc",
"../src/reader/wgsl/parser_impl_primary_expression_test.cc",
"../src/reader/wgsl/parser_impl_relational_expression_test.cc",
"../src/reader/wgsl/parser_impl_sampled_texture_type_test.cc",
"../src/reader/wgsl/parser_impl_sampler_type_test.cc",
"../src/reader/wgsl/parser_impl_shift_expression_test.cc",
+ "../src/reader/wgsl/parser_impl_singular_expression_test.cc",
"../src/reader/wgsl/parser_impl_statement_test.cc",
"../src/reader/wgsl/parser_impl_statements_test.cc",
"../src/reader/wgsl/parser_impl_storage_class_test.cc",
