Convert assignment_statement to new WGSL grammar.

This CL converts the `assignment_statement` to
`variable_updating_statement`. Some more test cases are added
around the phony assignment and usage of compound operators.

The `lhs_expression` and `core_lhs_expression` are converted to
return `Maybe`s.

Bug: tint:1633
Change-Id: Iaed6373e2f202609adf341b57dc9027e5a04af34
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/99380
Commit-Queue: Dan Sinclair <dsinclair@chromium.org>
Reviewed-by: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
This commit is contained in:
dan sinclair 2022-08-17 17:20:50 +00:00 committed by Dawn LUCI CQ
parent 6c8dc15d64
commit 873f92e741
6 changed files with 171 additions and 77 deletions

View File

@ -1762,9 +1762,7 @@ Maybe<const ast::Statement*> ParserImpl::statement() {
// | break_statement SEMICOLON
// | continue_statement SEMICOLON
// | DISCARD SEMICOLON
// | assignment_statement SEMICOLON
// | increment_statement SEMICOLON
// | decrement_statement SEMICOLON
// | variable_updating_statement SEMICOLON
// | static_assert_statement SEMICOLON
Maybe<const ast::Statement*> ParserImpl::non_block_statement() {
auto stmt = [&]() -> Maybe<const ast::Statement*> {
@ -1814,7 +1812,7 @@ Maybe<const ast::Statement*> ParserImpl::non_block_statement() {
}
// Note, this covers assignment, increment and decrement
auto assign = assignment_statement();
auto assign = variable_updating_statement();
if (assign.errored) {
return Failure::kErrored;
}
@ -2209,7 +2207,7 @@ ForHeader::ForHeader(const ast::Statement* init,
ForHeader::~ForHeader() = default;
// (variable_statement | increment_statement | decrement_statement | assignment_statement |
// (variable_statement | variable_updating_statement |
// func_call_statement)?
Maybe<const ast::Statement*> ParserImpl::for_header_initializer() {
auto call = func_call_statement();
@ -2228,7 +2226,7 @@ Maybe<const ast::Statement*> ParserImpl::for_header_initializer() {
return var.value;
}
auto assign = assignment_statement();
auto assign = variable_updating_statement();
if (assign.errored) {
return Failure::kErrored;
}
@ -2239,7 +2237,7 @@ Maybe<const ast::Statement*> ParserImpl::for_header_initializer() {
return Failure::kNoMatch;
}
// (increment_statement | decrement_statement | assignment_statement | func_call_statement)?
// (variable_updating_statement | func_call_statement)?
Maybe<const ast::Statement*> ParserImpl::for_header_continuing() {
auto call_stmt = func_call_statement();
if (call_stmt.errored) {
@ -2249,7 +2247,7 @@ Maybe<const ast::Statement*> ParserImpl::for_header_continuing() {
return call_stmt.value;
}
auto assign = assignment_statement();
auto assign = variable_updating_statement();
if (assign.errored) {
return Failure::kErrored;
}
@ -2261,10 +2259,10 @@ Maybe<const ast::Statement*> ParserImpl::for_header_continuing() {
}
// for_header
// : (variable_statement | assignment_statement | func_call_statement)?
// : (variable_statement | variable_updating_statement | func_call_statement)?
// SEMICOLON
// expression? SEMICOLON
// (assignment_statement | func_call_statement)?
// (variable_updating_statement | func_call_statement)?
Expect<std::unique_ptr<ForHeader>> ParserImpl::expect_for_header() {
auto initializer = for_header_initializer();
if (initializer.errored) {
@ -3118,7 +3116,7 @@ Maybe<ast::BinaryOp> ParserImpl::compound_assignment_operator() {
// core_lhs_expression
// : ident
// | PAREN_LEFT lhs_expression PAREN_RIGHT
Expect<const ast::Expression*> ParserImpl::core_lhs_expression() {
Maybe<const ast::Expression*> ParserImpl::core_lhs_expression() {
auto& t = peek();
if (t.IsIdentifier()) {
next();
@ -3133,15 +3131,19 @@ Expect<const ast::Expression*> ParserImpl::core_lhs_expression() {
if (expr.errored) {
return Failure::kErrored;
}
if (!expr.matched) {
return add_error(t, "invalid expression");
}
return expr.value;
});
}
return add_error(peek(), "missing expression");
return Failure::kNoMatch;
}
// lhs_expression
// : ( STAR | AND )* core_lhs_expression postfix_expression?
Expect<const ast::Expression*> ParserImpl::lhs_expression() {
Maybe<const ast::Expression*> ParserImpl::lhs_expression() {
std::vector<const Token*> prefixes;
while (peek_is(Token::Type::kStar) || peek_is(Token::Type::kAnd) ||
peek_is(Token::Type::kAndAnd)) {
@ -3158,6 +3160,12 @@ Expect<const ast::Expression*> ParserImpl::lhs_expression() {
auto core_expr = core_lhs_expression();
if (core_expr.errored) {
return Failure::kErrored;
} else if (!core_expr.matched) {
if (prefixes.empty()) {
return Failure::kNoMatch;
}
return add_error(peek(), "missing expression");
}
const auto* expr = core_expr.value;
@ -3177,16 +3185,15 @@ Expect<const ast::Expression*> ParserImpl::lhs_expression() {
return e.value;
}
// assignment_statement
// | lhs_expression ( EQUAL | compound_assignment_operator ) expression
// variable_updating_statement
// : lhs_expression ( EQUAL | compound_assignment_operator ) expression
// | lhs_expression MINUS_MINUS
// | lhs_expression PLUS_PLUS
// | UNDERSCORE EQUAL expression
//
// increment_statement
// | lhs_expression PLUS_PLUS
//
// decrement_statement
// | lhs_expression MINUS_MINUS
Maybe<const ast::Statement*> ParserImpl::assignment_statement() {
// Note, this is a simplification of the recursive grammar statement with the `lhs_expression`
// substituted back into the expression.
Maybe<const ast::Statement*> ParserImpl::variable_updating_statement() {
auto& t = peek();
// tint:295 - Test for `ident COLON` - this is invalid grammar, and without
@ -3196,36 +3203,47 @@ Maybe<const ast::Statement*> ParserImpl::assignment_statement() {
return add_error(peek(0).source(), "expected 'var' for variable declaration");
}
auto lhs = unary_expression();
if (lhs.errored) {
return Failure::kErrored;
}
if (!lhs.matched) {
Source source = t.source();
if (!match(Token::Type::kUnderscore, &source)) {
return Failure::kNoMatch;
}
lhs = create<ast::PhonyExpression>(source);
}
const ast::Expression* lhs = nullptr;
ast::BinaryOp compound_op = ast::BinaryOp::kNone;
if (peek_is(Token::Type::kUnderscore)) {
next(); // Consume the peek.
// Handle increment and decrement statements.
// We do this here because the parsing of the LHS expression overlaps with
// the assignment statement, and we cannot tell which we are parsing until we
// hit the ++/--/= token.
if (match(Token::Type::kPlusPlus)) {
return create<ast::IncrementDecrementStatement>(t.source(), lhs.value, true);
} else if (match(Token::Type::kMinusMinus)) {
return create<ast::IncrementDecrementStatement>(t.source(), lhs.value, false);
}
auto compound_op = compound_assignment_operator();
if (compound_op.errored) {
return Failure::kErrored;
}
if (!compound_op.matched) {
if (!expect("assignment", Token::Type::kEqual)) {
return Failure::kErrored;
}
lhs = create<ast::PhonyExpression>(t.source());
} else {
auto lhs_result = lhs_expression();
if (lhs_result.errored) {
return Failure::kErrored;
}
if (!lhs_result.matched) {
return Failure::kNoMatch;
}
lhs = lhs_result.value;
// Handle increment and decrement statements.
if (match(Token::Type::kPlusPlus)) {
return create<ast::IncrementDecrementStatement>(t.source(), lhs, true);
}
if (match(Token::Type::kMinusMinus)) {
return create<ast::IncrementDecrementStatement>(t.source(), lhs, false);
}
auto compound_op_result = compound_assignment_operator();
if (compound_op_result.errored) {
return Failure::kErrored;
}
if (compound_op_result.matched) {
compound_op = compound_op_result.value;
} else {
if (!expect("assignment", Token::Type::kEqual)) {
return Failure::kErrored;
}
}
}
auto rhs = expression();
@ -3236,12 +3254,10 @@ Maybe<const ast::Statement*> ParserImpl::assignment_statement() {
return add_error(peek(), "unable to parse right side of assignment");
}
if (compound_op.value != ast::BinaryOp::kNone) {
return create<ast::CompoundAssignmentStatement>(t.source(), lhs.value, rhs.value,
compound_op.value);
} else {
return create<ast::AssignmentStatement>(t.source(), lhs.value, rhs.value);
if (compound_op != ast::BinaryOp::kNone) {
return create<ast::CompoundAssignmentStatement>(t.source(), lhs, rhs.value, compound_op);
}
return create<ast::AssignmentStatement>(t.source(), lhs, rhs.value);
}
// const_literal

View File

@ -685,13 +685,13 @@ class ParserImpl {
Maybe<ast::BinaryOp> compound_assignment_operator();
/// Parses a `core_lhs_expression` grammar element
/// @returns the parsed expression or a non-kMatched failure
Expect<const ast::Expression*> core_lhs_expression();
Maybe<const ast::Expression*> core_lhs_expression();
/// Parses a `lhs_expression` grammar element
/// @returns the parsed expression or a non-kMatched failure
Expect<const ast::Expression*> lhs_expression();
/// Parses a `assignment_statement` grammar element
Maybe<const ast::Expression*> lhs_expression();
/// Parses a `variable_updating_statement` grammar element
/// @returns the parsed assignment or nullptr
Maybe<const ast::Statement*> assignment_statement();
Maybe<const ast::Statement*> variable_updating_statement();
/// Parses one or more attribute lists.
/// @return the parsed attribute list, or an empty list on error.
Maybe<AttributeList> attribute_list();

View File

@ -19,7 +19,7 @@ namespace {
TEST_F(ParserImplTest, AssignmentStmt_Parses_ToVariable) {
auto p = parser("a = 123");
auto e = p->assignment_statement();
auto e = p->variable_updating_statement();
EXPECT_TRUE(e.matched);
EXPECT_FALSE(e.errored);
EXPECT_FALSE(p->has_error()) << p->error();
@ -42,7 +42,7 @@ TEST_F(ParserImplTest, AssignmentStmt_Parses_ToVariable) {
TEST_F(ParserImplTest, AssignmentStmt_Parses_ToMember) {
auto p = parser("a.b.c[2].d = 123");
auto e = p->assignment_statement();
auto e = p->variable_updating_statement();
EXPECT_TRUE(e.matched);
EXPECT_FALSE(e.errored);
EXPECT_FALSE(p->has_error()) << p->error();
@ -92,7 +92,7 @@ TEST_F(ParserImplTest, AssignmentStmt_Parses_ToMember) {
TEST_F(ParserImplTest, AssignmentStmt_Parses_ToPhony) {
auto p = parser("_ = 123i");
auto e = p->assignment_statement();
auto e = p->variable_updating_statement();
EXPECT_TRUE(e.matched);
EXPECT_FALSE(e.errored);
EXPECT_FALSE(p->has_error()) << p->error();
@ -111,6 +111,38 @@ TEST_F(ParserImplTest, AssignmentStmt_Parses_ToPhony) {
ASSERT_TRUE(a->lhs->Is<ast::PhonyExpression>());
}
TEST_F(ParserImplTest, AssignmentStmt_Phony_CompoundOpFails) {
auto p = parser("_ += 123i");
auto e = p->variable_updating_statement();
EXPECT_FALSE(e.matched);
EXPECT_TRUE(e.errored);
EXPECT_TRUE(p->has_error());
EXPECT_EQ(e.value, nullptr);
EXPECT_EQ(p->error(), "1:3: expected '=' for assignment");
}
TEST_F(ParserImplTest, AssignmentStmt_Phony_IncrementFails) {
auto p = parser("_ ++");
auto e = p->variable_updating_statement();
EXPECT_FALSE(e.matched);
EXPECT_TRUE(e.errored);
EXPECT_TRUE(p->has_error());
EXPECT_EQ(e.value, nullptr);
EXPECT_EQ(p->error(), "1:3: expected '=' for assignment");
}
TEST_F(ParserImplTest, AssignmentStmt_Phony_EqualIncrementFails) {
auto p = parser("_ = ++");
auto e = p->variable_updating_statement();
EXPECT_FALSE(e.matched);
EXPECT_TRUE(e.errored);
EXPECT_TRUE(p->has_error());
EXPECT_EQ(e.value, nullptr);
EXPECT_EQ(
p->error(),
"1:5: prefix increment and decrement operators are reserved for a future WGSL version");
}
struct CompoundData {
std::string str;
ast::BinaryOp op;
@ -119,7 +151,7 @@ using CompoundOpTest = ParserImplTestWithParam<CompoundData>;
TEST_P(CompoundOpTest, CompoundOp) {
auto params = GetParam();
auto p = parser("a " + params.str + " 123u");
auto e = p->assignment_statement();
auto e = p->variable_updating_statement();
EXPECT_TRUE(e.matched);
EXPECT_FALSE(e.errored);
EXPECT_FALSE(p->has_error()) << p->error();
@ -155,7 +187,7 @@ INSTANTIATE_TEST_SUITE_P(ParserImplTest,
TEST_F(ParserImplTest, AssignmentStmt_MissingEqual) {
auto p = parser("a.b.c[2].d 123");
auto e = p->assignment_statement();
auto e = p->variable_updating_statement();
EXPECT_FALSE(e.matched);
EXPECT_TRUE(e.errored);
EXPECT_TRUE(p->has_error());
@ -165,7 +197,7 @@ TEST_F(ParserImplTest, AssignmentStmt_MissingEqual) {
TEST_F(ParserImplTest, AssignmentStmt_Compound_MissingEqual) {
auto p = parser("a + 123");
auto e = p->assignment_statement();
auto e = p->variable_updating_statement();
EXPECT_FALSE(e.matched);
EXPECT_TRUE(e.errored);
EXPECT_TRUE(p->has_error());
@ -175,7 +207,7 @@ TEST_F(ParserImplTest, AssignmentStmt_Compound_MissingEqual) {
TEST_F(ParserImplTest, AssignmentStmt_InvalidLHS) {
auto p = parser("if (true) {} = 123");
auto e = p->assignment_statement();
auto e = p->variable_updating_statement();
EXPECT_FALSE(e.matched);
EXPECT_FALSE(e.errored);
EXPECT_FALSE(p->has_error()) << p->error();
@ -184,7 +216,7 @@ TEST_F(ParserImplTest, AssignmentStmt_InvalidLHS) {
TEST_F(ParserImplTest, AssignmentStmt_InvalidRHS) {
auto p = parser("a.b.c[2].d = if (true) {}");
auto e = p->assignment_statement();
auto e = p->variable_updating_statement();
EXPECT_FALSE(e.matched);
EXPECT_TRUE(e.errored);
EXPECT_EQ(e.value, nullptr);
@ -194,7 +226,7 @@ TEST_F(ParserImplTest, AssignmentStmt_InvalidRHS) {
TEST_F(ParserImplTest, AssignmentStmt_InvalidCompoundOp) {
auto p = parser("a &&= true");
auto e = p->assignment_statement();
auto e = p->variable_updating_statement();
EXPECT_FALSE(e.matched);
EXPECT_TRUE(e.errored);
EXPECT_EQ(e.value, nullptr);

View File

@ -17,11 +17,20 @@
namespace tint::reader::wgsl {
namespace {
TEST_F(ParserImplTest, CoreLHS_NoMatch) {
auto p = parser("123");
auto e = p->core_lhs_expression();
ASSERT_FALSE(p->has_error()) << p->error();
ASSERT_FALSE(e.errored);
EXPECT_FALSE(e.matched);
}
TEST_F(ParserImplTest, CoreLHS_Ident) {
auto p = parser("identifier");
auto e = p->core_lhs_expression();
ASSERT_FALSE(p->has_error()) << p->error();
ASSERT_FALSE(e.errored);
EXPECT_TRUE(e.matched);
ASSERT_NE(e.value, nullptr);
ASSERT_TRUE(e->Is<ast::IdentifierExpression>());
@ -34,6 +43,7 @@ TEST_F(ParserImplTest, CoreLHS_ParenStmt) {
auto e = p->core_lhs_expression();
ASSERT_FALSE(p->has_error()) << p->error();
ASSERT_FALSE(e.errored);
EXPECT_TRUE(e.matched);
ASSERT_NE(e.value, nullptr);
ASSERT_TRUE(e->Is<ast::IdentifierExpression>());
@ -46,6 +56,7 @@ TEST_F(ParserImplTest, CoreLHS_MissingRightParen) {
auto e = p->core_lhs_expression();
ASSERT_TRUE(p->has_error());
ASSERT_TRUE(e.errored);
EXPECT_FALSE(e.matched);
ASSERT_EQ(e.value, nullptr);
EXPECT_EQ(p->error(), "1:3: expected ')'");
}
@ -55,8 +66,9 @@ TEST_F(ParserImplTest, CoreLHS_InvalidLHSExpression) {
auto e = p->core_lhs_expression();
ASSERT_TRUE(p->has_error());
ASSERT_TRUE(e.errored);
EXPECT_FALSE(e.matched);
ASSERT_EQ(e.value, nullptr);
EXPECT_EQ(p->error(), "1:2: missing expression");
EXPECT_EQ(p->error(), "1:1: invalid expression");
}
TEST_F(ParserImplTest, CoreLHS_MissingLHSExpression) {
@ -64,17 +76,17 @@ TEST_F(ParserImplTest, CoreLHS_MissingLHSExpression) {
auto e = p->core_lhs_expression();
ASSERT_TRUE(p->has_error());
ASSERT_TRUE(e.errored);
EXPECT_FALSE(e.matched);
ASSERT_EQ(e.value, nullptr);
EXPECT_EQ(p->error(), "1:2: missing expression");
EXPECT_EQ(p->error(), "1:1: invalid expression");
}
TEST_F(ParserImplTest, CoreLHS_Invalid) {
auto p = parser("1234");
auto e = p->core_lhs_expression();
ASSERT_TRUE(p->has_error());
ASSERT_TRUE(e.errored);
ASSERT_EQ(e.value, nullptr);
EXPECT_EQ(p->error(), "1:1: missing expression");
ASSERT_FALSE(p->has_error());
ASSERT_FALSE(e.errored);
EXPECT_FALSE(e.matched);
}
} // namespace

View File

@ -19,7 +19,7 @@ namespace {
TEST_F(ParserImplTest, IncrementDecrementStmt_Increment) {
auto p = parser("a++");
auto e = p->assignment_statement();
auto e = p->variable_updating_statement();
EXPECT_TRUE(e.matched);
EXPECT_FALSE(e.errored);
EXPECT_FALSE(p->has_error()) << p->error();
@ -38,7 +38,7 @@ TEST_F(ParserImplTest, IncrementDecrementStmt_Increment) {
TEST_F(ParserImplTest, IncrementDecrementStmt_Decrement) {
auto p = parser("a--");
auto e = p->assignment_statement();
auto e = p->variable_updating_statement();
EXPECT_TRUE(e.matched);
EXPECT_FALSE(e.errored);
EXPECT_FALSE(p->has_error()) << p->error();
@ -57,7 +57,7 @@ TEST_F(ParserImplTest, IncrementDecrementStmt_Decrement) {
TEST_F(ParserImplTest, IncrementDecrementStmt_Parenthesized) {
auto p = parser("(a)++");
auto e = p->assignment_statement();
auto e = p->variable_updating_statement();
EXPECT_TRUE(e.matched);
EXPECT_FALSE(e.errored);
EXPECT_FALSE(p->has_error()) << p->error();
@ -76,7 +76,7 @@ TEST_F(ParserImplTest, IncrementDecrementStmt_Parenthesized) {
TEST_F(ParserImplTest, IncrementDecrementStmt_ToMember) {
auto p = parser("a.b.c[2].d++");
auto e = p->assignment_statement();
auto e = p->variable_updating_statement();
EXPECT_TRUE(e.matched);
EXPECT_FALSE(e.errored);
EXPECT_FALSE(p->has_error()) << p->error();
@ -121,7 +121,7 @@ TEST_F(ParserImplTest, IncrementDecrementStmt_ToMember) {
TEST_F(ParserImplTest, IncrementDecrementStmt_InvalidLHS) {
auto p = parser("{}++");
auto e = p->assignment_statement();
auto e = p->variable_updating_statement();
EXPECT_FALSE(e.matched);
EXPECT_FALSE(e.errored);
EXPECT_FALSE(p->has_error()) << p->error();

View File

@ -22,15 +22,26 @@ TEST_F(ParserImplTest, LHSExpression_NoPrefix) {
auto e = p->lhs_expression();
ASSERT_FALSE(p->has_error()) << p->error();
ASSERT_FALSE(e.errored);
EXPECT_TRUE(e.matched);
ASSERT_NE(e.value, nullptr);
ASSERT_TRUE(e->Is<ast::IdentifierExpression>());
}
TEST_F(ParserImplTest, LHSExpression_NoMatch) {
auto p = parser("123");
auto e = p->lhs_expression();
ASSERT_FALSE(p->has_error()) << p->error();
ASSERT_FALSE(e.errored);
EXPECT_FALSE(e.matched);
ASSERT_EQ(e.value, nullptr);
}
TEST_F(ParserImplTest, LHSExpression_And) {
auto p = parser("&a");
auto e = p->lhs_expression();
ASSERT_FALSE(p->has_error()) << p->error();
ASSERT_FALSE(e.errored);
EXPECT_TRUE(e.matched);
ASSERT_NE(e.value, nullptr);
ASSERT_TRUE(e->Is<ast::UnaryOpExpression>());
@ -44,6 +55,7 @@ TEST_F(ParserImplTest, LHSExpression_Star) {
auto e = p->lhs_expression();
ASSERT_FALSE(p->has_error()) << p->error();
ASSERT_FALSE(e.errored);
EXPECT_TRUE(e.matched);
ASSERT_NE(e.value, nullptr);
ASSERT_TRUE(e->Is<ast::UnaryOpExpression>());
@ -52,11 +64,22 @@ TEST_F(ParserImplTest, LHSExpression_Star) {
EXPECT_TRUE(u->expr->Is<ast::IdentifierExpression>());
}
TEST_F(ParserImplTest, LHSExpression_InvalidCoreLHSExpr) {
auto p = parser("*123");
auto e = p->lhs_expression();
ASSERT_TRUE(p->has_error());
ASSERT_TRUE(e.errored);
EXPECT_FALSE(e.matched);
ASSERT_EQ(e.value, nullptr);
EXPECT_EQ(p->error(), "1:2: missing expression");
}
TEST_F(ParserImplTest, LHSExpression_Multiple) {
auto p = parser("*&**&&*a");
auto e = p->lhs_expression();
ASSERT_FALSE(p->has_error()) << p->error();
ASSERT_FALSE(e.errored);
EXPECT_TRUE(e.matched);
ASSERT_NE(e.value, nullptr);
std::vector<ast::UnaryOp> results = {ast::UnaryOp::kIndirection, ast::UnaryOp::kAddressOf,
@ -82,6 +105,7 @@ TEST_F(ParserImplTest, LHSExpression_PostfixExpression) {
auto e = p->lhs_expression();
ASSERT_FALSE(p->has_error()) << p->error();
ASSERT_FALSE(e.errored);
EXPECT_TRUE(e.matched);
ASSERT_NE(e.value, nullptr);
ASSERT_TRUE(e->Is<ast::MemberAccessorExpression>());
@ -100,5 +124,15 @@ TEST_F(ParserImplTest, LHSExpression_PostfixExpression) {
EXPECT_EQ(member_ident->symbol, p->builder().Symbols().Get("foo"));
}
TEST_F(ParserImplTest, LHSExpression_InvalidPostfixExpression) {
auto p = parser("*a.if");
auto e = p->lhs_expression();
ASSERT_TRUE(p->has_error());
ASSERT_TRUE(e.errored);
EXPECT_FALSE(e.matched);
ASSERT_EQ(e.value, nullptr);
EXPECT_EQ(p->error(), "1:4: expected identifier for member accessor");
}
} // namespace
} // namespace tint::reader::wgsl