From 4a6d62273371093bb1de0d603b5f6b3d59fb4953 Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Thu, 21 Oct 2021 14:28:39 -0700 Subject: [PATCH] Continue to refactor functions Now that we have immutable props, we can remove a bunch of 'helper' functions. --- src/builtin_functions.cpp | 21 ++++---- src/builtin_type.cpp | 12 +++-- src/complete.cpp | 9 ++-- src/function.cpp | 101 ++++++++++++++------------------------ src/function.h | 50 +++++++------------ src/parse_util.h | 2 +- src/parser.cpp | 3 +- 7 files changed, 79 insertions(+), 119 deletions(-) diff --git a/src/builtin_functions.cpp b/src/builtin_functions.cpp index cc20bc2dd..2d52b5145 100644 --- a/src/builtin_functions.cpp +++ b/src/builtin_functions.cpp @@ -135,7 +135,7 @@ static int parse_cmd_opts(functions_cmd_opts_t &opts, int *optind, //!OCLINT(hi return STATUS_CMD_OK; } -static int report_function_metadata(const wchar_t *funcname, bool verbose, io_streams_t &streams, +static int report_function_metadata(const wcstring &funcname, bool verbose, io_streams_t &streams, parser_t &parser, bool metadata_as_comments) { const wchar_t *path = L"n/a"; const wchar_t *autoloaded = L"n/a"; @@ -143,18 +143,16 @@ static int report_function_metadata(const wchar_t *funcname, bool verbose, io_st wcstring description = L"n/a"; int line_number = 0; - if (function_exists(funcname, parser)) { - auto props = function_get_props(funcname); - path = function_get_definition_file(funcname); + if (auto props = function_get_props_autoload(funcname, parser)) { + path = props->definition_file; if (path) { - autoloaded = function_is_autoloaded(funcname) ? L"autoloaded" : L"not-autoloaded"; - line_number = function_get_definition_lineno(funcname); + autoloaded = props->is_autoload ? L"autoloaded" : L"not-autoloaded"; + line_number = props->definition_lineno(); } else { path = L"stdin"; } shadows_scope = props->shadow_scope ? L"scope-shadowing" : L"no-scope-shadowing"; - function_get_desc(funcname, description); - description = escape_string(description, ESCAPE_NO_QUOTED); + description = escape_string(props->description, ESCAPE_NO_QUOTED); } if (metadata_as_comments) { @@ -347,16 +345,17 @@ maybe_t builtin_functions(parser_t &parser, io_streams_t &streams, const wc int res = STATUS_CMD_OK; for (int i = optind; i < argc; i++) { - if (!function_exists(argv[i], parser)) { + wcstring funcname = argv[i]; + auto func = function_get_props_autoload(argv[i], parser); + if (!func) { res++; } else { if (!opts.query) { if (i != optind) streams.out.append(L"\n"); - const wchar_t *funcname = argv[i]; if (!opts.no_metadata) { report_function_metadata(funcname, opts.verbose, streams, parser, true); } - wcstring def = functions_def(funcname); + wcstring def = func->annotated_definition(funcname); if (!streams.out_is_redirected && isatty(STDOUT_FILENO)) { std::vector colors; diff --git a/src/builtin_type.cpp b/src/builtin_type.cpp index 3ab6a2fb9..bf08bc4b9 100644 --- a/src/builtin_type.cpp +++ b/src/builtin_type.cpp @@ -125,11 +125,13 @@ maybe_t builtin_type(parser_t &parser, io_streams_t &streams, const wchar_t int found = 0; const wchar_t *name = argv[idx]; // Functions - if (!opts.force_path && !opts.no_functions && function_exists(name, parser)) { + function_properties_ref_t func{}; + if (!opts.force_path && !opts.no_functions && + (func = function_get_props_autoload(name, parser))) { ++found; res = true; if (!opts.query && !opts.type) { - auto path = function_get_definition_file(name); + auto path = func->definition_file; if (opts.path) { if (path) { streams.out.append(path); @@ -140,9 +142,9 @@ maybe_t builtin_type(parser_t &parser, io_streams_t &streams, const wchar_t streams.out.append(_(L" with definition")); streams.out.append(L"\n"); // Function path - wcstring def = functions_def(name); + wcstring def = func->annotated_definition(name); if (path) { - int line_number = function_get_definition_lineno(name); + int line_number = func->definition_lineno(); wcstring comment; if (std::wcscmp(path, L"-") != 0) { append_format(comment, L"# Defined in %ls @ line %d\n", path, @@ -165,7 +167,7 @@ maybe_t builtin_type(parser_t &parser, io_streams_t &streams, const wchar_t } } else { streams.out.append_format(_(L"%ls is a function"), name); - auto path = function_get_definition_file(name); + auto path = func->definition_file; if (path) { streams.out.append_format(_(L" (defined in %ls)"), path); } diff --git a/src/complete.cpp b/src/complete.cpp index 6ebdc268e..a863e5e3a 100644 --- a/src/complete.cpp +++ b/src/complete.cpp @@ -660,9 +660,10 @@ void completer_t::complete_cmd_desc(const wcstring &str) { /// Returns a description for the specified function, or an empty string if none. static wcstring complete_function_desc(const wcstring &fn) { - wcstring result; - function_get_desc(fn, result); - return result; + if (auto props = function_get_props(fn)) { + return props->description; + } + return wcstring{}; } /// Complete the specified command name. Search for executables in the path, executables defined @@ -854,7 +855,7 @@ static void complete_load(const wcstring &name) { // We have to load this as a function, since it may define a --wraps or signature. // See issue #2466. auto &parser = parser_t::principal_parser(); - function_load(name, parser); + function_get_props_autoload(name, parser); // It's important to NOT hold the lock around completion loading. // We need to take the lock to decide what to load, drop it to perform the load, then reacquire diff --git a/src/function.cpp b/src/function.cpp index c7ff951ee..df12ec156 100644 --- a/src/function.cpp +++ b/src/function.cpp @@ -163,20 +163,17 @@ function_properties_ref_t function_get_props(const wcstring &name) { return function_set.acquire()->get_props(name); } -int function_exists(const wcstring &cmd, parser_t &parser) { +function_properties_ref_t function_get_props_autoload(const wcstring &name, parser_t &parser) { ASSERT_IS_MAIN_THREAD(); - if (!valid_func_name(cmd)) return false; - if (parser_keywords_is_reserved(cmd)) return 0; - try_autoload(cmd, parser); - auto funcset = function_set.acquire(); - return funcset->funcs.find(cmd) != funcset->funcs.end(); + if (parser_keywords_is_reserved(name)) return nullptr; + try_autoload(name, parser); + return function_get_props(name); } -void function_load(const wcstring &cmd, parser_t &parser) { +bool function_exists(const wcstring &cmd, parser_t &parser) { ASSERT_IS_MAIN_THREAD(); - if (!parser_keywords_is_reserved(cmd)) { - try_autoload(cmd, parser); - } + if (!valid_func_name(cmd)) return false; + return function_get_props_autoload(cmd, parser) != nullptr; } bool function_exists_no_autoload(const wcstring &cmd) { @@ -203,29 +200,19 @@ void function_remove(const wcstring &name) { funcset->autoload_tombstones.insert(name); } -bool function_get_definition(const wcstring &name, wcstring &out_definition) { - auto props = function_get_props(name); - if (!props) return false; - +// \return the body of a function (everything after the header, up to but not including the 'end'). +static wcstring get_function_body_source(const function_properties_t &props) { // We want to preserve comments that the AST attaches to the header (#5285). // Take everything from the end of the header to the 'end' keyword. - auto header_src = props->func_node->header->try_source_range(); - auto end_kw_src = props->func_node->end.try_source_range(); + auto header_src = props.func_node->header->try_source_range(); + auto end_kw_src = props.func_node->end.try_source_range(); if (header_src && end_kw_src) { uint32_t body_start = header_src->start + header_src->length; uint32_t body_end = end_kw_src->start; assert(body_start <= body_end && "end keyword should come after header"); - out_definition = wcstring(props->parsed_source->src, body_start, body_end - body_start); + return wcstring(props.parsed_source->src, body_start, body_end - body_start); } - return true; -} - -bool function_get_desc(const wcstring &name, wcstring &out_desc) { - if (auto props = function_get_props(name)) { - out_desc = _(props->description.c_str()); - return true; - } - return false; + return wcstring{}; } void function_set_desc(const wcstring &name, const wcstring &desc, parser_t &parser) { @@ -278,34 +265,6 @@ wcstring_list_t function_get_names(int get_hidden) { return wcstring_list_t(names.begin(), names.end()); } -const wchar_t *function_get_definition_file(const wcstring &name) { - if (auto func = function_get_props(name)) { - return func->definition_file; - } - return nullptr; -} - -bool function_is_autoloaded(const wcstring &name) { - if (auto func = function_get_props(name)) { - return func->is_autoload; - } - return false; -} - -int function_get_definition_lineno(const wcstring &name) { - const auto props = function_set.acquire()->get_props(name); - if (!props) return -1; - // return one plus the number of newlines at offsets less than the start of our function's - // statement (which includes the header). - // TODO: merge with line_offset_of_character_at_offset? - auto source_range = props->func_node->try_source_range(); - assert(source_range && "Function has no source range"); - uint32_t func_start = source_range->start; - const wcstring &source = props->parsed_source->src; - assert(func_start <= source.size() && "function start out of bounds"); - return 1 + std::count(source.begin(), source.begin() + func_start, L'\n'); -} - void function_invalidate_path() { // Remove all autoloaded functions and update the autoload path. // Note we don't want to risk removal during iteration; we expect this to be called @@ -323,13 +282,10 @@ void function_invalidate_path() { funcset->autoloader.clear(); } -/// Return a definition of the specified function. Used by the functions builtin. -wcstring functions_def(const wcstring &name) { - assert(!name.empty() && "Empty name"); +wcstring function_properties_t::annotated_definition(const wcstring &name) const { wcstring out; - wcstring desc, def; - function_get_desc(name, desc); - function_get_definition(name, def); + wcstring desc = this->localized_description(); + wcstring def = get_function_body_source(*this); std::vector> ev = event_get_function_handlers(name); out.append(L"function "); @@ -353,9 +309,7 @@ wcstring functions_def(const wcstring &name) { out.append(esc_desc); } - auto props = function_get_props(name); - assert(props && "Should have function properties"); - if (!props->shadow_scope) { + if (!this->shadow_scope) { out.append(L" --no-scope-shadowing"); } @@ -393,7 +347,7 @@ wcstring functions_def(const wcstring &name) { } } - const wcstring_list_t &named = props->named_arguments; + const wcstring_list_t &named = this->named_arguments; if (!named.empty()) { append_format(out, L" --argument"); for (const auto &name : named) { @@ -408,7 +362,7 @@ wcstring functions_def(const wcstring &name) { } // Output any inherited variables as `set -l` lines. - for (const auto &kv : props->inherit_vars) { + for (const auto &kv : this->inherit_vars) { // We don't know what indentation style the function uses, // so we do what fish_indent would. append_format(out, L"\n set -l %ls", kv.first.c_str()); @@ -428,3 +382,20 @@ wcstring functions_def(const wcstring &name) { out.append(L"end\n"); return out; } + +const wchar_t *function_properties_t::localized_description() const { + if (description.empty()) return L""; + return _(description.c_str()); +} + +int function_properties_t::definition_lineno() const { + // return one plus the number of newlines at offsets less than the start of our function's + // statement (which includes the header). + // TODO: merge with line_offset_of_character_at_offset? + auto source_range = func_node->try_source_range(); + assert(source_range && "Function has no source range"); + uint32_t func_start = source_range->start; + const wcstring &source = parsed_source->src; + assert(func_start <= source.size() && "function start out of bounds"); + return 1 + std::count(source.begin(), source.begin() + func_start, L'\n'); +} diff --git a/src/function.h b/src/function.h index 63835debe..1b4199727 100644 --- a/src/function.h +++ b/src/function.h @@ -47,6 +47,17 @@ struct function_properties_t { /// The file from which the function was created (intern'd string), or nullptr if not from a /// file. const wchar_t *definition_file{}; + + /// \return the description, localized via _. + const wchar_t *localized_description() const; + + /// \return the line number where the definition of the specified function started. + int definition_lineno() const; + + /// \return a definition of the function, annotated with properties like event handlers and wrap + /// targets. This is to support the 'functions' builtin. + /// Note callers must provide the function name, since the function does not know its own name. + wcstring annotated_definition(const wcstring &name) const; }; using function_properties_ref_t = std::shared_ptr; @@ -60,27 +71,19 @@ void function_remove(const wcstring &name); /// \return the properties for a function, or nullptr if none. This does not trigger autoloading. function_properties_ref_t function_get_props(const wcstring &name); -/// Returns by reference the definition of the function with the name \c name. Returns true if -/// successful, false if no function with the given name exists. -/// This does not trigger autoloading. -bool function_get_definition(const wcstring &name, wcstring &out_definition); - -/// Returns by reference the description of the function with the name \c name. Returns true if the -/// function exists and has a nonempty description, false if it does not. -/// This does not trigger autoloading. -bool function_get_desc(const wcstring &name, wcstring &out_desc); +/// \return the properties for a function, or nullptr if none, perhaps triggering autoloading. +function_properties_ref_t function_get_props_autoload(const wcstring &name, parser_t &parser); /// Sets the description of the function with the name \c name. +/// This triggers autoloading. void function_set_desc(const wcstring &name, const wcstring &desc, parser_t &parser); -/// Returns true if the function with the name name exists. +/// Returns true if the function named \p cmd exists. /// This may autoload. -int function_exists(const wcstring &cmd, parser_t &parser); +bool function_exists(const wcstring &cmd, parser_t &parser); -/// Attempts to load a function if not yet loaded. This is used by the completion machinery. -void function_load(const wcstring &cmd, parser_t &parser); - -/// Returns true if the function with the name name exists, without triggering autoload. +/// Returns true if the function \p cmd either is loaded, or exists on disk in an autoload +/// directory. bool function_exists_no_autoload(const wcstring &cmd); /// Returns all function names. @@ -88,22 +91,6 @@ bool function_exists_no_autoload(const wcstring &cmd); /// \param get_hidden whether to include hidden functions, i.e. ones starting with an underscore. wcstring_list_t function_get_names(int get_hidden); -/// Returns true if the function was autoloaded. -bool function_is_autoloaded(const wcstring &name); - -/// Returns tha absolute path of the file where the specified function was defined. Returns 0 if the -/// file was defined on the commandline. -/// -/// This function does not autoload functions, it will only work on functions that have already been -/// defined. -/// -/// This returns an intern'd string. -const wchar_t *function_get_definition_file(const wcstring &name); - -/// Returns the linenumber where the definition of the specified function started. -/// This does not trigger autoloading. -int function_get_definition_lineno(const wcstring &name); - /// Creates a new function using the same definition as the specified function. Returns true if copy /// is successful. bool function_copy(const wcstring &name, const wcstring &new_name); @@ -111,5 +98,4 @@ bool function_copy(const wcstring &name, const wcstring &new_name); /// Observes that fish_function_path has changed. void function_invalidate_path(); -wcstring functions_def(const wcstring &name); #endif diff --git a/src/parse_util.h b/src/parse_util.h index 42396975a..dff4f06e1 100644 --- a/src/parse_util.h +++ b/src/parse_util.h @@ -78,7 +78,7 @@ void parse_util_token_extent(const wchar_t *buff, size_t cursor_pos, const wchar const wchar_t **tok_end, const wchar_t **prev_begin, const wchar_t **prev_end); -/// Get the linenumber at the specified character offset. +/// Get the line number at the specified character offset. int parse_util_lineno(const wcstring &str, size_t offset); /// Calculate the line number of the specified cursor position. diff --git a/src/parser.cpp b/src/parser.cpp index 83c7238a1..d61e6f00a 100644 --- a/src/parser.cpp +++ b/src/parser.cpp @@ -451,7 +451,8 @@ int parser_t::get_lineno() const { const wchar_t *parser_t::current_filename() const { for (const auto &b : block_list) { if (b.is_function_call()) { - return function_get_definition_file(b.function_name); + auto props = function_get_props(b.function_name); + return props ? props->definition_file : nullptr; } else if (b.type() == block_type_t::source) { return b.sourced_file; }