diff --git a/src/exec.cpp b/src/exec.cpp index 91cf99cd6..8ceae0aab 100644 --- a/src/exec.cpp +++ b/src/exec.cpp @@ -306,13 +306,14 @@ static bool io_transmogrify(const io_chain_t &in_chain, io_chain_t *out_chain, /// clean up morphed redirections. /// /// \param def the code to evaluate, or the empty string if none -/// \param node_offset the offset of the node to evalute, or NODE_OFFSET_INVALID +/// \param statement the statement node to evaluate, or empty /// \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, node_offset_t node_offset, +static void internal_exec_helper(parser_t &parser, const wcstring &def, + tnode_t statement, enum block_type_t block_type, const io_chain_t &ios) { - // If we have a valid node offset, then we must not have a string to execute. - assert(node_offset == NODE_OFFSET_INVALID || def.empty()); + // If we have a valid statement node, then we must not have a string to execute. + assert(!statement || def.empty()); io_chain_t morphed_chain; std::vector opened_fds; @@ -324,10 +325,10 @@ static void internal_exec_helper(parser_t &parser, const wcstring &def, node_off return; } - if (node_offset == NODE_OFFSET_INVALID) { + if (!statement) { parser.eval(def, morphed_chain, block_type); } else { - parser.eval_block_node(node_offset, morphed_chain, block_type); + parser.eval_node(statement, morphed_chain, block_type); } morphed_chain.clear(); @@ -810,8 +811,7 @@ void exec_job(parser_t &parser, job_t *j) { verify_buffer_output(); if (!exec_error) { - internal_exec_helper(parser, def, NODE_OFFSET_INVALID, TOP, - process_net_io_chain); + internal_exec_helper(parser, def, {}, TOP, process_net_io_chain); } parser.allow_function(); diff --git a/src/parse_execution.cpp b/src/parse_execution.cpp index a1fc4115c..5a0c10603 100644 --- a/src/parse_execution.cpp +++ b/src/parse_execution.cpp @@ -975,25 +975,27 @@ parse_execution_result_t parse_execution_context_t::populate_boolean_process( } template -parse_execution_result_t parse_execution_context_t::populate_block_process(job_t *job, - process_t *proc, - tnode_t node) { +parse_execution_result_t parse_execution_context_t::populate_block_process( + job_t *job, process_t *proc, tnode_t statement, + tnode_t specific_statement) { // We handle block statements by creating INTERNAL_BLOCK_NODE, that will bounce back to us when // it's time to execute them. UNUSED(job); static_assert(Type::token == symbol_block_statement || Type::token == symbol_if_statement || Type::token == symbol_switch_statement, "Invalid block process"); + assert(statement && "statement missing"); + assert(specific_statement && "specific_statement missing"); // The set of IO redirections that we construct for the process. // TODO: fix this ugly find_child. - auto arguments = node.template find_child(); + auto arguments = specific_statement.template find_child(); io_chain_t process_io_chain; bool errored = !this->determine_io_chain(arguments, &process_io_chain); if (errored) return parse_execution_errored; proc->type = INTERNAL_BLOCK_NODE; - proc->internal_block_node = this->get_offset(node); + proc->internal_block_node = statement; proc->set_io_chain(process_io_chain); return parse_execution_success; } @@ -1012,15 +1014,15 @@ parse_execution_result_t parse_execution_context_t::populate_job_process( } case symbol_block_statement: result = this->populate_block_process( - job, proc, tnode_t(&tree(), &specific_statement)); + job, proc, statement, tnode_t(&tree(), &specific_statement)); break; case symbol_if_statement: result = this->populate_block_process( - job, proc, tnode_t(&tree(), &specific_statement)); + job, proc, statement, tnode_t(&tree(), &specific_statement)); break; case symbol_switch_statement: result = this->populate_block_process( - job, proc, tnode_t(&tree(), &specific_statement)); + job, proc, statement, tnode_t(&tree(), &specific_statement)); break; case symbol_decorated_statement: { // Get the plain statement. It will pull out the decoration itself. @@ -1127,7 +1129,7 @@ parse_execution_result_t parse_execution_context_t::run_1_job(tnode_t jo } // When we encounter a block construct (e.g. while loop) in the general case, we create a "block - // process" that has a pointer to its source. This allows us to handle block-level redirections. + // process" containing its node. This allows us to handle block-level redirections. // However, if there are no redirections, then we can just jump into the block directly, which // is significantly faster. if (job_is_simple_block(job_node)) { @@ -1257,63 +1259,48 @@ parse_execution_result_t parse_execution_context_t::run_job_list(tnode_t j return result; } -parse_execution_result_t parse_execution_context_t::eval_node_at_offset( - node_offset_t offset, const block_t *associated_block, const io_chain_t &io) { - // Don't ever expect to have an empty tree if this is called. - assert(!tree().empty()); //!OCLINT(multiple unary operator) - assert(offset < tree().size()); - +parse_execution_result_t parse_execution_context_t::eval_node(tnode_t statement, + const block_t *associated_block, + const io_chain_t &io) { + assert(statement && "Empty node in eval_node"); + assert(statement.matches_node_tree(tree()) && "statement has unexpected tree"); // Apply this block IO for the duration of this function. scoped_push block_io_push(&block_io, io); - - const parse_node_t &node = tree().at(offset); - - // Currently, we only expect to execute the top level job list, or a block node. Assert that. - assert(node.type == symbol_job_list || specific_statement_type_is_redirectable_block(node)); - enum parse_execution_result_t status = parse_execution_success; - switch (node.type) { - case symbol_job_list: { - // We should only get a job list if it's the very first node. This is because this is - // the entry point for both top-level execution (the first node) and INTERNAL_BLOCK_NODE - // execution (which does block statements, but never job lists). - assert(offset == 0); - tnode_t job_list{&tree(), &node}; - wcstring func_name; - auto infinite_recursive_node = - this->infinite_recursive_statement_in_job_list(job_list, &func_name); - if (infinite_recursive_node) { - // We have an infinite recursion. - this->report_error(infinite_recursive_node, INFINITE_FUNC_RECURSION_ERR_MSG, - func_name.c_str()); - status = parse_execution_errored; - } else { - // No infinite recursion. - status = this->run_job_list(job_list, associated_block); - } - break; - } - case symbol_block_statement: { - status = this->run_block_statement({&tree(), &node}); - break; - } - case symbol_if_statement: { - status = this->run_if_statement({&tree(), &node}); - break; - } - case symbol_switch_statement: { - status = this->run_switch_statement({&tree(), &node}); - break; - } - default: { - // In principle, we could support other node types. However we never expect to be passed - // them - see above. - debug(0, "Unexpected node %ls found in %s", node.describe().c_str(), __FUNCTION__); - PARSER_DIE(); - break; - } + if (auto block = statement.try_get_child()) { + status = this->run_block_statement(block); + } else if (auto ifstat = statement.try_get_child()) { + status = this->run_if_statement(ifstat); + } else if (auto switchstat = statement.try_get_child()) { + status = this->run_switch_statement(switchstat); + } else { + debug(0, "Unexpected node %ls found in %s", statement.node()->describe().c_str(), + __FUNCTION__); + abort(); } + return status; +} +parse_execution_result_t parse_execution_context_t::eval_node(tnode_t job_list, + const block_t *associated_block, + const io_chain_t &io) { + // Apply this block IO for the duration of this function. + assert(job_list && "Empty node in eval_node"); + assert(job_list.matches_node_tree(tree()) && "job_list has unexpected tree"); + scoped_push block_io_push(&block_io, io); + enum parse_execution_result_t status = parse_execution_success; + wcstring func_name; + auto infinite_recursive_node = + this->infinite_recursive_statement_in_job_list(job_list, &func_name); + if (infinite_recursive_node) { + // We have an infinite recursion. + this->report_error(infinite_recursive_node, INFINITE_FUNC_RECURSION_ERR_MSG, + func_name.c_str()); + status = parse_execution_errored; + } else { + // No infinite recursion. + status = this->run_job_list(job_list, associated_block); + } return status; } diff --git a/src/parse_execution.h b/src/parse_execution.h index 1b7f305d7..17e178fcc 100644 --- a/src/parse_execution.h +++ b/src/parse_execution.h @@ -91,7 +91,8 @@ class parse_execution_context_t { template parse_execution_result_t populate_block_process(job_t *job, process_t *proc, - tnode_t statement_node); + tnode_t statement, + tnode_t specific_statement); // These encapsulate the actual logic of various (block) statements. parse_execution_result_t run_block_statement(tnode_t statement); @@ -146,11 +147,12 @@ class parse_execution_context_t { /// Return the parse tree. const parse_node_tree_t &tree() const { return pstree->tree; } - /// Start executing at the given node offset. Returns 0 if there was no error, 1 if there was an + /// Start executing at the given node. Returns 0 if there was no error, 1 if there was an /// error. - parse_execution_result_t eval_node_at_offset(node_offset_t offset, - const block_t *associated_block, - const io_chain_t &io); + parse_execution_result_t eval_node(tnode_t statement, + const block_t *associated_block, const io_chain_t &io); + parse_execution_result_t eval_node(tnode_t job_list, + const block_t *associated_block, const io_chain_t &io); }; #endif diff --git a/src/parser.cpp b/src/parser.cpp index 6f042f10a..9fdade353 100644 --- a/src/parser.cpp +++ b/src/parser.cpp @@ -670,7 +670,8 @@ int parser_t::eval(parsed_source_ref_t ps, const io_chain_t &io, enum block_type const parse_execution_context_t *ctx = execution_contexts.back().get(); // Execute the first node. - this->eval_block_node(0, io, block_type); + tnode_t start{&ps->tree, &ps->tree.front()}; + this->eval_node(start, io, block_type); // Clean up the execution context stack. assert(!execution_contexts.empty() && execution_contexts.back().get() == ctx); @@ -679,12 +680,11 @@ int parser_t::eval(parsed_source_ref_t ps, const io_chain_t &io, enum block_type return 0; } -int parser_t::eval_block_node(node_offset_t node_idx, const io_chain_t &io, - enum block_type_t block_type) { - // Paranoia. It's a little frightening that we're given only a node_idx and we interpret this in - // the topmost execution context's tree. What happens if two trees were to be interleaved? - // Fortunately that cannot happen (yet); in the future we probably want some sort of reference - // counted trees. +template +int parser_t::eval_node(tnode_t node, const io_chain_t &io, enum block_type_t block_type) { + static_assert( + std::is_same::value || std::is_same::value, + "Unexpected node type"); parse_execution_context_t *ctx = execution_contexts.back().get(); assert(ctx != NULL); @@ -711,13 +711,17 @@ int parser_t::eval_block_node(node_offset_t node_idx, const io_chain_t &io, // Start it up scope_block_t *scope_block = this->push_block(block_type); - int result = ctx->eval_node_at_offset(node_idx, scope_block, io); + int result = ctx->eval_node(node, scope_block, io); this->pop_block(scope_block); job_reap(0); // reap again return result; } +// Explicit instantiations. TODO: use overloads instead? +template int parser_t::eval_node(tnode_t, const io_chain_t &io, enum block_type_t block_type); +template int parser_t::eval_node(tnode_t, const io_chain_t &io, enum block_type_t block_type); + bool parser_t::detect_errors_in_argument_list(const wcstring &arg_list_src, wcstring *out, const wchar_t *prefix) { bool errored = false; diff --git a/src/parser.h b/src/parser.h index 5fcf6a849..04e36e278 100644 --- a/src/parser.h +++ b/src/parser.h @@ -251,8 +251,10 @@ 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 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); + /// Evaluates a node in the topmost execution context. + /// The node type must be grammar::statement or grammar::job_list. + template + int eval_node(tnode_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. diff --git a/src/proc.cpp b/src/proc.cpp index a9b313656..1e77e4561 100644 --- a/src/proc.cpp +++ b/src/proc.cpp @@ -344,25 +344,7 @@ static void handle_child_status(pid_t pid, int status) { return; } -process_t::process_t() - : is_first_in_job(), - is_last_in_job(), - type(), // gets set later - internal_block_node(NODE_OFFSET_INVALID), - pid(0), - pipe_write_fd(0), - pipe_read_fd(0), - completed(0), - stopped(0), - status(0), - count_help_magic(0) -#ifdef HAVE__PROC_SELF_STAT - , - last_time(), - last_jiffies(0) -#endif -{ -} +process_t::process_t() {} /// The constructor sets the pgid to -2 as a sentinel value /// 0 should not be used; although it is not a valid PGID in userspace, diff --git a/src/proc.h b/src/proc.h index 52c956571..227b45599 100644 --- a/src/proc.h +++ b/src/proc.h @@ -18,6 +18,7 @@ #include "common.h" #include "io.h" #include "parse_tree.h" +#include "tnode.h" /// Types of processes. enum process_type_t { @@ -73,15 +74,16 @@ class process_t { process_t(); // Note whether we are the first and/or last in the job - bool is_first_in_job; - bool is_last_in_job; + bool is_first_in_job{false}; + bool is_last_in_job{false}; /// Type of process. Can be one of \c EXTERNAL, \c INTERNAL_BUILTIN, \c INTERNAL_FUNCTION, \c /// INTERNAL_EXEC. - enum process_type_t type; + enum process_type_t type { EXTERNAL }; - /// For internal block processes only, the node offset of the block. - node_offset_t internal_block_node; + /// For internal block processes only, the node offset of the statement. + /// This is always either block, ifs, or switchs, never boolean or decorated. + tnode_t internal_block_node{}; /// Sets argv. void set_argv(const wcstring_list_t &argv) { argv_array.set(argv); } @@ -111,24 +113,24 @@ class process_t { /// Actual command to pass to exec in case of EXTERNAL or INTERNAL_EXEC. wcstring actual_cmd; /// Process ID - pid_t pid; + pid_t pid{0}; /// File descriptor that pipe output should bind to. - int pipe_write_fd; + int pipe_write_fd{0}; /// File descriptor that the _next_ process pipe input should bind to. - int pipe_read_fd; + int pipe_read_fd{0}; /// True if process has completed. - volatile int completed; + volatile int completed{false}; /// True if process has stopped. - volatile int stopped; + volatile int stopped{false}; /// Reported status value. - volatile int status; + volatile int status{0}; /// Special flag to tell the evaluation function for count to print the help information. - int count_help_magic; + int count_help_magic{0}; #ifdef HAVE__PROC_SELF_STAT /// Last time of cpu time check. - struct timeval last_time; + struct timeval last_time {}; /// Number of jiffies spent in process at last cpu time check. - unsigned long last_jiffies; + unsigned long last_jiffies{0}; #endif }; diff --git a/src/tnode.h b/src/tnode.h index 0aa95dc15..95c8f129f 100644 --- a/src/tnode.h +++ b/src/tnode.h @@ -88,6 +88,9 @@ class tnode_t { bool operator!=(const tnode_t &rhs) const { return !(*this == rhs); } + // Helper to return whether the given tree is the same as ours. + bool matches_node_tree(const parse_node_tree_t &t) const { return &t == tree; } + bool has_source() const { return nodeptr && nodeptr->has_source(); } // return the tag, or 0 if missing.