From 1d69915155b53d0feec998c937d7ae9e57574279 Mon Sep 17 00:00:00 2001 From: dan sinclair Date: Tue, 2 Jun 2020 17:18:56 +0000 Subject: [PATCH] Rename case statement conditions to selectors. The name conditions isn't quite correct for the case statement. This CL updates the code to use selectors instead of conditions. Change-Id: I98b8050b11e2328f97e4443469572ab47d7c1555 Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/22520 Reviewed-by: David Neto --- src/ast/case_statement.cc | 12 ++++----- src/ast/case_statement.h | 25 +++++++++---------- src/ast/case_statement_test.cc | 16 ++++++------ src/reader/wgsl/parser_impl.cc | 8 +++--- src/reader/wgsl/parser_impl_statement_test.cc | 2 +- .../wgsl/parser_impl_switch_body_test.cc | 4 +-- .../wgsl/parser_impl_switch_stmt_test.cc | 2 +- src/writer/spirv/builder.cc | 2 +- src/writer/wgsl/generator_impl.cc | 4 +-- 9 files changed, 37 insertions(+), 38 deletions(-) diff --git a/src/ast/case_statement.cc b/src/ast/case_statement.cc index 7eb4878426..bb63225d30 100644 --- a/src/ast/case_statement.cc +++ b/src/ast/case_statement.cc @@ -22,14 +22,14 @@ CaseStatement::CaseStatement() : Statement() {} CaseStatement::CaseStatement(StatementList body) : Statement(), body_(std::move(body)) {} -CaseStatement::CaseStatement(CaseSelectorList conditions, StatementList body) - : Statement(), conditions_(std::move(conditions)), body_(std::move(body)) {} +CaseStatement::CaseStatement(CaseSelectorList selectors, StatementList body) + : Statement(), selectors_(std::move(selectors)), body_(std::move(body)) {} CaseStatement::CaseStatement(const Source& source, - CaseSelectorList conditions, + CaseSelectorList selectors, StatementList body) : Statement(source), - conditions_(std::move(conditions)), + selectors_(std::move(selectors)), body_(std::move(body)) {} CaseStatement::CaseStatement(CaseStatement&&) = default; @@ -56,12 +56,12 @@ void CaseStatement::to_str(std::ostream& out, size_t indent) const { } else { out << "Case "; bool first = true; - for (const auto& lit : conditions_) { + for (const auto& selector : selectors_) { if (!first) out << ", "; first = false; - out << lit->to_str(); + out << selector->to_str(); } out << "{" << std::endl; } diff --git a/src/ast/case_statement.h b/src/ast/case_statement.h index b5b13d6b61..9b4150cac9 100644 --- a/src/ast/case_statement.h +++ b/src/ast/case_statement.h @@ -22,7 +22,6 @@ #include "src/ast/expression.h" #include "src/ast/literal.h" #include "src/ast/statement.h" -#include "src/ast/statement_condition.h" namespace tint { namespace ast { @@ -40,29 +39,29 @@ class CaseStatement : public Statement { /// @param body the case body explicit CaseStatement(StatementList body); /// Constructor - /// @param conditions the case conditions + /// @param selectors the case selectors /// @param body the case body - CaseStatement(CaseSelectorList conditions, StatementList body); + CaseStatement(CaseSelectorList selectors, StatementList body); /// Constructor /// @param source the source information - /// @param conditions the case conditions + /// @param selectors the case selectors /// @param body the case body CaseStatement(const Source& source, - CaseSelectorList conditions, + CaseSelectorList selectors, StatementList body); /// Move constructor CaseStatement(CaseStatement&&); ~CaseStatement() override; - /// Sets the conditions for the case statement - /// @param conditions the conditions to set - void set_conditions(CaseSelectorList conditions) { - conditions_ = std::move(conditions); + /// Sets the selectors for the case statement + /// @param selectors the selectors to set + void set_selectors(CaseSelectorList selectors) { + selectors_ = std::move(selectors); } - /// @returns the case condition, empty if none set - const CaseSelectorList& conditions() const { return conditions_; } + /// @returns the case selectors, empty if none set + const CaseSelectorList& selectors() const { return selectors_; } /// @returns true if this is a default statement - bool IsDefault() const { return conditions_.empty(); } + bool IsDefault() const { return selectors_.empty(); } /// Sets the case body /// @param body the case body @@ -84,7 +83,7 @@ class CaseStatement : public Statement { private: CaseStatement(const CaseStatement&) = delete; - CaseSelectorList conditions_; + CaseSelectorList selectors_; StatementList body_; }; diff --git a/src/ast/case_statement_test.cc b/src/ast/case_statement_test.cc index 8653cd7077..bc53f579a2 100644 --- a/src/ast/case_statement_test.cc +++ b/src/ast/case_statement_test.cc @@ -41,8 +41,8 @@ TEST_F(CaseStatementTest, Creation) { auto* kill_ptr = stmts[0].get(); CaseStatement c(std::move(b), std::move(stmts)); - ASSERT_EQ(c.conditions().size(), 1); - EXPECT_EQ(c.conditions()[0].get(), bool_ptr); + ASSERT_EQ(c.selectors().size(), 1); + EXPECT_EQ(c.selectors()[0].get(), bool_ptr); ASSERT_EQ(c.body().size(), 1u); EXPECT_EQ(c.body()[0].get(), kill_ptr); } @@ -61,7 +61,7 @@ TEST_F(CaseStatementTest, Creation_WithSource) { EXPECT_EQ(src.column, 2u); } -TEST_F(CaseStatementTest, IsDefault_WithoutCondition) { +TEST_F(CaseStatementTest, IsDefault_WithoutSelectors) { StatementList stmts; stmts.push_back(std::make_unique()); @@ -70,13 +70,13 @@ TEST_F(CaseStatementTest, IsDefault_WithoutCondition) { EXPECT_TRUE(c.IsDefault()); } -TEST_F(CaseStatementTest, IsDefault_WithCondition) { +TEST_F(CaseStatementTest, IsDefault_WithSelectors) { ast::type::BoolType bool_type; CaseSelectorList b; b.push_back(std::make_unique(&bool_type, true)); CaseStatement c; - c.set_conditions(std::move(b)); + c.set_selectors(std::move(b)); EXPECT_FALSE(c.IsDefault()); } @@ -115,7 +115,7 @@ TEST_F(CaseStatementTest, IsValid_InvalidBodyStatement) { EXPECT_FALSE(c.IsValid()); } -TEST_F(CaseStatementTest, ToStr_WithCondition) { +TEST_F(CaseStatementTest, ToStr_WithSelectors) { ast::type::BoolType bool_type; CaseSelectorList b; b.push_back(std::make_unique(&bool_type, true)); @@ -132,7 +132,7 @@ TEST_F(CaseStatementTest, ToStr_WithCondition) { )"); } -TEST_F(CaseStatementTest, ToStr_WithMultipleConditions) { +TEST_F(CaseStatementTest, ToStr_WithMultipleSelectors) { ast::type::I32Type i32; CaseSelectorList b; @@ -150,7 +150,7 @@ TEST_F(CaseStatementTest, ToStr_WithMultipleConditions) { )"); } -TEST_F(CaseStatementTest, ToStr_WithoutCondition) { +TEST_F(CaseStatementTest, ToStr_WithoutSelectors) { StatementList stmts; stmts.push_back(std::make_unique()); CaseStatement c(CaseSelectorList{}, std::move(stmts)); diff --git a/src/reader/wgsl/parser_impl.cc b/src/reader/wgsl/parser_impl.cc index e81ef0c37d..36d82e438d 100644 --- a/src/reader/wgsl/parser_impl.cc +++ b/src/reader/wgsl/parser_impl.cc @@ -1813,14 +1813,14 @@ std::unique_ptr ParserImpl::switch_body() { auto stmt = std::make_unique(); stmt->set_source(source); if (t.IsCase()) { - auto cond = case_selectors(); + auto selectors = case_selectors(); if (has_error()) return nullptr; - if (cond.empty()) { - set_error(peek(), "unable to parse case conditional"); + if (selectors.empty()) { + set_error(peek(), "unable to parse case selectors"); return nullptr; } - stmt->set_conditions(std::move(cond)); + stmt->set_selectors(std::move(selectors)); } t = next(); diff --git a/src/reader/wgsl/parser_impl_statement_test.cc b/src/reader/wgsl/parser_impl_statement_test.cc index 40a8bfcebe..1deaa12125 100644 --- a/src/reader/wgsl/parser_impl_statement_test.cc +++ b/src/reader/wgsl/parser_impl_statement_test.cc @@ -146,7 +146,7 @@ TEST_F(ParserImplTest, Statement_Switch_Invalid) { auto e = p->statement(); ASSERT_TRUE(p->has_error()); ASSERT_EQ(e, nullptr); - EXPECT_EQ(p->error(), "1:18: unable to parse case conditional"); + EXPECT_EQ(p->error(), "1:18: unable to parse case selectors"); } TEST_F(ParserImplTest, Statement_Loop) { diff --git a/src/reader/wgsl/parser_impl_switch_body_test.cc b/src/reader/wgsl/parser_impl_switch_body_test.cc index dc85463d99..c9458fd216 100644 --- a/src/reader/wgsl/parser_impl_switch_body_test.cc +++ b/src/reader/wgsl/parser_impl_switch_body_test.cc @@ -38,7 +38,7 @@ TEST_F(ParserImplTest, SwitchBody_Case_InvalidConstLiteral) { auto e = p->switch_body(); ASSERT_TRUE(p->has_error()); ASSERT_EQ(e, nullptr); - EXPECT_EQ(p->error(), "1:6: unable to parse case conditional"); + EXPECT_EQ(p->error(), "1:6: unable to parse case selectors"); } TEST_F(ParserImplTest, SwitchBody_Case_MissingConstLiteral) { @@ -46,7 +46,7 @@ TEST_F(ParserImplTest, SwitchBody_Case_MissingConstLiteral) { auto e = p->switch_body(); ASSERT_TRUE(p->has_error()); ASSERT_EQ(e, nullptr); - EXPECT_EQ(p->error(), "1:5: unable to parse case conditional"); + EXPECT_EQ(p->error(), "1:5: unable to parse case selectors"); } TEST_F(ParserImplTest, SwitchBody_Case_MissingColon) { diff --git a/src/reader/wgsl/parser_impl_switch_stmt_test.cc b/src/reader/wgsl/parser_impl_switch_stmt_test.cc index d0c98deae8..3d3a51542e 100644 --- a/src/reader/wgsl/parser_impl_switch_stmt_test.cc +++ b/src/reader/wgsl/parser_impl_switch_stmt_test.cc @@ -102,7 +102,7 @@ TEST_F(ParserImplTest, SwitchStmt_InvalidBody) { auto e = p->switch_stmt(); ASSERT_TRUE(p->has_error()); ASSERT_EQ(e, nullptr); - EXPECT_EQ(p->error(), "2:7: unable to parse case conditional"); + EXPECT_EQ(p->error(), "2:7: unable to parse case selectors"); } } // namespace diff --git a/src/writer/spirv/builder.cc b/src/writer/spirv/builder.cc index 7169411c68..102d849ddc 100644 --- a/src/writer/spirv/builder.cc +++ b/src/writer/spirv/builder.cc @@ -1365,7 +1365,7 @@ bool Builder::GenerateSwitchStatement(ast::SwitchStatement* stmt) { auto block_id = block.to_i(); case_ids.push_back(block_id); - for (const auto& selector : item->conditions()) { + for (const auto& selector : item->selectors()) { if (!selector->IsInt()) { error_ = "expected integer literal for switch case label"; return false; diff --git a/src/writer/wgsl/generator_impl.cc b/src/writer/wgsl/generator_impl.cc index 50dad688b8..9638f48bc4 100644 --- a/src/writer/wgsl/generator_impl.cc +++ b/src/writer/wgsl/generator_impl.cc @@ -725,13 +725,13 @@ bool GeneratorImpl::EmitCase(ast::CaseStatement* stmt) { out_ << "case "; bool first = true; - for (const auto& lit : stmt->conditions()) { + for (const auto& selector : stmt->selectors()) { if (!first) { out_ << ", "; } first = false; - if (!EmitLiteral(lit.get())) { + if (!EmitLiteral(selector.get())) { return false; } }