diff --git a/src/parse_execution.cpp b/src/parse_execution.cpp index 73398d730..8a01c056c 100644 --- a/src/parse_execution.cpp +++ b/src/parse_execution.cpp @@ -50,9 +50,13 @@ namespace g = grammar; /// These are the specific statement types that support redirections. +static constexpr bool type_is_redirectable_block(parse_token_type_t type) { + return type == symbol_block_statement || type == symbol_if_statement || + type == symbol_switch_statement; +} + static bool specific_statement_type_is_redirectable_block(const parse_node_t &node) { - return node.type == symbol_block_statement || node.type == symbol_if_statement || - node.type == symbol_switch_statement; + return type_is_redirectable_block(node.type); } /// Get the name of a redirectable block, for profiling purposes. @@ -200,30 +204,37 @@ parse_execution_context_t::cancellation_reason(const block_t *block) const { /// Return whether the job contains a single statement, of block type, with no redirections. bool parse_execution_context_t::job_is_simple_block(tnode_t job_node) const { - // Must have one statement. tnode_t statement = job_node.child<0>(); - const parse_node_t &specific_statement = statement.get_child_node<0>(); - if (!specific_statement_type_is_redirectable_block(specific_statement)) { - // Not an appropriate block type. - return false; - } // Must be no pipes. if (job_node.child<1>().try_get_child()) { return false; } - // Check for arguments and redirections. All of the above types have an arguments / redirections - // list. It must be empty. - const parse_node_t &args_and_redirections = - tree().find_child(specific_statement, symbol_arguments_or_redirections_list); - if (args_and_redirections.child_count > 0) { - // Non-empty, we have an argument or redirection. - return false; - } + // Helper to check if an argument or redirection list has no redirections. + auto is_empty = [](tnode_t lst) -> bool { + return !lst.next_in_list(); + }; - // Ok, we are a simple block! - return true; + // Check if we're a block statement with redirections. We do it this obnoxious way to preserve + // type safety (in case we add more specific statement types). + const parse_node_t &specific_statement = statement.get_child_node<0>(); + tnode_t args_and_redirs; + switch (specific_statement.type) { + case symbol_block_statement: + return is_empty(statement.require_get_child().child<3>()); + case symbol_switch_statement: + return is_empty(statement.require_get_child().child<5>()); + case symbol_if_statement: + return is_empty(statement.require_get_child().child<3>()); + case symbol_boolean_statement: + case symbol_decorated_statement: + // not block statements + return false; + default: + assert(0 && "Unexpected child block type"); + return false; + } } parse_execution_result_t parse_execution_context_t::run_if_statement( @@ -304,7 +315,8 @@ parse_execution_result_t parse_execution_context_t::run_if_statement( return result; } -parse_execution_result_t parse_execution_context_t::run_begin_statement(tnode_t contents) { +parse_execution_result_t parse_execution_context_t::run_begin_statement( + tnode_t contents) { // Basic begin/end block. Push a scope block, run jobs, pop it scope_block_t *sb = parser->push_block(BEGIN); parse_execution_result_t ret = run_job_list(contents, sb); @@ -674,7 +686,6 @@ static wcstring reconstruct_orig_str(wcstring tokenized_str) { /// Handle the case of command not found. parse_execution_result_t parse_execution_context_t::handle_command_not_found( const wcstring &cmd_str, tnode_t statement, int err_code) { - // We couldn't find the specified command. This is a non-fatal error. We want to set the exit // status to 127, which is the standard number used by other shells like bash and zsh. @@ -894,8 +905,10 @@ bool parse_execution_context_t::determine_io_chain(tnode_t(); - for (tnode_t redirect_node : redirect_nodes) { + while (auto arg_or_redir = node.next_in_list()) { + tnode_t redirect_node = arg_or_redir.try_get_child(); + if (!redirect_node) continue; + int source_fd = -1; // source fd wcstring target; // file path or target fd enum token_type redirect_type = diff --git a/src/parse_grammar.h b/src/parse_grammar.h index 972b22f1a..edca76448 100644 --- a/src/parse_grammar.h +++ b/src/parse_grammar.h @@ -17,6 +17,11 @@ namespace grammar { using production_element_t = uint8_t; +enum { + // The maximum length of any seq production. + MAX_PRODUCTION_LENGTH = 6 +}; + // Define primitive types. template struct primitive { @@ -145,6 +150,8 @@ struct seq { static constexpr production_t<1 + sizeof...(Ts)> production = { {element(), element()..., token_type_invalid}}; + static_assert(1 + sizeof...(Ts) <= MAX_PRODUCTION_LENGTH, "MAX_PRODUCTION_LENGTH too small"); + using type_tuple = std::tuple; template diff --git a/src/tnode.h b/src/tnode.h index 53b4d82b0..c8544c33f 100644 --- a/src/tnode.h +++ b/src/tnode.h @@ -17,7 +17,7 @@ constexpr bool child_type_possible_at_index() { } // Check if a child type is possible for a parent type at any index. -// 5 is arbitrary and represents the longest production we have. +// The number of cases here should match MAX_PRODUCTION_LENGTH. template constexpr bool child_type_possible() { return child_type_possible_at_index() || @@ -201,6 +201,9 @@ class tnode_t { /// Returns an empty item on failure. template tnode_t next_in_list() { + // We require that we can contain ourselves, and ItemType as well. + static_assert(child_type_possible(), "Is not a list"); + static_assert(child_type_possible(), "Is not a list of that type"); if (!nodeptr) return {}; const parse_node_t *next = tree->next_node_in_node_list(*nodeptr, ItemType::token, &nodeptr);