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.

7 files changed, 44 insertions, 34 deletions

diff --git a/src/common.h b/src/common.h
index 660bc012..2ba49854 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<long>(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<T> and then swap() out of it.
+ */
+template<typename T>
+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<std::string> &lst);
 
diff --git a/src/fish_tests.cpp b/src/fish_tests.cpp
index efc134d7..c5fa4385 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 c0c10a75..60885760 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<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)
 {
 }
 
diff --git a/src/parse_execution.h b/src/parse_execution.h
index a9ce32f5..ef796676 100644
--- a/src/parse_execution.h
+++ b/src/parse_execution.h
@@ -121,7 +121,7 @@ private:
     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<parse_node_tree_t> 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 d7b296ae..67632685 100644
--- a/src/parse_tree.h
+++ b/src/parse_tree.h
@@ -165,6 +165,13 @@ public:
 class parse_node_tree_t : public std::vector<parse_node_t>
 {
 public:
+    
+    parse_node_tree_t() {}
+    
+    parse_node_tree_t(moved_ref<parse_node_tree_t> 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 b89e6f62..2097aecf 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<parse_node_tree_t>(tree));
+}
 
-    //print_stderr(block_stack_description());
+int parser_t::eval_acquiring_tree(const wcstring &cmd, const io_chain_t &io, enum block_type_t block_type, moved_ref<parse_node_tree_t> 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 db35471b..af470d36 100644
--- a/src/parser.h
+++ b/src/parser.h
@@ -296,6 +296,11 @@ public:
       \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<parse_node_tree_t> 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);