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();
-  if (elseif.errored)
+  auto el = else_stmts();
+  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
-//   : ELSE_IF paren_rhs_stmt body_stmt elseif_stmt?
-Maybe<ast::ElseStatementList> ParserImpl::elseif_stmt() {
-  Source source;
-  if (!match(Token::Type::kElseIf, &source))
-    return Failure::kNoMatch;
-
-  ast::ElseStatementList ret;
-  while (continue_parsing()) {
-    auto condition = expect_paren_rhs_stmt();
-    if (condition.errored)
-      return Failure::kErrored;
-
-    auto body = expect_body_stmt();
-    if (body.errored)
-      return Failure::kErrored;
-
-    ret.push_back(
-        create<ast::ElseStatement>(source, condition.value, body.value));
-
-    if (!match(Token::Type::kElseIf, &source))
-      break;
   }
 
-  return ret;
+  return create<ast::IfStatement>(source, condition.value, body.value,
+                                  std::move(el.value));
 }
 
-// else_stmt
-//   : ELSE body_stmt
-Maybe<const ast::ElseStatement*> ParserImpl::else_stmt() {
-  Source source;
-  if (!match(Token::Type::kElse, &source))
-    return Failure::kNoMatch;
+// else_stmts
+//  : body_stmt
+//  | if_stmt
+Expect<ast::ElseStatementList> ParserImpl::else_stmts() {
+  ast::ElseStatementList stmts;
+  while (continue_parsing()) {
+    Source start;
 
-  auto body = expect_body_stmt();
-  if (body.errored)
-    return Failure::kErrored;
+    bool else_if = false;
+    if (match(Token::Type::kElse, &start)) {
+      else_if = match(Token::Type::kIf);
+    } else if (match(Token::Type::kElseIf, &start)) {
+      deprecated(start, "'elseif' is now 'else if'");
+      else_if = true;
+    } else {
+      break;
+    }
 
-  return create<ast::ElseStatement>(source, nullptr, body.value);
+    const ast::Expression* cond = nullptr;
+    if (else_if) {
+      auto condition = expect_paren_rhs_stmt();
+      if (condition.errored) {
+        return Failure::kErrored;
+      }
+      cond = condition.value;
+    }
+
+    auto body = expect_body_stmt();
+    if (body.errored) {
+      return Failure::kErrored;
+    }
+
+    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
-  /// @returns the parsed elements
-  Maybe<ast::ElseStatementList> elseif_stmt();
-  /// Parses a `else_stmt` grammar element
+  /// Parses a list of `else_stmt` grammar elements
   /// @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) {
-  auto p = parser("elseif (a == 4) { a = b; c = d; }");
-  auto e = p->elseif_stmt();
-  EXPECT_TRUE(e.matched);
-  EXPECT_FALSE(e.errored);
+TEST_F(ParserImplTest, ElseStmts) {
+  auto p = parser("else if (a == 4) { a = b; c = d; }");
+  auto e = p->else_stmts();
   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) {
-  auto p = parser("elseif (a == 4) { a = b; c = d; } elseif(c) { d = 2; }");
-  auto e = p->elseif_stmt();
-  EXPECT_TRUE(e.matched);
-  EXPECT_FALSE(e.errored);
+TEST_F(ParserImplTest, ElseStmts_Multiple) {
+  auto p = parser("else if (a == 4) { a = b; c = d; } else if(c) { d = 2; }");
+  auto e = p->else_stmts();
   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) {
-  auto p = parser("elseif (true) { fn main() {}}");
-  auto e = p->elseif_stmt();
-  EXPECT_FALSE(e.matched);
+TEST_F(ParserImplTest, ElseStmts_InvalidBody) {
+  auto p = parser("else if (true) { fn main() {}}");
+  auto e = p->else_stmts();
   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) {
-  auto p = parser("elseif (true)");
-  auto e = p->elseif_stmt();
-  EXPECT_FALSE(e.matched);
+TEST_F(ParserImplTest, ElseStmts_MissingBody) {
+  auto p = parser("else if (true)");
+  auto e = p->else_stmts();
   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);
@@ -98,13 +99,13 @@ TEST_F(ParserImplTest, IfStmt_MissingBody) {
 }
 
 TEST_F(ParserImplTest, IfStmt_InvalidElseif) {
-  auto p = parser("if (a) {} elseif (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:24: expected '}'");
+  EXPECT_EQ(p->error(), "1:25: expected '}'");
 }
 
 TEST_F(ParserImplTest, IfStmt_InvalidElse) {

src/resolver/compound_statement_test.cc

@@ -221,7 +221,7 @@ TEST_F(ResolverCompoundStatementTest, If) {
   // fn F() {
   //   if (cond_a) {
   //     stat_a;
-  //   } elseif (cond_b) {
+  //   } else if (cond_b) {
   //     stat_b;
   //   } else {
   //     stat_c;

src/writer/spirv/builder_if_test.cc

@@ -150,7 +150,7 @@ OpBranch %7
 TEST_F(BuilderTest, If_WithElseIf) {
   // if (true) {
   //   v = 2;
-  // } elseif (true) {
+  // } else if (true) {
   //   v = 3;
   // }
 
@@ -201,9 +201,9 @@ OpBranch %7
 TEST_F(BuilderTest, If_WithMultiple) {
   // if (true) {
   //   v = 2;
-  // } elseif (true) {
+  // } else if (true) {
   //   v = 3;
-  // } elseif (false) {
+  // } else if (false) {
   //   v = 4;
   // } else {
   //   v = 5;

src/writer/wgsl/generator_impl.cc

@@ -955,7 +955,7 @@ bool GeneratorImpl::EmitIf(const ast::IfStatement* stmt) {
   for (auto* e : stmt->else_statements) {
     if (e->condition) {
       auto out = line();
-      out << "} elseif (";
+      out << "} else if (";
       if (!EmitExpression(out, e->condition)) {
         return false;
       }

src/writer/wgsl/generator_impl_if_test.cc

@@ -61,7 +61,7 @@ TEST_F(WgslGeneratorImplTest, Emit_IfWithElseIf) {
   ASSERT_TRUE(gen.EmitStatement(i)) << gen.error();
   EXPECT_EQ(gen.result(), R"(  if (cond) {
     return;
-  } elseif (else_cond) {
+  } else if (else_cond) {
     return;
   }
 )");
@@ -118,7 +118,7 @@ TEST_F(WgslGeneratorImplTest, Emit_IfWithMultiple) {
   ASSERT_TRUE(gen.EmitStatement(i)) << gen.error();
   EXPECT_EQ(gen.result(), R"(  if (cond) {
     return;
-  } elseif (else_cond) {
+  } else if (else_cond) {
     return;
   } else {
     return;

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",

test/bug/tint/1046.wgsl

@@ -41,19 +41,19 @@ fn getColor(fragment : FragmentInput) -> vec4<f32>{
 
         color = fragment.color;
 
-    }elseif(uniforms.color_source == 1u){
+    } else if(uniforms.color_source == 1u){
         // NORMALS
 
         // color = vec4<f32>(0.0, 0.0, 1.0, 1.0);
         color = fragment.normal;
         color.a = 1.0;
 
-    }elseif(uniforms.color_source == 2u){
+    } else if(uniforms.color_source == 2u){
         // uniform color
 
         color = uniforms.color;
 
-    }elseif(uniforms.color_source == 3u){
+    } else if(uniforms.color_source == 3u){
         // TEXTURE
 
         color = textureSample(myTexture, mySampler, fragment.uv);

test/bug/tint/1046.wgsl.expected.wgsl

@@ -44,12 +44,12 @@ fn getColor(fragment : FragmentInput) -> vec4<f32> {
   var color : vec4<f32>;
   if ((uniforms.color_source == 0u)) {
     color = fragment.color;
-  } elseif ((uniforms.color_source == 1u)) {
+  } else if ((uniforms.color_source == 1u)) {
     color = fragment.normal;
     color.a = 1.0;
-  } elseif ((uniforms.color_source == 2u)) {
+  } else if ((uniforms.color_source == 2u)) {
     color = uniforms.color;
-  } elseif ((uniforms.color_source == 3u)) {
+  } else if ((uniforms.color_source == 3u)) {
     color = textureSample(myTexture, mySampler, fragment.uv);
   }
   return color;