From 96da62ea874e01cfaee8ac230ce177a8d5fa8115 Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Sat, 29 Sep 2018 00:22:24 -0400 Subject: [PATCH] Revert "Convert list of builtins from sorted array to unordered_set" This reverts commit d8a1928c243b35fd399a56d69ed526f8c2b64a0a. It is better to have builtins as static const data to avoid unnecessary initialization and allocation during startup. --- src/builtin.cpp | 50 ++++++++++++++++++++++++++++++------------------- src/builtin.h | 20 +++++--------------- 2 files changed, 36 insertions(+), 34 deletions(-) diff --git a/src/builtin.cpp b/src/builtin.cpp index d73867ee2..e6b9b422d 100644 --- a/src/builtin.cpp +++ b/src/builtin.cpp @@ -26,7 +26,6 @@ #include #include #include -#include #include "builtin.h" #include "builtin_argparse.h" @@ -76,6 +75,14 @@ #include "wgetopt.h" #include "wutil.h" // IWYU pragma: keep +bool builtin_data_t::operator<(const wcstring &other) const { + return wcscmp(this->name, other.c_str()) < 0; +} + +bool builtin_data_t::operator<(const builtin_data_t *other) const { + return wcscmp(this->name, other->name) < 0; +} + /// Counts the number of arguments in the specified null-terminated array int builtin_count_args(const wchar_t *const *argv) { int argc; @@ -394,10 +401,12 @@ int builtin_false(parser_t &parser, io_streams_t &streams, wchar_t **argv) { // END OF BUILTIN COMMANDS // Below are functions for handling the builtin commands. +// THESE MUST BE SORTED BY NAME! Completion lookup uses binary search. // Data about all the builtin commands in fish. // Functions that are bound to builtin_generic are handled directly by the parser. -static const std::unordered_set builtin_datas = { +// NOTE: These must be kept in sorted order! +static const builtin_data_t builtin_datas[] = { {L"[", &builtin_test, N_(L"Test a condition")}, {L"and", &builtin_generic, N_(L"Execute command if previous command suceeded")}, {L"argparse", &builtin_argparse, N_(L"Parse options in fish script")}, @@ -406,7 +415,8 @@ static const std::unordered_set builtin_datas = { {L"bind", &builtin_bind, N_(L"Handle fish key bindings")}, {L"block", &builtin_block, N_(L"Temporarily block delivery of events")}, {L"break", &builtin_break_continue, N_(L"Stop the innermost loop")}, - {L"breakpoint", &builtin_breakpoint, N_(L"Temporarily halt execution of a script and launch an interactive debug prompt")}, + {L"breakpoint", &builtin_breakpoint, + N_(L"Temporarily halt execution of a script and launch an interactive debug prompt")}, {L"builtin", &builtin_builtin, N_(L"Run a builtin command instead of a function")}, {L"case", &builtin_generic, N_(L"Conditionally execute a block of commands")}, {L"cd", &builtin_cd, N_(L"Change working directory")}, @@ -414,7 +424,8 @@ static const std::unordered_set builtin_datas = { {L"commandline", &builtin_commandline, N_(L"Set or get the commandline")}, {L"complete", &builtin_complete, N_(L"Edit command specific completions")}, {L"contains", &builtin_contains, N_(L"Search for a specified string in a list")}, - {L"continue", &builtin_break_continue, N_(L"Skip the rest of the current lap of the innermost loop")}, + {L"continue", &builtin_break_continue, + N_(L"Skip the rest of the current lap of the innermost loop")}, {L"count", &builtin_count, N_(L"Count the number of arguments")}, {L"disown", &builtin_disown, N_(L"Remove job from job list")}, {L"echo", &builtin_echo, N_(L"Print arguments")}, @@ -452,6 +463,8 @@ static const std::unordered_set builtin_datas = { {L"wait", &builtin_wait, N_(L"Wait for background processes completed")}, {L"while", &builtin_generic, N_(L"Perform a command multiple times")}}; +#define BUILTIN_COUNT (sizeof builtin_datas / sizeof *builtin_datas) + /// Look up a builtin_data_t for a specified builtin /// /// @param name @@ -461,19 +474,18 @@ static const std::unordered_set builtin_datas = { /// Pointer to a builtin_data_t /// static const builtin_data_t *builtin_lookup(const wcstring &name) { - auto search = builtin_data_t { name }; - auto result = builtin_datas.find(search); - if (result == builtin_datas.end()) { - return NULL; + const builtin_data_t *array_end = builtin_datas + BUILTIN_COUNT; + const builtin_data_t *found = std::lower_bound(builtin_datas, array_end, name); + if (found != array_end && name == found->name) { + return found; } - - return &*result; + return NULL; } /// Initialize builtin data. void builtin_init() { - for (auto &bi : builtin_datas) { - intern_static(bi.name.c_str()); + for (size_t i = 0; i < BUILTIN_COUNT; i++) { + intern_static(builtin_datas[i].name); } } @@ -518,9 +530,9 @@ int builtin_run(parser_t &parser, int job_pgid, wchar_t **argv, io_streams_t &st /// Returns a list of all builtin names. wcstring_list_t builtin_get_names() { wcstring_list_t result; - result.reserve(builtin_datas.size()); - for (auto &bi : builtin_datas) { - result.push_back(bi.name); + result.reserve(BUILTIN_COUNT); + for (size_t i = 0; i < BUILTIN_COUNT; i++) { + result.push_back(builtin_datas[i].name); } return result; } @@ -528,9 +540,9 @@ wcstring_list_t builtin_get_names() { /// Insert all builtin names into list. void builtin_get_names(std::vector *list) { assert(list != NULL); - list->reserve(list->size() + builtin_datas.size()); - for (auto &bi : builtin_datas) { - append_completion(list, bi.name); + list->reserve(list->size() + BUILTIN_COUNT); + for (size_t i = 0; i < BUILTIN_COUNT; i++) { + append_completion(list, builtin_datas[i].name); } } @@ -539,7 +551,7 @@ wcstring builtin_get_desc(const wcstring &name) { wcstring result; const builtin_data_t *builtin = builtin_lookup(name); if (builtin) { - result = _(builtin->desc.c_str()); + result = _(builtin->desc); } return result; } diff --git a/src/builtin.h b/src/builtin.h index ac64f2dc7..46c079d29 100644 --- a/src/builtin.h +++ b/src/builtin.h @@ -16,26 +16,16 @@ struct io_streams_t; /// Data structure to describe a builtin. struct builtin_data_t { // Name of the builtin. - const wcstring name; - // Function pointer to the builtin implementation. + const wchar_t *name; + // Function pointer tothe builtin implementation. int (*func)(parser_t &parser, io_streams_t &streams, wchar_t **argv); // Description of what the builtin does. - const wcstring desc; + const wchar_t *desc; - bool operator == (const builtin_data_t &other) const { - return name == other.name; - } + bool operator<(const wcstring &) const; + bool operator<(const builtin_data_t *) const; }; -namespace std { - template<> - struct hash { - std::size_t operator()(const builtin_data_t &data) const { - return std::hash()(data.name); - } - }; -} - /// The default prompt for the read command. #define DEFAULT_READ_PROMPT L"set_color green; echo -n read; set_color normal; echo -n \"> \""