Move the ast node ownership from Context to Module
Fixes: tint:335
Change-Id: I128e229daa56d43e7227ecab72269be33b3ee012
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/33240
Commit-Queue: Ben Clayton <bclayton@google.com>
Auto-Submit: Ben Clayton <bclayton@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
diff --git a/src/ast/builder.cc b/src/ast/builder.cc
index de6b3ae..b4aacca 100644
--- a/src/ast/builder.cc
+++ b/src/ast/builder.cc
@@ -25,7 +25,8 @@
void_(tm->Get<ast::type::VoidType>()),
tm_(tm) {}
-Builder::Builder(tint::Context* c) : ctx(c), ty(&c->type_mgr()) {}
+Builder::Builder(tint::Context* c, tint::ast::Module* m)
+ : ctx(c), mod(m), ty(&c->type_mgr()) {}
Builder::~Builder() = default;
ast::Variable* Builder::Var(const std::string& name,
@@ -36,9 +37,11 @@
return var;
}
-BuilderWithContext::BuilderWithContext() : Builder(new Context()) {}
-BuilderWithContext::~BuilderWithContext() {
+BuilderWithContextAndModule::BuilderWithContextAndModule()
+ : Builder(new Context(), new ast::Module()) {}
+BuilderWithContextAndModule::~BuilderWithContextAndModule() {
delete ctx;
+ delete mod;
}
} // namespace ast
diff --git a/src/ast/builder.h b/src/ast/builder.h
index 364bfd3..dcee4a2 100644
--- a/src/ast/builder.h
+++ b/src/ast/builder.h
@@ -24,6 +24,7 @@
#include "src/ast/expression.h"
#include "src/ast/float_literal.h"
#include "src/ast/identifier_expression.h"
+#include "src/ast/module.h"
#include "src/ast/scalar_constructor_expression.h"
#include "src/ast/sint_literal.h"
#include "src/ast/type/array_type.h"
@@ -185,7 +186,8 @@
/// Constructor
/// @param ctx the context to use in the builder
- explicit Builder(tint::Context* ctx);
+ /// @param mod the module to use in the builder
+ explicit Builder(tint::Context* ctx, tint::ast::Module* mod);
virtual ~Builder();
/// @param expr the expression
@@ -441,17 +443,19 @@
ExprList(std::forward<ARGS>(args)...)};
}
- /// Creates a new `ast::Node` owned by the Context. When the Context is
+ /// Creates a new `ast::Node` owned by the Module. When the Module is
/// destructed, the `ast::Node` will also be destructed.
/// @param args the arguments to pass to the type constructor
/// @returns the node pointer
template <typename T, typename... ARGS>
T* create(ARGS&&... args) {
- return ctx->create<T>(std::forward<ARGS>(args)...);
+ return mod->create<T>(std::forward<ARGS>(args)...);
}
- /// The builder context
+ /// The builder module
tint::Context* const ctx;
+ /// The builder module
+ tint::ast::Module* const mod;
/// The builder types
const TypesBuilder ty;
@@ -460,11 +464,12 @@
virtual void OnVariableBuilt(ast::Variable*) {}
};
-/// BuilderWithContext is a `Builder` that constructs and owns its `Context`.
-class BuilderWithContext : public Builder {
+/// BuilderWithContextAndModule is a `Builder` that constructs and owns its
+/// `Context` and `Module`.
+class BuilderWithContextAndModule : public Builder {
public:
- BuilderWithContext();
- ~BuilderWithContext() override;
+ BuilderWithContextAndModule();
+ ~BuilderWithContextAndModule() override;
};
//! @cond Doxygen_Suppress
diff --git a/src/ast/module.h b/src/ast/module.h
index c0b1e7b..6ec3c78 100644
--- a/src/ast/module.h
+++ b/src/ast/module.h
@@ -77,6 +77,20 @@
/// @returns a string representation of the module
std::string to_str() const;
+ /// Creates a new `ast::Node` owned by the Module. When the Module is
+ /// destructed, the `ast::Node` will also be destructed.
+ /// @param args the arguments to pass to the type constructor
+ /// @returns the node pointer
+ template <typename T, typename... ARGS>
+ T* create(ARGS&&... args) {
+ static_assert(std::is_base_of<ast::Node, T>::value,
+ "T does not derive from ast::Node");
+ auto uptr = std::make_unique<T>(std::forward<ARGS>(args)...);
+ auto ptr = uptr.get();
+ ast_nodes_.emplace_back(std::move(uptr));
+ return ptr;
+ }
+
private:
Module(const Module&) = delete;
@@ -84,6 +98,7 @@
// The constructed types are owned by the type manager
std::vector<type::Type*> constructed_types_;
FunctionList functions_;
+ std::vector<std::unique_ptr<ast::Node>> ast_nodes_;
};
} // namespace ast
diff --git a/src/ast/test_helper.h b/src/ast/test_helper.h
index c97284c..18ad0cf 100644
--- a/src/ast/test_helper.h
+++ b/src/ast/test_helper.h
@@ -19,7 +19,7 @@
#include <utility>
#include "gtest/gtest.h"
-#include "src/context.h"
+#include "src/ast/module.h"
namespace tint {
namespace ast {
@@ -31,17 +31,17 @@
TestHelperBase() {}
~TestHelperBase() = default;
- /// Creates a new `ast::Node` owned by the Context. When the Context is
+ /// Creates a new `ast::Node` owned by the Module. When the Module is
/// destructed, the `ast::Node` will also be destructed.
/// @param args the arguments to pass to the type constructor
/// @returns the node pointer
template <typename T, typename... ARGS>
T* create(ARGS&&... args) {
- return ctx.create<T>(std::forward<ARGS>(args)...);
+ return mod.create<T>(std::forward<ARGS>(args)...);
}
- /// The context
- Context ctx;
+ /// The module
+ Module mod;
};
using TestHelper = TestHelperBase<testing::Test>;
diff --git a/src/context.h b/src/context.h
index edec3e7..300b113 100644
--- a/src/context.h
+++ b/src/context.h
@@ -51,24 +51,9 @@
/// @returns the namer object
Namer* namer() const { return namer_.get(); }
- /// Creates a new `ast::Node` owned by the Context. When the Context is
- /// destructed, the `ast::Node` will also be destructed.
- /// @param args the arguments to pass to the type constructor
- /// @returns the node pointer
- template <typename T, typename... ARGS>
- T* create(ARGS&&... args) {
- static_assert(std::is_base_of<ast::Node, T>::value,
- "T does not derive from ast::Node");
- auto uptr = std::make_unique<T>(std::forward<ARGS>(args)...);
- auto ptr = uptr.get();
- ast_nodes_.emplace_back(std::move(uptr));
- return ptr;
- }
-
private:
TypeManager type_mgr_;
std::unique_ptr<Namer> namer_;
- std::vector<std::unique_ptr<ast::Node>> ast_nodes_;
};
} // namespace tint
diff --git a/src/inspector/inspector_test.cc b/src/inspector/inspector_test.cc
index 4487192..d1d39e6 100644
--- a/src/inspector/inspector_test.cc
+++ b/src/inspector/inspector_test.cc
@@ -635,13 +635,13 @@
return &comparison_sampler_type_;
}
- /// Creates a new `ast::Node` owned by the Context. When the Context is
+ /// Creates a new `ast::Node` owned by the Module. When the Module is
/// destructed, the `ast::Node` will also be destructed.
/// @param args the arguments to pass to the type constructor
/// @returns the node pointer
template <typename T, typename... ARGS>
T* create(ARGS&&... args) {
- return ctx_.create<T>(std::forward<ARGS>(args)...);
+ return mod_.create<T>(std::forward<ARGS>(args)...);
}
private:
diff --git a/src/reader/reader.h b/src/reader/reader.h
index 249c8d2..481752f 100644
--- a/src/reader/reader.h
+++ b/src/reader/reader.h
@@ -59,15 +59,6 @@
/// @param diags the list of diagnostic messages
void set_diagnostics(const diag::List& diags) { diags_ = diags; }
- /// Creates a new `ast::Node` owned by the Context. When the Context is
- /// destructed, the `ast::Node` will also be destructed.
- /// @param args the arguments to pass to the type constructor
- /// @returns the node pointer
- template <typename T, typename... ARGS>
- T* create(ARGS&&... args) const {
- return ctx_.create<T>(std::forward<ARGS>(args)...);
- }
-
/// The Tint context object
Context& ctx_;
diff --git a/src/reader/spirv/function.h b/src/reader/spirv/function.h
index e9c54ad..3211dc9 100644
--- a/src/reader/spirv/function.h
+++ b/src/reader/spirv/function.h
@@ -797,14 +797,13 @@
/// @returns a boolean false expression.
ast::Expression* MakeFalse() const;
- /// Creates a new `ast::Node` owned by the Context. When the Context is
+ /// Creates a new `ast::Node` owned by the Module. When the Module is
/// destructed, the `ast::Node` will also be destructed.
/// @param args the arguments to pass to the type constructor
/// @returns the node pointer
template <typename T, typename... ARGS>
T* create(ARGS&&... args) const {
- auto& ctx = parser_impl_.context();
- return ctx.create<T>(std::forward<ARGS>(args)...);
+ return ast_module_.create<T>(std::forward<ARGS>(args)...);
}
ParserImpl& parser_impl_;
diff --git a/src/reader/spirv/parser_impl.h b/src/reader/spirv/parser_impl.h
index 48f8d20..f0a23ca 100644
--- a/src/reader/spirv/parser_impl.h
+++ b/src/reader/spirv/parser_impl.h
@@ -450,6 +450,15 @@
bool ApplyArrayDecorations(const spvtools::opt::analysis::Type* spv_type,
ast::type::ArrayType* ast_type);
+ /// Creates a new `ast::Node` owned by the Module. When the Module is
+ /// destructed, the `ast::Node` will also be destructed.
+ /// @param args the arguments to pass to the type constructor
+ /// @returns the node pointer
+ template <typename T, typename... ARGS>
+ T* create(ARGS&&... args) {
+ return ast_module_.create<T>(std::forward<ARGS>(args)...);
+ }
+
// The SPIR-V binary we're parsing
std::vector<uint32_t> spv_binary_;
diff --git a/src/reader/wgsl/parser_impl.h b/src/reader/wgsl/parser_impl.h
index daf4789..09c5f5d 100644
--- a/src/reader/wgsl/parser_impl.h
+++ b/src/reader/wgsl/parser_impl.h
@@ -734,13 +734,13 @@
Maybe<ast::Statement*> for_header_initializer();
Maybe<ast::Statement*> for_header_continuing();
- /// Creates a new `ast::Node` owned by the Context. When the Context is
+ /// Creates a new `ast::Node` owned by the Module. When the Module is
/// destructed, the `ast::Node` will also be destructed.
/// @param args the arguments to pass to the type constructor
/// @returns the node pointer
template <typename T, typename... ARGS>
T* create(ARGS&&... args) {
- return ctx_.create<T>(std::forward<ARGS>(args)...);
+ return module_.create<T>(std::forward<ARGS>(args)...);
}
Context& ctx_;
diff --git a/src/reader/wgsl/parser_impl_test_helper.h b/src/reader/wgsl/parser_impl_test_helper.h
index d77a909..6a6f1f5 100644
--- a/src/reader/wgsl/parser_impl_test_helper.h
+++ b/src/reader/wgsl/parser_impl_test_helper.h
@@ -76,7 +76,6 @@
private:
std::vector<std::unique_ptr<Source::File>> files_;
- std::unique_ptr<ParserImpl> impl;
Context ctx_;
};
diff --git a/src/transform/bound_array_accessors_transform_test.cc b/src/transform/bound_array_accessors_transform_test.cc
index 78cd50c..6302423 100644
--- a/src/transform/bound_array_accessors_transform_test.cc
+++ b/src/transform/bound_array_accessors_transform_test.cc
@@ -76,13 +76,13 @@
Manager* manager() { return manager_.get(); }
- /// Creates a new `ast::Node` owned by the Context. When the Context is
+ /// Creates a new `ast::Node` owned by the Module. When the Module is
/// destructed, the `ast::Node` will also be destructed.
/// @param args the arguments to pass to the type constructor
/// @returns the node pointer
template <typename T, typename... ARGS>
T* create(ARGS&&... args) {
- return ctx_.create<T>(std::forward<ARGS>(args)...);
+ return mod_.create<T>(std::forward<ARGS>(args)...);
}
private:
diff --git a/src/transform/transformer.h b/src/transform/transformer.h
index 7fbcb58..d1ec990 100644
--- a/src/transform/transformer.h
+++ b/src/transform/transformer.h
@@ -44,13 +44,13 @@
const std::string& error() { return error_; }
protected:
- /// Creates a new `ast::Node` owned by the Context. When the Context is
+ /// Creates a new `ast::Node` owned by the Module. When the Module is
/// destructed, the `ast::Node` will also be destructed.
/// @param args the arguments to pass to the type constructor
/// @returns the node pointer
template <typename T, typename... ARGS>
T* create(ARGS&&... args) {
- return ctx_->create<T>(std::forward<ARGS>(args)...);
+ return mod_->create<T>(std::forward<ARGS>(args)...);
}
/// The context
diff --git a/src/transform/vertex_pulling_transform_test.cc b/src/transform/vertex_pulling_transform_test.cc
index 4da530c..91b632b 100644
--- a/src/transform/vertex_pulling_transform_test.cc
+++ b/src/transform/vertex_pulling_transform_test.cc
@@ -86,13 +86,13 @@
Manager* manager() { return manager_.get(); }
VertexPullingTransform* transform() { return transform_; }
- /// Creates a new `ast::Node` owned by the Context. When the Context is
+ /// Creates a new `ast::Node` owned by the Module. When the Module is
/// destructed, the `ast::Node` will also be destructed.
/// @param args the arguments to pass to the type constructor
/// @returns the node pointer
template <typename T, typename... ARGS>
T* create(ARGS&&... args) {
- return ctx_.create<T>(std::forward<ARGS>(args)...);
+ return mod_->create<T>(std::forward<ARGS>(args)...);
}
private:
diff --git a/src/type_determiner_test.cc b/src/type_determiner_test.cc
index 7a25151..c12c0d2 100644
--- a/src/type_determiner_test.cc
+++ b/src/type_determiner_test.cc
@@ -88,13 +88,13 @@
ast::Module* mod() { return &mod_; }
Context* ctx() { return &ctx_; }
- /// Creates a new `ast::Node` owned by the Context. When the Context is
+ /// Creates a new `ast::Node` owned by the Module. When the Module is
/// destructed, the `ast::Node` will also be destructed.
/// @param args the arguments to pass to the type constructor
/// @returns the node pointer
template <typename T, typename... ARGS>
T* create(ARGS&&... args) {
- return ctx_.create<T>(std::forward<ARGS>(args)...);
+ return mod_.create<T>(std::forward<ARGS>(args)...);
}
private:
diff --git a/src/validator/validator_test_helper.h b/src/validator/validator_test_helper.h
index 92edc34..b4db450 100644
--- a/src/validator/validator_test_helper.h
+++ b/src/validator/validator_test_helper.h
@@ -41,13 +41,13 @@
/// @return a pointer to the test module
ast::Module* mod() { return &mod_; }
- /// Creates a new `ast::Node` owned by the Context. When the Context is
+ /// Creates a new `ast::Node` owned by the Module. When the Module is
/// destructed, the `ast::Node` will also be destructed.
/// @param args the arguments to pass to the type constructor
/// @returns the node pointer
template <typename T, typename... ARGS>
T* create(ARGS&&... args) {
- return ctx_.create<T>(std::forward<ARGS>(args)...);
+ return mod_.create<T>(std::forward<ARGS>(args)...);
}
private:
diff --git a/src/writer/hlsl/test_helper.h b/src/writer/hlsl/test_helper.h
index b55f542..aea3938 100644
--- a/src/writer/hlsl/test_helper.h
+++ b/src/writer/hlsl/test_helper.h
@@ -43,13 +43,13 @@
/// @returns the pre result string
std::string pre_result() const { return pre.str(); }
- /// Creates a new `ast::Node` owned by the Context. When the Context is
+ /// Creates a new `ast::Node` owned by the Module. When the Module is
/// destructed, the `ast::Node` will also be destructed.
/// @param args the arguments to pass to the type constructor
/// @returns the node pointer template <typename T, typename... ARGS>
template <typename T, typename... ARGS>
T* create(ARGS&&... args) {
- return ctx.create<T>(std::forward<ARGS>(args)...);
+ return mod.create<T>(std::forward<ARGS>(args)...);
}
/// The context
diff --git a/src/writer/msl/test_helper.h b/src/writer/msl/test_helper.h
index 149ba43..b336a47 100644
--- a/src/writer/msl/test_helper.h
+++ b/src/writer/msl/test_helper.h
@@ -35,13 +35,13 @@
TestHelperBase() : td(&ctx, &mod), gen(&ctx, &mod) {}
~TestHelperBase() = default;
- /// Creates a new `ast::Node` owned by the Context. When the Context is
+ /// Creates a new `ast::Node` owned by the Module. When the Module is
/// destructed, the `ast::Node` will also be destructed.
/// @param args the arguments to pass to the type constructor
/// @returns the node pointer
template <typename T, typename... ARGS>
T* create(ARGS&&... args) {
- return ctx.create<T>(std::forward<ARGS>(args)...);
+ return mod.create<T>(std::forward<ARGS>(args)...);
}
/// The context
diff --git a/src/writer/spirv/builder_function_decoration_test.cc b/src/writer/spirv/builder_function_decoration_test.cc
index 8b59b2e..161966c 100644
--- a/src/writer/spirv/builder_function_decoration_test.cc
+++ b/src/writer/spirv/builder_function_decoration_test.cc
@@ -105,9 +105,9 @@
EXPECT_TRUE(b.GenerateGlobalVariable(v_out)) << b.error();
EXPECT_TRUE(b.GenerateGlobalVariable(v_wg)) << b.error();
- mod.AddGlobalVariable(v_in);
- mod.AddGlobalVariable(v_out);
- mod.AddGlobalVariable(v_wg);
+ mod->AddGlobalVariable(v_in);
+ mod->AddGlobalVariable(v_out);
+ mod->AddGlobalVariable(v_wg);
ASSERT_TRUE(b.GenerateFunction(&func)) << b.error();
EXPECT_EQ(DumpInstructions(b.debug()), R"(OpName %1 "tint_6d795f696e"
@@ -168,9 +168,9 @@
EXPECT_TRUE(b.GenerateGlobalVariable(v_out)) << b.error();
EXPECT_TRUE(b.GenerateGlobalVariable(v_wg)) << b.error();
- mod.AddGlobalVariable(v_in);
- mod.AddGlobalVariable(v_out);
- mod.AddGlobalVariable(v_wg);
+ mod->AddGlobalVariable(v_in);
+ mod->AddGlobalVariable(v_out);
+ mod->AddGlobalVariable(v_wg);
ASSERT_TRUE(b.GenerateFunction(&func)) << b.error();
EXPECT_EQ(DumpInstructions(b.debug()), R"(OpName %1 "tint_6d795f696e"
diff --git a/src/writer/spirv/builder_function_test.cc b/src/writer/spirv/builder_function_test.cc
index 1a6de92..20cbf8a 100644
--- a/src/writer/spirv/builder_function_test.cc
+++ b/src/writer/spirv/builder_function_test.cc
@@ -252,10 +252,10 @@
decos.push_back(create<ast::SetDecoration>(0, Source{}));
data_var->set_decorations(decos);
- mod.AddConstructedType(&s);
+ mod->AddConstructedType(&s);
td.RegisterVariableForTesting(data_var);
- mod.AddGlobalVariable(data_var);
+ mod->AddGlobalVariable(data_var);
{
ast::VariableList params;
@@ -272,7 +272,7 @@
func->add_decoration(
create<ast::StageDecoration>(ast::PipelineStage::kCompute, Source{}));
- mod.AddFunction(func);
+ mod->AddFunction(func);
}
{
@@ -290,7 +290,7 @@
func->add_decoration(
create<ast::StageDecoration>(ast::PipelineStage::kCompute, Source{}));
- mod.AddFunction(func);
+ mod->AddFunction(func);
}
ASSERT_TRUE(td.Determine()) << td.error();
diff --git a/src/writer/spirv/builder_intrinsic_test.cc b/src/writer/spirv/builder_intrinsic_test.cc
index c877885..3b05637 100644
--- a/src/writer/spirv/builder_intrinsic_test.cc
+++ b/src/writer/spirv/builder_intrinsic_test.cc
@@ -51,16 +51,15 @@
namespace spirv {
namespace {
-class IntrinsicBuilderTest : public ast::BuilderWithContext,
+class IntrinsicBuilderTest : public ast::BuilderWithContextAndModule,
public testing::Test {
protected:
void OnVariableBuilt(ast::Variable* var) override {
td.RegisterVariableForTesting(var);
}
- ast::Module mod;
- TypeDeterminer td{ctx, &mod};
- spirv::Builder b{ctx, &mod};
+ TypeDeterminer td{ctx, mod};
+ spirv::Builder b{ctx, mod};
};
template <typename T>
diff --git a/src/writer/spirv/test_helper.h b/src/writer/spirv/test_helper.h
index 98637ba..c8194b4 100644
--- a/src/writer/spirv/test_helper.h
+++ b/src/writer/spirv/test_helper.h
@@ -31,13 +31,11 @@
/// Helper class for testing
template <typename BASE>
-class TestHelperBase : public ast::BuilderWithContext, public BASE {
+class TestHelperBase : public ast::BuilderWithContextAndModule, public BASE {
public:
- TestHelperBase() : td(ctx, &mod), b(ctx, &mod) {}
+ TestHelperBase() : td(ctx, mod), b(ctx, mod) {}
~TestHelperBase() override = default;
- /// The module
- ast::Module mod;
/// The type determiner
TypeDeterminer td;
/// The generator
diff --git a/src/writer/wgsl/test_helper.h b/src/writer/wgsl/test_helper.h
index 76e434f..57b14ce 100644
--- a/src/writer/wgsl/test_helper.h
+++ b/src/writer/wgsl/test_helper.h
@@ -36,13 +36,13 @@
~TestHelperBase() = default;
- /// Creates a new `ast::Node` owned by the Context. When the Context is
+ /// Creates a new `ast::Node` owned by the Module. When the Module is
/// destructed, the `ast::Node` will also be destructed.
/// @param args the arguments to pass to the type constructor
/// @returns the node pointer
template <typename T, typename... ARGS>
T* create(ARGS&&... args) {
- return ctx.create<T>(std::forward<ARGS>(args)...);
+ return mod.create<T>(std::forward<ARGS>(args)...);
}
/// The context
