From e6c7fd711071ed6f1fb31f27f8f0908323fbce1e Mon Sep 17 00:00:00 2001 From: James Price Date: Tue, 5 Apr 2022 21:01:09 +0000 Subject: [PATCH] wgsl: Make colon optional for case statements Fixed: tint:1485 Change-Id: I76af5a284cb455170bed27e852a4bab47c5dc491 Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/85525 Reviewed-by: Ben Clayton Kokoro: Kokoro --- docs/tint/origin-trial-changes.md | 1 + src/tint/reader/wgsl/parser_impl.cc | 11 ++- .../reader/wgsl/parser_impl_error_msg_test.cc | 8 -- .../wgsl/parser_impl_switch_body_test.cc | 87 +++++++++++++++---- 4 files changed, 74 insertions(+), 33 deletions(-) diff --git a/docs/tint/origin-trial-changes.md b/docs/tint/origin-trial-changes.md index 3bb1e4e75a..3f3c61d824 100644 --- a/docs/tint/origin-trial-changes.md +++ b/docs/tint/origin-trial-changes.md @@ -6,6 +6,7 @@ * Parentheses are no longer required around expressions for if and switch statements [tint:1424](crbug.com/tint/1424) * Compound assignment statements are now supported. [tint:1325](https://crbug.com/tint/1325) +* The colon in case statements is now optional. [tint:1485](crbug.com/tint/1485) ### Breaking changes diff --git a/src/tint/reader/wgsl/parser_impl.cc b/src/tint/reader/wgsl/parser_impl.cc index 03d6ca3227..2798c77d8b 100644 --- a/src/tint/reader/wgsl/parser_impl.cc +++ b/src/tint/reader/wgsl/parser_impl.cc @@ -1761,8 +1761,8 @@ Maybe ParserImpl::switch_stmt() { } // switch_body -// : CASE case_selectors COLON BRACKET_LEFT case_body BRACKET_RIGHT -// | DEFAULT COLON BRACKET_LEFT case_body BRACKET_RIGHT +// : CASE case_selectors COLON? BRACKET_LEFT case_body BRACKET_RIGHT +// | DEFAULT COLON? BRACKET_LEFT case_body BRACKET_RIGHT Maybe ParserImpl::switch_body() { if (!peek_is(Token::Type::kCase) && !peek_is(Token::Type::kDefault)) return Failure::kNoMatch; @@ -1779,11 +1779,10 @@ Maybe ParserImpl::switch_body() { selector_list = std::move(selectors.value); } + // Consume the optional colon if present. + match(Token::Type::kColon); + const char* use = "case statement"; - - if (!expect(use, Token::Type::kColon)) - return Failure::kErrored; - auto body = expect_brace_block(use, [&] { return case_body(); }); if (body.errored) 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 8610ae561b..f1444ebd00 100644 --- a/src/tint/reader/wgsl/parser_impl_error_msg_test.cc +++ b/src/tint/reader/wgsl/parser_impl_error_msg_test.cc @@ -1174,14 +1174,6 @@ fn f() { switch(1) { case false: } } )"); } -TEST_F(ParserImplErrorTest, SwitchStmtCaseMissingColon) { - EXPECT("fn f() { switch(1) { case 1 {} } }", - R"(test.wgsl:1:29 error: expected ':' for case statement -fn f() { switch(1) { case 1 {} } } - ^ -)"); -} - TEST_F(ParserImplErrorTest, SwitchStmtCaseMissingLBrace) { EXPECT("fn f() { switch(1) { case 1: } }", R"(test.wgsl:1:30 error: expected '{' for case statement diff --git a/src/tint/reader/wgsl/parser_impl_switch_body_test.cc b/src/tint/reader/wgsl/parser_impl_switch_body_test.cc index 3f9ffc9037..e983cfab48 100644 --- a/src/tint/reader/wgsl/parser_impl_switch_body_test.cc +++ b/src/tint/reader/wgsl/parser_impl_switch_body_test.cc @@ -20,6 +20,22 @@ namespace wgsl { namespace { TEST_F(ParserImplTest, SwitchBody_Case) { + auto p = parser("case 1 { a = 4; }"); + 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()); + EXPECT_FALSE(e->IsDefault()); + auto* stmt = e->As(); + ASSERT_EQ(stmt->selectors.size(), 1u); + EXPECT_EQ(stmt->selectors[0]->ValueAsU32(), 1u); + ASSERT_EQ(e->body->statements.size(), 1u); + EXPECT_TRUE(e->body->statements[0]->Is()); +} + +TEST_F(ParserImplTest, SwitchBody_Case_WithColon) { auto p = parser("case 1: { a = 4; }"); auto e = p->switch_body(); EXPECT_FALSE(p->has_error()) << p->error(); @@ -36,6 +52,21 @@ TEST_F(ParserImplTest, SwitchBody_Case) { } 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()); + EXPECT_FALSE(e->IsDefault()); + auto* stmt = e->As(); + ASSERT_EQ(stmt->selectors.size(), 2u); + EXPECT_EQ(stmt->selectors[0]->ValueAsU32(), 1u); + EXPECT_EQ(stmt->selectors[1]->ValueAsU32(), 2u); +} + +TEST_F(ParserImplTest, SwitchBody_Case_TrailingComma_WithColon) { auto p = parser("case 1, 2,: { }"); auto e = p->switch_body(); EXPECT_FALSE(p->has_error()) << p->error(); @@ -80,17 +111,17 @@ TEST_F(ParserImplTest, SwitchBody_Case_MissingConstLiteral) { EXPECT_EQ(p->error(), "1:5: unable to parse case selectors"); } -TEST_F(ParserImplTest, SwitchBody_Case_MissingColon) { - auto p = parser("case 1 { a = 4; }"); +TEST_F(ParserImplTest, SwitchBody_Case_MissingBracketLeft) { + auto p = parser("case 1 a = 4; }"); 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:8: expected ':' for case statement"); + EXPECT_EQ(p->error(), "1:8: expected '{' for case statement"); } -TEST_F(ParserImplTest, SwitchBody_Case_MissingBracketLeft) { +TEST_F(ParserImplTest, SwitchBody_Case_MissingBracketLeft_WithColon) { auto p = parser("case 1: a = 4; }"); auto e = p->switch_body(); EXPECT_TRUE(p->has_error()); @@ -121,6 +152,21 @@ TEST_F(ParserImplTest, SwitchBody_Case_InvalidCaseBody) { } TEST_F(ParserImplTest, SwitchBody_Case_MultipleSelectors) { + 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()); + EXPECT_FALSE(e->IsDefault()); + ASSERT_EQ(e->body->statements.size(), 0u); + ASSERT_EQ(e->selectors.size(), 2u); + ASSERT_EQ(e->selectors[0]->ValueAsI32(), 1); + ASSERT_EQ(e->selectors[1]->ValueAsI32(), 2); +} + +TEST_F(ParserImplTest, SwitchBody_Case_MultipleSelectors_WithColon) { auto p = parser("case 1, 2: { }"); auto e = p->switch_body(); EXPECT_FALSE(p->has_error()) << p->error(); @@ -135,16 +181,6 @@ TEST_F(ParserImplTest, SwitchBody_Case_MultipleSelectors) { ASSERT_EQ(e->selectors[1]->ValueAsI32(), 2); } -TEST_F(ParserImplTest, SwitchBody_Case_MultipleSelectorsMissingColon) { - 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: expected ':' for case statement"); -} - TEST_F(ParserImplTest, SwitchBody_Case_MultipleSelectorsMissingComma) { auto p = parser("case 1 2: { }"); auto e = p->switch_body(); @@ -152,7 +188,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 ':' for case statement"); + EXPECT_EQ(p->error(), "1:8: expected '{' for case statement"); } TEST_F(ParserImplTest, SwitchBody_Case_MultipleSelectorsStartsWithComma) { @@ -166,6 +202,19 @@ TEST_F(ParserImplTest, SwitchBody_Case_MultipleSelectorsStartsWithComma) { } TEST_F(ParserImplTest, SwitchBody_Default) { + auto p = parser("default { a = 4; }"); + 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()); + EXPECT_TRUE(e->IsDefault()); + ASSERT_EQ(e->body->statements.size(), 1u); + EXPECT_TRUE(e->body->statements[0]->Is()); +} + +TEST_F(ParserImplTest, SwitchBody_Default_WithColon) { auto p = parser("default: { a = 4; }"); auto e = p->switch_body(); EXPECT_FALSE(p->has_error()) << p->error(); @@ -178,17 +227,17 @@ TEST_F(ParserImplTest, SwitchBody_Default) { EXPECT_TRUE(e->body->statements[0]->Is()); } -TEST_F(ParserImplTest, SwitchBody_Default_MissingColon) { - auto p = parser("default { a = 4; }"); +TEST_F(ParserImplTest, SwitchBody_Default_MissingBracketLeft) { + auto p = parser("default a = 4; }"); 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:9: expected ':' for case statement"); + EXPECT_EQ(p->error(), "1:9: expected '{' for case statement"); } -TEST_F(ParserImplTest, SwitchBody_Default_MissingBracketLeft) { +TEST_F(ParserImplTest, SwitchBody_Default_MissingBracketLeft_WithColon) { auto p = parser("default: a = 4; }"); auto e = p->switch_body(); EXPECT_TRUE(p->has_error());