From db33ed0fc757bfcf7b2fffc0b4dda049d5a397c3 Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Sun, 11 Feb 2018 16:37:38 -0800 Subject: [PATCH] Migrate some function properties into a shared_ptr The idea is that we can return the shared pointer directly, avoiding lots of annoying little getter functions that each need to take locks. It also helps to pull together the data structures used to initialize functions versus store them. --- src/builtin_function.cpp | 17 ++++------ src/function.cpp | 71 ++++++++++++++++------------------------ src/function.h | 29 ++++++++++------ 3 files changed, 55 insertions(+), 62 deletions(-) diff --git a/src/builtin_function.cpp b/src/builtin_function.cpp index e865c2a4c..ca66b301c 100644 --- a/src/builtin_function.cpp +++ b/src/builtin_function.cpp @@ -249,23 +249,20 @@ int builtin_function(parser_t &parser, io_streams_t &streams, const wcstring_lis if (!opts.description.empty()) d.description = opts.description; // d.description = opts.description; d.events.swap(opts.events); - d.shadow_scope = opts.shadow_scope; - d.named_arguments.swap(opts.named_arguments); - d.inherit_vars.swap(opts.inherit_vars); + d.props.shadow_scope = opts.shadow_scope; + d.props.named_arguments = std::move(opts.named_arguments); + d.inherit_vars = std::move(opts.inherit_vars); for (size_t i = 0; i < d.events.size(); i++) { event_t &e = d.events.at(i); e.function_name = d.name; } - d.parsed_source = source; - d.body_node = body; - function_add(d, parser); + d.props.parsed_source = source; + d.props.body_node = body; + function_add(std::move(d), parser); // Handle wrap targets by creating the appropriate completions. - for (size_t w = 0; w < opts.wrap_targets.size(); w++) { - complete_add_wrapper(function_name, opts.wrap_targets.at(w)); - } - + for (const wcstring &wt : opts.wrap_targets) complete_add_wrapper(function_name, wt); return STATUS_CMD_OK; } diff --git a/src/function.cpp b/src/function.cpp index bd0d4b107..a39a34871 100644 --- a/src/function.cpp +++ b/src/function.cpp @@ -31,29 +31,23 @@ class function_info_t { public: - /// Parsed source containing the function. - const parsed_source_ref_t parsed_source; - /// Node containing the function body, pointing into parsed_source. - const tnode_t body_node; - /// Function description. Only the description may be changed after the function is created. - wcstring description; - /// File where this function was defined (intern'd string). - const wchar_t *const definition_file; - /// List of all named arguments for this function. - const wcstring_list_t named_arguments; - /// Mapping of all variables that were inherited from the function definition scope to their - /// values. - const std::map inherit_vars; - /// Flag for specifying that this function was automatically loaded. - const bool is_autoload; - /// Set to true if invoking this function shadows the variables of the underlying function. - const bool shadow_scope; + /// Immutable properties of the function. + std::shared_ptr props; + /// Function description. This may be changed after the function is created. + wcstring description; + /// File where this function was defined (intern'd string). + const wchar_t *const definition_file; + /// Mapping of all variables that were inherited from the function definition scope to their + /// values. + const std::map inherit_vars; + /// Flag for specifying that this function was automatically loaded. + const bool is_autoload; - /// Constructs relevant information from the function_data. - function_info_t(const function_data_t &data, const wchar_t *filename, bool autoload); + /// Constructs relevant information from the function_data. + function_info_t(function_data_t data, const wchar_t *filename, bool autoload); - /// Used by function_copy. - function_info_t(const function_info_t &data, const wchar_t *filename, bool autoload); + /// Used by function_copy. + function_info_t(const function_info_t &data, const wchar_t *filename, bool autoload); }; @@ -138,34 +132,27 @@ static void autoload_names(std::unordered_set &names, int get_hidden) static std::map snapshot_vars(const wcstring_list_t &vars) { std::map result; - for (wcstring_list_t::const_iterator it = vars.begin(), end = vars.end(); it != end; ++it) { - auto var = env_get(*it); - if (var) result.insert(std::make_pair(*it, std::move(*var))); + for (const wcstring &name : vars) { + auto var = env_get(name); + if (var) result[name] = std::move(*var); } return result; } -function_info_t::function_info_t(const function_data_t &data, const wchar_t *filename, - bool autoload) - : parsed_source(data.parsed_source), - body_node(data.body_node), - description(data.description), +function_info_t::function_info_t(function_data_t data, const wchar_t *filename, bool autoload) + : props(std::make_shared(std::move(data.props))), + description(std::move(data.description)), definition_file(intern(filename)), - named_arguments(data.named_arguments), inherit_vars(snapshot_vars(data.inherit_vars)), - is_autoload(autoload), - shadow_scope(data.shadow_scope) {} + is_autoload(autoload) {} function_info_t::function_info_t(const function_info_t &data, const wchar_t *filename, bool autoload) - : parsed_source(data.parsed_source), - body_node(data.body_node), + : props(data.props), description(data.description), definition_file(intern(filename)), - named_arguments(data.named_arguments), inherit_vars(data.inherit_vars), - is_autoload(autoload), - shadow_scope(data.shadow_scope) {} + is_autoload(autoload) {} void function_add(const function_data_t &data, const parser_t &parser) { UNUSED(parser); @@ -250,7 +237,7 @@ bool function_get_definition(const wcstring &name, wcstring *out_definition) { scoped_rlock locker(functions_lock); const function_info_t *func = function_get(name); if (func && out_definition) { - out_definition->assign(func->body_node.get_source(func->parsed_source->src)); + out_definition->assign(func->props->body_node.get_source(func->props->parsed_source->src)); } return func != NULL; } @@ -258,7 +245,7 @@ bool function_get_definition(const wcstring &name, wcstring *out_definition) { wcstring_list_t function_get_named_arguments(const wcstring &name) { scoped_rlock locker(functions_lock); const function_info_t *func = function_get(name); - return func ? func->named_arguments : wcstring_list_t(); + return func ? func->props->named_arguments : wcstring_list_t(); } std::map function_get_inherit_vars(const wcstring &name) { @@ -270,7 +257,7 @@ std::map function_get_inherit_vars(const wcstring &name) { bool function_get_shadow_scope(const wcstring &name) { scoped_rlock locker(functions_lock); const function_info_t *func = function_get(name); - return func ? func->shadow_scope : false; + return func ? func->props->shadow_scope : false; } bool function_get_desc(const wcstring &name, wcstring *out_desc) { @@ -344,12 +331,12 @@ int function_get_definition_lineno(const wcstring &name) { if (!func) 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 block_stat = func->body_node.try_get_parent(); + auto block_stat = func->props->body_node.try_get_parent(); assert(block_stat && "Function body is not part of block statement"); auto source_range = block_stat.source_range(); assert(source_range && "Function has no source range"); uint32_t func_start = source_range->start; - const wcstring &source = func->parsed_source->src; + const wcstring &source = func->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'); } diff --git a/src/function.h b/src/function.h index 637faaa45..ed796df50 100644 --- a/src/function.h +++ b/src/function.h @@ -15,6 +15,21 @@ class parser_t; +/// A function's constant properties. These do not change once initialized. +struct function_properties_t { + /// Parsed source containing the function. + parsed_source_ref_t parsed_source; + + /// Node containing the function body, pointing into parsed_source. + tnode_t body_node; + + /// List of all named arguments for this function. + wcstring_list_t named_arguments; + + /// Set to true if invoking this function shadows the variables of the underlying function. + bool shadow_scope; +}; + /// Structure describing a function. This is used by the parser to store data on a function while /// parsing it. It is not used internally to store functions, the function_info_t structure /// is used for that purpose. Parhaps this should be merged with function_info_t. @@ -23,19 +38,13 @@ struct function_data_t { wcstring name; /// Description of function. wcstring description; - /// Parsed source containing the function. - parsed_source_ref_t parsed_source; - /// Node containing the function body, pointing into parsed_source. - tnode_t body_node; - /// List of all event handlers for this function. - std::vector events; - /// List of all named arguments for this function. - wcstring_list_t named_arguments; /// List of all variables that are inherited from the function definition scope. The variable /// values are snapshotted when function_add() is called. wcstring_list_t inherit_vars; - /// Set to true if invoking this function shadows the variables of the underlying function. - bool shadow_scope; + /// Function's metadata fields + function_properties_t props; + /// List of all event handlers for this function. + std::vector events; }; /// Add a function.