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>
diff --git a/docs/origin-trial-changes.md b/docs/origin-trial-changes.md
index 4acefa6..ef7731a 100644
--- a/docs/origin-trial-changes.md
+++ b/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
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index 10aeea6..62fd9fc 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -826,7 +826,6 @@
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
diff --git a/src/reader/wgsl/parser_impl.cc b/src/reader/wgsl/parser_impl.cc
index 9112b18..17a5e5f 100644
--- a/src/reader/wgsl/parser_impl.cc
+++ b/src/reader/wgsl/parser_impl.cc
@@ -1540,7 +1540,7 @@
}
// 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 @@
}
// 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 @@
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
diff --git a/src/reader/wgsl/parser_impl.h b/src/reader/wgsl/parser_impl.h
index e059f09..6f51e0e 100644
--- a/src/reader/wgsl/parser_impl.h
+++ b/src/reader/wgsl/parser_impl.h
@@ -522,12 +522,9 @@
/// 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();
diff --git a/src/reader/wgsl/parser_impl_else_stmt_test.cc b/src/reader/wgsl/parser_impl_else_stmt_test.cc
deleted file mode 100644
index 822602b..0000000
--- a/src/reader/wgsl/parser_impl_else_stmt_test.cc
+++ /dev/null
@@ -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
diff --git a/src/reader/wgsl/parser_impl_elseif_stmt_test.cc b/src/reader/wgsl/parser_impl_elseif_stmt_test.cc
index ade3c99..11099d4 100644
--- a/src/reader/wgsl/parser_impl_elseif_stmt_test.cc
+++ b/src/reader/wgsl/parser_impl_elseif_stmt_test.cc
@@ -19,11 +19,9 @@
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 @@
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 @@
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
diff --git a/src/reader/wgsl/parser_impl_if_stmt_test.cc b/src/reader/wgsl/parser_impl_if_stmt_test.cc
index 1c93c19..d7d86b2 100644
--- a/src/reader/wgsl/parser_impl_if_stmt_test.cc
+++ b/src/reader/wgsl/parser_impl_if_stmt_test.cc
@@ -35,7 +35,8 @@
}
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_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) {
diff --git a/src/resolver/compound_statement_test.cc b/src/resolver/compound_statement_test.cc
index 7de565c..1eb4f0f 100644
--- a/src/resolver/compound_statement_test.cc
+++ b/src/resolver/compound_statement_test.cc
@@ -221,7 +221,7 @@
// fn F() {
// if (cond_a) {
// stat_a;
- // } elseif (cond_b) {
+ // } else if (cond_b) {
// stat_b;
// } else {
// stat_c;
diff --git a/src/writer/spirv/builder_if_test.cc b/src/writer/spirv/builder_if_test.cc
index 2fda0e1..b67a4aa 100644
--- a/src/writer/spirv/builder_if_test.cc
+++ b/src/writer/spirv/builder_if_test.cc
@@ -150,7 +150,7 @@
TEST_F(BuilderTest, If_WithElseIf) {
// if (true) {
// v = 2;
- // } elseif (true) {
+ // } else if (true) {
// v = 3;
// }
@@ -201,9 +201,9 @@
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;
diff --git a/src/writer/wgsl/generator_impl.cc b/src/writer/wgsl/generator_impl.cc
index 0140fe7..eef9aaa 100644
--- a/src/writer/wgsl/generator_impl.cc
+++ b/src/writer/wgsl/generator_impl.cc
@@ -955,7 +955,7 @@
for (auto* e : stmt->else_statements) {
if (e->condition) {
auto out = line();
- out << "} elseif (";
+ out << "} else if (";
if (!EmitExpression(out, e->condition)) {
return false;
}
diff --git a/src/writer/wgsl/generator_impl_if_test.cc b/src/writer/wgsl/generator_impl_if_test.cc
index a929a69..36fe5d2 100644
--- a/src/writer/wgsl/generator_impl_if_test.cc
+++ b/src/writer/wgsl/generator_impl_if_test.cc
@@ -61,7 +61,7 @@
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 @@
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;
diff --git a/test/BUILD.gn b/test/BUILD.gn
index 4403a24..e85e787 100644
--- a/test/BUILD.gn
+++ b/test/BUILD.gn
@@ -462,7 +462,6 @@
"../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",
diff --git a/test/bug/tint/1046.wgsl b/test/bug/tint/1046.wgsl
index 1c8ac15..8fdfe17 100644
--- a/test/bug/tint/1046.wgsl
+++ b/test/bug/tint/1046.wgsl
@@ -41,19 +41,19 @@
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);
diff --git a/test/bug/tint/1046.wgsl.expected.wgsl b/test/bug/tint/1046.wgsl.expected.wgsl
index 3c654b0..310472c 100644
--- a/test/bug/tint/1046.wgsl.expected.wgsl
+++ b/test/bug/tint/1046.wgsl.expected.wgsl
@@ -44,12 +44,12 @@
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;
