From 3319e308d0f438ecd594d103d27043c8e61027ca Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Sun, 12 Jul 2020 15:52:02 -0700 Subject: [PATCH] Make ast::node_t non-virtual Eliminate its vtable to save 8 bytes per node, which is a lot! --- src/ast.cpp | 26 +++++++++++++------ src/ast.h | 74 ++++++++++++++++++++++++++++++++++------------------- 2 files changed, 66 insertions(+), 34 deletions(-) diff --git a/src/ast.cpp b/src/ast.cpp index 0d1685e6a..2d82c60ca 100644 --- a/src/ast.cpp +++ b/src/ast.cpp @@ -291,6 +291,18 @@ const wchar_t *ast_type_to_string(type_t type) { return L"(unknown)"; } +/// Delete an untyped node. +void node_deleter_t::operator()(node_t *n) { + if (!n) return; + switch (n->type) { +#define ELEM(T) \ + case type_t::T: \ + delete n->as(); \ + break; +#include "ast_node_types.inc" + } +} + wcstring node_t::describe() const { wcstring res = ast_type_to_string(this->type); if (const auto *n = this->try_as()) { @@ -301,8 +313,6 @@ wcstring node_t::describe() const { return res; } -node_t::~node_t() = default; - /// From C++14. template using enable_if_t = typename std::enable_if::type; @@ -395,12 +405,12 @@ class ast_t::populator_t { if (top_type == type_t::job_list) { unique_ptr list = allocate(); this->populate_list(*list, true /* exhaust_stream */); - this->ast_->top_ = std::move(list); + this->ast_->top_.reset(list.release()); } else { unique_ptr list = allocate(); this->populate_list(list->arguments, true /* exhaust_stream */); - this->ast_->top_ = std::move(list); + this->ast_->top_.reset(list.release()); } // Chomp trailing extras, etc. chomp_extras(type_t::job_list); @@ -1185,15 +1195,15 @@ class ast_t::populator_t { assert(ptr && "Statement contents must never be null"); } - void visit_union_field(argument_or_redirection_t::contents_ptr_t &ptr) { + void visit_union_field(argument_or_redirection_t::contents_ptr_t &contents) { if (auto arg = try_parse()) { - ptr.contents = std::move(arg); + contents = std::move(arg); } else if (auto redir = try_parse()) { - ptr.contents = std::move(redir); + contents = std::move(redir); } else { internal_error(__FUNCTION__, L"Unable to parse argument or redirection"); } - assert(ptr && "Statement contents must never be null"); + assert(contents && "Statement contents must never be null"); } void visit_union_field(block_statement_t::header_ptr_t &ptr) { diff --git a/src/ast.h b/src/ast.h index 3ace88827..2a394d2b8 100644 --- a/src/ast.h +++ b/src/ast.h @@ -103,24 +103,18 @@ const wchar_t *ast_type_to_string(type_t type); * }; */ -namespace template_goo { -/// \return true if type Type is in the Candidates list. -template -constexpr bool type_in_list() { - return false; -} - -template -constexpr bool type_in_list() { - return std::is_same::value || type_in_list(); -} -} // namespace template_goo +// Our node base type is not virtual, so we must not invoke its destructor directly. +// If you want to delete a node and don't know its concrete type, use this deleter type. +struct node_deleter_t { + void operator()(node_t *node); +}; +using node_unique_ptr_t = std::unique_ptr; // A union pointer field is a pointer to one of a fixed set of node types. // It is never null after construction. template struct union_ptr_t { - std::unique_ptr contents{}; + node_unique_ptr_t contents{}; /// \return a pointer to the node contents. const node_t *get() const { @@ -133,16 +127,15 @@ struct union_ptr_t { const node_t *operator->() const { return get(); } - /// \return whether this union pointer can hold the given node. - static inline bool allows_node(const node_t &node); - union_ptr_t() = default; + // Allow setting a typed unique pointer. template - /* implicit */ union_ptr_t(std::unique_ptr n) : contents(std::move(n)) { - static_assert(template_goo::type_in_list(), - "Cannot construct from this node type"); - } + inline void operator=(std::unique_ptr n); + + // Construct from a typed unique pointer. + template + inline union_ptr_t(std::unique_ptr n); }; // A pointer to something, or nullptr if not present. @@ -312,10 +305,11 @@ struct node_t { return *storage; } - // We are a pure virtual class. - // Note that it is NOT necessary to declare virtual destructors for all subclasses - these will - // be made virtual automatically. - virtual ~node_t() = 0; + protected: + // We are NOT a virtual class - we have no vtable or virtual methods and our destructor is not + // virtual, so as to keep the size down. Only typed nodes should invoke the destructor. + // Use node_deleter_t to delete an untyped node. + ~node_t() = default; }; // Base class for all "branch" nodes: nodes with at least one ast child. @@ -409,7 +403,7 @@ struct list_t : public node_t { } list_t() : node_t(ListType, Category) {} - ~list_t() override { delete[] contents; } + ~list_t() { delete[] contents; } // Disallow moving as we own a raw pointer. list_t(list_t &&) = delete; @@ -790,6 +784,34 @@ bool keyword_t::allows_keyword(parse_keyword_t kw) { return false; } +namespace template_goo { +/// \return true if type Type is in the Candidates list. +template +constexpr bool type_in_list() { + return false; +} + +template +constexpr bool type_in_list() { + return std::is_same::value || type_in_list(); +} +} // namespace template_goo + +template +template +void union_ptr_t::operator=(std::unique_ptr n) { + static_assert(template_goo::type_in_list(), + "Cannot construct from this node type"); + contents.reset(n.release()); +} + +template +template +union_ptr_t::union_ptr_t(std::unique_ptr n) : contents(n.release()) { + static_assert(template_goo::type_in_list(), + "Cannot construct from this node type"); +} + /** * A node visitor is like a field visitor, but adapted to only visit actual nodes, as const * references. It calls the visit() function of its visitor with a const reference to each node @@ -1025,7 +1047,7 @@ class ast_t { // The top node. // Its type depends on what was requested to parse. - std::unique_ptr top_{}; + node_unique_ptr_t top_{}; /// Whether any errors were encountered during parsing. bool any_error_{false};