From 5b24aac2660c27d27c9f3192821cd063fd07f9c0 Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Sun, 15 Dec 2013 16:05:37 -0800 Subject: [PATCH] Initial work on backtrace support with new parser --- builtin_complete.cpp | 2 +- parse_constants.h | 7 ++ parse_util.cpp | 201 +++++++++++++++++++++++++++++++++++++++++-- parse_util.h | 3 + parser.cpp | 183 +++++---------------------------------- parser.h | 12 +-- reader.cpp | 19 ++-- 7 files changed, 237 insertions(+), 190 deletions(-) diff --git a/builtin_complete.cpp b/builtin_complete.cpp index 0cc3b7e7d..f3773f487 100644 --- a/builtin_complete.cpp +++ b/builtin_complete.cpp @@ -499,7 +499,7 @@ static int builtin_complete(parser_t &parser, wchar_t **argv) { const wcstring condition_string = condition; parse_error_list_t errors; - if (parser.detect_errors(condition_string, &errors)) + if (parse_util_detect_errors(condition_string, &errors)) { append_format(stderr_buffer, L"%ls: Condition '%ls' contained a syntax error", diff --git a/parse_constants.h b/parse_constants.h index 7ccc962c2..e3eebbf5d 100644 --- a/parse_constants.h +++ b/parse_constants.h @@ -119,6 +119,13 @@ enum parse_error_code_t parse_error_unbalancing_case, //case outside of switch }; +enum { + PARSER_TEST_ERROR = 1, + PARSER_TEST_INCOMPLETE = 2 +}; +typedef unsigned int parser_test_error_bits_t; + + /** Error message for tokenizer error. The tokenizer message is diff --git a/parse_util.cpp b/parse_util.cpp index cb33915e3..f95679591 100644 --- a/parse_util.cpp +++ b/parse_util.cpp @@ -39,18 +39,12 @@ #include "signal.h" #include "wildcard.h" #include "parse_tree.h" +#include "parser.h" /** - Maximum number of autoloaded items opf a specific type to keep in - memory at a time. + Error message for improper use of the exec builtin */ -#define AUTOLOAD_MAX 10 - -/** - Minimum time, in seconds, before an autoloaded item will be - unloaded -*/ -#define AUTOLOAD_MIN_AGE 60 +#define EXEC_ERR_MSG _(L"This command can not be used in a pipeline") int parse_util_lineno(const wchar_t *str, size_t offset) { @@ -940,3 +934,192 @@ std::vector parse_util_compute_indents(const wcstring &src) return indents; } + +/* Append a syntax error to the given error list */ +static bool append_syntax_error(parse_error_list_t *errors, const parse_node_t &node, const wchar_t *fmt, ...) +{ + parse_error_t error; + error.source_start = node.source_start; + error.source_length = node.source_length; + error.code = parse_error_syntax; + + va_list va; + va_start(va, fmt); + error.text = vformat_string(fmt, va); + va_end(va); + + errors->push_back(error); + return true; +} + +/** + Returns 1 if the specified command is a builtin that may not be used in a pipeline +*/ +static int parser_is_pipe_forbidden(const wcstring &word) +{ + return contains(word, + L"exec", + L"case", + L"break", + L"return", + L"continue"); +} + +// Check if the first argument under the given node is --help +static bool first_argument_is_help(const parse_node_tree_t &node_tree, const parse_node_t &node, const wcstring &src) +{ + bool is_help = false; + const parse_node_tree_t::parse_node_list_t arg_nodes = node_tree.find_nodes(node, symbol_argument, 1); + if (! arg_nodes.empty()) + { + // Check the first argument only + const parse_node_t &arg = *arg_nodes.at(0); + const wcstring first_arg_src = arg.get_source(src); + is_help = parser_t::is_help(first_arg_src.c_str(), 3); + } + return is_help; +} + +parser_test_error_bits_t parse_util_detect_errors(const wcstring &buff_src, parse_error_list_t *out_errors) +{ + parse_node_tree_t node_tree; + parse_error_list_t parse_errors; + + // Whether we encountered a parse error + bool errored = false; + + // Whether we encountered an unclosed block + // We detect this via an 'end_command' block without source + bool has_unclosed_block = false; + + // Parse the input string into a parse tree + // Some errors are detected here + bool parsed = parse_t::parse(buff_src, 0, &node_tree, &parse_errors); + if (! parsed) + { + errored = true; + } + + // Expand all commands + // Verify 'or' and 'and' not used inside pipelines + // Verify pipes via parser_is_pipe_forbidden + // Verify return only within a function + + if (! errored) + { + const size_t node_tree_size = node_tree.size(); + for (size_t i=0; i < node_tree_size; i++) + { + const parse_node_t &node = node_tree.at(i); + if (node.type == symbol_end_command && ! node.has_source()) + { + // an 'end' without source is an unclosed block + has_unclosed_block = true; + } + else if (node.type == symbol_plain_statement) + { + wcstring command; + if (node_tree.command_for_plain_statement(node, buff_src, &command)) + { + // Check that we can expand the command + if (! expand_one(command, EXPAND_SKIP_CMDSUBST | EXPAND_SKIP_VARIABLES | EXPAND_SKIP_JOBS)) + { + errored = append_syntax_error(&parse_errors, node, ILLEGAL_CMD_ERR_MSG, command.c_str()); + } + + // Check that pipes are sound + bool is_boolean_command = contains(command, L"or", L"and"); + bool is_pipe_forbidden = parser_is_pipe_forbidden(command); + if (! errored && (is_boolean_command || is_pipe_forbidden)) + { + // 'or' and 'and' can be first in the pipeline. forbidden commands cannot be in a pipeline at all + if (node_tree.plain_statement_is_in_pipeline(node, is_pipe_forbidden)) + { + errored = append_syntax_error(&parse_errors, node, EXEC_ERR_MSG); + } + } + + // Check that we don't return from outside a function + // But we allow it if it's 'return --help' + if (! errored && command == L"return") + { + const parse_node_t *ancestor = &node; + bool found_function = false; + while (ancestor != NULL) + { + const parse_node_t *possible_function_header = node_tree.header_node_for_block_statement(*ancestor); + if (possible_function_header != NULL && possible_function_header->type == symbol_function_header) + { + found_function = true; + break; + } + ancestor = node_tree.get_parent(*ancestor); + + } + if (! found_function && ! first_argument_is_help(node_tree, node, buff_src)) + { + errored = append_syntax_error(&parse_errors, node, INVALID_RETURN_ERR_MSG); + } + } + + // Check that we don't return from outside a function + if (! errored && (command == L"break" || command == L"continue")) + { + // Walk up until we hit a 'for' or 'while' loop. If we hit a function first, stop the search; we can't break an outer loop from inside a function. + // This is a little funny because we can't tell if it's a 'for' or 'while' loop from the ancestor alone; we need the header. That is, we hit a block_statement, and have to check its header. + bool found_loop = false, end_search = false; + const parse_node_t *ancestor = &node; + while (ancestor != NULL && ! end_search) + { + const parse_node_t *loop_or_function_header = node_tree.header_node_for_block_statement(*ancestor); + if (loop_or_function_header != NULL) + { + switch (loop_or_function_header->type) + { + case symbol_while_header: + case symbol_for_header: + // this is a loop header, so we can break or continue + found_loop = true; + end_search = true; + break; + + case symbol_function_header: + // this is a function header, so we cannot break or continue. We stop our search here. + found_loop = false; + end_search = true; + break; + + default: + // most likely begin / end style block, which makes no difference + break; + } + } + ancestor = node_tree.get_parent(*ancestor); + } + + if (! found_loop && ! first_argument_is_help(node_tree, node, buff_src)) + { + errored = append_syntax_error(&parse_errors, node, INVALID_LOOP_ERR_MSG); + } + } + } + } + } + } + + parser_test_error_bits_t res = 0; + + if (errored) + res |= PARSER_TEST_ERROR; + + if (has_unclosed_block) + res |= PARSER_TEST_INCOMPLETE; + + if (out_errors) + { + out_errors->swap(parse_errors); + } + + return res; + +} diff --git a/parse_util.h b/parse_util.h index b5b5262a3..28e263ed9 100644 --- a/parse_util.h +++ b/parse_util.h @@ -8,6 +8,7 @@ #define FISH_PARSE_UTIL_H #include "autoload.h" +#include "parse_tree.h" #include #include #include @@ -162,4 +163,6 @@ wcstring parse_util_escape_string_with_quote(const wcstring &cmd, wchar_t quote) /** Given a string, parse it as fish code and then return the indents. The return value has the same size as the string */ std::vector parse_util_compute_indents(const wcstring &src); +parser_test_error_bits_t parse_util_detect_errors(const wcstring &buff_src, parse_error_list_t *out_errors); + #endif diff --git a/parser.cpp b/parser.cpp index 136bc74a4..1c6162a74 100644 --- a/parser.cpp +++ b/parser.cpp @@ -801,7 +801,7 @@ void parser_t::eval_args(const wchar_t *line, std::vector &args) proc_pop_interactive(); } -void parser_t::stack_trace(block_t *b, wcstring &buff) +void parser_t::stack_trace(block_t *b, wcstring &buff) const { /* Check if we should end the recursion @@ -844,7 +844,7 @@ void parser_t::stack_trace(block_t *b, wcstring &buff) { const source_block_t *sb = static_cast(b); const wchar_t *source_dest = sb->source_file; - append_format(buff, _(L"in . (source) call of file '%ls',\n"), source_dest); + append_format(buff, _(L"from sourcing file '%ls',\n"), source_dest); break; } case FUNCTION_CALL: @@ -2762,7 +2762,7 @@ int parser_t::parser_test_argument(const wchar_t *arg, wcstring *out, const wcha // debug( 1, L"%ls -> %ls %ls", arg_cpy, subst, tmp.buff ); parse_error_list_t errors; - err |= parser_t::detect_errors(subst, &errors); + err |= parse_util_detect_errors(subst, &errors); if (out && ! errors.empty()) { out->append(parse_errors_description(errors, subst, prefix)); @@ -2890,21 +2890,6 @@ int parser_t::test_args(const wchar_t * buff, wcstring *out, const wchar_t *pre return err; } -// Check if the first argument under the given node is --help -static bool first_argument_is_help(const parse_node_tree_t &node_tree, const parse_node_t &node, const wcstring &src) -{ - bool is_help = false; - const parse_node_tree_t::parse_node_list_t arg_nodes = node_tree.find_nodes(node, symbol_argument, 1); - if (! arg_nodes.empty()) - { - // Check the first argument only - const parse_node_t &arg = *arg_nodes.at(0); - const wcstring first_arg_src = arg.get_source(src); - is_help = parser_t::is_help(first_arg_src.c_str(), 3); - } - return is_help; -} - // helper type used in parser::test below struct block_info_t { @@ -2929,153 +2914,31 @@ static bool append_syntax_error(parse_error_list_t *errors, const parse_node_t & return true; } -parser_test_error_bits_t parser_t::detect_errors(const wcstring &buff_src, parse_error_list_t *out_errors, const wchar_t *prefix) +void parser_t::get_backtrace(const wcstring &src, const parse_error_list_t &errors, wcstring *output) const { - ASSERT_IS_MAIN_THREAD(); - - parse_node_tree_t node_tree; - parse_error_list_t parse_errors; - - // Whether we encountered a parse error - bool errored = false; - - // Whether we encountered an unclosed block - // We detect this via an 'end_command' block without source - bool has_unclosed_block = false; - - // Parse the input string into a parse tree - // Some errors are detected here - bool parsed = parse_t::parse(buff_src, 0, &node_tree, &parse_errors); - if (! parsed) + assert(output != NULL); + if (! errors.empty()) { - errored = true; - } - - // Expand all commands - // Verify 'or' and 'and' not used inside pipelines - // Verify pipes via parser_is_pipe_forbidden - // Verify return only within a function - - if (! errored) - { - const size_t node_tree_size = node_tree.size(); - for (size_t i=0; i < node_tree_size; i++) + const parse_error_t err = errors.at(0); + output->append(err.describe(src)); + output->push_back(L'\n'); + + // Determine which line we're on + assert(err.source_start <= src.size()); + size_t which_line = 1 + std::count(src.begin(), src.begin() + err.source_start, L'\n'); + + const wchar_t *filename = this->current_filename(); + if (filename) { - const parse_node_t &node = node_tree.at(i); - if (node.type == symbol_end_command && ! node.has_source()) - { - // an 'end' without source is an unclosed block - has_unclosed_block = true; - } - else if (node.type == symbol_plain_statement) - { - wcstring command; - if (node_tree.command_for_plain_statement(node, buff_src, &command)) - { - // Check that we can expand the command - if (! expand_one(command, EXPAND_SKIP_CMDSUBST | EXPAND_SKIP_VARIABLES | EXPAND_SKIP_JOBS)) - { - errored = append_syntax_error(&parse_errors, node, ILLEGAL_CMD_ERR_MSG, command.c_str()); - } - - // Check that pipes are sound - bool is_boolean_command = contains(command, L"or", L"and"); - bool is_pipe_forbidden = parser_is_pipe_forbidden(command); - if (! errored && (is_boolean_command || is_pipe_forbidden)) - { - // 'or' and 'and' can be first in the pipeline. forbidden commands cannot be in a pipeline at all - if (node_tree.plain_statement_is_in_pipeline(node, is_pipe_forbidden)) - { - errored = append_syntax_error(&parse_errors, node, EXEC_ERR_MSG); - } - } - - // Check that we don't return from outside a function - // But we allow it if it's 'return --help' - if (! errored && command == L"return") - { - const parse_node_t *ancestor = &node; - bool found_function = false; - while (ancestor != NULL) - { - const parse_node_t *possible_function_header = node_tree.header_node_for_block_statement(*ancestor); - if (possible_function_header != NULL && possible_function_header->type == symbol_function_header) - { - found_function = true; - break; - } - ancestor = node_tree.get_parent(*ancestor); - - } - if (! found_function && ! first_argument_is_help(node_tree, node, buff_src)) - { - errored = append_syntax_error(&parse_errors, node, INVALID_RETURN_ERR_MSG); - } - } - - // Check that we don't return from outside a function - if (! errored && (command == L"break" || command == L"continue")) - { - // Walk up until we hit a 'for' or 'while' loop. If we hit a function first, stop the search; we can't break an outer loop from inside a function. - // This is a little funny because we can't tell if it's a 'for' or 'while' loop from the ancestor alone; we need the header. That is, we hit a block_statement, and have to check its header. - bool found_loop = false, end_search = false; - const parse_node_t *ancestor = &node; - while (ancestor != NULL && ! end_search) - { - const parse_node_t *loop_or_function_header = node_tree.header_node_for_block_statement(*ancestor); - if (loop_or_function_header != NULL) - { - switch (loop_or_function_header->type) - { - case symbol_while_header: - case symbol_for_header: - // this is a loop header, so we can break or continue - found_loop = true; - end_search = true; - break; - - case symbol_function_header: - // this is a function header, so we cannot break or continue. We stop our search here. - found_loop = false; - end_search = true; - break; - - default: - // most likely begin / end style block, which makes no difference - break; - } - } - ancestor = node_tree.get_parent(*ancestor); - } - - if (! found_loop && ! first_argument_is_help(node_tree, node, buff_src)) - { - errored = append_syntax_error(&parse_errors, node, INVALID_LOOP_ERR_MSG); - } - } - } - } + append_format(*output, _(L"line %lu of '%ls'\n"), which_line, filename); } + else + { + append_format(*output, L"%ls: ", _(L"Standard input"), which_line); + } + + this->stack_trace(current_block, *output); } - - parser_test_error_bits_t res = 0; - - if (errored) - res |= PARSER_TEST_ERROR; - - if (has_unclosed_block) - res |= PARSER_TEST_INCOMPLETE; - - if (out_errors) - { - out_errors->swap(parse_errors); - } - - error_code=0; - - - return res; - } parser_test_error_bits_t parser_t::detect_errors2(const wchar_t *buff, wcstring *out, const wchar_t *prefix) diff --git a/parser.h b/parser.h index fb3efad85..90b6a1c43 100644 --- a/parser.h +++ b/parser.h @@ -14,12 +14,6 @@ #include "parse_tree.h" #include -enum { - PARSER_TEST_ERROR = 1, - PARSER_TEST_INCOMPLETE = 2 -}; -typedef unsigned int parser_test_error_bits_t; - /** event_blockage_t represents a block on events of the specified type */ @@ -488,8 +482,8 @@ public: \param out if non-null, any errors in the command will be filled out into this buffer \param prefix the prefix string to prepend to each error message written to the \c out buffer */ - parser_test_error_bits_t detect_errors(const wcstring &buff, parse_error_list_t *out_errors = NULL, const wchar_t *prefix = NULL); - parser_test_error_bits_t detect_errors2(const wchar_t * buff, wcstring *out = NULL, const wchar_t *prefix = NULL); + parser_test_error_bits_t detect_errors2(const wchar_t *buff, wcstring *out_error_desc, const wchar_t *prefix); + void get_backtrace(const wcstring &src, const parse_error_list_t &errors, wcstring *output) const; /** Test if the specified string can be parsed as an argument list, @@ -538,7 +532,7 @@ public: /** Write a stack trace starting at the specified block to the specified wcstring */ - void stack_trace(block_t *b, wcstring &buff); + void stack_trace(block_t *b, wcstring &buff) const; int get_block_type(const wchar_t *cmd) const; const wchar_t *get_block_command(int type) const; diff --git a/reader.cpp b/reader.cpp index 3eeae6271..87c014642 100644 --- a/reader.cpp +++ b/reader.cpp @@ -2473,7 +2473,8 @@ void reader_run_command(parser_t &parser, const wcstring &cmd) int reader_shell_test(const wchar_t *b) { wcstring bstr = b; - int res = parser_t::principal_parser().detect_errors(bstr); + parse_error_list_t errors; + int res = parse_util_detect_errors(bstr, &errors); if (res & PARSER_TEST_ERROR) { @@ -2490,14 +2491,9 @@ int reader_shell_test(const wchar_t *b) tmp2, 0); - parse_error_list_t errors; - parser_t::principal_parser().detect_errors(bstr, &errors, L"fish"); - - if (! errors.empty()) - { - const wcstring sb = parse_errors_description(errors, b, L"fish"); - fwprintf(stderr, L"%ls", sb.c_str()); - } + wcstring sb; + parser_t::principal_parser().get_backtrace(bstr, errors, &sb); + fwprintf(stderr, L"%ls", sb.c_str()); } return res; } @@ -3908,13 +3904,14 @@ static int read_ni(int fd, const io_chain_t &io) } parse_error_list_t errors; - if (! parser.detect_errors(str, &errors, L"fish")) + if (! parse_util_detect_errors(str, &errors)) { parser.eval(str, io, TOP); } else { - const wcstring sb = parse_errors_description(errors, str); + wcstring sb; + parser.get_backtrace(str, errors, &sb); fwprintf(stderr, L"%ls", sb.c_str()); res = 1; }