encounter
/
dawn-cmake
mirror of https://github.com/encounter/dawn-cmake.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

reader/wgsl: Allow trailing commas in lists 

Also allows empty constexpr type constructors.

Fixed: tint:739
Change-Id: Ic4729c13b6ac538491d5d1d3c7960e78fac80127
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/49443
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:56:54 +00:00 committed by Commit Bot service account
parent 4248a46bfb
commit 3189bf02e6
11 changed files with 171 additions and 106 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

82
src/reader/wgsl/parser_impl.cc
View File

@ -1416,16 +1416,16 @@ Maybe<ParserImpl::FunctionHeader> ParserImpl::function_header() {
// param_list
// :
// | (param COMMA)* param
// | (param COMMA)* param COMMA?
Expect<ast::VariableList> ParserImpl::expect_param_list() {
// Check for an empty list.
auto t = peek();
if (!t.IsIdentifier() && !t.IsAttrLeft()) {
return ast::VariableList{};
}
ast::VariableList ret;
while (synchronized_) {
// Check for the end of the list.
auto t = peek();
if (!t.IsIdentifier() && !t.IsAttrLeft()) {
break;
}
auto param = expect_param();
if (param.errored)
return Failure::kErrored;
@ -1912,34 +1912,26 @@ Maybe<ast::CaseStatement*> ParserImpl::switch_body() {
}
// case_selectors
// : const_literal (COMMA const_literal)*
// : const_literal (COMMA const_literal)* COMMA?
Expect<ast::CaseSelectorList> ParserImpl::expect_case_selectors() {
ast::CaseSelectorList selectors;
for (;;) {
auto t = peek();
auto matched_comma = match(Token::Type::kComma);
if (selectors.empty() && matched_comma)
return add_error(t, "a selector is expected before the comma");
if (matched_comma)
t = peek();
while (synchronized_) {
auto cond = const_literal();
if (cond.errored)
if (cond.errored) {
return Failure::kErrored;
if (!cond.matched) {
if (matched_comma) {
return add_error(t, "a selector is expected after the comma");
}
} else if (!cond.matched) {
break;
} else if (!cond->Is<ast::IntLiteral>()) {
return add_error(cond.value->source(),
"invalid case selector must be an integer value");
}
if (!cond->Is<ast::IntLiteral>())
return add_error(t, "invalid case selector must be an integer value");
if (!selectors.empty() && !matched_comma)
return add_error(t, "expected a comma after the previous selector");
selectors.push_back(cond.value->As<ast::IntLiteral>());
if (!match(Token::Type::kComma)) {
break;
}
}
if (selectors.empty())
@ -2305,25 +2297,18 @@ Maybe<ast::Expression*> ParserImpl::singular_expression() {
}
// argument_expression_list
// : PAREN_LEFT (logical_or_expression COMMA)* logical_or_expression
// : PAREN_LEFT ((logical_or_expression COMMA)* logical_or_expression COMMA?)?
// 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{};
}
ast::ExpressionList ret;
while (synchronized_) {
auto arg = logical_or_expression();
if (arg.errored)
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");
} else if (!arg.matched) {
break;
}
ret.push_back(arg.value);
@ -2838,7 +2823,7 @@ Maybe<ast::Literal*> ParserImpl::const_literal() {
}
// const_expr
// : type_decl PAREN_LEFT (const_expr COMMA)? const_expr PAREN_RIGHT
// : type_decl PAREN_LEFT ((const_expr COMMA)? const_expr COMMA?)? PAREN_RIGHT
// | const_literal
Expect<ast::ConstructorExpression*> ParserImpl::expect_const_expr() {
auto t = peek();
@ -2852,15 +2837,20 @@ Expect<ast::ConstructorExpression*> ParserImpl::expect_const_expr() {
auto params = expect_paren_block("type constructor",
[&]() -> Expect<ast::ExpressionList> {
ast::ExpressionList list;
auto param = expect_const_expr();
if (param.errored)
return Failure::kErrored;
list.emplace_back(param.value);
while (match(Token::Type::kComma)) {
param = expect_const_expr();
if (param.errored)
while (synchronized_) {
if (peek().IsParenRight()) {
break;
}
auto arg = expect_const_expr();
if (arg.errored) {
return Failure::kErrored;
list.emplace_back(param.value);
}
list.emplace_back(arg.value);
if (!match(Token::Type::kComma)) {
break;
}
}
return list;
});

27
src/reader/wgsl/parser_impl_argument_expression_list_test.cc
View File

@ -50,6 +50,17 @@ TEST_F(ParserImplTest, ArgumentExpressionList_ParsesMultiple) {
ASSERT_TRUE(e.value[2]->Is<ast::BinaryExpression>());
}
TEST_F(ParserImplTest, ArgumentExpressionList_TrailingComma) {
auto p = parser("(a, 42,)");
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(), 2u);
ASSERT_TRUE(e.value[0]->Is<ast::IdentifierExpression>());
ASSERT_TRUE(e.value[1]->Is<ast::ConstructorExpression>());
}
TEST_F(ParserImplTest, ArgumentExpressionList_HandlesMissingLeftParen) {
auto p = parser("a)");
auto e = p->expect_argument_expression_list("argument list");
@ -66,12 +77,20 @@ TEST_F(ParserImplTest, ArgumentExpressionList_HandlesMissingRightParen) {
EXPECT_EQ(p->error(), "1:3: expected ')' for argument list");
}
TEST_F(ParserImplTest, ArgumentExpressionList_HandlesMissingExpression) {
auto p = parser("(a, )");
TEST_F(ParserImplTest, ArgumentExpressionList_HandlesMissingExpression_0) {
auto p = parser("(,)");
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");
EXPECT_EQ(p->error(), "1:2: expected ')' for argument list");
}
TEST_F(ParserImplTest, ArgumentExpressionList_HandlesMissingExpression_1) {
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: expected ')' for argument list");
}
TEST_F(ParserImplTest, ArgumentExpressionList_HandlesInvalidExpression) {
@ -79,7 +98,7 @@ TEST_F(ParserImplTest, ArgumentExpressionList_HandlesInvalidExpression) {
auto e = p->expect_argument_expression_list("argument list");
ASSERT_TRUE(p->has_error());
ASSERT_TRUE(e.errored);
EXPECT_EQ(p->error(), "1:2: unable to parse argument expression");
EXPECT_EQ(p->error(), "1:2: expected ')' for argument list");
}
} // namespace

22
src/reader/wgsl/parser_impl_call_stmt_test.cc
View File

@ -59,13 +59,33 @@ TEST_F(ParserImplTest, Statement_Call_WithParams) {
EXPECT_TRUE(c->params()[2]->Is<ast::BinaryExpression>());
}
TEST_F(ParserImplTest, Statement_Call_WithParams_TrailingComma) {
auto p = parser("a(1, b,);");
auto e = p->statement();
ASSERT_FALSE(p->has_error()) << p->error();
ASSERT_NE(e.value, nullptr);
EXPECT_TRUE(e.matched);
EXPECT_FALSE(e.errored);
ASSERT_TRUE(e->Is<ast::CallStatement>());
auto* c = e->As<ast::CallStatement>()->expr();
ASSERT_TRUE(c->func()->Is<ast::IdentifierExpression>());
auto* ident = c->func()->As<ast::IdentifierExpression>();
EXPECT_EQ(ident->symbol(), p->builder().Symbols().Get("a"));
EXPECT_EQ(c->params().size(), 2u);
EXPECT_TRUE(c->params()[0]->Is<ast::ConstructorExpression>());
EXPECT_TRUE(c->params()[1]->Is<ast::IdentifierExpression>());
}
TEST_F(ParserImplTest, Statement_Call_Missing_RightParen) {
auto p = parser("a(");
auto e = p->statement();
EXPECT_TRUE(p->has_error());
EXPECT_TRUE(e.errored);
EXPECT_FALSE(e.matched);
EXPECT_EQ(p->error(), "1:3: unable to parse argument expression");
EXPECT_EQ(p->error(), "1:3: expected ')' for function call");
}
TEST_F(ParserImplTest, Statement_Call_Missing_Semi) {

50
src/reader/wgsl/parser_impl_const_expr_test.cc
View File

@ -47,6 +47,38 @@ TEST_F(ParserImplTest, ConstExpr_TypeDecl) {
EXPECT_FLOAT_EQ(c->literal()->As<ast::FloatLiteral>()->value(), 2.);
}
TEST_F(ParserImplTest, ConstExpr_TypeDecl_Empty) {
auto p = parser("vec2<f32>()");
auto e = p->expect_const_expr();
ASSERT_FALSE(p->has_error()) << p->error();
ASSERT_FALSE(e.errored);
ASSERT_TRUE(e->Is<ast::ConstructorExpression>());
ASSERT_TRUE(e->Is<ast::TypeConstructorExpression>());
auto* t = e->As<ast::TypeConstructorExpression>();
ASSERT_TRUE(t->type()->Is<sem::Vector>());
EXPECT_EQ(t->type()->As<sem::Vector>()->size(), 2u);
ASSERT_EQ(t->values().size(), 0u);
}
TEST_F(ParserImplTest, ConstExpr_TypeDecl_TrailingComma) {
auto p = parser("vec2<f32>(1., 2.,)");
auto e = p->expect_const_expr();
ASSERT_FALSE(p->has_error()) << p->error();
ASSERT_FALSE(e.errored);
ASSERT_TRUE(e->Is<ast::ConstructorExpression>());
ASSERT_TRUE(e->Is<ast::TypeConstructorExpression>());
auto* t = e->As<ast::TypeConstructorExpression>();
ASSERT_TRUE(t->type()->Is<sem::Vector>());
EXPECT_EQ(t->type()->As<sem::Vector>()->size(), 2u);
ASSERT_EQ(t->values().size(), 2u);
ASSERT_TRUE(t->values()[0]->Is<ast::ScalarConstructorExpression>());
ASSERT_TRUE(t->values()[1]->Is<ast::ScalarConstructorExpression>());
}
TEST_F(ParserImplTest, ConstExpr_TypeDecl_MissingRightParen) {
auto p = parser("vec2<f32>(1., 2.");
auto e = p->expect_const_expr();
@ -65,15 +97,6 @@ TEST_F(ParserImplTest, ConstExpr_TypeDecl_MissingLeftParen) {
EXPECT_EQ(p->error(), "1:11: expected '(' for type constructor");
}
TEST_F(ParserImplTest, ConstExpr_TypeDecl_HangingComma) {
auto p = parser("vec2<f32>(1.,)");
auto e = p->expect_const_expr();
ASSERT_TRUE(p->has_error());
ASSERT_TRUE(e.errored);
ASSERT_EQ(e.value, nullptr);
EXPECT_EQ(p->error(), "1:14: unable to parse constant literal");
}
TEST_F(ParserImplTest, ConstExpr_TypeDecl_MissingComma) {
auto p = parser("vec2<f32>(1. 2.");
auto e = p->expect_const_expr();
@ -83,15 +106,6 @@ TEST_F(ParserImplTest, ConstExpr_TypeDecl_MissingComma) {
EXPECT_EQ(p->error(), "1:14: expected ')' for type constructor");
}
TEST_F(ParserImplTest, ConstExpr_MissingExpr) {
auto p = parser("vec2<f32>()");
auto e = p->expect_const_expr();
ASSERT_TRUE(p->has_error());
ASSERT_TRUE(e.errored);
ASSERT_EQ(e.value, nullptr);
EXPECT_EQ(p->error(), "1:11: unable to parse constant literal");
}
TEST_F(ParserImplTest, ConstExpr_InvalidExpr) {
auto p = parser("vec2<f32>(1., if(a) {})");
auto e = p->expect_const_expr();

10
src/reader/wgsl/parser_impl_error_msg_test.cc
View File

@ -137,14 +137,14 @@ TEST_F(ParserImplErrorTest, CallStmtMissingRParen) {
TEST_F(ParserImplErrorTest, CallStmtInvalidArgument0) {
EXPECT("fn f() { f(<); }",
"test.wgsl:1:12 error: unable to parse argument expression\n"
"test.wgsl:1:12 error: expected ')' for function call\n"
"fn f() { f(<); }\n"
" ^\n");
}
TEST_F(ParserImplErrorTest, CallStmtInvalidArgument1) {
EXPECT("fn f() { f(1.0, <); }",
"test.wgsl:1:17 error: unable to parse argument expression\n"
"test.wgsl:1:17 error: expected ')' for function call\n"
"fn f() { f(1.0, <); }\n"
" ^\n");
}
@ -446,9 +446,9 @@ TEST_F(ParserImplErrorTest, FunctionDeclParamInvalidType) {
}
TEST_F(ParserImplErrorTest, FunctionDeclParamMissing) {
EXPECT("fn f(x : i32, ) {}",
"test.wgsl:1:15 error: expected identifier for parameter\n"
"fn f(x : i32, ) {}\n"
EXPECT("fn f(x : i32, ,) {}",
"test.wgsl:1:15 error: expected ')' for function declaration\n"
"fn f(x : i32, ,) {}\n"
" ^\n");
}

2
src/reader/wgsl/parser_impl_for_stmt_test.cc
View File

@ -250,7 +250,7 @@ TEST_F(ForStmtErrorTest, InvalidBreakConditionMatch) {
// Test a for loop with an invalid continuing statement.
TEST_F(ForStmtErrorTest, InvalidContinuingAsFuncCall) {
std::string for_str = "for (;; a(,) ) { }";
std::string error_str = "1:11: unable to parse argument expression";
std::string error_str = "1:11: expected ')' for function call";
TestForWithError(for_str, error_str);
}

16
src/reader/wgsl/parser_impl_function_header_test.cc
View File

@ -33,6 +33,18 @@ TEST_F(ParserImplTest, FunctionHeader) {
EXPECT_TRUE(f->return_type->Is<sem::Void>());
}
TEST_F(ParserImplTest, FunctionHeader_TrailingComma) {
auto p = parser("fn main(a :i32,)");
auto f = p->function_header();
EXPECT_TRUE(f.matched);
EXPECT_FALSE(f.errored);
EXPECT_EQ(f->name, "main");
ASSERT_EQ(f->params.size(), 1u);
EXPECT_EQ(f->params[0]->symbol(), p->builder().Symbols().Get("a"));
EXPECT_TRUE(f->return_type->Is<sem::Void>());
}
TEST_F(ParserImplTest, FunctionHeader_DecoratedReturnType) {
auto p = parser("fn main() -> [[location(1)]] f32");
auto f = p->function_header();
@ -77,12 +89,12 @@ TEST_F(ParserImplTest, FunctionHeader_MissingParenLeft) {
}
TEST_F(ParserImplTest, FunctionHeader_InvalidParamList) {
auto p = parser("fn main(a :i32,) -> i32");
auto p = parser("fn main(a :i32, ,) -> i32");
auto f = p->function_header();
EXPECT_FALSE(f.matched);
EXPECT_TRUE(f.errored);
EXPECT_TRUE(p->has_error());
EXPECT_EQ(p->error(), "1:16: expected identifier for parameter");
EXPECT_EQ(p->error(), "1:17: expected ')' for function declaration");
}
TEST_F(ParserImplTest, FunctionHeader_MissingParenRight) {

8
src/reader/wgsl/parser_impl_param_list_test.cc
View File

@ -87,12 +87,12 @@ TEST_F(ParserImplTest, ParamList_Empty) {
EXPECT_EQ(e.value.size(), 0u);
}
TEST_F(ParserImplTest, ParamList_HangingComma) {
TEST_F(ParserImplTest, ParamList_TrailingComma) {
auto p = parser("a : i32,");
auto e = p->expect_param_list();
ASSERT_TRUE(p->has_error());
ASSERT_TRUE(e.errored);
EXPECT_EQ(p->error(), "1:9: expected identifier for parameter");
ASSERT_FALSE(p->has_error());
ASSERT_FALSE(e.errored);
EXPECT_EQ(e.value.size(), 1u);
}
TEST_F(ParserImplTest, ParamList_Decorations) {

2
src/reader/wgsl/parser_impl_primary_expression_test.cc
View File

@ -121,7 +121,7 @@ TEST_F(ParserImplTest, PrimaryExpression_TypeDecl_InvalidValue) {
EXPECT_TRUE(e.errored);
EXPECT_EQ(e.value, nullptr);
ASSERT_TRUE(p->has_error());
EXPECT_EQ(p->error(), "1:5: unable to parse argument expression");
EXPECT_EQ(p->error(), "1:5: expected ')' for type constructor");
}
TEST_F(ParserImplTest, PrimaryExpression_TypeDecl_StructConstructor_Empty) {

26
src/reader/wgsl/parser_impl_singular_expression_test.cc
View File

@ -128,6 +128,18 @@ TEST_F(ParserImplTest, SingularExpression_Call_WithArgs) {
EXPECT_TRUE(c->params()[2]->Is<ast::BinaryExpression>());
}
TEST_F(ParserImplTest, SingularExpression_Call_TrailingComma) {
auto p = parser("a(b, )");
auto e = p->singular_expression();
EXPECT_TRUE(e.matched);
EXPECT_FALSE(e.errored);
ASSERT_NE(e.value, nullptr);
ASSERT_TRUE(e->Is<ast::CallExpression>());
auto* c = e->As<ast::CallExpression>();
EXPECT_EQ(c->params().size(), 1u);
}
TEST_F(ParserImplTest, SingularExpression_Call_InvalidArg) {
auto p = parser("a(if(a) {})");
auto e = p->singular_expression();
@ -135,17 +147,7 @@ TEST_F(ParserImplTest, SingularExpression_Call_InvalidArg) {
EXPECT_TRUE(e.errored);
EXPECT_EQ(e.value, nullptr);
EXPECT_TRUE(p->has_error());
EXPECT_EQ(p->error(), "1:3: unable to parse argument expression");
}
TEST_F(ParserImplTest, SingularExpression_Call_HangingComma) {
auto p = parser("a(b, )");
auto e = p->singular_expression();
EXPECT_FALSE(e.matched);
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");
EXPECT_EQ(p->error(), "1:3: expected ')' for function call");
}
TEST_F(ParserImplTest, SingularExpression_Call_MissingRightParen) {
@ -155,7 +157,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: unable to parse argument expression");
EXPECT_EQ(p->error(), "1:3: expected ')' for function call");
}
TEST_F(ParserImplTest, SingularExpression_MemberAccessor) {

32
src/reader/wgsl/parser_impl_switch_body_test.cc
View File

@ -28,10 +28,28 @@ TEST_F(ParserImplTest, SwitchBody_Case) {
ASSERT_NE(e.value, nullptr);
ASSERT_TRUE(e->Is<ast::CaseStatement>());
EXPECT_FALSE(e->IsDefault());
auto* stmt = e->As<ast::CaseStatement>();
ASSERT_EQ(stmt->selectors().size(), 1u);
EXPECT_EQ(stmt->selectors()[0]->value_as_u32(), 1u);
ASSERT_EQ(e->body()->size(), 1u);
EXPECT_TRUE(e->body()->get(0)->Is<ast::AssignmentStatement>());
}
TEST_F(ParserImplTest, SwitchBody_Case_TrailingComma) {
auto p = parser("case 1, 2,: { }");
auto e = p->switch_body();
EXPECT_FALSE(p->has_error()) << p->error();
EXPECT_TRUE(e.matched);
EXPECT_FALSE(e.errored);
ASSERT_NE(e.value, nullptr);
ASSERT_TRUE(e->Is<ast::CaseStatement>());
EXPECT_FALSE(e->IsDefault());
auto* stmt = e->As<ast::CaseStatement>();
ASSERT_EQ(stmt->selectors().size(), 2u);
EXPECT_EQ(stmt->selectors()[0]->value_as_u32(), 1u);
EXPECT_EQ(stmt->selectors()[1]->value_as_u32(), 2u);
}
TEST_F(ParserImplTest, SwitchBody_Case_InvalidConstLiteral) {
auto p = parser("case a == 4: { a = 4; }");
auto e = p->switch_body();
@ -134,17 +152,7 @@ TEST_F(ParserImplTest, SwitchBody_Case_MultipleSelectorsMissingComma) {
EXPECT_TRUE(e.errored);
EXPECT_FALSE(e.matched);
EXPECT_EQ(e.value, nullptr);
EXPECT_EQ(p->error(), "1:8: expected a comma after the previous selector");
}
TEST_F(ParserImplTest, SwitchBody_Case_MultipleSelectorsEndsWithComma) {
auto p = parser("case 1, 2,: { }");
auto e = p->switch_body();
EXPECT_TRUE(p->has_error());
EXPECT_TRUE(e.errored);
EXPECT_FALSE(e.matched);
EXPECT_EQ(e.value, nullptr);
EXPECT_EQ(p->error(), "1:11: a selector is expected after the comma");
EXPECT_EQ(p->error(), "1:8: expected ':' for case statement");
}
TEST_F(ParserImplTest, SwitchBody_Case_MultipleSelectorsStartsWithComma) {
@ -154,7 +162,7 @@ TEST_F(ParserImplTest, SwitchBody_Case_MultipleSelectorsStartsWithComma) {
EXPECT_TRUE(e.errored);
EXPECT_FALSE(e.matched);
EXPECT_EQ(e.value, nullptr);
EXPECT_EQ(p->error(), "1:6: a selector is expected before the comma");
EXPECT_EQ(p->error(), "1:6: unable to parse case selectors");
}
TEST_F(ParserImplTest, SwitchBody_Default) {