From 33f51b45e4ef90cd75e820fc4594601e7424eaeb Mon Sep 17 00:00:00 2001 From: Johannes Altmanninger Date: Wed, 19 Apr 2023 22:25:50 +0200 Subject: [PATCH] Tease apart parser.eval() overloads The most common overload takes a string and an io chain so let that one keep its name. --- src/builtins/eval.cpp | 2 +- src/exec.cpp | 3 ++- src/fish.cpp | 2 +- src/fish_tests.cpp | 34 ---------------------------------- src/parser.cpp | 15 ++++++++++----- src/parser.h | 14 +++++++------- src/reader.cpp | 2 +- 7 files changed, 22 insertions(+), 50 deletions(-) diff --git a/src/builtins/eval.cpp b/src/builtins/eval.cpp index 19dd5f9b8..9ba68f119 100644 --- a/src/builtins/eval.cpp +++ b/src/builtins/eval.cpp @@ -58,7 +58,7 @@ maybe_t builtin_eval(parser_t &parser, io_streams_t &streams, const wchar_t } int status = STATUS_CMD_OK; - auto res = parser.eval(new_cmd, ios, streams.job_group); + auto res = parser.eval_with(new_cmd, ios, streams.job_group, block_type_t::top); if (res.was_empty) { // Issue #5692, in particular, to catch `eval ""`, `eval "begin; end;"`, etc. // where we have an argument but nothing is executed. diff --git a/src/exec.cpp b/src/exec.cpp index b7e934069..ecf22caf9 100644 --- a/src/exec.cpp +++ b/src/exec.cpp @@ -1213,7 +1213,8 @@ static int exec_subshell_internal(const wcstring &cmd, parser_t &parser, *break_expand = true; return STATUS_CMD_ERROR; } - eval_res_t eval_res = parser.eval(cmd, io_chain_t{bufferfill}, job_group, block_type_t::subst); + eval_res_t eval_res = + parser.eval_with(cmd, io_chain_t{bufferfill}, job_group, block_type_t::subst); separated_buffer_t buffer = io_bufferfill_t::finish(std::move(bufferfill)); if (buffer.discarded()) { *break_expand = true; diff --git a/src/fish.cpp b/src/fish.cpp index b212b698e..637a17510 100644 --- a/src/fish.cpp +++ b/src/fish.cpp @@ -274,7 +274,7 @@ static int run_command_list(parser_t &parser, const std::vector &cm // Construct a parsed source ref. // Be careful to transfer ownership, this could be a very large string. auto ps = new_parsed_source_ref(cmd_wcs, *ast); - parser.eval(*ps, io); + parser.eval_parsed_source(*ps, io, {}, block_type_t::top); } else { wcstring sb; parser.get_backtrace(cmd_wcs, *errors, sb); diff --git a/src/fish_tests.cpp b/src/fish_tests.cpp index 4ee6c0acf..fad0735ed 100644 --- a/src/fish_tests.cpp +++ b/src/fish_tests.cpp @@ -2034,39 +2034,6 @@ static void test_abbreviations() { } } -/// Test path functions. -static void test_path() { - say(L"Testing path functions"); - - wcstring path = L"//foo//////bar/"; - path_make_canonical(path); - if (path != L"/foo/bar") { - err(L"Bug in canonical PATH code"); - } - - path = L"/"; - path_make_canonical(path); - if (path != L"/") { - err(L"Bug in canonical PATH code"); - } - - if (paths_are_equivalent(L"/foo/bar/baz", L"foo/bar/baz")) - err(L"Bug in canonical PATH code on line %ld", (long)__LINE__); - if (!paths_are_equivalent(L"///foo///bar/baz", L"/foo/bar////baz//")) - err(L"Bug in canonical PATH code on line %ld", (long)__LINE__); - if (!paths_are_equivalent(L"/foo/bar/baz", L"/foo/bar/baz")) - err(L"Bug in canonical PATH code on line %ld", (long)__LINE__); - if (!paths_are_equivalent(L"/", L"/")) - err(L"Bug in canonical PATH code on line %ld", (long)__LINE__); - - do_test(path_apply_working_directory(L"abc", L"/def/") == L"/def/abc"); - do_test(path_apply_working_directory(L"abc/", L"/def/") == L"/def/abc/"); - do_test(path_apply_working_directory(L"/abc/", L"/def/") == L"/abc/"); - do_test(path_apply_working_directory(L"/abc", L"/def/") == L"/abc"); - do_test(path_apply_working_directory(L"", L"/def/").empty()); - do_test(path_apply_working_directory(L"abc", L"") == L"abc"); -} - static void test_pager_navigation() { say(L"Testing pager navigation"); @@ -6433,7 +6400,6 @@ static const test_t s_tests[]{ {TEST_GROUP("wcstod"), test_wcstod}, {TEST_GROUP("dup2s"), test_dup2s}, {TEST_GROUP("dup2s"), test_dup2s_fd_for_target_fd}, - {TEST_GROUP("path"), test_path}, {TEST_GROUP("pager_navigation"), test_pager_navigation}, {TEST_GROUP("pager_layout"), test_pager_layout}, {TEST_GROUP("word_motion"), test_word_motion}, diff --git a/src/parser.cpp b/src/parser.cpp index 121cba8e0..9a1e9a873 100644 --- a/src/parser.cpp +++ b/src/parser.cpp @@ -525,13 +525,17 @@ profile_item_t *parser_t::create_profile_item() { return nullptr; } -eval_res_t parser_t::eval(const wcstring &cmd, const io_chain_t &io, - const job_group_ref_t &job_group, enum block_type_t block_type) { +eval_res_t parser_t::eval(const wcstring &cmd, const io_chain_t &io) { + return eval_with(cmd, io, {}, block_type_t::top); +} + +eval_res_t parser_t::eval_with(const wcstring &cmd, const io_chain_t &io, + const job_group_ref_t &job_group, enum block_type_t block_type) { // Parse the source into a tree, if we can. auto error_list = new_parse_error_list(); auto ps = parse_source(wcstring{cmd}, parse_flag_none, &*error_list); if (ps->has_value()) { - return this->eval(*ps, io, job_group, block_type); + return this->eval_parsed_source(*ps, io, job_group, block_type); } else { // Get a backtrace. This includes the message. wcstring backtrace_and_desc; @@ -549,8 +553,9 @@ eval_res_t parser_t::eval(const wcstring &cmd, const io_chain_t &io, eval_res_t parser_t::eval_string_ffi1(const wcstring &cmd) { return eval(cmd, io_chain_t()); } -eval_res_t parser_t::eval(const parsed_source_ref_t &ps, const io_chain_t &io, - const job_group_ref_t &job_group, enum block_type_t block_type) { +eval_res_t parser_t::eval_parsed_source(const parsed_source_ref_t &ps, const io_chain_t &io, + const job_group_ref_t &job_group, + enum block_type_t block_type) { assert(block_type == block_type_t::top || block_type == block_type_t::subst); const auto &job_list = ps.ast().top()->as_job_list(); if (!job_list.empty()) { diff --git a/src/parser.h b/src/parser.h index 1572cc899..e897229bd 100644 --- a/src/parser.h +++ b/src/parser.h @@ -324,6 +324,8 @@ class parser_t : public std::enable_shared_from_this { /// Global event blocks. uint64_t global_event_blocks{}; + eval_res_t eval(const wcstring &cmd, const io_chain_t &io); + /// Evaluate the expressions contained in cmd. /// /// \param cmd the string to evaluate @@ -332,18 +334,16 @@ class parser_t : public std::enable_shared_from_this { /// \param block_type The type of block to push on the block stack, which must be either 'top' /// or 'subst'. /// \return the result of evaluation. - eval_res_t eval(const wcstring &cmd, const io_chain_t &io, - const job_group_ref_t &job_group = {}, - block_type_t block_type = block_type_t::top); + eval_res_t eval_with(const wcstring &cmd, const io_chain_t &io, + const job_group_ref_t &job_group, block_type_t block_type); - /// An ffi overload of `eval(const wcstring &cmd, ...)` but without the extra parameters. eval_res_t eval_string_ffi1(const wcstring &cmd); /// Evaluate the parsed source ps. /// Because the source has been parsed, a syntax error is impossible. - eval_res_t eval(const parsed_source_ref_t &ps, const io_chain_t &io, - const job_group_ref_t &job_group = {}, - block_type_t block_type = block_type_t::top); + eval_res_t eval_parsed_source(const parsed_source_ref_t &ps, const io_chain_t &io, + const job_group_ref_t &job_group = {}, + block_type_t block_type = block_type_t::top); /// Evaluates a node. /// The node type must be ast_t::statement_t or ast::job_list_t. diff --git a/src/reader.cpp b/src/reader.cpp index 8939018f8..a854b3c17 100644 --- a/src/reader.cpp +++ b/src/reader.cpp @@ -4750,7 +4750,7 @@ static int read_ni(parser_t &parser, int fd, const io_chain_t &io) { // Construct a parsed source ref. // Be careful to transfer ownership, this could be a very large string. auto ps = new_parsed_source_ref(str, *ast); - parser.eval(*ps, io); + parser.eval_parsed_source(*ps, io); return 0; } else { wcstring sb;