[transformer] Remove deprecated Manager constructor
Also moves the type determiner call out of the transformers into the
manager.
Cleans up the code to not have anything directly calling
Run() on the transformers other then the manager.
Bug: tint:308
Change-Id: I3343f2ba16dae6fb33f35e390ae4c797f2a05522
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/33262
Auto-Submit: Ryan Harrison <rharrison@chromium.org>
Commit-Queue: dan sinclair <dsinclair@chromium.org>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
diff --git a/samples/main.cc b/samples/main.cc
index 19750cb..6e2456f 100644
--- a/samples/main.cc
+++ b/samples/main.cc
@@ -507,7 +507,7 @@
return 1;
}
- tint::transform::Manager transform_manager;
+ tint::transform::Manager transform_manager(&ctx, &mod);
for (const auto& name : options.transforms) {
// TODO(dsinclair): The vertex pulling transform requires setup code to
// be run that needs user input. Should we find a way to support that here
diff --git a/src/transform/bound_array_accessors_transform.h b/src/transform/bound_array_accessors_transform.h
index f67834b..fb20428 100644
--- a/src/transform/bound_array_accessors_transform.h
+++ b/src/transform/bound_array_accessors_transform.h
@@ -40,6 +40,9 @@
explicit BoundArrayAccessorsTransform(Context* ctx, ast::Module* mod);
~BoundArrayAccessorsTransform() override;
+ /// Users of Tint should register the transform with transform manager and
+ /// invoke its Run(), instead of directly calling the transform's Run().
+ /// Calling Run() directly does not perform module state cleanup operations.
/// @returns true if the transformation was successful
bool Run() override;
diff --git a/src/transform/bound_array_accessors_transform_test.cc b/src/transform/bound_array_accessors_transform_test.cc
index aa994ae..78cd50c 100644
--- a/src/transform/bound_array_accessors_transform_test.cc
+++ b/src/transform/bound_array_accessors_transform_test.cc
@@ -41,6 +41,7 @@
#include "src/ast/variable.h"
#include "src/ast/variable_decl_statement.h"
#include "src/context.h"
+#include "src/transform/manager.h"
#include "src/type_determiner.h"
namespace tint {
@@ -49,7 +50,13 @@
class BoundArrayAccessorsTest : public testing::Test {
public:
- BoundArrayAccessorsTest() : td_(&ctx_, &mod_), transform_(&ctx_, &mod_) {}
+ BoundArrayAccessorsTest() : td_(&ctx_, &mod_) {
+ auto transform =
+ std::make_unique<BoundArrayAccessorsTransform>(&ctx_, &mod_);
+ transform_ = transform.get();
+ manager_ = std::make_unique<Manager>(&ctx_, &mod_);
+ manager_->append(std::move(transform));
+ }
ast::BlockStatement* SetupFunctionAndBody() {
auto* block = create<ast::BlockStatement>();
@@ -67,7 +74,7 @@
TypeDeterminer* td() { return &td_; }
- BoundArrayAccessorsTransform* transform() { return &transform_; }
+ Manager* manager() { return manager_.get(); }
/// Creates a new `ast::Node` owned by the Context. When the Context is
/// destructed, the `ast::Node` will also be destructed.
@@ -83,7 +90,8 @@
ast::Module mod_;
TypeDeterminer td_;
ast::type::VoidType void_type_;
- BoundArrayAccessorsTransform transform_;
+ std::unique_ptr<Manager> manager_;
+ BoundArrayAccessorsTransform* transform_;
ast::BlockStatement* body_ = nullptr;
};
@@ -120,7 +128,7 @@
ASSERT_TRUE(td()->Determine()) << td()->error();
- ASSERT_TRUE(transform()->Run());
+ ASSERT_TRUE(manager()->Run());
ASSERT_TRUE(ptr->IsArrayAccessor());
ASSERT_TRUE(ptr->idx_expr()->IsCall());
@@ -183,7 +191,7 @@
ASSERT_TRUE(td()->Determine()) << td()->error();
- ASSERT_TRUE(transform()->Run());
+ ASSERT_TRUE(manager()->Run());
ASSERT_TRUE(ptr->IsArrayAccessor());
ASSERT_TRUE(ptr->idx_expr()->IsCall());
@@ -256,7 +264,7 @@
ASSERT_TRUE(td()->Determine()) << td()->error();
- ASSERT_TRUE(transform()->Run());
+ ASSERT_TRUE(manager()->Run());
ASSERT_TRUE(ptr->IsArrayAccessor());
ASSERT_TRUE(ptr->idx_expr()->IsConstructor());
ASSERT_TRUE(ptr->idx_expr()->AsConstructor()->IsScalarConstructor());
@@ -304,7 +312,7 @@
ASSERT_TRUE(td()->Determine()) << td()->error();
- ASSERT_TRUE(transform()->Run());
+ ASSERT_TRUE(manager()->Run());
ASSERT_TRUE(ptr->IsArrayAccessor());
ASSERT_TRUE(ptr->idx_expr()->IsCall());
@@ -357,7 +365,7 @@
ASSERT_TRUE(td()->Determine()) << td()->error();
- ASSERT_TRUE(transform()->Run());
+ ASSERT_TRUE(manager()->Run());
ASSERT_TRUE(ptr->IsArrayAccessor());
ASSERT_TRUE(ptr->idx_expr()->IsConstructor());
ASSERT_TRUE(ptr->idx_expr()->AsConstructor()->IsScalarConstructor());
@@ -396,7 +404,7 @@
ASSERT_TRUE(td()->Determine()) << td()->error();
- ASSERT_TRUE(transform()->Run());
+ ASSERT_TRUE(manager()->Run());
ASSERT_TRUE(ptr->IsArrayAccessor());
ASSERT_TRUE(ptr->idx_expr()->IsConstructor());
ASSERT_TRUE(ptr->idx_expr()->AsConstructor()->IsScalarConstructor());
@@ -435,7 +443,7 @@
ASSERT_TRUE(td()->Determine()) << td()->error();
- ASSERT_TRUE(transform()->Run());
+ ASSERT_TRUE(manager()->Run());
ASSERT_TRUE(ptr->IsArrayAccessor());
ASSERT_TRUE(ptr->idx_expr()->IsConstructor());
ASSERT_TRUE(ptr->idx_expr()->AsConstructor()->IsScalarConstructor());
@@ -483,7 +491,7 @@
ASSERT_TRUE(td()->Determine()) << td()->error();
- ASSERT_TRUE(transform()->Run());
+ ASSERT_TRUE(manager()->Run());
ASSERT_TRUE(ptr->IsArrayAccessor());
ASSERT_TRUE(ptr->idx_expr()->IsCall());
@@ -535,7 +543,7 @@
ASSERT_TRUE(td()->Determine()) << td()->error();
- ASSERT_TRUE(transform()->Run());
+ ASSERT_TRUE(manager()->Run());
ASSERT_TRUE(ptr->IsArrayAccessor());
ASSERT_TRUE(ptr->idx_expr()->IsConstructor());
ASSERT_TRUE(ptr->idx_expr()->AsConstructor()->IsScalarConstructor());
@@ -574,7 +582,7 @@
ASSERT_TRUE(td()->Determine()) << td()->error();
- ASSERT_TRUE(transform()->Run());
+ ASSERT_TRUE(manager()->Run());
ASSERT_TRUE(ptr->IsArrayAccessor());
ASSERT_TRUE(ptr->idx_expr()->IsConstructor());
ASSERT_TRUE(ptr->idx_expr()->AsConstructor()->IsScalarConstructor());
@@ -616,7 +624,7 @@
ASSERT_TRUE(td()->Determine()) << td()->error();
- ASSERT_TRUE(transform()->Run());
+ ASSERT_TRUE(manager()->Run());
ASSERT_TRUE(ptr->IsArrayAccessor());
ASSERT_TRUE(ptr->array()->IsArrayAccessor());
@@ -680,7 +688,7 @@
ASSERT_TRUE(td()->Determine()) << td()->error();
- ASSERT_TRUE(transform()->Run());
+ ASSERT_TRUE(manager()->Run());
ASSERT_TRUE(ptr->IsArrayAccessor());
ASSERT_TRUE(ptr->array()->IsArrayAccessor());
@@ -759,7 +767,7 @@
ASSERT_TRUE(td()->Determine()) << td()->error();
- ASSERT_TRUE(transform()->Run());
+ ASSERT_TRUE(manager()->Run());
ASSERT_TRUE(ptr->IsArrayAccessor());
ASSERT_TRUE(ptr->array()->IsArrayAccessor());
@@ -827,7 +835,7 @@
ASSERT_TRUE(td()->Determine()) << td()->error();
- ASSERT_TRUE(transform()->Run());
+ ASSERT_TRUE(manager()->Run());
ASSERT_TRUE(ptr->IsArrayAccessor());
ASSERT_TRUE(ptr->array()->IsArrayAccessor());
@@ -881,7 +889,7 @@
ASSERT_TRUE(td()->Determine()) << td()->error();
- ASSERT_TRUE(transform()->Run());
+ ASSERT_TRUE(manager()->Run());
ASSERT_TRUE(ptr->IsArrayAccessor());
ASSERT_TRUE(ptr->array()->IsArrayAccessor());
@@ -936,7 +944,7 @@
ASSERT_TRUE(td()->Determine()) << td()->error();
- ASSERT_TRUE(transform()->Run());
+ ASSERT_TRUE(manager()->Run());
ASSERT_TRUE(ptr->IsArrayAccessor());
ASSERT_TRUE(ptr->array()->IsArrayAccessor());
@@ -991,7 +999,7 @@
ASSERT_TRUE(td()->Determine()) << td()->error();
- ASSERT_TRUE(transform()->Run());
+ ASSERT_TRUE(manager()->Run());
ASSERT_TRUE(ptr->IsArrayAccessor());
ASSERT_TRUE(ptr->array()->IsArrayAccessor());
diff --git a/src/transform/manager.cc b/src/transform/manager.cc
index 2447605..0cf8a55 100644
--- a/src/transform/manager.cc
+++ b/src/transform/manager.cc
@@ -19,8 +19,6 @@
namespace tint {
namespace transform {
-Manager::Manager() : context_(nullptr), module_(nullptr) {}
-
Manager::Manager(Context* context, ast::Module* module)
: context_(context), module_(module) {}
diff --git a/src/transform/manager.h b/src/transform/manager.h
index 0d193a9..84f614e 100644
--- a/src/transform/manager.h
+++ b/src/transform/manager.h
@@ -31,8 +31,6 @@
class Manager {
public:
/// Constructor
- /// DEPRECATED
- Manager();
/// @param context the tint context
/// @param module the module to transform
Manager(Context* context, ast::Module* module);
diff --git a/src/transform/transformer.h b/src/transform/transformer.h
index 7608db6..7fbcb58 100644
--- a/src/transform/transformer.h
+++ b/src/transform/transformer.h
@@ -34,6 +34,9 @@
Transformer(Context* ctx, ast::Module* mod);
virtual ~Transformer();
+ /// Users of Tint should register the transform with transform manager and
+ /// invoke its Run(), instead of directly calling the transform's Run().
+ /// Calling Run() directly does not perform module state cleanup operations.
/// @returns true if the transformation was successful
virtual bool Run() = 0;
diff --git a/src/transform/vertex_pulling_transform.cc b/src/transform/vertex_pulling_transform.cc
index 7707194..d099d8f 100644
--- a/src/transform/vertex_pulling_transform.cc
+++ b/src/transform/vertex_pulling_transform.cc
@@ -38,7 +38,6 @@
#include "src/ast/type_constructor_expression.h"
#include "src/ast/uint_literal.h"
#include "src/ast/variable_decl_statement.h"
-#include "src/type_determiner.h"
namespace tint {
namespace transform {
@@ -101,11 +100,7 @@
AddVertexStorageBuffers();
AddVertexPullingPreamble(vertex_func);
- // We've potentially inserted nodes into the AST so we have to make sure to
- // re-run type determination else those nodes will be missing their
- // result_type
- TypeDeterminer td(ctx_, mod_);
- return td.Determine();
+ return true;
}
std::string VertexPullingTransform::GetVertexBufferName(uint32_t index) {
diff --git a/src/transform/vertex_pulling_transform.h b/src/transform/vertex_pulling_transform.h
index b92219d..ab876d9 100644
--- a/src/transform/vertex_pulling_transform.h
+++ b/src/transform/vertex_pulling_transform.h
@@ -164,6 +164,9 @@
/// @param number the set number we will use
void SetPullingBufferBindingSet(uint32_t number);
+ /// Users of Tint should register the transform with transform manager and
+ /// invoke its Run(), instead of directly calling the transform's Run().
+ /// Calling Run() directly does not perform module state cleanup operations.
/// @returns true if the transformation was successful
bool Run() override;
diff --git a/src/transform/vertex_pulling_transform_test.cc b/src/transform/vertex_pulling_transform_test.cc
index f8af132..4da530c 100644
--- a/src/transform/vertex_pulling_transform_test.cc
+++ b/src/transform/vertex_pulling_transform_test.cc
@@ -25,6 +25,7 @@
#include "src/ast/type/f32_type.h"
#include "src/ast/type/i32_type.h"
#include "src/ast/type/void_type.h"
+#include "src/transform/manager.h"
#include "src/type_determiner.h"
#include "src/validator/validator.h"
@@ -36,7 +37,11 @@
public:
VertexPullingTransformHelper() {
mod_ = std::make_unique<ast::Module>();
- transform_ = std::make_unique<VertexPullingTransform>(&ctx_, mod_.get());
+ manager_ = std::make_unique<Manager>(&ctx_, mod_.get());
+ auto transform =
+ std::make_unique<VertexPullingTransform>(&ctx_, mod_.get());
+ transform_ = transform.get();
+ manager_->append(std::move(transform));
}
// Create basic module with an entry point and vertex function
@@ -78,7 +83,8 @@
Context* ctx() { return &ctx_; }
ast::Module* mod() { return mod_.get(); }
- VertexPullingTransform* transform() { return transform_.get(); }
+ Manager* manager() { return manager_.get(); }
+ VertexPullingTransform* transform() { return transform_; }
/// Creates a new `ast::Node` owned by the Context. When the Context is
/// destructed, the `ast::Node` will also be destructed.
@@ -92,21 +98,22 @@
private:
Context ctx_;
std::unique_ptr<ast::Module> mod_;
- std::unique_ptr<VertexPullingTransform> transform_;
+ std::unique_ptr<Manager> manager_;
+ VertexPullingTransform* transform_;
};
class VertexPullingTransformTest : public VertexPullingTransformHelper,
public testing::Test {};
TEST_F(VertexPullingTransformTest, Error_NoVertexState) {
- EXPECT_FALSE(transform()->Run());
- EXPECT_EQ(transform()->error(), "SetVertexState not called");
+ EXPECT_FALSE(manager()->Run());
+ EXPECT_EQ(manager()->error(), "SetVertexState not called");
}
TEST_F(VertexPullingTransformTest, Error_NoEntryPoint) {
transform()->SetVertexState(std::make_unique<VertexStateDescriptor>());
- EXPECT_FALSE(transform()->Run());
- EXPECT_EQ(transform()->error(), "Vertex stage entry point not found");
+ EXPECT_FALSE(manager()->Run());
+ EXPECT_EQ(manager()->error(), "Vertex stage entry point not found");
}
TEST_F(VertexPullingTransformTest, Error_InvalidEntryPoint) {
@@ -114,8 +121,8 @@
InitTransform({});
transform()->SetEntryPoint("_");
- EXPECT_FALSE(transform()->Run());
- EXPECT_EQ(transform()->error(), "Vertex stage entry point not found");
+ EXPECT_FALSE(manager()->Run());
+ EXPECT_EQ(manager()->error(), "Vertex stage entry point not found");
}
TEST_F(VertexPullingTransformTest, Error_EntryPointWrongStage) {
@@ -128,14 +135,14 @@
mod()->AddFunction(func);
InitTransform({});
- EXPECT_FALSE(transform()->Run());
- EXPECT_EQ(transform()->error(), "Vertex stage entry point not found");
+ EXPECT_FALSE(manager()->Run());
+ EXPECT_EQ(manager()->error(), "Vertex stage entry point not found");
}
TEST_F(VertexPullingTransformTest, BasicModule) {
InitBasicModule();
InitTransform({});
- EXPECT_TRUE(transform()->Run());
+ EXPECT_TRUE(manager()->Run());
}
TEST_F(VertexPullingTransformTest, OneAttribute) {
@@ -146,7 +153,7 @@
InitTransform({{{4, InputStepMode::kVertex, {{VertexFormat::kF32, 0, 0}}}}});
- EXPECT_TRUE(transform()->Run());
+ EXPECT_TRUE(manager()->Run());
EXPECT_EQ(R"(Module{
TintVertexData Struct{
@@ -231,7 +238,7 @@
InitTransform(
{{{4, InputStepMode::kInstance, {{VertexFormat::kF32, 0, 0}}}}});
- EXPECT_TRUE(transform()->Run());
+ EXPECT_TRUE(manager()->Run());
EXPECT_EQ(R"(Module{
TintVertexData Struct{
@@ -316,7 +323,7 @@
InitTransform({{{4, InputStepMode::kVertex, {{VertexFormat::kF32, 0, 0}}}}});
transform()->SetPullingBufferBindingSet(5);
- EXPECT_TRUE(transform()->Run());
+ EXPECT_TRUE(manager()->Run());
EXPECT_EQ(R"(Module{
TintVertexData Struct{
@@ -431,7 +438,7 @@
{{{4, InputStepMode::kVertex, {{VertexFormat::kF32, 0, 0}}},
{4, InputStepMode::kInstance, {{VertexFormat::kF32, 0, 1}}}}});
- EXPECT_TRUE(transform()->Run());
+ EXPECT_TRUE(manager()->Run());
EXPECT_EQ(R"(Module{
TintVertexData Struct{
@@ -571,7 +578,7 @@
InputStepMode::kVertex,
{{VertexFormat::kF32, 0, 0}, {VertexFormat::kVec4F32, 0, 1}}}}});
- EXPECT_TRUE(transform()->Run());
+ EXPECT_TRUE(manager()->Run());
EXPECT_EQ(R"(Module{
TintVertexData Struct{
@@ -756,7 +763,7 @@
{12, InputStepMode::kVertex, {{VertexFormat::kVec3F32, 0, 1}}},
{16, InputStepMode::kVertex, {{VertexFormat::kVec4F32, 0, 2}}}}});
- EXPECT_TRUE(transform()->Run());
+ EXPECT_TRUE(manager()->Run());
EXPECT_EQ(R"(Module{
TintVertexData Struct{
