From a28bcceb152774b123ad9a390f8255be5255073d Mon Sep 17 00:00:00 2001 From: dan sinclair Date: Tue, 14 Apr 2020 14:46:58 +0000 Subject: [PATCH] Remove premerge. This CL removes the premerge statement and replaces it with a `premerge` reserved word. Change-Id: Ic9bc13878ed26e1733eb65dd1ba30d9bef095cb6 Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/19380 Reviewed-by: Ryan Harrison --- src/CMakeLists.txt | 1 - src/ast/if_statement.cc | 26 +--- src/ast/if_statement.h | 7 -- src/ast/if_statement_test.cc | 118 ------------------ src/reader/wgsl/lexer.cc | 4 +- src/reader/wgsl/lexer_test.cc | 2 +- src/reader/wgsl/parser_impl.cc | 20 +-- src/reader/wgsl/parser_impl.h | 3 - src/reader/wgsl/parser_impl_if_stmt_test.cc | 34 ----- .../wgsl/parser_impl_premerge_stmt_test.cc | 43 ------- src/reader/wgsl/token.h | 2 - 11 files changed, 5 insertions(+), 255 deletions(-) delete mode 100644 src/reader/wgsl/parser_impl_premerge_stmt_test.cc diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 084a38ba54..83cee3e716 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -385,7 +385,6 @@ if(${TINT_BUILD_WGSL_READER}) reader/wgsl/parser_impl_paren_rhs_stmt_test.cc reader/wgsl/parser_impl_pipeline_stage_test.cc reader/wgsl/parser_impl_postfix_expression_test.cc - reader/wgsl/parser_impl_premerge_stmt_test.cc reader/wgsl/parser_impl_primary_expression_test.cc reader/wgsl/parser_impl_relational_expression_test.cc reader/wgsl/parser_impl_shift_expression_test.cc diff --git a/src/ast/if_statement.cc b/src/ast/if_statement.cc index 9447c6c0ba..3fc380864d 100644 --- a/src/ast/if_statement.cc +++ b/src/ast/if_statement.cc @@ -62,20 +62,6 @@ bool IfStatement::IsValid() const { found_else = true; } - for (const auto& stmt : premerge_) { - if (stmt == nullptr || !stmt->IsValid()) - return false; - } - - if (premerge_.size() > 0) { - // Premerge only with a single else statement - if (else_statements_.size() != 1) - return false; - // Must be an else, not an elseif - if (else_statements_[0]->condition() != nullptr) - return false; - } - return true; } @@ -108,18 +94,8 @@ void IfStatement::to_str(std::ostream& out, size_t indent) const { make_indent(out, indent); out << "}" << std::endl; - for (const auto& e : else_statements_) + for (const auto& e : else_statements_) { e->to_str(out, indent); - - if (premerge_.size() > 0) { - make_indent(out, indent); - out << "premerge{" << std::endl; - - for (const auto& stmt : premerge_) - stmt->to_str(out, indent + 2); - - make_indent(out, indent); - out << "}" << std::endl; } } diff --git a/src/ast/if_statement.h b/src/ast/if_statement.h index 7acb72b09c..3e57f17306 100644 --- a/src/ast/if_statement.h +++ b/src/ast/if_statement.h @@ -67,12 +67,6 @@ class IfStatement : public Statement { /// @returns the else statements const ElseStatementList& else_statements() const { return else_statements_; } - /// Sets the premerge statements - /// @param premerge the premerge statements - void set_premerge(StatementList premerge) { premerge_ = std::move(premerge); } - /// @returns the premerge statements - const StatementList& premerge() const { return premerge_; } - /// @returns true if this is a if statement bool IsIf() const override; @@ -90,7 +84,6 @@ class IfStatement : public Statement { std::unique_ptr condition_; StatementList body_; ElseStatementList else_statements_; - StatementList premerge_; }; } // namespace ast diff --git a/src/ast/if_statement_test.cc b/src/ast/if_statement_test.cc index 4786ece659..7a96899efa 100644 --- a/src/ast/if_statement_test.cc +++ b/src/ast/if_statement_test.cc @@ -79,23 +79,6 @@ TEST_F(IfStatementTest, IsValid_WithElseStatements) { EXPECT_TRUE(stmt.IsValid()); } -TEST_F(IfStatementTest, IsValid_WithPremerge) { - auto cond = std::make_unique("cond"); - StatementList body; - body.push_back(std::make_unique()); - - ElseStatementList else_stmts; - else_stmts.push_back(std::make_unique()); - - StatementList premerge; - premerge.push_back(std::make_unique()); - - IfStatement stmt(std::move(cond), std::move(body)); - stmt.set_else_statements(std::move(else_stmts)); - stmt.set_premerge(std::move(premerge)); - EXPECT_TRUE(stmt.IsValid()); -} - TEST_F(IfStatementTest, IsValid_MissingCondition) { StatementList body; body.push_back(std::make_unique()); @@ -163,72 +146,6 @@ TEST_F(IfStatementTest, IsValid_InvalidElseStatement) { EXPECT_FALSE(stmt.IsValid()); } -TEST_F(IfStatementTest, IsValid_NullPremergeStatement) { - auto cond = std::make_unique("cond"); - StatementList body; - body.push_back(std::make_unique()); - - ElseStatementList else_stmts; - else_stmts.push_back(std::make_unique()); - - StatementList premerge; - premerge.push_back(std::make_unique()); - premerge.push_back(nullptr); - - IfStatement stmt(std::move(cond), std::move(body)); - stmt.set_else_statements(std::move(else_stmts)); - stmt.set_premerge(std::move(premerge)); - EXPECT_FALSE(stmt.IsValid()); -} - -TEST_F(IfStatementTest, IsValid_InvalidPremergeStatement) { - auto cond = std::make_unique("cond"); - StatementList body; - body.push_back(std::make_unique()); - - ElseStatementList else_stmts; - else_stmts.push_back(std::make_unique()); - - StatementList premerge; - premerge.push_back(std::make_unique()); - - IfStatement stmt(std::move(cond), std::move(body)); - stmt.set_else_statements(std::move(else_stmts)); - stmt.set_premerge(std::move(premerge)); - EXPECT_FALSE(stmt.IsValid()); -} - -TEST_F(IfStatementTest, IsValid_PremergeWithElseIf) { - auto cond = std::make_unique("cond"); - StatementList body; - body.push_back(std::make_unique()); - - ElseStatementList else_stmts; - else_stmts.push_back(std::make_unique()); - else_stmts[0]->set_condition(std::make_unique("ident")); - - StatementList premerge; - premerge.push_back(std::make_unique()); - - IfStatement stmt(std::move(cond), std::move(body)); - stmt.set_else_statements(std::move(else_stmts)); - stmt.set_premerge(std::move(premerge)); - EXPECT_FALSE(stmt.IsValid()); -} - -TEST_F(IfStatementTest, IsValid_PremergeWithoutElse) { - auto cond = std::make_unique("cond"); - StatementList body; - body.push_back(std::make_unique()); - - StatementList premerge; - premerge.push_back(std::make_unique()); - - IfStatement stmt(std::move(cond), std::move(body)); - stmt.set_premerge(std::move(premerge)); - EXPECT_FALSE(stmt.IsValid()); -} - TEST_F(IfStatementTest, IsValid_MultipleElseWiththoutCondition) { auto cond = std::make_unique("cond"); StatementList body; @@ -327,41 +244,6 @@ TEST_F(IfStatementTest, ToStr_WithElseStatements) { )"); } -TEST_F(IfStatementTest, ToStr_WithPremerge) { - auto cond = std::make_unique("cond"); - StatementList body; - body.push_back(std::make_unique()); - - ElseStatementList else_stmts; - else_stmts.push_back(std::make_unique()); - - StatementList premerge; - premerge.push_back(std::make_unique()); - - IfStatement stmt(std::move(cond), std::move(body)); - stmt.set_else_statements(std::move(else_stmts)); - stmt.set_premerge(std::move(premerge)); - - std::ostringstream out; - stmt.to_str(out, 2); - EXPECT_EQ(out.str(), R"( If{ - ( - Identifier{cond} - ) - { - Nop{} - } - } - Else{ - { - } - } - premerge{ - Nop{} - } -)"); -} - } // namespace } // namespace ast } // namespace tint diff --git a/src/reader/wgsl/lexer.cc b/src/reader/wgsl/lexer.cc index 3d4e989957..d1a6982962 100644 --- a/src/reader/wgsl/lexer.cc +++ b/src/reader/wgsl/lexer.cc @@ -598,8 +598,6 @@ Token Lexer::check_keyword(const Source& source, const std::string& str) { return {Token::Type::kOuterProduct, source, "outer_product"}; if (str == "position") return {Token::Type::kPosition, source, "position"}; - if (str == "premerge") - return {Token::Type::kPremerge, source, "premerge"}; if (str == "private") return {Token::Type::kPrivate, source, "private"}; if (str == "ptr") @@ -673,6 +671,8 @@ Token Lexer::check_reserved(const Source& source, const std::string& str) { return {Token::Type::kReservedKeyword, source, "i64"}; if (str == "let") return {Token::Type::kReservedKeyword, source, "let"}; + if (str == "premerge") + return {Token::Type::kReservedKeyword, source, "premerge"}; if (str == "typedef") return {Token::Type::kReservedKeyword, source, "typedef"}; if (str == "u8") diff --git a/src/reader/wgsl/lexer_test.cc b/src/reader/wgsl/lexer_test.cc index fb8b3cafd1..29b5ee29b4 100644 --- a/src/reader/wgsl/lexer_test.cc +++ b/src/reader/wgsl/lexer_test.cc @@ -474,7 +474,6 @@ INSTANTIATE_TEST_SUITE_P( TokenData{"out", Token::Type::kOut}, TokenData{"outer_product", Token::Type::kOuterProduct}, TokenData{"position", Token::Type::kPosition}, - TokenData{"premerge", Token::Type::kPremerge}, TokenData{"private", Token::Type::kPrivate}, TokenData{"ptr", Token::Type::kPtr}, TokenData{"push_constant", Token::Type::kPushConstant}, @@ -521,6 +520,7 @@ INSTANTIATE_TEST_SUITE_P(LexerTest, "i16", "i64", "let", + "premerge", "typedef", "u8", "u16", diff --git a/src/reader/wgsl/parser_impl.cc b/src/reader/wgsl/parser_impl.cc index d1a1148b35..db8a8619d1 100644 --- a/src/reader/wgsl/parser_impl.cc +++ b/src/reader/wgsl/parser_impl.cc @@ -1720,7 +1720,7 @@ std::unique_ptr ParserImpl::variable_stmt() { // if_stmt // : IF paren_rhs_stmt body_stmt -// {(elseif_stmt else_stmt?) | (else_stmt premerge_stmt?)} +// {(elseif_stmt else_stmt?) | (else_stmt?)} std::unique_ptr ParserImpl::if_stmt() { auto t = peek(); if (!t.IsIf()) @@ -1758,13 +1758,6 @@ std::unique_ptr ParserImpl::if_stmt() { auto stmt = std::make_unique(source, std::move(condition), std::move(body)); if (el != nullptr) { - if (elseif.size() == 0) { - auto premerge = premerge_stmt(); - if (has_error()) - return nullptr; - - stmt->set_premerge(std::move(premerge)); - } elseif.push_back(std::move(el)); } stmt->set_else_statements(std::move(elseif)); @@ -1836,17 +1829,6 @@ std::unique_ptr ParserImpl::else_stmt() { return std::make_unique(source, std::move(body)); } -// premerge_stmt -// : PREMERGE body_stmt -ast::StatementList ParserImpl::premerge_stmt() { - auto t = peek(); - if (!t.IsPremerge()) - return {}; - - next(); // Consume the peek - return body_stmt(); -} - // unless_stmt // : UNLESS paren_rhs_stmt body_stmt std::unique_ptr ParserImpl::unless_stmt() { diff --git a/src/reader/wgsl/parser_impl.h b/src/reader/wgsl/parser_impl.h index faf77810be..4f33cf0740 100644 --- a/src/reader/wgsl/parser_impl.h +++ b/src/reader/wgsl/parser_impl.h @@ -208,9 +208,6 @@ class ParserImpl { /// Parses a `else_stmt` grammar element /// @returns the parsed statement or nullptr std::unique_ptr else_stmt(); - /// Parses a `premerge_stmt` grammar element - /// @returns the parsed statements - ast::StatementList premerge_stmt(); /// Parses a `unless_stmt` grammar element /// @returns the parsed element or nullptr std::unique_ptr unless_stmt(); diff --git a/src/reader/wgsl/parser_impl_if_stmt_test.cc b/src/reader/wgsl/parser_impl_if_stmt_test.cc index 40a6661a55..56bd415da0 100644 --- a/src/reader/wgsl/parser_impl_if_stmt_test.cc +++ b/src/reader/wgsl/parser_impl_if_stmt_test.cc @@ -34,7 +34,6 @@ TEST_F(ParserImplTest, IfStmt) { ASSERT_TRUE(e->condition()->IsBinary()); EXPECT_EQ(e->body().size(), 2); EXPECT_EQ(e->else_statements().size(), 0); - EXPECT_EQ(e->premerge().size(), 0); } TEST_F(ParserImplTest, IfStmt_WithElse) { @@ -57,31 +56,6 @@ TEST_F(ParserImplTest, IfStmt_WithElse) { EXPECT_EQ(e->else_statements()[1]->body().size(), 0); } -TEST_F(ParserImplTest, IfStmt_WithPremerge) { - auto p = parser(R"(if (a == 4) { - a = b; - c = d; -} else { - d = 2; -} premerge { - a = 2; -})"); - auto e = p->if_stmt(); - ASSERT_FALSE(p->has_error()) << p->error(); - ASSERT_NE(e, nullptr); - - ASSERT_TRUE(e->IsIf()); - ASSERT_NE(e->condition(), nullptr); - ASSERT_TRUE(e->condition()->IsBinary()); - EXPECT_EQ(e->body().size(), 2); - - ASSERT_EQ(e->else_statements().size(), 1); - ASSERT_EQ(e->else_statements()[0]->condition(), nullptr); - EXPECT_EQ(e->else_statements()[0]->body().size(), 1); - - ASSERT_EQ(e->premerge().size(), 1); -} - TEST_F(ParserImplTest, IfStmt_InvalidCondition) { auto p = parser("if (a = 3) {}"); auto e = p->if_stmt(); @@ -130,14 +104,6 @@ TEST_F(ParserImplTest, IfStmt_InvalidElse) { EXPECT_EQ(p->error(), "1:18: missing }"); } -TEST_F(ParserImplTest, IfStmt_InvalidPremerge) { - auto p = parser("if (a) {} else {} premerge { fn main() -> a{}}"); - auto e = p->if_stmt(); - ASSERT_TRUE(p->has_error()); - ASSERT_EQ(e, nullptr); - EXPECT_EQ(p->error(), "1:30: missing }"); -} - } // namespace } // namespace wgsl } // namespace reader diff --git a/src/reader/wgsl/parser_impl_premerge_stmt_test.cc b/src/reader/wgsl/parser_impl_premerge_stmt_test.cc deleted file mode 100644 index 377b74bace..0000000000 --- a/src/reader/wgsl/parser_impl_premerge_stmt_test.cc +++ /dev/null @@ -1,43 +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 "gtest/gtest.h" -#include "src/reader/wgsl/parser_impl.h" -#include "src/reader/wgsl/parser_impl_test_helper.h" - -namespace tint { -namespace reader { -namespace wgsl { -namespace { - -TEST_F(ParserImplTest, PremergeStmt) { - auto p = parser("premerge { nop; }"); - auto e = p->premerge_stmt(); - ASSERT_FALSE(p->has_error()) << p->error(); - ASSERT_EQ(e.size(), 1); - ASSERT_TRUE(e[0]->IsNop()); -} - -TEST_F(ParserImplTest, PremergeStmt_InvalidBody) { - auto p = parser("premerge { nop }"); - auto e = p->premerge_stmt(); - ASSERT_TRUE(p->has_error()); - ASSERT_EQ(e.size(), 0); - EXPECT_EQ(p->error(), "1:16: missing ;"); -} - -} // namespace -} // namespace wgsl -} // namespace reader -} // namespace tint diff --git a/src/reader/wgsl/token.h b/src/reader/wgsl/token.h index 234e5e24ee..418979de4f 100644 --- a/src/reader/wgsl/token.h +++ b/src/reader/wgsl/token.h @@ -561,8 +561,6 @@ class Token { bool IsOuterProduct() const { return type_ == Type::kOuterProduct; } /// @returns true if token is a 'position' bool IsPosition() const { return type_ == Type::kPosition; } - /// @returns true if token is a 'premerge' - bool IsPremerge() const { return type_ == Type::kPremerge; } /// @returns true if token is a 'private' bool IsPrivate() const { return type_ == Type::kPrivate; } /// @returns true if token is a 'ptr'