reader/wgsl: Handle parentheses inside expect_argument_expression_list

This simplifies the callsites, which were previously each having to
handle the "empty list" case (and soon: trailing commas). This is also
a better match for the grammar rules in the WGSL spec.

Change-Id: I88ed54f94964f7b23a0fd9b584659037abb567ff
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/49465
Commit-Queue: James Price <jrprice@google.com>
Auto-Submit: James Price <jrprice@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
This commit is contained in:
James Price 2021-04-29 15:49:44 +00:00 committed by Commit Bot service account
parent 961dc6fbf5
commit e80887d14c
6 changed files with 93 additions and 95 deletions

View File

@ -2131,31 +2131,20 @@ Maybe<ast::Statement*> ParserImpl::for_stmt() {
}
// func_call_stmt
// : IDENT PAREN_LEFT argument_expression_list* PAREN_RIGHT
// : IDENT argument_expression_list
Maybe<ast::CallStatement*> ParserImpl::func_call_stmt() {
auto t = peek();
auto t2 = peek(1);
if (!t.IsIdentifier() || !t2.IsParenLeft())
return Failure::kNoMatch;
next(); // Consume the first peek
auto source = t.source();
next(); // Consume the peek
next(); // Consume the 2nd peek
auto name = t.to_str();
ast::ExpressionList params;
t = peek();
if (!t.IsParenRight() && !t.IsEof()) {
auto list = expect_argument_expression_list();
if (list.errored)
return Failure::kErrored;
params = std::move(list.value);
}
if (!expect("call statement", Token::Type::kParenRight))
auto params = expect_argument_expression_list("function call");
if (params.errored)
return Failure::kErrored;
return create<ast::CallStatement>(
@ -2163,7 +2152,7 @@ Maybe<ast::CallStatement*> ParserImpl::func_call_stmt() {
source,
create<ast::IdentifierExpression>(
source, builder_.Symbols().Register(name)),
std::move(params)));
std::move(params.value)));
}
// break_stmt
@ -2197,7 +2186,7 @@ Maybe<ast::BlockStatement*> ParserImpl::continuing_stmt() {
// primary_expression
// : IDENT argument_expression_list?
// | type_decl PAREN_LEFT argument_expression_list* PAREN_RIGHT
// | type_decl argument_expression_list
// | const_literal
// | paren_rhs_stmt
// | BITCAST LESS_THAN type_decl GREATER_THAN paren_rhs_stmt
@ -2239,23 +2228,13 @@ Maybe<ast::Expression*> ParserImpl::primary_expression() {
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;
if (peek().IsParenLeft()) {
auto params = expect_argument_expression_list("function call");
if (params.errored)
return Failure::kErrored;
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 create<ast::CallExpression>(source, ident,
std::move(params.value));
}
return ident;
@ -2265,25 +2244,12 @@ Maybe<ast::Expression*> ParserImpl::primary_expression() {
if (type.errored)
return Failure::kErrored;
if (type.matched) {
auto expr = expect_paren_block(
"type constructor", [&]() -> Expect<ast::TypeConstructorExpression*> {
t = peek();
if (t.IsParenRight() || t.IsEof())
return create<ast::TypeConstructorExpression>(
source, type.value, ast::ExpressionList{});
auto params = expect_argument_expression_list();
if (params.errored)
return Failure::kErrored;
return create<ast::TypeConstructorExpression>(source, type.value,
params.value);
});
if (expr.errored)
auto params = expect_argument_expression_list("type constructor");
if (params.errored)
return Failure::kErrored;
return expr.value;
return create<ast::TypeConstructorExpression>(source, type.value,
std::move(params.value));
}
return Failure::kNoMatch;
@ -2339,28 +2305,34 @@ Maybe<ast::Expression*> ParserImpl::singular_expression() {
}
// argument_expression_list
// : (logical_or_expression COMMA)* logical_or_expression
Expect<ast::ExpressionList> ParserImpl::expect_argument_expression_list() {
auto arg = logical_or_expression();
if (arg.errored)
return Failure::kErrored;
if (!arg.matched)
return add_error(peek(), "unable to parse argument expression");
ast::ExpressionList ret;
ret.push_back(arg.value);
while (match(Token::Type::kComma)) {
arg = logical_or_expression();
if (arg.errored)
return Failure::kErrored;
if (!arg.matched) {
return add_error(peek(),
"unable to parse argument expression after comma");
// : PAREN_LEFT (logical_or_expression COMMA)* logical_or_expression
// PAREN_RIGHT
Expect<ast::ExpressionList> ParserImpl::expect_argument_expression_list(
const std::string& use) {
return expect_paren_block(use, [&]() -> Expect<ast::ExpressionList> {
// Check for empty list.
// TODO(crbug.com/tint/739): Remove this (handled by !arg.matched).
if (peek().IsParenRight()) {
return ast::ExpressionList{};
}
ret.push_back(arg.value);
}
return ret;
ast::ExpressionList ret;
while (synchronized_) {
auto arg = logical_or_expression();
if (arg.errored)
return Failure::kErrored;
if (!arg.matched) {
// TODO(crbug.com/tint/739): remove error to allow trailing commas.
return add_error(peek(), "unable to parse argument expression");
}
ret.push_back(arg.value);
if (!match(Token::Type::kComma)) {
break;
}
}
return ret;
});
}
// unary_expression

View File

@ -552,8 +552,10 @@ class ParserImpl {
Maybe<ast::Expression*> primary_expression();
/// Parses a `argument_expression_list` grammar element, erroring on parse
/// failure.
/// @param use a description of what was being parsed if an error was raised
/// @returns the list of arguments
Expect<ast::ExpressionList> expect_argument_expression_list();
Expect<ast::ExpressionList> expect_argument_expression_list(
const std::string& use);
/// Parses the recursive portion of the postfix_expression
/// @param prefix the left side of the expression
/// @returns the parsed expression or nullptr

View File

@ -20,8 +20,8 @@ namespace wgsl {
namespace {
TEST_F(ParserImplTest, ArgumentExpressionList_Parses) {
auto p = parser("a");
auto e = p->expect_argument_expression_list();
auto p = parser("(a)");
auto e = p->expect_argument_expression_list("argument list");
ASSERT_FALSE(p->has_error()) << p->error();
ASSERT_FALSE(e.errored);
@ -29,9 +29,18 @@ TEST_F(ParserImplTest, ArgumentExpressionList_Parses) {
ASSERT_TRUE(e.value[0]->Is<ast::IdentifierExpression>());
}
TEST_F(ParserImplTest, ArgumentExpressionList_ParsesEmptyList) {
auto p = parser("()");
auto e = p->expect_argument_expression_list("argument list");
ASSERT_FALSE(p->has_error()) << p->error();
ASSERT_FALSE(e.errored);
ASSERT_EQ(e.value.size(), 0u);
}
TEST_F(ParserImplTest, ArgumentExpressionList_ParsesMultiple) {
auto p = parser("a, -33, 1+2");
auto e = p->expect_argument_expression_list();
auto p = parser("(a, -33, 1+2)");
auto e = p->expect_argument_expression_list("argument list");
ASSERT_FALSE(p->has_error()) << p->error();
ASSERT_FALSE(e.errored);
@ -41,20 +50,36 @@ TEST_F(ParserImplTest, ArgumentExpressionList_ParsesMultiple) {
ASSERT_TRUE(e.value[2]->Is<ast::BinaryExpression>());
}
TEST_F(ParserImplTest, ArgumentExpressionList_HandlesMissingExpression) {
auto p = parser("a, ");
auto e = p->expect_argument_expression_list();
TEST_F(ParserImplTest, ArgumentExpressionList_HandlesMissingLeftParen) {
auto p = parser("a)");
auto e = p->expect_argument_expression_list("argument list");
ASSERT_TRUE(p->has_error());
ASSERT_TRUE(e.errored);
EXPECT_EQ(p->error(), "1:4: unable to parse argument expression after comma");
EXPECT_EQ(p->error(), "1:1: expected '(' for argument list");
}
TEST_F(ParserImplTest, ArgumentExpressionList_HandlesMissingRightParen) {
auto p = parser("(a");
auto e = p->expect_argument_expression_list("argument list");
ASSERT_TRUE(p->has_error());
ASSERT_TRUE(e.errored);
EXPECT_EQ(p->error(), "1:3: expected ')' for argument list");
}
TEST_F(ParserImplTest, ArgumentExpressionList_HandlesMissingExpression) {
auto p = parser("(a, )");
auto e = p->expect_argument_expression_list("argument list");
ASSERT_TRUE(p->has_error());
ASSERT_TRUE(e.errored);
EXPECT_EQ(p->error(), "1:5: unable to parse argument expression");
}
TEST_F(ParserImplTest, ArgumentExpressionList_HandlesInvalidExpression) {
auto p = parser("if(a) {}");
auto e = p->expect_argument_expression_list();
auto p = parser("(if(a) {})");
auto e = p->expect_argument_expression_list("argument list");
ASSERT_TRUE(p->has_error());
ASSERT_TRUE(e.errored);
EXPECT_EQ(p->error(), "1:1: unable to parse argument expression");
EXPECT_EQ(p->error(), "1:2: unable to parse argument expression");
}
} // namespace

View File

@ -65,7 +65,7 @@ TEST_F(ParserImplTest, Statement_Call_Missing_RightParen) {
EXPECT_TRUE(p->has_error());
EXPECT_TRUE(e.errored);
EXPECT_FALSE(e.matched);
EXPECT_EQ(p->error(), "1:3: expected ')' for call statement");
EXPECT_EQ(p->error(), "1:3: unable to parse argument expression");
}
TEST_F(ParserImplTest, Statement_Call_Missing_Semi) {
@ -83,7 +83,7 @@ TEST_F(ParserImplTest, Statement_Call_Bad_ArgList) {
EXPECT_TRUE(p->has_error());
EXPECT_TRUE(e.errored);
EXPECT_FALSE(e.matched);
EXPECT_EQ(p->error(), "1:5: expected ')' for call statement");
EXPECT_EQ(p->error(), "1:5: expected ')' for function call");
}
} // namespace

View File

@ -123,14 +123,14 @@ TEST_F(ParserImplErrorTest, BreakStmtMissingSemicolon) {
TEST_F(ParserImplErrorTest, CallExprMissingRParen) {
EXPECT("fn f() { x = f(1.; }",
"test.wgsl:1:18 error: expected ')' for call expression\n"
"test.wgsl:1:18 error: expected ')' for function call\n"
"fn f() { x = f(1.; }\n"
" ^\n");
}
TEST_F(ParserImplErrorTest, CallStmtMissingRParen) {
EXPECT("fn f() { f(1.; }",
"test.wgsl:1:14 error: expected ')' for call statement\n"
"test.wgsl:1:14 error: expected ')' for function call\n"
"fn f() { f(1.; }\n"
" ^\n");
}
@ -143,11 +143,10 @@ TEST_F(ParserImplErrorTest, CallStmtInvalidArgument0) {
}
TEST_F(ParserImplErrorTest, CallStmtInvalidArgument1) {
EXPECT(
"fn f() { f(1.0, <); }",
"test.wgsl:1:17 error: unable to parse argument expression after comma\n"
"fn f() { f(1.0, <); }\n"
" ^\n");
EXPECT("fn f() { f(1.0, <); }",
"test.wgsl:1:17 error: unable to parse argument expression\n"
"fn f() { f(1.0, <); }\n"
" ^\n");
}
TEST_F(ParserImplErrorTest, CallStmtMissingSemicolon) {

View File

@ -145,7 +145,7 @@ TEST_F(ParserImplTest, SingularExpression_Call_HangingComma) {
EXPECT_TRUE(e.errored);
EXPECT_EQ(e.value, nullptr);
EXPECT_TRUE(p->has_error());
EXPECT_EQ(p->error(), "1:6: unable to parse argument expression after comma");
EXPECT_EQ(p->error(), "1:6: unable to parse argument expression");
}
TEST_F(ParserImplTest, SingularExpression_Call_MissingRightParen) {
@ -155,7 +155,7 @@ TEST_F(ParserImplTest, SingularExpression_Call_MissingRightParen) {
EXPECT_TRUE(e.errored);
EXPECT_EQ(e.value, nullptr);
EXPECT_TRUE(p->has_error());
EXPECT_EQ(p->error(), "1:3: expected ')' for call expression");
EXPECT_EQ(p->error(), "1:3: unable to parse argument expression");
}
TEST_F(ParserImplTest, SingularExpression_MemberAccessor) {