From 1c5208cf5cbefdeb69d035c8048022903c3f3033 Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Thu, 21 Oct 2021 13:56:32 -0700 Subject: [PATCH] Migrate a function's description into its immutable properties No functional change here. --- src/builtin_function.cpp | 3 ++- src/fish_tests.cpp | 4 ++-- src/function.cpp | 30 +++++++++++++----------------- src/function.h | 6 ++++-- 4 files changed, 21 insertions(+), 22 deletions(-) diff --git a/src/builtin_function.cpp b/src/builtin_function.cpp index 4f3830862..dcdb0d201 100644 --- a/src/builtin_function.cpp +++ b/src/builtin_function.cpp @@ -271,6 +271,7 @@ maybe_t builtin_function(parser_t &parser, io_streams_t &streams, props->named_arguments = std::move(opts.named_arguments); props->parsed_source = source; props->func_node = &func_node; + props->description = opts.description; props->definition_file = parser.libdata().current_filename; // Populate inherit_vars. @@ -281,7 +282,7 @@ maybe_t builtin_function(parser_t &parser, io_streams_t &streams, } // Add the function itself. - function_add(function_name, opts.description, props); + function_add(function_name, props); // Handle wrap targets by creating the appropriate completions. for (const wcstring &wt : opts.wrap_targets) { diff --git a/src/fish_tests.cpp b/src/fish_tests.cpp index 2e087114c..17aad019a 100644 --- a/src/fish_tests.cpp +++ b/src/fish_tests.cpp @@ -3271,7 +3271,7 @@ static void test_complete() { #endif // Add a function and test completing it in various ways. - function_add(L"scuttlebutt", {}, func_props); + function_add(L"scuttlebutt", func_props); // Complete a function name. completions = do_complete(L"echo (scuttlebut", {}); @@ -3361,7 +3361,7 @@ static void test_complete() { // Test abbreviations. auto &pvars = parser_t::principal_parser().vars(); - function_add(L"testabbrsonetwothreefour", {}, func_props); + function_add(L"testabbrsonetwothreefour", func_props); int ret = pvars.set_one(L"_fish_abbr_testabbrsonetwothreezero", ENV_LOCAL, L"expansion"); completions = complete(L"testabbrsonetwothree", {}, parser->context()); do_test(ret == 0); diff --git a/src/function.cpp b/src/function.cpp index f04b0a644..b2ff93e9a 100644 --- a/src/function.cpp +++ b/src/function.cpp @@ -38,10 +38,7 @@ class function_info_t { public: /// Immutable properties of the function. function_properties_ref_t props; - /// Function description. This may be changed after the function is created. - wcstring description; - - function_info_t(function_properties_ref_t props, wcstring desc); + explicit function_info_t(function_properties_ref_t props); }; /// Type wrapping up the set of all functions. @@ -145,11 +142,9 @@ static void autoload_names(std::unordered_set &names, int get_hidden) } } -function_info_t::function_info_t(function_properties_ref_t props, wcstring desc) - : props(std::move(props)), description(std::move(desc)) {} +function_info_t::function_info_t(function_properties_ref_t props) : props(std::move(props)) {} -void function_add(wcstring name, wcstring description, - std::shared_ptr props) { +void function_add(wcstring name, std::shared_ptr props) { ASSERT_IS_MAIN_THREAD(); assert(props && "Null props"); auto funcset = function_set.acquire(); @@ -166,13 +161,12 @@ void function_add(wcstring name, wcstring description, props->is_autoload = funcset->autoloader.autoload_in_progress(name); // Create and store a new function. - auto ins = funcset->funcs.emplace(std::move(name), - function_info_t(std::move(props), std::move(description))); + auto ins = funcset->funcs.emplace(std::move(name), function_info_t(std::move(props))); assert(ins.second && "Function should not already be present in the table"); (void)ins; } -std::shared_ptr function_get_properties(const wcstring &name) { +function_properties_ref_t function_get_properties(const wcstring &name) { if (parser_keywords_is_reserved(name)) return nullptr; auto funcset = function_set.acquire(); if (auto info = funcset->get_info(name)) { @@ -240,10 +234,8 @@ bool function_get_definition(const wcstring &name, wcstring &out_definition) { } bool function_get_desc(const wcstring &name, wcstring &out_desc) { - const auto funcset = function_set.acquire(); - const function_info_t *func = funcset->get_info(name); - if (func && !func->description.empty()) { - out_desc = _(func->description.c_str()); + if (auto props = function_get_properties(name)) { + out_desc = _(props->description.c_str()); return true; } return false; @@ -255,7 +247,11 @@ void function_set_desc(const wcstring &name, const wcstring &desc, parser_t &par auto funcset = function_set.acquire(); auto iter = funcset->funcs.find(name); if (iter != funcset->funcs.end()) { - iter->second.description = desc; + // Note the description is immutable, as it may be accessed on another thread, so we copy + // the properties to modify it. + auto new_props = std::make_shared(*iter->second.props); + new_props->description = desc; + iter->second.props = new_props; } } @@ -277,7 +273,7 @@ bool function_copy(const wcstring &name, const wcstring &new_name) { // Note this will NOT overwrite an existing function with the new name. // TODO: rationalize if this behavior is desired. - funcset->funcs.emplace(new_name, function_info_t(new_props, src_func.description)); + funcset->funcs.emplace(new_name, function_info_t(new_props)); return true; } diff --git a/src/function.h b/src/function.h index 916e64540..9fb341e19 100644 --- a/src/function.h +++ b/src/function.h @@ -31,6 +31,9 @@ struct function_properties_t { /// List of all named arguments for this function. wcstring_list_t named_arguments; + /// Description of the function. + wcstring description; + /// Mapping of all variables that were inherited from the function definition scope to their /// values. std::map inherit_vars; @@ -49,8 +52,7 @@ struct function_properties_t { using function_properties_ref_t = std::shared_ptr; /// Add a function. This may mutate \p props to set is_autoload. -void function_add(wcstring name, wcstring description, - std::shared_ptr props); +void function_add(wcstring name, std::shared_ptr props); /// Remove the function with the specified name. void function_remove(const wcstring &name);