wgsl: Replace 'elseif' with 'else if'

Bug: tint:1289
Change-Id: I72432391e60cf5ff173aa51a6d4a2bc8ef58fbf2
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/75240
Reviewed-by: James Price <jrprice@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>

docs/origin-trial-changes.md

@@ -4,7 +4,10 @@
 
 ### Deprecated Features
 
-* The `[[block]]` attribute has been deprecated and will be removed in M102. [tint:1324](https://crbug.com/tint/1324)
+The following features have been deprecated and will be removed in M102:
+
+* The `[[block]]` attribute has been deprecated. [tint:1324](https://crbug.com/tint/1324)
+* `elseif` has been replaced with `else if` [tint:1289](https://crbug.com/tint/1289)
 
 ### New Features
 

src/CMakeLists.txt

@@ -826,7 +826,6 @@ if(${TINT_BUILD_TESTS})
       reader/wgsl/parser_impl_continuing_stmt_test.cc
       reader/wgsl/parser_impl_depth_texture_type_test.cc
       reader/wgsl/parser_impl_external_texture_type_test.cc
-      reader/wgsl/parser_impl_else_stmt_test.cc
       reader/wgsl/parser_impl_elseif_stmt_test.cc
       reader/wgsl/parser_impl_equality_expression_test.cc
       reader/wgsl/parser_impl_error_msg_test.cc

src/reader/wgsl/parser_impl.cc

@@ -1540,7 +1540,7 @@ Expect<ast::Builtin> ParserImpl::expect_builtin() {
 }
 
 // body_stmt
-//   : BRACKET_LEFT statements BRACKET_RIGHT
+//   : BRACE_LEFT statements BRACE_RIGHT
 Expect<ast::BlockStatement*> ParserImpl::expect_body_stmt() {
   return expect_brace_block("", [&]() -> Expect<ast::BlockStatement*> {
     auto stmts = expect_statements();
@@ -1791,7 +1791,7 @@ Maybe<const ast::VariableDeclStatement*> ParserImpl::variable_stmt() {
 }
 
 // if_stmt
-//   : IF paren_rhs_stmt body_stmt elseif_stmt? else_stmt?
+//   : IF paren_rhs_stmt body_stmt ( ELSE else_stmts ) ?
 Maybe<const ast::IfStatement*> ParserImpl::if_stmt() {
   Source source;
   if (!match(Token::Type::kIf, &source))
@@ -1805,59 +1805,52 @@ Maybe<const ast::IfStatement*> ParserImpl::if_stmt() {
   if (body.errored)
     return Failure::kErrored;
 
-  auto elseif = elseif_stmt();
+  auto el = else_stmts();
-  if (elseif.errored)
+  if (el.errored) {
     return Failure::kErrored;
-
-  auto el = else_stmt();
-  if (el.errored)
-    return Failure::kErrored;
-  if (el.matched)
-    elseif.value.push_back(el.value);
-
-  return create<ast::IfStatement>(source, condition.value, body.value,
-                                  elseif.value);
   }
 
-// elseif_stmt
+  return create<ast::IfStatement>(source, condition.value, body.value,
-//   : ELSE_IF paren_rhs_stmt body_stmt elseif_stmt?
+                                  std::move(el.value));
-Maybe<ast::ElseStatementList> ParserImpl::elseif_stmt() {
+}
-  Source source;
-  if (!match(Token::Type::kElseIf, &source))
-    return Failure::kNoMatch;
 
-  ast::ElseStatementList ret;
+// else_stmts
+//  : body_stmt
+//  | if_stmt
+Expect<ast::ElseStatementList> ParserImpl::else_stmts() {
+  ast::ElseStatementList stmts;
   while (continue_parsing()) {
-    auto condition = expect_paren_rhs_stmt();
+    Source start;
-    if (condition.errored)
-      return Failure::kErrored;
 
-    auto body = expect_body_stmt();
+    bool else_if = false;
-    if (body.errored)
+    if (match(Token::Type::kElse, &start)) {
-      return Failure::kErrored;
+      else_if = match(Token::Type::kIf);
-
+    } else if (match(Token::Type::kElseIf, &start)) {
-    ret.push_back(
+      deprecated(start, "'elseif' is now 'else if'");
-        create<ast::ElseStatement>(source, condition.value, body.value));
+      else_if = true;
-
+    } else {
-    if (!match(Token::Type::kElseIf, &source))
       break;
     }
 
-  return ret;
+    const ast::Expression* cond = nullptr;
+    if (else_if) {
+      auto condition = expect_paren_rhs_stmt();
+      if (condition.errored) {
+        return Failure::kErrored;
+      }
+      cond = condition.value;
     }
 
-// else_stmt
-//   : ELSE body_stmt
-Maybe<const ast::ElseStatement*> ParserImpl::else_stmt() {
-  Source source;
-  if (!match(Token::Type::kElse, &source))
-    return Failure::kNoMatch;
-
     auto body = expect_body_stmt();
-  if (body.errored)
+    if (body.errored) {
       return Failure::kErrored;
+    }
 
-  return create<ast::ElseStatement>(source, nullptr, body.value);
+    Source source = make_source_range_from(start);
+    stmts.emplace_back(create<ast::ElseStatement>(source, cond, body.value));
+  }
+
+  return stmts;
 }
 
 // switch_stmt

src/reader/wgsl/parser_impl.h

@@ -522,12 +522,9 @@ class ParserImpl {
   /// Parses a `if_stmt` grammar element
   /// @returns the parsed statement or nullptr
   Maybe<const ast::IfStatement*> if_stmt();
-  /// Parses a `elseif_stmt` grammar element
+  /// Parses a list of `else_stmt` grammar elements
-  /// @returns the parsed elements
-  Maybe<ast::ElseStatementList> elseif_stmt();
-  /// Parses a `else_stmt` grammar element
   /// @returns the parsed statement or nullptr
-  Maybe<const ast::ElseStatement*> else_stmt();
+  Expect<ast::ElseStatementList> else_stmts();
   /// Parses a `switch_stmt` grammar element
   /// @returns the parsed statement or nullptr
   Maybe<const ast::SwitchStatement*> switch_stmt();

src/reader/wgsl/parser_impl_else_stmt_test.cc

@@ -1,57 +0,0 @@
-// Copyright 2020 The Tint Authors.
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//     http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-#include "src/reader/wgsl/parser_impl_test_helper.h"
-
-namespace tint {
-namespace reader {
-namespace wgsl {
-namespace {
-
-TEST_F(ParserImplTest, ElseStmt) {
-  auto p = parser("else { a = b; c = d; }");
-  auto e = p->else_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::ElseStatement>());
-  ASSERT_EQ(e->condition, nullptr);
-  EXPECT_EQ(e->body->statements.size(), 2u);
-}
-
-TEST_F(ParserImplTest, ElseStmt_InvalidBody) {
-  auto p = parser("else { fn main() {}}");
-  auto e = p->else_stmt();
-  EXPECT_FALSE(e.matched);
-  EXPECT_TRUE(e.errored);
-  EXPECT_EQ(e.value, nullptr);
-  EXPECT_TRUE(p->has_error());
-  EXPECT_EQ(p->error(), "1:8: expected '}'");
-}
-
-TEST_F(ParserImplTest, ElseStmt_MissingBody) {
-  auto p = parser("else");
-  auto e = p->else_stmt();
-  EXPECT_FALSE(e.matched);
-  EXPECT_TRUE(e.errored);
-  EXPECT_EQ(e.value, nullptr);
-  EXPECT_TRUE(p->has_error());
-  EXPECT_EQ(p->error(), "1:5: expected '{'");
-}
-
-}  // namespace
-}  // namespace wgsl
-}  // namespace reader
-}  // namespace tint

src/reader/wgsl/parser_impl_elseif_stmt_test.cc

@@ -19,11 +19,9 @@ namespace reader {
 namespace wgsl {
 namespace {
 
-TEST_F(ParserImplTest, ElseIfStmt) {
+TEST_F(ParserImplTest, ElseStmts) {
   auto p = parser("else if (a == 4) { a = b; c = d; }");
-  auto e = p->elseif_stmt();
+  auto e = p->else_stmts();
-  EXPECT_TRUE(e.matched);
-  EXPECT_FALSE(e.errored);
   EXPECT_FALSE(p->has_error()) << p->error();
   ASSERT_EQ(e.value.size(), 1u);
 
@@ -33,11 +31,9 @@ TEST_F(ParserImplTest, ElseIfStmt) {
   EXPECT_EQ(e.value[0]->body->statements.size(), 2u);
 }
 
-TEST_F(ParserImplTest, ElseIfStmt_Multiple) {
+TEST_F(ParserImplTest, ElseStmts_Multiple) {
   auto p = parser("else if (a == 4) { a = b; c = d; } else if(c) { d = 2; }");
-  auto e = p->elseif_stmt();
+  auto e = p->else_stmts();
-  EXPECT_TRUE(e.matched);
-  EXPECT_FALSE(e.errored);
   EXPECT_FALSE(p->has_error()) << p->error();
   ASSERT_EQ(e.value.size(), 2u);
 
@@ -52,22 +48,82 @@ TEST_F(ParserImplTest, ElseIfStmt_Multiple) {
   EXPECT_EQ(e.value[1]->body->statements.size(), 1u);
 }
 
-TEST_F(ParserImplTest, ElseIfStmt_InvalidBody) {
+TEST_F(ParserImplTest, ElseStmts_InvalidBody) {
   auto p = parser("else if (true) { fn main() {}}");
-  auto e = p->elseif_stmt();
+  auto e = p->else_stmts();
-  EXPECT_FALSE(e.matched);
   EXPECT_TRUE(e.errored);
   EXPECT_TRUE(p->has_error());
-  EXPECT_EQ(p->error(), "1:17: expected '}'");
+  EXPECT_EQ(p->error(), "1:18: expected '}'");
 }
 
-TEST_F(ParserImplTest, ElseIfStmt_MissingBody) {
+TEST_F(ParserImplTest, ElseStmts_MissingBody) {
   auto p = parser("else if (true)");
-  auto e = p->elseif_stmt();
+  auto e = p->else_stmts();
-  EXPECT_FALSE(e.matched);
   EXPECT_TRUE(e.errored);
   EXPECT_TRUE(p->has_error());
-  EXPECT_EQ(p->error(), "1:14: expected '{'");
+  EXPECT_EQ(p->error(), "1:15: expected '{'");
+}
+
+////////////////////////////////////////////////////////////////////////////////
+// The tests below use the deprecated 'elseif' syntax
+////////////////////////////////////////////////////////////////////////////////
+
+TEST_F(ParserImplTest, DEPRECATED_ElseStmts) {
+  auto p = parser("elseif (a == 4) { a = b; c = d; }");
+  auto e = p->else_stmts();
+  EXPECT_FALSE(p->has_error()) << p->error();
+  EXPECT_EQ(
+      p->error(),
+      R"(1:1: use of deprecated language feature: 'elseif' is now 'else if')");
+  ASSERT_EQ(e.value.size(), 1u);
+
+  ASSERT_TRUE(e.value[0]->Is<ast::ElseStatement>());
+  ASSERT_NE(e.value[0]->condition, nullptr);
+  ASSERT_TRUE(e.value[0]->condition->Is<ast::BinaryExpression>());
+  EXPECT_EQ(e.value[0]->body->statements.size(), 2u);
+}
+
+TEST_F(ParserImplTest, DEPRECATED_ElseStmts_Multiple) {
+  auto p = parser("elseif (a == 4) { a = b; c = d; } elseif(c) { d = 2; }");
+  auto e = p->else_stmts();
+  EXPECT_FALSE(p->has_error()) << p->error();
+  EXPECT_EQ(
+      p->error(),
+      R"(1:1: use of deprecated language feature: 'elseif' is now 'else if'
+1:35: use of deprecated language feature: 'elseif' is now 'else if')");
+  ASSERT_EQ(e.value.size(), 2u);
+
+  ASSERT_TRUE(e.value[0]->Is<ast::ElseStatement>());
+  ASSERT_NE(e.value[0]->condition, nullptr);
+  ASSERT_TRUE(e.value[0]->condition->Is<ast::BinaryExpression>());
+  EXPECT_EQ(e.value[0]->body->statements.size(), 2u);
+
+  ASSERT_TRUE(e.value[1]->Is<ast::ElseStatement>());
+  ASSERT_NE(e.value[1]->condition, nullptr);
+  ASSERT_TRUE(e.value[1]->condition->Is<ast::IdentifierExpression>());
+  EXPECT_EQ(e.value[1]->body->statements.size(), 1u);
+}
+
+TEST_F(ParserImplTest, DEPRECATED_ElseStmts_InvalidBody) {
+  auto p = parser("elseif (true) { fn main() {}}");
+  auto e = p->else_stmts();
+  EXPECT_TRUE(e.errored);
+  EXPECT_TRUE(p->has_error());
+  EXPECT_EQ(
+      p->error(),
+      R"(1:1: use of deprecated language feature: 'elseif' is now 'else if'
+1:17: expected '}')");
+}
+
+TEST_F(ParserImplTest, DEPRECATED_ElseStmts_MissingBody) {
+  auto p = parser("elseif (true)");
+  auto e = p->else_stmts();
+  EXPECT_TRUE(e.errored);
+  EXPECT_TRUE(p->has_error());
+  EXPECT_EQ(
+      p->error(),
+      R"(1:1: use of deprecated language feature: 'elseif' is now 'else if'
+1:14: expected '{')");
 }
 
 }  // namespace

src/reader/wgsl/parser_impl_if_stmt_test.cc

@@ -35,7 +35,8 @@ TEST_F(ParserImplTest, IfStmt) {
 }
 
 TEST_F(ParserImplTest, IfStmt_WithElse) {
-  auto p = parser("if (a == 4) { a = b; c = d; } elseif(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);
@@ -104,7 +105,7 @@ TEST_F(ParserImplTest, IfStmt_InvalidElseif) {
   EXPECT_TRUE(e.errored);
   EXPECT_EQ(e.value, nullptr);
   EXPECT_TRUE(p->has_error());
-  EXPECT_EQ(p->error(), "1:24: expected '}'");
+  EXPECT_EQ(p->error(), "1:25: expected '}'");
 }
 
 TEST_F(ParserImplTest, IfStmt_InvalidElse) {

test/BUILD.gn

@@ -462,7 +462,6 @@ tint_unittests_source_set("tint_unittests_wgsl_reader_src") {
     "../src/reader/wgsl/parser_impl_continue_stmt_test.cc",
     "../src/reader/wgsl/parser_impl_continuing_stmt_test.cc",
     "../src/reader/wgsl/parser_impl_depth_texture_type_test.cc",
-    "../src/reader/wgsl/parser_impl_else_stmt_test.cc",
     "../src/reader/wgsl/parser_impl_elseif_stmt_test.cc",
     "../src/reader/wgsl/parser_impl_equality_expression_test.cc",
     "../src/reader/wgsl/parser_impl_error_msg_test.cc",