Add tint::Cloneable base class

The CloneContext was previously dealing with pointers to CastableBase, which has no guarantees that the object was actually cloneable.
Add a Cloneable base class that CloneContext can use instead.

Improves readability and produces cleaner compiler errors if you try to clone a non-cloneable object.

Change-Id: I4352fc5dab3da434e4ab160a54c4c82d50e427b4
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/41722
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
This commit is contained in:
Ben Clayton 2021-02-17 13:17:39 +00:00 committed by Commit Bot service account
parent f81df124f6
commit 52296de528
5 changed files with 73 additions and 86 deletions

View File

@ -19,7 +19,7 @@
#include <string>
#include <vector>
#include "src/castable.h"
#include "src/clone_context.h"
#include "src/source.h"
namespace tint {
@ -36,16 +36,10 @@ class Info;
namespace ast {
/// AST base class node
class Node : public Castable<Node> {
class Node : public Castable<Node, Cloneable> {
public:
~Node() override;
/// Clones this node and all transitive child nodes using the `CloneContext`
/// `ctx`.
/// @param ctx the clone context
/// @return the newly cloned node
virtual Node* Clone(CloneContext* ctx) const = 0;
/// @returns the node source data
const Source& source() const { return source_; }

View File

@ -19,6 +19,8 @@
#include "src/program.h"
#include "src/program_builder.h"
TINT_INSTANTIATE_CLASS_ID(tint::Cloneable);
namespace tint {
CloneContext::CloneContext(ProgramBuilder* to, Program const* from)

View File

@ -28,15 +28,23 @@
namespace tint {
// Forward declarations
class CloneContext;
class Program;
class ProgramBuilder;
namespace ast {
class FunctionList;
} // namespace ast
/// Cloneable is the base class for all objects that can be cloned
class Cloneable : public Castable<Cloneable> {
public:
/// Performs a deep clone of this object using the CloneContext `ctx`.
/// @param ctx the clone context
/// @return the newly cloned object
virtual Cloneable* Clone(CloneContext* ctx) const = 0;
};
/// CloneContext holds the state used while cloning AST nodes and types.
class CloneContext {
public:
@ -186,7 +194,7 @@ class CloneContext {
///
/// `replacer` must be function-like with the signature:
/// `T* (CloneContext*, T*)`
/// where `T` is a type deriving from CastableBase.
/// where `T` is a type deriving from Cloneable.
///
/// If `replacer` returns a nullptr then Clone() will attempt the next
/// registered replacer function that matches the object type. If no replacers
@ -210,13 +218,13 @@ class CloneContext {
/// references of the original object. A type mismatch will result in an
/// assertion in debug builds, and undefined behavior in release builds.
/// @param replacer a function or function-like object with the signature
/// `T* (CloneContext*, T*)`, where `T` derives from CastableBase
/// `T* (CloneContext*, T*)`, where `T` derives from Cloneable
/// @returns this CloneContext so calls can be chained
template <typename F>
CloneContext& ReplaceAll(F replacer) {
using TPtr = traits::ParamTypeT<F, 1>;
using T = typename std::remove_pointer<TPtr>::type;
transforms_.emplace_back([=](CastableBase* in) {
transforms_.emplace_back([=](Cloneable* in) {
auto* in_as_t = in->As<T>();
return in_as_t != nullptr ? replacer(this, in_as_t) : nullptr;
});
@ -264,7 +272,7 @@ class CloneContext {
Program const* const src;
private:
using Transform = std::function<CastableBase*(CastableBase*)>;
using Transform = std::function<Cloneable*(Cloneable*)>;
CloneContext(const CloneContext&) = delete;
CloneContext& operator=(const CloneContext&) = delete;
@ -272,7 +280,7 @@ class CloneContext {
/// LookupOrTransform is the template-independent logic of Clone().
/// This is outside of Clone() to reduce the amount of template-instantiated
/// code.
CastableBase* LookupOrTransform(CastableBase* a) {
Cloneable* LookupOrTransform(Cloneable* a) {
// Have we seen this object before? If so, return the previously cloned
// version instead of making yet another copy.
auto it = cloned_.find(a);
@ -282,7 +290,7 @@ class CloneContext {
// Attempt to clone using the registered replacer functions.
for (auto& f : transforms_) {
if (CastableBase* c = f(a)) {
if (Cloneable* c = f(a)) {
cloned_.emplace(a, c);
return c;
}
@ -301,16 +309,16 @@ class CloneContext {
return cast;
}
/// A vector of CastableBase*
using CastableList = std::vector<CastableBase*>;
/// A vector of Cloneable*
using CloneableList = std::vector<Cloneable*>;
/// A map of object in #src to their cloned equivalent in #dst
std::unordered_map<CastableBase*, CastableBase*> cloned_;
std::unordered_map<Cloneable*, Cloneable*> cloned_;
/// A map of object in #src to the list of cloned objects in #dst.
/// Clone(const std::vector<T*>& v) will use this to insert the map-value list
/// into the target vector/ before cloning and inserting the map-key.
std::unordered_map<CastableBase*, CastableList> insert_before_;
std::unordered_map<Cloneable*, CloneableList> insert_before_;
/// Transform functions registered with ReplaceAll()
std::vector<Transform> transforms_;

View File

@ -25,18 +25,17 @@
namespace tint {
namespace {
struct Cloneable : public Castable<Cloneable, ast::Node> {
explicit Cloneable(const Source& source, std::string n)
: Base(source), name(n) {}
struct Node : public Castable<Node, ast::Node> {
explicit Node(const Source& source, std::string n) : Base(source), name(n) {}
std::string name;
Cloneable* a = nullptr;
Cloneable* b = nullptr;
Cloneable* c = nullptr;
std::vector<Cloneable*> vec;
Node* a = nullptr;
Node* b = nullptr;
Node* c = nullptr;
std::vector<Node*> vec;
Cloneable* Clone(CloneContext* ctx) const override {
auto* out = ctx->dst->create<Cloneable>(name);
Node* Clone(CloneContext* ctx) const override {
auto* out = ctx->dst->create<Node>(name);
out->a = ctx->Clone(a);
out->b = ctx->Clone(b);
out->c = ctx->Clone(c);
@ -48,18 +47,18 @@ struct Cloneable : public Castable<Cloneable, ast::Node> {
void to_str(const semantic::Info&, std::ostream&, size_t) const override {}
};
struct Replaceable : public Castable<Replaceable, Cloneable> {
struct Replaceable : public Castable<Replaceable, Node> {
explicit Replaceable(const Source& source, std::string n) : Base(source, n) {}
};
struct Replacement : public Castable<Replacement, Replaceable> {
explicit Replacement(const Source& source, std::string n) : Base(source, n) {}
};
struct NotACloneable : public Castable<NotACloneable, ast::Node> {
explicit NotACloneable(const Source& source) : Base(source) {}
struct NotANode : public Castable<NotANode, ast::Node> {
explicit NotANode(const Source& source) : Base(source) {}
NotACloneable* Clone(CloneContext* ctx) const override {
return ctx->dst->create<NotACloneable>();
NotANode* Clone(CloneContext* ctx) const override {
return ctx->dst->create<NotANode>();
}
bool IsValid() const override { return true; }
@ -68,24 +67,24 @@ struct NotACloneable : public Castable<NotACloneable, ast::Node> {
TEST(CloneContext, Clone) {
ProgramBuilder builder;
auto* original_root = builder.create<Cloneable>("root");
original_root->a = builder.create<Cloneable>("a");
original_root->a->b = builder.create<Cloneable>("a->b");
original_root->b = builder.create<Cloneable>("b");
auto* original_root = builder.create<Node>("root");
original_root->a = builder.create<Node>("a");
original_root->a->b = builder.create<Node>("a->b");
original_root->b = builder.create<Node>("b");
original_root->b->a = original_root->a; // Aliased
original_root->b->b = builder.create<Cloneable>("b->b");
original_root->b->b = builder.create<Node>("b->b");
original_root->c = original_root->b; // Aliased
Program original(std::move(builder));
// root
// ╭──────────────────┼──────────────────╮
// (a) (b) (c)
// C <──────┐ C <───────────────┘
// N <──────┐ N <───────────────┘
// ╭────┼────╮ │ ╭────┼────╮
// (a) (b) (c) │ (a) (b) (c)
// C └───┘ C
// N └───┘ N
//
// C: Clonable
// N: Node
ProgramBuilder cloned;
auto* cloned_root = CloneContext(&cloned, &original).Clone(original_root);
@ -119,8 +118,8 @@ TEST(CloneContext, Clone) {
TEST(CloneContext, CloneWithReplacements) {
ProgramBuilder builder;
auto* original_root = builder.create<Cloneable>("root");
original_root->a = builder.create<Cloneable>("a");
auto* original_root = builder.create<Node>("root");
original_root->a = builder.create<Node>("a");
original_root->a->b = builder.create<Replaceable>("a->b");
original_root->b = builder.create<Replaceable>("b");
original_root->b->a = original_root->a; // Aliased
@ -130,12 +129,12 @@ TEST(CloneContext, CloneWithReplacements) {
// root
// ╭──────────────────┼──────────────────╮
// (a) (b) (c)
// C <──────┐ R <───────────────┘
// N <──────┐ R <───────────────┘
// ╭────┼────╮ │ ╭────┼────╮
// (a) (b) (c) │ (a) (b) (c)
// R └───┘
//
// C: Clonable
// N: Node
// R: Replaceable
ProgramBuilder cloned;
@ -143,7 +142,7 @@ TEST(CloneContext, CloneWithReplacements) {
CloneContext(&cloned, &original)
.ReplaceAll([&](CloneContext* ctx, Replaceable* in) {
auto* out = cloned.create<Replacement>("replacement:" + in->name);
out->b = cloned.create<Cloneable>("replacement-child:" + in->name);
out->b = cloned.create<Node>("replacement-child:" + in->name);
out->c = ctx->Clone(in->a);
return out;
})
@ -152,15 +151,15 @@ TEST(CloneContext, CloneWithReplacements) {
// root
// ╭─────────────────┼──────────────────╮
// (a) (b) (c)
// C <──────┐ R <───────────────┘
// N <──────┐ R <───────────────┘
// ╭────┼────╮ │ ╭────┼────╮
// (a) (b) (c) │ (a) (b) (c)
// R │ C |
// R │ N |
// ╭────┼────╮ └────────────┘
// (a) (b) (c)
// C
// N
//
// C: Clonable
// N: Node
// R: Replacement
EXPECT_NE(cloned_root->a, nullptr);
@ -201,10 +200,10 @@ TEST(CloneContext, CloneWithReplacements) {
TEST(CloneContext, CloneWithReplace) {
ProgramBuilder builder;
auto* original_root = builder.create<Cloneable>("root");
original_root->a = builder.create<Cloneable>("a");
original_root->b = builder.create<Cloneable>("b");
original_root->c = builder.create<Cloneable>("c");
auto* original_root = builder.create<Node>("root");
original_root->a = builder.create<Node>("a");
original_root->b = builder.create<Node>("b");
original_root->c = builder.create<Node>("c");
Program original(std::move(builder));
// root
@ -213,7 +212,7 @@ TEST(CloneContext, CloneWithReplace) {
// Replaced
ProgramBuilder cloned;
auto* replacement = cloned.create<Cloneable>("replacement");
auto* replacement = cloned.create<Node>("replacement");
auto* cloned_root = CloneContext(&cloned, &original)
.Replace(original_root->b, replacement)
@ -231,15 +230,15 @@ TEST(CloneContext, CloneWithReplace) {
TEST(CloneContext, CloneWithInsertBefore) {
ProgramBuilder builder;
auto* original_root = builder.create<Cloneable>("root");
original_root->a = builder.create<Cloneable>("a");
original_root->b = builder.create<Cloneable>("b");
original_root->c = builder.create<Cloneable>("c");
auto* original_root = builder.create<Node>("root");
original_root->a = builder.create<Node>("a");
original_root->b = builder.create<Node>("b");
original_root->c = builder.create<Node>("c");
original_root->vec = {original_root->a, original_root->b, original_root->c};
Program original(std::move(builder));
ProgramBuilder cloned;
auto* insertion = cloned.create<Cloneable>("insertion");
auto* insertion = cloned.create<Node>("insertion");
auto* cloned_root = CloneContext(&cloned, &original)
.InsertBefore(original_root->b, insertion)
@ -257,12 +256,12 @@ TEST(CloneContext, CloneWithInsertBefore) {
EXPECT_EQ(cloned_root->vec[3]->name, "c");
}
TEST(CloneContext, CloneWithReplace_WithNotACloneable) {
TEST(CloneContext, CloneWithReplace_WithNotANode) {
ProgramBuilder builder;
auto* original_root = builder.create<Cloneable>("root");
original_root->a = builder.create<Cloneable>("a");
original_root->b = builder.create<Cloneable>("b");
original_root->c = builder.create<Cloneable>("c");
auto* original_root = builder.create<Node>("root");
original_root->a = builder.create<Node>("a");
original_root->b = builder.create<Node>("b");
original_root->c = builder.create<Node>("c");
Program original(std::move(builder));
// root
@ -271,7 +270,7 @@ TEST(CloneContext, CloneWithReplace_WithNotACloneable) {
// Replaced
ProgramBuilder cloned;
auto* replacement = cloned.create<NotACloneable>();
auto* replacement = cloned.create<NotANode>();
CloneContext ctx(&cloned, &original);
ctx.Replace(original_root->b, replacement);
@ -289,9 +288,9 @@ TEST(CloneContext, CloneWithReplace_WithNotACloneable) {
} // namespace
TINT_INSTANTIATE_CLASS_ID(Cloneable);
TINT_INSTANTIATE_CLASS_ID(Node);
TINT_INSTANTIATE_CLASS_ID(Replaceable);
TINT_INSTANTIATE_CLASS_ID(Replacement);
TINT_INSTANTIATE_CLASS_ID(NotACloneable);
TINT_INSTANTIATE_CLASS_ID(NotANode);
} // namespace tint

View File

@ -17,7 +17,6 @@
#include <string>
#include "src/castable.h"
#include "src/clone_context.h"
namespace tint {
@ -32,17 +31,12 @@ namespace type {
enum class MemoryLayout { kUniformBuffer, kStorageBuffer };
/// Base class for a type in the system
class Type : public Castable<Type> {
class Type : public Castable<Type, Cloneable> {
public:
/// Move constructor
Type(Type&&);
~Type() override;
/// Clones this type and all transitive types using the `CloneContext` `ctx`.
/// @param ctx the clone context
/// @return the newly cloned type
virtual Type* Clone(CloneContext* ctx) const = 0;
/// @returns the name for this type. The type name is unique over all types.
virtual std::string type_name() const = 0;
@ -114,16 +108,6 @@ class Type : public Castable<Type> {
protected:
Type();
/// A helper method for cloning the `Type` `t` if it is not null.
/// If `t` is null, then `Clone()` returns null.
/// @param b the program builder to clone `n` into
/// @param t the `Type` to clone (if not null)
/// @return the cloned type
template <typename T>
static T* Clone(ProgramBuilder* b, const T* t) {
return (t != nullptr) ? static_cast<T*>(t->Clone(b)) : nullptr;
}
};
} // namespace type