wgsl: make if/switch parentheses optional
Fixed: tint:1424
Change-Id: Id135c74fbbba941cce7fb96970d3c23417bc14ec
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/84340
Kokoro: Kokoro <noreply+kokoro@google.com>
Auto-Submit: James Price <jrprice@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
diff --git a/docs/tint/origin-trial-changes.md b/docs/tint/origin-trial-changes.md
index fb48d13..8680fd3 100644
--- a/docs/tint/origin-trial-changes.md
+++ b/docs/tint/origin-trial-changes.md
@@ -2,6 +2,10 @@
## Changes for M102
+### New Features
+
+* Parentheses are no longer required around expressions for if and switch statements [tint:1424](crbug.com/tint/1424)
+
### Breaking changes
* The `@block` attribute has been removed. [tint:1324](crbug.com/tint/1324)
diff --git a/src/tint/reader/wgsl/parser_impl.cc b/src/tint/reader/wgsl/parser_impl.cc
index 20e8d38..d2150b1 100644
--- a/src/tint/reader/wgsl/parser_impl.cc
+++ b/src/tint/reader/wgsl/parser_impl.cc
@@ -1650,15 +1650,18 @@
}
// if_stmt
-// : IF paren_rhs_stmt body_stmt ( ELSE else_stmts ) ?
+// : IF expression compound_stmt ( ELSE else_stmts ) ?
Maybe<const ast::IfStatement*> ParserImpl::if_stmt() {
Source source;
if (!match(Token::Type::kIf, &source))
return Failure::kNoMatch;
- auto condition = expect_paren_rhs_stmt();
+ auto condition = logical_or_expression();
if (condition.errored)
return Failure::kErrored;
+ if (!condition.matched) {
+ return add_error(peek(), "unable to parse condition expression");
+ }
auto body = expect_body_stmt();
if (body.errored)
@@ -1690,10 +1693,14 @@
const ast::Expression* cond = nullptr;
if (else_if) {
- auto condition = expect_paren_rhs_stmt();
+ auto condition = logical_or_expression();
if (condition.errored) {
return Failure::kErrored;
}
+ if (!condition.matched) {
+ return add_error(peek(), "unable to parse condition expression");
+ }
+
cond = condition.value;
}
@@ -1716,9 +1723,12 @@
if (!match(Token::Type::kSwitch, &source))
return Failure::kNoMatch;
- auto condition = expect_paren_rhs_stmt();
+ auto condition = logical_or_expression();
if (condition.errored)
return Failure::kErrored;
+ if (!condition.matched) {
+ return add_error(peek(), "unable to parse selector expression");
+ }
auto body = expect_brace_block("switch statement",
[&]() -> Expect<ast::CaseStatementList> {
diff --git a/src/tint/reader/wgsl/parser_impl_error_msg_test.cc b/src/tint/reader/wgsl/parser_impl_error_msg_test.cc
index f68c973..021b266 100644
--- a/src/tint/reader/wgsl/parser_impl_error_msg_test.cc
+++ b/src/tint/reader/wgsl/parser_impl_error_msg_test.cc
@@ -1033,13 +1033,6 @@
)");
}
-TEST_F(ParserImplErrorTest, IfStmtMissingLParen) {
- EXPECT("fn f() { if true) {} }", R"(test.wgsl:1:13 error: expected '('
-fn f() { if true) {} }
- ^^^^
-)");
-}
-
TEST_F(ParserImplErrorTest, IfStmtMissingRParen) {
EXPECT("fn f() { if (true {} }", R"(test.wgsl:1:19 error: expected ')'
fn f() { if (true {} }
diff --git a/src/tint/reader/wgsl/parser_impl_if_stmt_test.cc b/src/tint/reader/wgsl/parser_impl_if_stmt_test.cc
index 8f97550..2a57452 100644
--- a/src/tint/reader/wgsl/parser_impl_if_stmt_test.cc
+++ b/src/tint/reader/wgsl/parser_impl_if_stmt_test.cc
@@ -20,7 +20,7 @@
namespace {
TEST_F(ParserImplTest, IfStmt) {
- auto p = parser("if (a == 4) { a = b; c = d; }");
+ auto p = parser("if a == 4 { a = b; c = d; }");
auto e = p->if_stmt();
EXPECT_TRUE(e.matched);
EXPECT_FALSE(e.errored);
@@ -35,8 +35,30 @@
}
TEST_F(ParserImplTest, IfStmt_WithElse) {
- auto p =
- parser("if (a == 4) { a = b; c = d; } else if(c) { d = 2; } else {}");
+ auto p = parser("if a == 4 { a = b; c = d; } else if(c) { d = 2; } else {}");
+ auto e = p->if_stmt();
+ EXPECT_TRUE(e.matched);
+ EXPECT_FALSE(e.errored);
+ EXPECT_FALSE(p->has_error()) << p->error();
+ ASSERT_NE(e.value, nullptr);
+
+ ASSERT_TRUE(e->Is<ast::IfStatement>());
+ ASSERT_NE(e->condition, nullptr);
+ ASSERT_TRUE(e->condition->Is<ast::BinaryExpression>());
+ EXPECT_EQ(e->body->statements.size(), 2u);
+
+ ASSERT_EQ(e->else_statements.size(), 2u);
+ ASSERT_NE(e->else_statements[0]->condition, nullptr);
+ ASSERT_TRUE(
+ e->else_statements[0]->condition->Is<ast::IdentifierExpression>());
+ EXPECT_EQ(e->else_statements[0]->body->statements.size(), 1u);
+
+ ASSERT_EQ(e->else_statements[1]->condition, nullptr);
+ EXPECT_EQ(e->else_statements[1]->body->statements.size(), 0u);
+}
+
+TEST_F(ParserImplTest, IfStmt_WithElse_WithParens) {
+ auto p = parser("if(a==4) { a = b; c = d; } else if(c) { d = 2; } else {}");
auto e = p->if_stmt();
EXPECT_TRUE(e.matched);
EXPECT_FALSE(e.errored);
@@ -59,13 +81,13 @@
}
TEST_F(ParserImplTest, IfStmt_InvalidCondition) {
- auto p = parser("if (a = 3) {}");
+ auto p = parser("if a = 3 {}");
auto e = p->if_stmt();
EXPECT_FALSE(e.matched);
EXPECT_TRUE(e.errored);
EXPECT_EQ(e.value, nullptr);
EXPECT_TRUE(p->has_error());
- EXPECT_EQ(p->error(), "1:7: expected ')'");
+ EXPECT_EQ(p->error(), "1:6: expected '{'");
}
TEST_F(ParserImplTest, IfStmt_MissingCondition) {
@@ -75,47 +97,47 @@
EXPECT_TRUE(e.errored);
EXPECT_EQ(e.value, nullptr);
EXPECT_TRUE(p->has_error());
- EXPECT_EQ(p->error(), "1:4: expected '('");
+ EXPECT_EQ(p->error(), "1:4: unable to parse condition expression");
}
TEST_F(ParserImplTest, IfStmt_InvalidBody) {
- auto p = parser("if (a) { fn main() {}}");
+ auto p = parser("if a { fn main() {}}");
auto e = p->if_stmt();
EXPECT_FALSE(e.matched);
EXPECT_TRUE(e.errored);
EXPECT_EQ(e.value, nullptr);
EXPECT_TRUE(p->has_error());
- EXPECT_EQ(p->error(), "1:10: expected '}'");
+ EXPECT_EQ(p->error(), "1:8: expected '}'");
}
TEST_F(ParserImplTest, IfStmt_MissingBody) {
- auto p = parser("if (a)");
+ auto p = parser("if a");
auto e = p->if_stmt();
EXPECT_FALSE(e.matched);
EXPECT_TRUE(e.errored);
EXPECT_EQ(e.value, nullptr);
EXPECT_TRUE(p->has_error());
- EXPECT_EQ(p->error(), "1:7: expected '{'");
+ EXPECT_EQ(p->error(), "1:5: expected '{'");
}
TEST_F(ParserImplTest, IfStmt_InvalidElseif) {
- auto p = parser("if (a) {} else if (a) { fn main() -> a{}}");
+ auto p = parser("if a {} else if a { fn main() -> a{}}");
auto e = p->if_stmt();
EXPECT_FALSE(e.matched);
EXPECT_TRUE(e.errored);
EXPECT_EQ(e.value, nullptr);
EXPECT_TRUE(p->has_error());
- EXPECT_EQ(p->error(), "1:25: expected '}'");
+ EXPECT_EQ(p->error(), "1:21: expected '}'");
}
TEST_F(ParserImplTest, IfStmt_InvalidElse) {
- auto p = parser("if (a) {} else { fn main() -> a{}}");
+ auto p = parser("if a {} else { fn main() -> a{}}");
auto e = p->if_stmt();
EXPECT_FALSE(e.matched);
EXPECT_TRUE(e.errored);
EXPECT_EQ(e.value, nullptr);
EXPECT_TRUE(p->has_error());
- EXPECT_EQ(p->error(), "1:18: expected '}'");
+ EXPECT_EQ(p->error(), "1:16: expected '}'");
}
} // namespace
diff --git a/src/tint/reader/wgsl/parser_impl_switch_stmt_test.cc b/src/tint/reader/wgsl/parser_impl_switch_stmt_test.cc
index 1d8fb2f..f2e97de 100644
--- a/src/tint/reader/wgsl/parser_impl_switch_stmt_test.cc
+++ b/src/tint/reader/wgsl/parser_impl_switch_stmt_test.cc
@@ -20,7 +20,7 @@
namespace {
TEST_F(ParserImplTest, SwitchStmt_WithoutDefault) {
- auto p = parser(R"(switch(a) {
+ auto p = parser(R"(switch a {
case 1: {}
case 2: {}
})");
@@ -36,7 +36,7 @@
}
TEST_F(ParserImplTest, SwitchStmt_Empty) {
- auto p = parser("switch(a) { }");
+ auto p = parser("switch a { }");
auto e = p->switch_stmt();
EXPECT_TRUE(e.matched);
EXPECT_FALSE(e.errored);
@@ -47,7 +47,7 @@
}
TEST_F(ParserImplTest, SwitchStmt_DefaultInMiddle) {
- auto p = parser(R"(switch(a) {
+ auto p = parser(R"(switch a {
case 1: {}
default: {}
case 2: {}
@@ -65,14 +65,25 @@
ASSERT_FALSE(e->body[2]->IsDefault());
}
+TEST_F(ParserImplTest, SwitchStmt_WithParens) {
+ auto p = parser("switch(a+b) { }");
+ auto e = p->switch_stmt();
+ EXPECT_TRUE(e.matched);
+ EXPECT_FALSE(e.errored);
+ EXPECT_FALSE(p->has_error()) << p->error();
+ ASSERT_NE(e.value, nullptr);
+ ASSERT_TRUE(e->Is<ast::SwitchStatement>());
+ ASSERT_EQ(e->body.size(), 0u);
+}
+
TEST_F(ParserImplTest, SwitchStmt_InvalidExpression) {
- auto p = parser("switch(a=b) {}");
+ auto p = parser("switch a=b {}");
auto e = p->switch_stmt();
EXPECT_FALSE(e.matched);
EXPECT_TRUE(e.errored);
EXPECT_EQ(e.value, nullptr);
EXPECT_TRUE(p->has_error());
- EXPECT_EQ(p->error(), "1:9: expected ')'");
+ EXPECT_EQ(p->error(), "1:9: expected '{' for switch statement");
}
TEST_F(ParserImplTest, SwitchStmt_MissingExpression) {
@@ -82,31 +93,31 @@
EXPECT_TRUE(e.errored);
EXPECT_EQ(e.value, nullptr);
EXPECT_TRUE(p->has_error());
- EXPECT_EQ(p->error(), "1:8: expected '('");
+ EXPECT_EQ(p->error(), "1:8: unable to parse selector expression");
}
TEST_F(ParserImplTest, SwitchStmt_MissingBracketLeft) {
- auto p = parser("switch(a) }");
+ auto p = parser("switch a }");
auto e = p->switch_stmt();
EXPECT_FALSE(e.matched);
EXPECT_TRUE(e.errored);
EXPECT_EQ(e.value, nullptr);
EXPECT_TRUE(p->has_error());
- EXPECT_EQ(p->error(), "1:11: expected '{' for switch statement");
+ EXPECT_EQ(p->error(), "1:10: expected '{' for switch statement");
}
TEST_F(ParserImplTest, SwitchStmt_MissingBracketRight) {
- auto p = parser("switch(a) {");
+ auto p = parser("switch a {");
auto e = p->switch_stmt();
EXPECT_FALSE(e.matched);
EXPECT_TRUE(e.errored);
EXPECT_EQ(e.value, nullptr);
EXPECT_TRUE(p->has_error());
- EXPECT_EQ(p->error(), "1:12: expected '}' for switch statement");
+ EXPECT_EQ(p->error(), "1:11: expected '}' for switch statement");
}
TEST_F(ParserImplTest, SwitchStmt_InvalidBody) {
- auto p = parser(R"(switch(a) {
+ auto p = parser(R"(switch a {
case: {}
})");
auto e = p->switch_stmt();
