From e3970f9cbbd00289c551f947db0f4644654bd8d9 Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Sun, 28 Feb 2016 00:33:11 -0800 Subject: [PATCH] Allow parse_execution_context to take ownership of a parse tree Introduces a new template moved_ref which is like an rvalue reference. This allows passing around objects while being explicit that the receiver may acquire ownership. This will help reduce some allocations. --- src/common.h | 11 +++++++++++ src/fish_tests.cpp | 11 ----------- src/parse_execution.cpp | 2 +- src/parse_execution.h | 2 +- src/parse_tree.h | 7 +++++++ src/parser.cpp | 42 ++++++++++++++++++++--------------------- src/parser.h | 5 +++++ 7 files changed, 45 insertions(+), 35 deletions(-) diff --git a/src/common.h b/src/common.h index 660bc0124..2ba498545 100644 --- a/src/common.h +++ b/src/common.h @@ -450,6 +450,17 @@ inline wcstring to_string(const int &x) return to_string(static_cast(x)); } +/* A hackish thing to simulate rvalue references in C++98. + The idea is that you can define a constructor to take a moved_ref and then swap() out of it. + */ +template +struct moved_ref +{ + T &val; + + explicit moved_ref(T &v) : val(v) { } +}; + wchar_t **make_null_terminated_array(const wcstring_list_t &lst); char **make_null_terminated_array(const std::vector &lst); diff --git a/src/fish_tests.cpp b/src/fish_tests.cpp index efc134d73..c5fa43856 100644 --- a/src/fish_tests.cpp +++ b/src/fish_tests.cpp @@ -751,17 +751,6 @@ static void test_parser() } say(L"Testing basic evaluation"); -#if 0 - /* This fails now since the parser takes a wcstring&, and NULL converts to wchar_t * converts to wcstring which crashes (thanks C++) */ - if (!parser.eval(0, 0, TOP)) - { - err(L"Null input when evaluating undetected"); - } -#endif - if (!parser.eval(L"ls", io_chain_t(), WHILE)) - { - err(L"Invalid block mode when evaluating undetected"); - } /* Ensure that we don't crash on infinite self recursion and mutual recursion. These must use the principal parser because we cannot yet execute jobs on other parsers (!) */ say(L"Testing recursion detection"); diff --git a/src/parse_execution.cpp b/src/parse_execution.cpp index c0c10a756..608857604 100644 --- a/src/parse_execution.cpp +++ b/src/parse_execution.cpp @@ -66,7 +66,7 @@ static wcstring profiling_cmd_name_for_redirectable_block(const parse_node_t &no return result; } -parse_execution_context_t::parse_execution_context_t(const parse_node_tree_t &t, const wcstring &s, parser_t *p, int initial_eval_level) : tree(t), src(s), parser(p), eval_level(initial_eval_level), executing_node_idx(NODE_OFFSET_INVALID), cached_lineno_offset(0), cached_lineno_count(0) +parse_execution_context_t::parse_execution_context_t(moved_ref t, const wcstring &s, parser_t *p, int initial_eval_level) : tree(t), src(s), parser(p), eval_level(initial_eval_level), executing_node_idx(NODE_OFFSET_INVALID), cached_lineno_offset(0), cached_lineno_count(0) { } diff --git a/src/parse_execution.h b/src/parse_execution.h index a9ce32f55..ef796676c 100644 --- a/src/parse_execution.h +++ b/src/parse_execution.h @@ -121,7 +121,7 @@ class parse_execution_context_t int line_offset_of_character_at_offset(size_t char_idx); public: - parse_execution_context_t(const parse_node_tree_t &t, const wcstring &s, parser_t *p, int initial_eval_level); + parse_execution_context_t(moved_ref t, const wcstring &s, parser_t *p, int initial_eval_level); /* Returns the current eval level */ int current_eval_level() const diff --git a/src/parse_tree.h b/src/parse_tree.h index d7b296ae8..67632685f 100644 --- a/src/parse_tree.h +++ b/src/parse_tree.h @@ -165,6 +165,13 @@ class parse_node_t class parse_node_tree_t : public std::vector { public: + + parse_node_tree_t() {} + + parse_node_tree_t(moved_ref t) + { + this->swap(t.val); + } /* Get the node corresponding to a child of the given node, or NULL if there is no such child. If expected_type is provided, assert that the node has that type. */ diff --git a/src/parser.cpp b/src/parser.cpp index b89e6f62f..2097aecf6 100644 --- a/src/parser.cpp +++ b/src/parser.cpp @@ -813,18 +813,8 @@ profile_item_t *parser_t::create_profile_item() return result; } - int parser_t::eval(const wcstring &cmd, const io_chain_t &io, enum block_type_t block_type) { - CHECK_BLOCK(1); - - if (block_type != TOP && block_type != SUBST) - { - debug(1, INVALID_SCOPE_ERR_MSG, parser_t::get_block_desc(block_type)); - bugreport(); - return 1; - } - /* Parse the source into a tree, if we can */ parse_node_tree_t tree; parse_error_list_t error_list; @@ -833,34 +823,42 @@ int parser_t::eval(const wcstring &cmd, const io_chain_t &io, enum block_type_t /* Get a backtrace. This includes the message. */ wcstring backtrace_and_desc; this->get_backtrace(cmd, error_list, &backtrace_and_desc); - + /* Print it */ fprintf(stderr, "%ls", backtrace_and_desc.c_str()); - + return 1; } + return this->eval_acquiring_tree(cmd, io, block_type, moved_ref(tree)); +} + +int parser_t::eval_acquiring_tree(const wcstring &cmd, const io_chain_t &io, enum block_type_t block_type, moved_ref tree) +{ + CHECK_BLOCK(1); + assert(block_type == TOP || block_type == SUBST); + + if (tree.val.empty()) + { + return 0; + } //print_stderr(block_stack_description()); - - + /* Determine the initial eval level. If this is the first context, it's -1; otherwise it's the eval level of the top context. This is sort of wonky because we're stitching together a global notion of eval level from these separate objects. A better approach would be some profile object that all contexts share, and that tracks the eval levels on its own. */ int exec_eval_level = (execution_contexts.empty() ? -1 : execution_contexts.back()->current_eval_level()); - + /* Append to the execution context stack */ parse_execution_context_t *ctx = new parse_execution_context_t(tree, cmd, this, exec_eval_level); execution_contexts.push_back(ctx); - + /* Execute the first node */ - if (! tree.empty()) - { - this->eval_block_node(0, io, block_type); - } - + this->eval_block_node(0, io, block_type); + /* Clean up the execution context stack */ assert(! execution_contexts.empty() && execution_contexts.back() == ctx); execution_contexts.pop_back(); delete ctx; - + return 0; } diff --git a/src/parser.h b/src/parser.h index db35471be..af470d36a 100644 --- a/src/parser.h +++ b/src/parser.h @@ -296,6 +296,11 @@ class parser_t \return 0 on success, 1 otherwise */ int eval(const wcstring &cmd, const io_chain_t &io, enum block_type_t block_type); + + /** + Evaluate the expressions contained in cmd, which has been parsed into the given parse tree. This takes ownership of the tree. + */ + int eval_acquiring_tree(const wcstring &cmd, const io_chain_t &io, enum block_type_t block_type, moved_ref t); /** Evaluates a block node at the given node offset in the topmost execution context */ int eval_block_node(node_offset_t node_idx, const io_chain_t &io, enum block_type_t block_type);