diff --git a/src/highlight.cpp b/src/highlight.cpp index cc69010ec..91ef533c2 100644 --- a/src/highlight.cpp +++ b/src/highlight.cpp @@ -1033,7 +1033,7 @@ const highlighter_t::color_array_t &highlighter_t::highlight() { case symbol_if_clause: case symbol_else_clause: case symbol_case_item: - case symbol_boolean_statement: + case symbol_not_statement: case symbol_decorated_statement: case symbol_if_statement: { this->color_children(node, parse_token_type_string, highlight_spec_command); diff --git a/src/parse_constants.h b/src/parse_constants.h index ef66ec9b1..b4f679b6a 100644 --- a/src/parse_constants.h +++ b/src/parse_constants.h @@ -19,6 +19,7 @@ enum parse_token_type_t { symbol_job_list, symbol_job_conjunction, symbol_job_conjunction_continuation, + symbol_job_decorator, symbol_job, symbol_job_continuation, symbol_statement, @@ -35,7 +36,7 @@ enum parse_token_type_t { symbol_switch_statement, symbol_case_item_list, symbol_case_item, - symbol_boolean_statement, + symbol_not_statement, symbol_decorated_statement, symbol_plain_statement, symbol_arguments_or_redirections_list, @@ -84,36 +85,9 @@ const enum_map token_enum_map[] = { {parse_token_type_andand, L"parse_token_type_andand"}, {parse_token_type_oror, L"parse_token_type_oror"}, {parse_token_type_terminate, L"parse_token_type_terminate"}, - {symbol_andor_job_list, L"symbol_andor_job_list"}, - {symbol_argument, L"symbol_argument"}, - {symbol_argument_list, L"symbol_argument_list"}, - {symbol_arguments_or_redirections_list, L"symbol_arguments_or_redirections_list"}, - {symbol_begin_header, L"symbol_begin_header"}, - {symbol_block_header, L"symbol_block_header"}, - {symbol_block_statement, L"symbol_block_statement"}, - {symbol_boolean_statement, L"symbol_boolean_statement"}, - {symbol_case_item, L"symbol_case_item"}, - {symbol_case_item_list, L"symbol_case_item_list"}, - {symbol_decorated_statement, L"symbol_decorated_statement"}, - {symbol_else_clause, L"symbol_else_clause"}, - {symbol_else_continuation, L"symbol_else_continuation"}, - {symbol_end_command, L"symbol_end_command"}, - {symbol_for_header, L"symbol_for_header"}, - {symbol_freestanding_argument_list, L"symbol_freestanding_argument_list"}, - {symbol_function_header, L"symbol_function_header"}, - {symbol_if_clause, L"symbol_if_clause"}, - {symbol_if_statement, L"symbol_if_statement"}, - {symbol_job, L"symbol_job"}, - {symbol_job_conjunction, L"symbol_job_conjunction"}, - {symbol_job_continuation, L"symbol_job_continuation"}, - {symbol_job_list, L"symbol_job_list"}, - {symbol_optional_newlines, L"symbol_optional_newlines"}, - {symbol_optional_background, L"symbol_optional_background"}, - {symbol_plain_statement, L"symbol_plain_statement"}, - {symbol_redirection, L"symbol_redirection"}, - {symbol_statement, L"symbol_statement"}, - {symbol_switch_statement, L"symbol_switch_statement"}, - {symbol_while_header, L"symbol_while_header"}, +// Define all symbols +#define ELEM(sym) {symbol_##sym, L"symbol_" #sym}, +#include "parse_grammar_elements.inc" {token_type_invalid, L"token_type_invalid"}, {token_type_invalid, NULL}}; #define token_enum_map_len (sizeof token_enum_map / sizeof *token_enum_map) @@ -165,7 +139,7 @@ enum parse_statement_decoration_t { }; // Boolean statement types, stored in node tag. -enum parse_bool_statement_type_t { parse_bool_and, parse_bool_or, parse_bool_not }; +enum parse_bool_statement_type_t { parse_bool_none, parse_bool_and, parse_bool_or }; // Whether a statement is backgrounded. enum parse_optional_background_t { parse_no_background, parse_background }; diff --git a/src/parse_execution.cpp b/src/parse_execution.cpp index 06f31a624..a54f6fb49 100644 --- a/src/parse_execution.cpp +++ b/src/parse_execution.cpp @@ -107,7 +107,7 @@ tnode_t parse_execution_context_t::infinite_recursive_statem const wcstring &forbidden_function_name = parser->forbidden_function.back(); // Get the first job in the job list. - tnode_t first_job = job_list.try_get_child().child<0>(); + tnode_t first_job = job_list.try_get_child().child<0>(); if (!first_job) { return {}; } @@ -215,7 +215,7 @@ bool parse_execution_context_t::job_is_simple_block(tnode_t job_node) co 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_not_statement: case symbol_decorated_statement: // not block statements return false; @@ -921,33 +921,10 @@ bool parse_execution_context_t::determine_io_chain(tnode_t bool_statement) { - // Handle a boolean statement. - bool skip_job = false; - switch (bool_statement_type(bool_statement)) { - case parse_bool_and: { - // AND. Skip if the last job failed. - skip_job = (proc_get_last_status() != 0); - break; - } - case parse_bool_or: { - // OR. Skip if the last job succeeded. - skip_job = (proc_get_last_status() == 0); - break; - } - case parse_bool_not: { - // NOT. Negate it. - job->set_flag(JOB_NEGATE, !job->get_flag(JOB_NEGATE)); - break; - } - } - - if (skip_job) { - return parse_execution_skipped; - } - return this->populate_job_process(job, proc, - bool_statement.require_get_child()); +parse_execution_result_t parse_execution_context_t::populate_not_process( + job_t *job, process_t *proc, tnode_t not_statement) { + job->set_flag(JOB_NEGATE, !job->get_flag(JOB_NEGATE)); + return this->populate_job_process(job, proc, not_statement.child<1>()); } template @@ -985,8 +962,8 @@ parse_execution_result_t parse_execution_context_t::populate_job_process( parse_execution_result_t result = parse_execution_success; switch (specific_statement.type) { - case symbol_boolean_statement: { - result = this->populate_boolean_process(job, proc, {&tree(), &specific_statement}); + case symbol_not_statement: { + result = this->populate_not_process(job, proc, {&tree(), &specific_statement}); break; } case symbol_block_statement: @@ -1224,6 +1201,7 @@ parse_execution_result_t parse_execution_context_t::run_job_conjunction( tnode_t job_expr, const block_t *associated_block) { parse_execution_result_t result = parse_execution_success; tnode_t cursor = job_expr; + // continuation is the parent of the cursor tnode_t continuation; while (cursor) { if (should_cancel_execution(associated_block)) break; @@ -1232,13 +1210,7 @@ parse_execution_result_t parse_execution_context_t::run_job_conjunction( // Check the conjunction type. parse_bool_statement_type_t conj = bool_statement_type(continuation); assert((conj == parse_bool_and || conj == parse_bool_or) && "Unexpected conjunction"); - if (conj == parse_bool_and) { - // Skip if last job failed. - skip = (proc_get_last_status() != 0); - } else if (conj == parse_bool_or) { - // Skip if last job succeeded. - skip = (proc_get_last_status() == 0); - } + skip = should_skip(conj); } if (! skip) { result = run_1_job(cursor.child<0>(), associated_block); @@ -1249,20 +1221,40 @@ parse_execution_result_t parse_execution_context_t::run_job_conjunction( return result; } +bool parse_execution_context_t::should_skip(parse_bool_statement_type_t type) const { + switch (type) { + case parse_bool_and: + // AND. Skip if the last job failed. + return proc_get_last_status() != 0; + case parse_bool_or: + // OR. Skip if the last job succeeded. + return proc_get_last_status() == 0; + default: + return false; + } +} + template parse_execution_result_t parse_execution_context_t::run_job_list(tnode_t job_list, const block_t *associated_block) { + // We handle both job_list and andor_job_list uniformly. static_assert(Type::token == symbol_job_list || Type::token == symbol_andor_job_list, "Not a job list"); parse_execution_result_t result = parse_execution_success; - while (tnode_t job_expr = - job_list.template next_in_list()) { + while (auto job_conj = job_list.template next_in_list()) { if (should_cancel_execution(associated_block)) break; - result = this->run_job_conjunction(job_expr, associated_block); + + // Maybe skip the job if it has a leading and/or. + // Skipping is treated as success. + if (should_skip(get_decorator(job_conj))) { + result = parse_execution_success; + } else { + result = this->run_job_conjunction(job_conj, associated_block); + } } - // Returns the last job executed. + // Returns the result of the last job executed or skipped. return result; } diff --git a/src/parse_execution.h b/src/parse_execution.h index d15e69469..dceb87235 100644 --- a/src/parse_execution.h +++ b/src/parse_execution.h @@ -72,6 +72,9 @@ class parse_execution_context_t { tnode_t job_list, wcstring *out_func_name) const; bool is_function_context() const; + /// Return whether we should skip a job with the given bool statement type. + bool should_skip(parse_bool_statement_type_t type) const; + /// Indicates whether a job is a simple block (one block, no redirections). bool job_is_simple_block(tnode_t job) const; @@ -81,8 +84,8 @@ class parse_execution_context_t { // These create process_t structures from statements. parse_execution_result_t populate_job_process(job_t *job, process_t *proc, tnode_t statement); - parse_execution_result_t populate_boolean_process( - job_t *job, process_t *proc, tnode_t bool_statement); + parse_execution_result_t populate_not_process(job_t *job, process_t *proc, + tnode_t not_statement); parse_execution_result_t populate_plain_process(job_t *job, process_t *proc, tnode_t statement); diff --git a/src/parse_grammar.h b/src/parse_grammar.h index 99b021f4a..e08ccb62d 100644 --- a/src/parse_grammar.h +++ b/src/parse_grammar.h @@ -199,12 +199,20 @@ struct alternative {}; // A job_list is a list of job_conjunctions, separated by semicolons or newlines DEF_ALT(job_list) { - using normal = seq; + using normal = seq; using empty_line = seq; using empty = grammar::empty; ALT_BODY(job_list, normal, empty_line, empty); }; +// Job decorators are 'and' and 'or'. These apply to the whole job. +DEF_ALT(job_decorator) { + using ands = single>; + using ors = single>; + using empty = grammar::empty; + ALT_BODY(job_decorator, ands, ors, empty); +}; + // A job_conjunction is a job followed by a continuation. DEF(job_conjunction) produces_sequence { BODY(job_conjunction); @@ -231,12 +239,12 @@ DEF_ALT(job_continuation) { // A statement is a normal command, or an if / while / and etc DEF_ALT(statement) { - using boolean = single; + using nots = single; using block = single; using ifs = single; using switchs = single; using decorated = single; - ALT_BODY(statement, boolean, block, ifs, switchs, decorated); + ALT_BODY(statement, nots, block, ifs, switchs, decorated); }; // A block is a conditional, loop, or begin/end @@ -304,19 +312,15 @@ DEF(function_header) produces_sequence, argument, argument_list, tok_end>{ BODY(function_header)}; -// A boolean statement is AND or OR or NOT -DEF_ALT(boolean_statement) { - using ands = seq, statement>; // foo ; and bar - using ors = seq, statement>; // foo ; or bar - using nots = seq, statement>; // not foo - ALT_BODY(boolean_statement, ands, ors, nots); +DEF(not_statement) produces_sequence, statement> { + BODY(not_statement); }; // An andor_job_list is zero or more job lists, where each starts with an `and` or `or` boolean // statement. DEF_ALT(andor_job_list) { using empty = grammar::empty; - using andor_job = seq; + using andor_job = seq; using empty_line = seq; ALT_BODY(andor_job_list, empty, andor_job, empty_line); }; diff --git a/src/parse_grammar_elements.inc b/src/parse_grammar_elements.inc index 1543a2a93..79339b6cb 100644 --- a/src/parse_grammar_elements.inc +++ b/src/parse_grammar_elements.inc @@ -1,6 +1,7 @@ // Define ELEM before including this file. ELEM(job_list) ELEM(job) +ELEM(job_decorator) ELEM(job_conjunction) ELEM(job_conjunction_continuation) ELEM(job_continuation) @@ -18,7 +19,7 @@ ELEM(for_header) ELEM(while_header) ELEM(begin_header) ELEM(function_header) -ELEM(boolean_statement) +ELEM(not_statement) ELEM(andor_job_list) ELEM(decorated_statement) ELEM(plain_statement) diff --git a/src/parse_productions.cpp b/src/parse_productions.cpp index 94c56a38b..a09e28e13 100644 --- a/src/parse_productions.cpp +++ b/src/parse_productions.cpp @@ -61,6 +61,26 @@ RESOLVE(job_list) { } } +// A job decorator is AND or OR +RESOLVE(job_decorator) { + UNUSED(token2); + + switch (token1.keyword) { + case parse_keyword_and: { + *out_tag = parse_bool_and; + return production_for(); + } + case parse_keyword_or: { + *out_tag = parse_bool_or; + return production_for(); + } + default: { + *out_tag = parse_bool_none; + return production_for(); + } + } +} + RESOLVE(job_conjunction_continuation) { UNUSED(token2); UNUSED(out_tag); @@ -123,10 +143,8 @@ RESOLVE(statement) { switch (token1.type) { case parse_token_type_string: { switch (token1.keyword) { - case parse_keyword_and: - case parse_keyword_or: case parse_keyword_not: { - return production_for(); + return production_for(); } case parse_keyword_for: case parse_keyword_while: @@ -260,27 +278,6 @@ RESOLVE(block_header) { } } -// A boolean statement is AND or OR or NOT. -RESOLVE(boolean_statement) { - UNUSED(token2); - - switch (token1.keyword) { - case parse_keyword_and: { - *out_tag = parse_bool_and; - return production_for(); - } - case parse_keyword_or: { - *out_tag = parse_bool_or; - return production_for(); - } - case parse_keyword_not: { - *out_tag = parse_bool_not; - return production_for(); - } - default: { return NO_PRODUCTION; } - } -} - RESOLVE(decorated_statement) { // If this is e.g. 'command --help' then the command is 'command' and not a decoration. If the diff --git a/src/parse_util.cpp b/src/parse_util.cpp index 83860912a..2927763ee 100644 --- a/src/parse_util.cpp +++ b/src/parse_util.cpp @@ -1072,17 +1072,15 @@ static bool detect_errors_in_backgrounded_job(tnode_t job, "Expected first job to be the node we found"); (void)first_jconj; - // Try getting the next job as a boolean statement. - tnode_t next_job = jlist.next_in_list().child<0>(); - tnode_t next_stmt = next_job.child<0>(); - if (auto bool_stmt = next_stmt.try_get_child()) { + // Try getting the next job's decorator. + if (auto next_job_dec = jlist.next_in_list()) { // The next job is indeed a boolean statement. - parse_bool_statement_type_t bool_type = bool_statement_type(bool_stmt); - if (bool_type == parse_bool_and) { // this is not allowed - errored = append_syntax_error(parse_errors, bool_stmt.source_range()->start, + parse_bool_statement_type_t bool_type = bool_statement_type(next_job_dec); + if (bool_type == parse_bool_and) { + errored = append_syntax_error(parse_errors, next_job_dec.source_range()->start, BOOL_AFTER_BACKGROUND_ERROR_MSG, L"and"); - } else if (bool_type == parse_bool_or) { // this is not allowed - errored = append_syntax_error(parse_errors, bool_stmt.source_range()->start, + } else if (bool_type == parse_bool_or) { + errored = append_syntax_error(parse_errors, next_job_dec.source_range()->start, BOOL_AFTER_BACKGROUND_ERROR_MSG, L"or"); } } @@ -1100,7 +1098,8 @@ static bool detect_errors_in_plain_statement(const wcstring &buff_src, // In a few places below, we want to know if we are in a pipeline. tnode_t st = pst.try_get_parent().try_get_parent(); - const bool is_in_pipeline = statement_is_in_pipeline(st, true /* count first */); + pipeline_position_t pipe_pos = get_pipeline_position(st); + bool is_in_pipeline = (pipe_pos != pipeline_position_t::none); // We need to know the decoration. const enum parse_statement_decoration_t decoration = get_decoration(pst); @@ -1110,6 +1109,19 @@ static bool detect_errors_in_plain_statement(const wcstring &buff_src, errored = append_syntax_error(parse_errors, source_start, EXEC_ERR_MSG, L"exec"); } + // This is a somewhat stale check that 'and' and 'or' are not in pipelines, except at the + // beginning. We can't disallow them as commands entirely because we need to support 'and + // --help', etc. + if (pipe_pos == pipeline_position_t::subsequent) { + // check if our command is 'and' or 'or'. This is very clumsy; we don't catch e.g. quoted + // commands. + wcstring command = pst.child<0>().get_source(buff_src); + if (command == L"and" || command == L"or") { + errored = + append_syntax_error(parse_errors, source_start, EXEC_ERR_MSG, command.c_str()); + } + } + if (maybe_t mcommand = command_for_plain_statement(pst, buff_src)) { wcstring command = std::move(*mcommand); // Check that we can expand the command. @@ -1254,16 +1266,9 @@ parser_test_error_bits_t parse_util_detect_errors(const wcstring &buff_src, has_unclosed_block = true; } else if (node.type == symbol_statement && !node.has_source()) { // Check for a statement without source in a pipeline, i.e. unterminated pipeline. - has_unclosed_pipe |= statement_is_in_pipeline({&node_tree, &node}, false); - } else if (node.type == symbol_boolean_statement) { - // 'or' and 'and' can be in a pipeline, as long as they're first. - tnode_t gbs{&node_tree, &node}; - parse_bool_statement_type_t type = bool_statement_type(gbs); - if ((type == parse_bool_and || type == parse_bool_or) && - statement_is_in_pipeline(gbs.try_get_parent(), - false /* don't count first */)) { - errored = append_syntax_error(&parse_errors, node.source_start, EXEC_ERR_MSG, - (type == parse_bool_and) ? L"and" : L"or"); + auto pipe_pos = get_pipeline_position({&node_tree, &node}); + if (pipe_pos != pipeline_position_t::none) { + has_unclosed_pipe = true; } } else if (node.type == symbol_argument) { tnode_t arg{&node_tree, &node}; diff --git a/src/tnode.cpp b/src/tnode.cpp index 64b93d124..8e4a447fb 100644 --- a/src/tnode.cpp +++ b/src/tnode.cpp @@ -46,7 +46,7 @@ enum parse_statement_decoration_t get_decoration(tnode_t stmt) { +enum parse_bool_statement_type_t bool_statement_type(tnode_t stmt) { return static_cast(stmt.tag()); } @@ -111,24 +111,35 @@ bool job_node_is_background(tnode_t job) { return bg.tag() == parse_background; } -bool statement_is_in_pipeline(tnode_t st, bool include_first) { +parse_bool_statement_type_t get_decorator(tnode_t conj) { + using namespace grammar; + tnode_t dec; + // We have two possible parents: job_list and andor_job_list. + if (auto p = conj.try_get_parent()) { + dec = p.require_get_child(); + } else if (auto p = conj.try_get_parent()) { + dec = p.require_get_child(); + } + // note this returns 0 (none) if dec is empty. + return bool_statement_type(dec); +} + +pipeline_position_t get_pipeline_position(tnode_t st) { using namespace grammar; if (!st) { - return false; + return pipeline_position_t::none; } // If we're part of a job continuation, we're definitely in a pipeline. if (st.try_get_parent()) { - return true; + return pipeline_position_t::subsequent; } - // If include_first is set, check if we're the beginning of a job, and if so, whether that job + // Check if we're the beginning of a job, and if so, whether that job // has a non-empty continuation. - if (include_first) { - tnode_t jc = st.try_get_parent().child<1>(); - if (jc.try_get_child()) { - return true; - } + tnode_t jc = st.try_get_parent().child<1>(); + if (jc.try_get_child()) { + return pipeline_position_t::first; } - return false; + return pipeline_position_t::none; } diff --git a/src/tnode.h b/src/tnode.h index 92fde8da4..60fac38b6 100644 --- a/src/tnode.h +++ b/src/tnode.h @@ -234,7 +234,7 @@ maybe_t command_for_plain_statement(tnode_t parse_statement_decoration_t get_decoration(tnode_t stmt); /// Return the type for a boolean statement. -enum parse_bool_statement_type_t bool_statement_type(tnode_t stmt); +enum parse_bool_statement_type_t bool_statement_type(tnode_t stmt); enum parse_bool_statement_type_t bool_statement_type(tnode_t stmt); @@ -253,9 +253,19 @@ arguments_node_list_t get_argument_nodes(tnode_t); -/// Return whether the statement is part of a pipeline. If include_first is set, the first command -/// in a pipeline is considered part of it; otherwise only the second or additional commands are. -bool statement_is_in_pipeline(tnode_t st, bool include_first); +/// If the conjunction is has a decorator (and/or), return it; otherwise return none. This only +/// considers the leading conjunction, e.g. in `and true || false` only the 'true' conjunction will +/// return 'and'. +parse_bool_statement_type_t get_decorator(tnode_t); + +/// Return whether the statement is part of a pipeline. +/// This doesn't detect e.g. pipelines involving our parent's block statements. +enum class pipeline_position_t { + none, // not part of a pipeline + first, // first command in a pipeline + subsequent // second or further command in a pipeline +}; +pipeline_position_t get_pipeline_position(tnode_t st); /// Check whether an argument_list is a root list. inline bool argument_list_is_root(tnode_t list) { diff --git a/tests/andandoror.in b/tests/andandoror.in index f2093039b..f739cc1ea 100644 --- a/tests/andandoror.in +++ b/tests/andandoror.in @@ -20,7 +20,7 @@ set beta 0 set gamma 0 set delta 0 while [ $alpha -lt 2 ] && [ $beta -lt 3 ] - and [ $gamma -lt 4 ] || [ $delta -lt 5 ] + or [ $gamma -lt 4 ] || [ $delta -lt 5 ] echo $alpha $beta $gamma set alpha ( math $alpha + 1 ) set beta ( math $beta + 1 ) @@ -31,3 +31,8 @@ end logmsg "Complex scenarios" begin; echo 1 ; false ; end || begin ; echo 2 && echo 3 ; end + +if false && true + or not false + echo 4 +end diff --git a/tests/andandoror.out b/tests/andandoror.out index e8f8f8bed..84f9f04f2 100644 --- a/tests/andandoror.out +++ b/tests/andandoror.out @@ -28,3 +28,4 @@ if test 4 ok 1 2 3 +4