From eeac0c5f63559f0d79d7a000cb5330da91b40a28 Mon Sep 17 00:00:00 2001 From: Ben Clayton Date: Mon, 16 Nov 2020 15:15:37 +0000 Subject: [PATCH] ast: Remove no-arg constructor for ast::IfStatement In a near-future change, AST nodes, such as ast::BlockStatement will no longer be std::unique_ptrs, and will have to be constructed and owned by an external class. This means AST nodes can no longer allocate default child nodes. Bug: tint:322 Change-Id: I36a1cf55c31a1dabccde272b2be415f98c16b18f Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/32677 Commit-Queue: Ben Clayton Commit-Queue: David Neto Reviewed-by: David Neto Reviewed-by: dan sinclair --- src/ast/block_statement_test.cc | 2 +- src/ast/case_statement_test.cc | 2 +- src/ast/else_statement_test.cc | 2 +- src/ast/if_statement.cc | 3 --- src/ast/if_statement.h | 2 -- src/ast/if_statement_test.cc | 32 ++++++++++++++++---------------- src/ast/loop_statement_test.cc | 4 ++-- src/reader/spirv/function.cc | 29 ++++++++++++++++++----------- 8 files changed, 39 insertions(+), 37 deletions(-) diff --git a/src/ast/block_statement_test.cc b/src/ast/block_statement_test.cc index 802bb01767..542ece5908 100644 --- a/src/ast/block_statement_test.cc +++ b/src/ast/block_statement_test.cc @@ -91,7 +91,7 @@ TEST_F(BlockStatementTest, IsValid_NullBodyStatement) { TEST_F(BlockStatementTest, IsValid_InvalidBodyStatement) { BlockStatement b; - b.append(create()); + b.append(create(nullptr, create())); EXPECT_FALSE(b.IsValid()); } diff --git a/src/ast/case_statement_test.cc b/src/ast/case_statement_test.cc index 0a67de374e..0e9cb90d04 100644 --- a/src/ast/case_statement_test.cc +++ b/src/ast/case_statement_test.cc @@ -129,7 +129,7 @@ TEST_F(CaseStatementTest, IsValid_InvalidBodyStatement) { b.push_back(create(&i32, 2)); auto body = create(); - body->append(create()); + body->append(create(nullptr, create())); CaseStatement c({std::move(b)}, std::move(body)); EXPECT_FALSE(c.IsValid()); diff --git a/src/ast/else_statement_test.cc b/src/ast/else_statement_test.cc index b61c02ab10..8c3acdcfbb 100644 --- a/src/ast/else_statement_test.cc +++ b/src/ast/else_statement_test.cc @@ -98,7 +98,7 @@ TEST_F(ElseStatementTest, IsValid_InvalidCondition) { TEST_F(ElseStatementTest, IsValid_InvalidBodyStatement) { auto body = create(); - body->append(create()); + body->append(create(nullptr, create())); ElseStatement e(std::move(body)); EXPECT_FALSE(e.IsValid()); diff --git a/src/ast/if_statement.cc b/src/ast/if_statement.cc index 5008c7db9e..3f869bdfe5 100644 --- a/src/ast/if_statement.cc +++ b/src/ast/if_statement.cc @@ -19,9 +19,6 @@ namespace tint { namespace ast { -IfStatement::IfStatement() - : Statement(), body_(std::make_unique()) {} - IfStatement::IfStatement(std::unique_ptr condition, std::unique_ptr body) : Statement(), condition_(std::move(condition)), body_(std::move(body)) {} diff --git a/src/ast/if_statement.h b/src/ast/if_statement.h index 53af7b732d..5c1c8613a9 100644 --- a/src/ast/if_statement.h +++ b/src/ast/if_statement.h @@ -29,8 +29,6 @@ namespace ast { /// An if statement class IfStatement : public Statement { public: - /// Constructor - IfStatement(); /// Constructor /// @param condition the if condition /// @param body the if body diff --git a/src/ast/if_statement_test.cc b/src/ast/if_statement_test.cc index c9458bab9d..d465f785a9 100644 --- a/src/ast/if_statement_test.cc +++ b/src/ast/if_statement_test.cc @@ -26,7 +26,7 @@ using IfStatementTest = TestHelper; TEST_F(IfStatementTest, Creation) { auto cond = create("cond"); - auto body = create(); + auto body = create(); body->append(create()); auto* cond_ptr = cond.get(); @@ -40,7 +40,7 @@ TEST_F(IfStatementTest, Creation) { TEST_F(IfStatementTest, Creation_WithSource) { auto cond = create("cond"); - auto body = create(); + auto body = create(); body->append(create()); IfStatement stmt(Source{Source::Location{20, 2}}, std::move(cond), @@ -51,13 +51,13 @@ TEST_F(IfStatementTest, Creation_WithSource) { } TEST_F(IfStatementTest, IsIf) { - IfStatement stmt; + IfStatement stmt(nullptr, create()); EXPECT_TRUE(stmt.IsIf()); } TEST_F(IfStatementTest, IsValid) { auto cond = create("cond"); - auto body = create(); + auto body = create(); body->append(create()); IfStatement stmt(std::move(cond), std::move(body)); @@ -66,7 +66,7 @@ TEST_F(IfStatementTest, IsValid) { TEST_F(IfStatementTest, IsValid_WithElseStatements) { auto cond = create("cond"); - auto body = create(); + auto body = create(); body->append(create()); ElseStatementList else_stmts; @@ -80,7 +80,7 @@ TEST_F(IfStatementTest, IsValid_WithElseStatements) { } TEST_F(IfStatementTest, IsValid_MissingCondition) { - auto body = create(); + auto body = create(); body->append(create()); IfStatement stmt(nullptr, std::move(body)); @@ -89,7 +89,7 @@ TEST_F(IfStatementTest, IsValid_MissingCondition) { TEST_F(IfStatementTest, IsValid_InvalidCondition) { auto cond = create(""); - auto body = create(); + auto body = create(); body->append(create()); IfStatement stmt(std::move(cond), std::move(body)); @@ -98,7 +98,7 @@ TEST_F(IfStatementTest, IsValid_InvalidCondition) { TEST_F(IfStatementTest, IsValid_NullBodyStatement) { auto cond = create("cond"); - auto body = create(); + auto body = create(); body->append(create()); body->append(nullptr); @@ -108,9 +108,9 @@ TEST_F(IfStatementTest, IsValid_NullBodyStatement) { TEST_F(IfStatementTest, IsValid_InvalidBodyStatement) { auto cond = create("cond"); - auto body = create(); + auto body = create(); body->append(create()); - body->append(create()); + body->append(create(nullptr, create())); IfStatement stmt(std::move(cond), std::move(body)); EXPECT_FALSE(stmt.IsValid()); @@ -118,7 +118,7 @@ TEST_F(IfStatementTest, IsValid_InvalidBodyStatement) { TEST_F(IfStatementTest, IsValid_NullElseStatement) { auto cond = create("cond"); - auto body = create(); + auto body = create(); body->append(create()); ElseStatementList else_stmts; @@ -134,7 +134,7 @@ TEST_F(IfStatementTest, IsValid_NullElseStatement) { TEST_F(IfStatementTest, IsValid_InvalidElseStatement) { auto cond = create("cond"); - auto body = create(); + auto body = create(); body->append(create()); ElseStatementList else_stmts; @@ -148,7 +148,7 @@ TEST_F(IfStatementTest, IsValid_InvalidElseStatement) { TEST_F(IfStatementTest, IsValid_MultipleElseWiththoutCondition) { auto cond = create("cond"); - auto body = create(); + auto body = create(); body->append(create()); ElseStatementList else_stmts; @@ -162,7 +162,7 @@ TEST_F(IfStatementTest, IsValid_MultipleElseWiththoutCondition) { TEST_F(IfStatementTest, IsValid_ElseNotLast) { auto cond = create("cond"); - auto body = create(); + auto body = create(); body->append(create()); ElseStatementList else_stmts; @@ -177,7 +177,7 @@ TEST_F(IfStatementTest, IsValid_ElseNotLast) { TEST_F(IfStatementTest, ToStr) { auto cond = create("cond"); - auto body = create(); + auto body = create(); body->append(create()); IfStatement stmt(std::move(cond), std::move(body)); @@ -197,7 +197,7 @@ TEST_F(IfStatementTest, ToStr) { TEST_F(IfStatementTest, ToStr_WithElseStatements) { auto cond = create("cond"); - auto body = create(); + auto body = create(); body->append(create()); auto else_if_body = create(); diff --git a/src/ast/loop_statement_test.cc b/src/ast/loop_statement_test.cc index 946ce27ad7..56e477d956 100644 --- a/src/ast/loop_statement_test.cc +++ b/src/ast/loop_statement_test.cc @@ -120,7 +120,7 @@ TEST_F(LoopStatementTest, IsValid_NullBodyStatement) { TEST_F(LoopStatementTest, IsValid_InvalidBodyStatement) { auto body = create(); body->append(create()); - body->append(create()); + body->append(create(nullptr, create())); auto continuing = create(); continuing->append(create()); @@ -147,7 +147,7 @@ TEST_F(LoopStatementTest, IsValid_InvalidContinuingStatement) { auto continuing = create(); continuing->append(create()); - continuing->append(create()); + continuing->append(create(nullptr, create())); LoopStatement l(std::move(body), std::move(continuing)); EXPECT_FALSE(l.IsValid()); diff --git a/src/reader/spirv/function.cc b/src/reader/spirv/function.cc index f2e90d0d1a..dafb7ce381 100644 --- a/src/reader/spirv/function.cc +++ b/src/reader/spirv/function.cc @@ -515,10 +515,11 @@ void FunctionEmitter::PushGuard(const std::string& guard_name, // if-selection with a then-clause ending at the same block // as the statement block at the top of the stack. const auto& top = statements_stack_.back(); - auto* const guard_stmt = - AddStatement(std::make_unique())->AsIf(); - guard_stmt->set_condition( - std::make_unique(guard_name)); + auto cond = std::make_unique(guard_name); + auto body = std::make_unique(); + auto* const guard_stmt = AddStatement(std::make_unique( + std::move(cond), std::move(body))) + ->AsIf(); PushNewStatementBlock(top.construct_, end_id, [guard_stmt](StatementBlock* s) { guard_stmt->set_body(std::move(s->statements_)); @@ -528,8 +529,11 @@ void FunctionEmitter::PushGuard(const std::string& guard_name, void FunctionEmitter::PushTrueGuard(uint32_t end_id) { assert(!statements_stack_.empty()); const auto& top = statements_stack_.back(); - auto* const guard_stmt = - AddStatement(std::make_unique())->AsIf(); + auto cond = MakeTrue(); + auto body = std::make_unique(); + auto* const guard_stmt = AddStatement(std::make_unique( + std::move(cond), std::move(body))) + ->AsIf(); guard_stmt->set_condition(MakeTrue()); PushNewStatementBlock(top.construct_, end_id, [guard_stmt](StatementBlock* s) { @@ -1977,12 +1981,15 @@ bool FunctionEmitter::EmitIfStart(const BlockInfo& block_info) { AddStatement(std::move(guard_decl)); } - auto* const if_stmt = - AddStatement(std::make_unique())->AsIf(); const auto condition_id = block_info.basic_block->terminator()->GetSingleWordInOperand(0); + auto cond = MakeExpression(condition_id).expr; + auto body = std::make_unique(); + auto* const if_stmt = AddStatement(std::make_unique( + std::move(cond), std::move(body))) + ->AsIf(); + // Generate the code for the condition. - if_stmt->set_condition(std::move(MakeExpression(condition_id).expr)); // Compute the block IDs that should end the then-clause and the else-clause. @@ -2413,8 +2420,8 @@ std::unique_ptr FunctionEmitter::MakeSimpleIf( if ((then_stmt == nullptr) && (else_stmt == nullptr)) { return nullptr; } - auto if_stmt = std::make_unique(); - if_stmt->set_condition(std::move(condition)); + auto if_stmt = std::make_unique( + std::move(condition), std::make_unique()); if (then_stmt != nullptr) { auto stmts = std::make_unique(); stmts->append(std::move(then_stmt));