Stop copying out function definition when executing a function

This switches function execution from the function's source code to
its stored node and pstree. This means we no longer have to re-parse
the function every time we execute it.

src/exec.cpp

@@ -305,15 +305,13 @@ static bool io_transmogrify(const io_chain_t &in_chain, io_chain_t *out_chain,
 /// Morph an io redirection chain into redirections suitable for passing to eval, call eval, and
 /// clean up morphed redirections.
 ///
-/// \param def the code to evaluate, or the empty string if none
+/// \param parsed_source the parsed source code containing the node to evaluate
-/// \param statement the statement node to evaluate, or empty
+/// \param node the node to evaluate
-/// \param block_type the type of block to push on evaluation
 /// \param ios the io redirections to be performed on this block
-static void internal_exec_helper(parser_t &parser, const wcstring &def,
+template <typename T>
-                                 tnode_t<grammar::statement> statement,
+void internal_exec_helper(parser_t &parser, parsed_source_ref_t parsed_source, tnode_t<T> node,
-                                 enum block_type_t block_type, const io_chain_t &ios) {
+                          const io_chain_t &ios) {
-    // If we have a valid statement node, then we must not have a string to execute.
+    assert(parsed_source && node && "exec_helper missing source or without node");
-    assert(!statement || def.empty());
 
     io_chain_t morphed_chain;
     std::vector<int> opened_fds;
@@ -325,11 +323,7 @@ static void internal_exec_helper(parser_t &parser, const wcstring &def,
         return;
     }
 
-    if (!statement) {
+    parser.eval_node(parsed_source, node, morphed_chain, TOP);
-        parser.eval(def, morphed_chain, block_type);
-    } else {
-        parser.eval_node(statement, morphed_chain, block_type);
-    }
 
     morphed_chain.clear();
     io_cleanup_fds(opened_fds);
@@ -798,8 +792,6 @@ void exec_job(parser_t &parser, job_t *j) {
                     break;
                 }
 
-                wcstring def;
-                function_get_definition(func_name, &def);
                 const std::map<wcstring, env_var_t> inherit_vars =
                     function_get_inherit_vars(func_name);
 
@@ -811,7 +803,8 @@ void exec_job(parser_t &parser, job_t *j) {
                 verify_buffer_output();
 
                 if (!exec_error) {
-                    internal_exec_helper(parser, def, {}, TOP, process_net_io_chain);
+                    internal_exec_helper(parser, props->parsed_source, props->body_node,
+                                         process_net_io_chain);
                 }
 
                 parser.allow_function();
@@ -824,7 +817,9 @@ void exec_job(parser_t &parser, job_t *j) {
                 verify_buffer_output();
 
                 if (!exec_error) {
-                    internal_exec_helper(parser, wcstring(), p->internal_block_node, TOP,
+                    assert(p->block_node_source && p->internal_block_node &&
+                           "Process is missing node info");
+                    internal_exec_helper(parser, p->block_node_source, p->internal_block_node,
                                          process_net_io_chain);
                 }
                 break;

src/parse_execution.cpp

@@ -995,6 +995,7 @@ parse_execution_result_t parse_execution_context_t::populate_block_process(
     if (errored) return parse_execution_errored;
 
     proc->type = INTERNAL_BLOCK_NODE;
+    proc->block_node_source = pstree;
     proc->internal_block_node = statement;
     proc->set_io_chain(process_io_chain);
     return parse_execution_success;

src/parser.cpp

@@ -658,36 +658,19 @@ int parser_t::eval(parsed_source_ref_t ps, const io_chain_t &io, enum block_type
         return 0;
     }
 
-    // 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.
-    execution_contexts.push_back(make_unique<parse_execution_context_t>(ps, this, exec_eval_level));
-    const parse_execution_context_t *ctx = execution_contexts.back().get();
-
     // Execute the first node.
     tnode_t<grammar::job_list> start{&ps->tree, &ps->tree.front()};
-    this->eval_node(start, io, block_type);
+    this->eval_node(ps, start, io, block_type);
-
-    // Clean up the execution context stack.
-    assert(!execution_contexts.empty() && execution_contexts.back().get() == ctx);
-    execution_contexts.pop_back();
 
     return 0;
 }
 
 template <typename T>
-int parser_t::eval_node(tnode_t<T> node, const io_chain_t &io, enum block_type_t block_type) {
+int parser_t::eval_node(parsed_source_ref_t ps, tnode_t<T> node, const io_chain_t &io,
+                        enum block_type_t block_type) {
     static_assert(
         std::is_same<T, grammar::statement>::value || std::is_same<T, grammar::job_list>::value,
         "Unexpected node type");
-    parse_execution_context_t *ctx = execution_contexts.back().get();
-    assert(ctx != NULL);
-
     CHECK_BLOCK(1);
 
     // Handle cancellation requests. If our block stack is currently empty, then we already did
@@ -711,16 +694,34 @@ int parser_t::eval_node(tnode_t<T> node, const io_chain_t &io, enum block_type_t
 
     // Start it up
     scope_block_t *scope_block = this->push_block<scope_block_t>(block_type);
+
+    // 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.
+    execution_contexts.push_back(make_unique<parse_execution_context_t>(ps, this, exec_eval_level));
+    parse_execution_context_t *ctx = execution_contexts.back().get();
+
     int result = ctx->eval_node(node, scope_block, io);
     this->pop_block(scope_block);
 
+    // Clean up the execution context stack.
+    assert(!execution_contexts.empty() && execution_contexts.back().get() == ctx);
+    execution_contexts.pop_back();
+
     job_reap(0);  // reap again
     return result;
 }
 
 // Explicit instantiations. TODO: use overloads instead?
-template int parser_t::eval_node(tnode_t<grammar::statement>, const io_chain_t &io, enum block_type_t block_type);
+template int parser_t::eval_node(parsed_source_ref_t, tnode_t<grammar::statement>,
-template int parser_t::eval_node(tnode_t<grammar::job_list>, const io_chain_t &io, enum block_type_t block_type);
+                                 const io_chain_t &, enum block_type_t);
+template int parser_t::eval_node(parsed_source_ref_t, tnode_t<grammar::job_list>,
+                                 const io_chain_t &, enum block_type_t);
 
 bool parser_t::detect_errors_in_argument_list(const wcstring &arg_list_src, wcstring *out,
                                               const wchar_t *prefix) {

src/parser.h

@@ -251,10 +251,11 @@ class parser_t {
     /// Evaluate the parsed source ps.
     int eval(parsed_source_ref_t ps, const io_chain_t &io, enum block_type_t block_type);
 
-    /// Evaluates a node in the topmost execution context.
+    /// Evaluates a node.
     /// The node type must be grammar::statement or grammar::job_list.
     template <typename T>
-    int eval_node(tnode_t<T> node, const io_chain_t &io, enum block_type_t block_type);
+    int eval_node(parsed_source_ref_t ps, tnode_t<T> node, const io_chain_t &io,
+                  enum block_type_t block_type);
 
     /// Evaluate line as a list of parameters, i.e. tokenize it and perform parameter expansion and
     /// cmdsubst execution on the tokens. The output is inserted into output. Errors are ignored.

src/proc.h

@@ -83,6 +83,7 @@ class process_t {
 
     /// For internal block processes only, the node offset of the statement.
     /// This is always either block, ifs, or switchs, never boolean or decorated.
+    parsed_source_ref_t block_node_source{};
     tnode_t<grammar::statement> internal_block_node{};
 
     /// Sets argv.

tests/test_cmdsub.err

@@ -13,8 +13,8 @@ set b (string repeat -n 512 x)
 # Command sub over the limit should fail
 fish: Too much data emitted by command substitution so it was discarded
 
-set -l x (string repeat -n $argv x)
+    set -l x (string repeat -n $argv x)
-         ^
+             ^
 in function 'subme'
 	called on standard input
 	with parameter list '513'