ast: Remove Function::set_body()
Bug: tint:390
Change-Id: I525086868c6470ba39fe8c4ede7390e74c9489ff
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/35012
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
diff --git a/src/ast/function.h b/src/ast/function.h
index ba819ab..894525a 100644
--- a/src/ast/function.h
+++ b/src/ast/function.h
@@ -153,9 +153,6 @@
// function is empty
const Statement* get_last_statement() const;
- /// Sets the body of the function
- /// @param body the function body
- void set_body(BlockStatement* body) { body_ = body; }
/// @returns the function body
const BlockStatement* body() const { return body_; }
/// @returns the function body
diff --git a/src/ast/function_test.cc b/src/ast/function_test.cc
index db0e995..1413792 100644
--- a/src/ast/function_test.cc
+++ b/src/ast/function_test.cc
@@ -185,12 +185,11 @@
params.push_back(
create<Variable>(Source{}, "var", StorageClass::kNone, &i32));
- auto* block = create<BlockStatement>();
- block->append(create<DiscardStatement>());
+ auto* body = create<BlockStatement>();
+ body->append(create<DiscardStatement>());
- Function f(Source{}, "func", params, &void_type, create<BlockStatement>(),
+ Function f(Source{}, "func", params, &void_type, body,
FunctionDecorationList{});
- f.set_body(block);
EXPECT_TRUE(f.IsValid());
}
@@ -253,13 +252,13 @@
params.push_back(
create<Variable>(Source{}, "var", StorageClass::kNone, &i32));
- auto* block = create<BlockStatement>();
- block->append(create<DiscardStatement>());
- block->append(nullptr);
+ auto* body = create<BlockStatement>();
+ body->append(create<DiscardStatement>());
+ body->append(nullptr);
- Function f(Source{}, "func", params, &void_type, create<BlockStatement>(),
+ Function f(Source{}, "func", params, &void_type, body,
FunctionDecorationList{});
- f.set_body(block);
+
EXPECT_FALSE(f.IsValid());
}
@@ -271,13 +270,12 @@
params.push_back(
create<Variable>(Source{}, "var", StorageClass::kNone, &i32));
- auto* block = create<BlockStatement>();
- block->append(create<DiscardStatement>());
- block->append(nullptr);
+ auto* body = create<BlockStatement>();
+ body->append(create<DiscardStatement>());
+ body->append(nullptr);
- Function f(Source{}, "func", params, &void_type, create<BlockStatement>(),
+ Function f(Source{}, "func", params, &void_type, body,
FunctionDecorationList{});
- f.set_body(block);
EXPECT_FALSE(f.IsValid());
}
@@ -285,12 +283,10 @@
type::Void void_type;
type::I32 i32;
- auto* block = create<BlockStatement>();
- block->append(create<DiscardStatement>());
+ auto* body = create<BlockStatement>();
+ body->append(create<DiscardStatement>());
- Function f(Source{}, "func", {}, &void_type, create<BlockStatement>(),
- FunctionDecorationList{});
- f.set_body(block);
+ Function f(Source{}, "func", {}, &void_type, body, FunctionDecorationList{});
std::ostringstream out;
f.to_str(out, 2);
@@ -306,11 +302,11 @@
type::Void void_type;
type::I32 i32;
- auto* block = create<BlockStatement>();
- block->append(create<DiscardStatement>());
+ auto* body = create<BlockStatement>();
+ body->append(create<DiscardStatement>());
Function f(
- Source{}, "func", {}, &void_type, block,
+ Source{}, "func", {}, &void_type, body,
FunctionDecorationList{create<WorkgroupDecoration>(2, 4, 6, Source{})});
std::ostringstream out;
@@ -332,12 +328,11 @@
params.push_back(
create<Variable>(Source{}, "var", StorageClass::kNone, &i32));
- auto* block = create<BlockStatement>();
- block->append(create<DiscardStatement>());
+ auto* body = create<BlockStatement>();
+ body->append(create<DiscardStatement>());
- Function f(Source{}, "func", params, &void_type, create<BlockStatement>(),
+ Function f(Source{}, "func", params, &void_type, body,
FunctionDecorationList{});
- f.set_body(block);
std::ostringstream out;
f.to_str(out, 2);
@@ -386,9 +381,8 @@
auto* body = create<BlockStatement>();
auto* stmt = create<DiscardStatement>();
body->append(stmt);
- Function f(Source{}, "func", params, &void_type, create<BlockStatement>(),
+ Function f(Source{}, "func", params, &void_type, body,
FunctionDecorationList{});
- f.set_body(body);
EXPECT_EQ(f.get_last_statement(), stmt);
}
@@ -398,9 +392,8 @@
VariableList params;
auto* body = create<BlockStatement>();
- Function f(Source{}, "func", params, &void_type, create<BlockStatement>(),
+ Function f(Source{}, "func", params, &void_type, body,
FunctionDecorationList{});
- f.set_body(body);
EXPECT_EQ(f.get_last_statement(), nullptr);
}
diff --git a/src/reader/spirv/function.cc b/src/reader/spirv/function.cc
index a2d90f7..d6daa3d 100644
--- a/src/reader/spirv/function.cc
+++ b/src/reader/spirv/function.cc
@@ -702,7 +702,8 @@
return true;
}
- if (!EmitFunctionDeclaration()) {
+ FunctionDeclaration decl;
+ if (!ParseFunctionDeclaration(&decl)) {
return false;
}
@@ -716,8 +717,12 @@
"element but has "
<< statements_stack_.size();
}
+
auto* body = statements_stack_[0].statements_;
- parser_impl_.get_module().functions().back()->set_body(body);
+ ast_module_.AddFunction(create<ast::Function>(
+ decl.source, decl.name, std::move(decl.params), decl.return_type, body,
+ std::move(decl.decorations)));
+
// Maintain the invariant by repopulating the one and only element.
statements_stack_.clear();
PushNewStatementBlock(constructs_[0].get(), 0, nullptr, nullptr, nullptr);
@@ -725,7 +730,7 @@
return success();
}
-bool FunctionEmitter::EmitFunctionDeclaration() {
+bool FunctionEmitter::ParseFunctionDeclaration(FunctionDeclaration* decl) {
if (failed()) {
return false;
}
@@ -774,11 +779,11 @@
if (ep_info_ != nullptr) {
decos.emplace_back(create<ast::StageDecoration>(ep_info_->stage, Source{}));
}
- auto* ast_fn =
- create<ast::Function>(Source{}, name, std::move(ast_params), ret_ty,
- create<ast::BlockStatement>(), std::move(decos));
- ast_module_.AddFunction(ast_fn);
+ decl->name = name;
+ decl->params = std::move(ast_params);
+ decl->return_type = ret_ty;
+ decl->decorations = std::move(decos);
return success();
}
@@ -4114,6 +4119,9 @@
return ast_module_.create<ast::BitcastExpression>(dest_type, texel_prefix);
}
+FunctionEmitter::FunctionDeclaration::FunctionDeclaration() = default;
+FunctionEmitter::FunctionDeclaration::~FunctionDeclaration() = default;
+
} // namespace spirv
} // namespace reader
} // namespace tint
diff --git a/src/reader/spirv/function.h b/src/reader/spirv/function.h
index 3bc36f2..d276a54 100644
--- a/src/reader/spirv/function.h
+++ b/src/reader/spirv/function.h
@@ -328,12 +328,6 @@
/// @returns the parser implementation
ParserImpl* parser() { return &parser_impl_; }
- /// Emits the declaration, which comprises the name, parameters, and
- /// return type. The function AST node is appended to the module
- /// AST node.
- /// @returns true if emission has not yet failed.
- bool EmitFunctionDeclaration();
-
/// Emits the function body, populating the bottom entry of the statements
/// stack.
/// @returns false if emission failed.
@@ -700,6 +694,31 @@
const spvtools::opt::Instruction& image_access);
private:
+ /// FunctionDeclaration contains the parsed information for a function header.
+ struct FunctionDeclaration {
+ /// Constructor
+ FunctionDeclaration();
+ /// Destructor
+ ~FunctionDeclaration();
+
+ /// Parsed header source
+ Source source;
+ /// Function name
+ std::string name;
+ /// Function parameters
+ ast::VariableList params;
+ /// Function return type
+ ast::type::Type* return_type;
+ /// Function decorations
+ ast::FunctionDecorationList decorations;
+ };
+
+ /// Parse the function declaration, which comprises the name, parameters, and
+ /// return type, populating `decl`.
+ /// @param decl the FunctionDeclaration to populate
+ /// @returns true if emission has not yet failed.
+ bool ParseFunctionDeclaration(FunctionDeclaration* decl);
+
/// @returns the store type for the OpVariable instruction, or
/// null on failure.
ast::type::Type* GetVariableStoreType(
diff --git a/src/reader/spirv/function_decl_test.cc b/src/reader/spirv/function_decl_test.cc
index ed1afb6..2223893 100644
--- a/src/reader/spirv/function_decl_test.cc
+++ b/src/reader/spirv/function_decl_test.cc
@@ -49,7 +49,7 @@
)";
}
-TEST_F(SpvParserTest, EmitFunctionDeclaration_VoidFunctionWithoutParams) {
+TEST_F(SpvParserTest, Emit_VoidFunctionWithoutParams) {
auto p = parser(test::Assemble(CommonTypes() + R"(
%100 = OpFunction %void None %voidfn
%entry = OpLabel
@@ -58,15 +58,20 @@
)"));
ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions());
FunctionEmitter fe(p.get(), *spirv_function(p.get(), 100));
- EXPECT_TRUE(fe.EmitFunctionDeclaration());
- EXPECT_THAT(p->module().to_str(), HasSubstr(R"(
+ EXPECT_TRUE(fe.Emit());
+ auto got = p->module().to_str();
+ auto* expect = R"(Module{
Function x_100 -> __void
()
{
- })"));
+ Return{}
+ }
+}
+)";
+ EXPECT_EQ(got, expect);
}
-TEST_F(SpvParserTest, EmitFunctionDeclaration_NonVoidResultType) {
+TEST_F(SpvParserTest, Emit_NonVoidResultType) {
auto p = parser(test::Assemble(CommonTypes() + R"(
%fn_ret_float = OpTypeFunction %float
%100 = OpFunction %float None %fn_ret_float
@@ -76,16 +81,25 @@
)"));
ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions());
FunctionEmitter fe(p.get(), *spirv_function(p.get(), 100));
- EXPECT_TRUE(fe.EmitFunctionDeclaration());
+ EXPECT_TRUE(fe.Emit());
- EXPECT_THAT(p->module().to_str(), HasSubstr(R"(
+ auto got = p->module().to_str();
+ auto* expect = R"(Module{
Function x_100 -> __f32
()
{
- })"));
+ Return{
+ {
+ ScalarConstructor[not set]{0.000000}
+ }
+ }
+ }
+}
+)";
+ EXPECT_EQ(got, expect);
}
-TEST_F(SpvParserTest, EmitFunctionDeclaration_MixedParamTypes) {
+TEST_F(SpvParserTest, Emit_MixedParamTypes) {
auto p = parser(test::Assemble(Names({"a", "b", "c"}) + CommonTypes() + R"(
%fn_mixed_params = OpTypeFunction %float %uint %float %int
@@ -99,9 +113,10 @@
)"));
ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions());
FunctionEmitter fe(p.get(), *spirv_function(p.get(), 100));
- EXPECT_TRUE(fe.EmitFunctionDeclaration());
+ EXPECT_TRUE(fe.Emit());
- EXPECT_THAT(p->module().to_str(), HasSubstr(R"(
+ auto got = p->module().to_str();
+ auto* expect = R"(Module{
Function x_100 -> __void
(
VariableConst{
@@ -121,10 +136,14 @@
}
)
{
- })"));
+ Return{}
+ }
+}
+)";
+ EXPECT_EQ(got, expect);
}
-TEST_F(SpvParserTest, EmitFunctionDeclaration_GenerateParamNames) {
+TEST_F(SpvParserTest, Emit_GenerateParamNames) {
auto p = parser(test::Assemble(CommonTypes() + R"(
%fn_mixed_params = OpTypeFunction %float %uint %float %int
@@ -138,9 +157,10 @@
)"));
ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions());
FunctionEmitter fe(p.get(), *spirv_function(p.get(), 100));
- EXPECT_TRUE(fe.EmitFunctionDeclaration());
+ EXPECT_TRUE(fe.Emit());
- EXPECT_THAT(p->module().to_str(), HasSubstr(R"(
+ auto got = p->module().to_str();
+ auto* expect = R"(Module{
Function x_100 -> __void
(
VariableConst{
@@ -160,7 +180,11 @@
}
)
{
- })"));
+ Return{}
+ }
+}
+)";
+ EXPECT_EQ(got, expect);
}
} // namespace
diff --git a/src/writer/spirv/builder_function_decoration_test.cc b/src/writer/spirv/builder_function_decoration_test.cc
index cb5da8a..b796d34 100644
--- a/src/writer/spirv/builder_function_decoration_test.cc
+++ b/src/writer/spirv/builder_function_decoration_test.cc
@@ -143,12 +143,6 @@
ast::type::F32 f32;
ast::type::Void void_type;
- ast::Function func(
- Source{}, "main", {}, &void_type, create<ast::BlockStatement>(),
- ast::FunctionDecorationList{
- create<ast::StageDecoration>(ast::PipelineStage::kVertex, Source{}),
- });
-
auto* body = create<ast::BlockStatement>();
body->append(create<ast::AssignmentStatement>(
create<ast::IdentifierExpression>("my_out"),
@@ -160,7 +154,12 @@
body->append(create<ast::AssignmentStatement>(
create<ast::IdentifierExpression>("my_out"),
create<ast::IdentifierExpression>("my_in")));
- func.set_body(body);
+
+ ast::Function func(
+ Source{}, "main", {}, &void_type, body,
+ ast::FunctionDecorationList{
+ create<ast::StageDecoration>(ast::PipelineStage::kVertex, Source{}),
+ });
auto* v_in =
create<ast::Variable>(Source{}, "my_in", ast::StorageClass::kInput, &f32);
