From d3fa58d6213810b0ffd69051828ec96199c24ffd Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Sun, 3 Feb 2019 16:06:10 -0800 Subject: [PATCH] Cleanup common.h Remove a bunch of headers, simplify lots of code, migrate it into .cpp files. Debug build time improves by ~3 seconds on my Mac. --- configure.ac | 2 +- src/builtin_math.cpp | 1 + src/builtin_pwd.cpp | 2 + src/builtin_realpath.cpp | 1 + src/builtin_test.cpp | 2 + src/common.cpp | 105 +++++++++++++++++----------- src/common.h | 145 +++++++-------------------------------- src/complete.cpp | 27 ++++---- src/complete.h | 2 - src/env.cpp | 10 +-- src/exec.cpp | 2 +- src/expand.cpp | 2 +- src/fallback.h | 4 -- src/fish.cpp | 2 +- src/maybe.h | 1 - src/proc.cpp | 4 +- 16 files changed, 121 insertions(+), 191 deletions(-) diff --git a/configure.ac b/configure.ac index 82f859368..77be63258 100644 --- a/configure.ac +++ b/configure.ac @@ -283,7 +283,7 @@ fi # Check presense of various header files # -AC_CHECK_HEADERS([getopt.h termios.h sys/resource.h term.h ncurses/term.h ncurses.h ncurses/curses.h curses.h stropts.h siginfo.h sys/select.h sys/ioctl.h execinfo.h spawn.h sys/sysctl.h]) +AC_CHECK_HEADERS([getopt.h termios.h sys/resource.h term.h ncurses/term.h ncurses.h ncurses/curses.h curses.h stropts.h siginfo.h sys/select.h sys/ioctl.h execinfo.h spawn.h sys/sysctl.h xlocale.h]) if test x$local_gettext != xno; then AC_CHECK_HEADERS([libintl.h]) diff --git a/src/builtin_math.cpp b/src/builtin_math.cpp index cf51111d2..284e47a2b 100644 --- a/src/builtin_math.cpp +++ b/src/builtin_math.cpp @@ -3,6 +3,7 @@ #include #include +#include #include #include diff --git a/src/builtin_pwd.cpp b/src/builtin_pwd.cpp index f9d7fda9c..1dce15da3 100644 --- a/src/builtin_pwd.cpp +++ b/src/builtin_pwd.cpp @@ -1,6 +1,8 @@ // Implementation of the pwd builtin. #include "config.h" // IWYU pragma: keep +#include + #include "builtin.h" #include "builtin_pwd.h" #include "common.h" diff --git a/src/builtin_realpath.cpp b/src/builtin_realpath.cpp index b87fe905a..89a1dab56 100644 --- a/src/builtin_realpath.cpp +++ b/src/builtin_realpath.cpp @@ -3,6 +3,7 @@ #include #include +#include #include #include "builtin.h" diff --git a/src/builtin_test.cpp b/src/builtin_test.cpp index 85cd32d03..327b0ea79 100644 --- a/src/builtin_test.cpp +++ b/src/builtin_test.cpp @@ -5,10 +5,12 @@ #include #include +#include #include #include #include #include +#include #include #include diff --git a/src/common.cpp b/src/common.cpp index c66639000..7a88e569f 100644 --- a/src/common.cpp +++ b/src/common.cpp @@ -38,6 +38,7 @@ #include #include +#include #include // IWYU pragma: keep #include @@ -692,58 +693,54 @@ void debug_safe(int level, const char *msg, const char *param1, const char *para errno = errno_old; } -void format_long_safe(char buff[64], long val) { - if (val == 0) { - strcpy(buff, "0"); - } else { - // Generate the string in reverse. - size_t idx = 0; - bool negative = (val < 0); +// Careful to not negate LLONG_MIN. +static unsigned long long absolute_value(long long x) { + if (x >= 0) return static_cast(x); + x = -(x + 1); + return static_cast(x) + 1; +} - // Note that we can't just negate val if it's negative, because it may be the most negative - // value. We do rely on round-towards-zero division though. +template +void format_safe_impl(CharT *buff, size_t size, unsigned long long val) { + size_t idx = 0; + if (val == 0) { + buff[idx++] = '0'; + } else { + // Generate the string backwards, then reverse it. while (val != 0) { - long rem = val % 10; - buff[idx++] = '0' + (rem < 0 ? -rem : rem); + buff[idx++] = (val % 10) + '0'; val /= 10; } - if (negative) buff[idx++] = '-'; - buff[idx] = 0; + std::reverse(buff, buff + idx); + } + buff[idx++] = '\0'; + assert(idx <= size && "Buffer overflowed"); +} - size_t left = 0, right = idx - 1; - while (left < right) { - char tmp = buff[left]; - buff[left++] = buff[right]; - buff[right--] = tmp; - } +void format_long_safe(char buff[64], long val) { + unsigned long long uval = absolute_value(val); + if (val >= 0) { + format_safe_impl(buff, 64, uval); + } else { + buff[0] = '-'; + format_safe_impl(buff + 1, 63, uval); } } void format_long_safe(wchar_t buff[64], long val) { - if (val == 0) { - wcscpy(buff, L"0"); + unsigned long long uval = absolute_value(val); + if (val >= 0) { + format_safe_impl(buff, 64, uval); } else { - // Generate the string in reverse. - size_t idx = 0; - bool negative = (val < 0); - - while (val != 0) { - long rem = val % 10; - buff[idx++] = L'0' + (wchar_t)(rem < 0 ? -rem : rem); - val /= 10; - } - if (negative) buff[idx++] = L'-'; - buff[idx] = 0; - - size_t left = 0, right = idx - 1; - while (left < right) { - wchar_t tmp = buff[left]; - buff[left++] = buff[right]; - buff[right--] = tmp; - } + buff[0] = '-'; + format_safe_impl(buff + 1, 63, uval); } } +void format_ullong_safe(wchar_t buff[64], unsigned long long val) { + return format_safe_impl(buff, 64, val); +} + void narrow_string_safe(char buff[64], const wchar_t *s) { size_t idx = 0; for (size_t widx = 0; s[widx] != L'\0'; widx++) { @@ -1956,6 +1953,36 @@ int string_fuzzy_match_t::compare(const string_fuzzy_match_t &rhs) const { return 0; // equal } +template +size_t ifind_impl(const T &haystack, const T &needle) { + using char_t = typename T::value_type; + std::locale locale; + + auto ieq = [&locale](char_t c1, char_t c2) { + if (c1 == c2 || std::toupper(c1, locale) == std::toupper(c2, locale)) return true; + + // In fuzzy matching treat treat `-` and `_` as equal (#3584). + if (Fuzzy) { + if ((c1 == '-' || c1 == '_') && (c2 == '-' || c2 == '_')) return true; + } + return false; + }; + + auto result = std::search(haystack.begin(), haystack.end(), needle.begin(), needle.end(), ieq); + if (result != haystack.end()) { + return result - haystack.begin(); + } + return T::npos; +} + +size_t ifind(const wcstring &haystack, const wcstring &needle, bool fuzzy) { + return fuzzy ? ifind_impl(haystack, needle) : ifind_impl(haystack, needle); +} + +size_t ifind(const std::string &haystack, const std::string &needle, bool fuzzy) { + return fuzzy ? ifind_impl(haystack, needle) : ifind_impl(haystack, needle); +} + wcstring_list_t split_string(const wcstring &val, wchar_t sep) { wcstring_list_t out; size_t pos = 0, end = val.size(); diff --git a/src/common.h b/src/common.h index 5fa2119ba..8e94364be 100644 --- a/src/common.h +++ b/src/common.h @@ -5,33 +5,19 @@ #include #include -#include -#include // IWYU pragma: keep -#include -#include -#include -#include -#include -#include #ifdef HAVE_SYS_IOCTL_H #include // IWYU pragma: keep #endif #include #include -#include #include #include -#include #include -#include -#include -#include #include #include "fallback.h" // IWYU pragma: keep #include "maybe.h" -#include "signal.h" // IWYU pragma: keep // Define a symbol we can use elsewhere in our code to determine if we're being built on MS Windows // under Cygwin. @@ -361,61 +347,13 @@ bool string_suffixes_string_case_insensitive(const wcstring &proposed_suffix, bool string_prefixes_string_case_insensitive(const wcstring &proposed_prefix, const wcstring &value); -/// Case-insensitive string search, templated for use with both std::string and std::wstring. -/// Modeled after std::string::find(). +/// Case-insensitive string search, modeled after std::string::find(). /// \param fuzzy indicates this is being used for fuzzy matching and case insensitivity is /// expanded to include symbolic characters (#3584). /// \return the offset of the first case-insensitive matching instance of `needle` within /// `haystack`, or `string::npos()` if no results were found. -template -size_t ifind(const T &haystack, const T &needle, bool fuzzy = false) { - using char_t = typename T::value_type; - auto locale = std::locale(); - - std::function icase_eq; - - if (!fuzzy) { - icase_eq = [&locale](char_t c1, char_t c2) { - return std::toupper(c1, locale) == std::toupper(c2, locale); - }; - } else { - icase_eq = [&locale](char_t c1, char_t c2) { - // This `ifind()` call is being used for fuzzy string matching. Further extend case - // insensitivity to treat `-` and `_` as equal (#3584). - - // The two lines below were tested to be 27% faster than - // (c1 == '_' || c1 == '-') && (c2 == '-' || c2 == '_') - // while returning no false positives for all (c1, c2) combinations in the printable - // range (0x20-0x7E). It might return false positives outside that range, but fuzzy - // comparisons are typically called for file names only, which are unlikely to have - // such characters and this entire function is 100% broken on unicode so there's no - // point in worrying about anything outside of the ANSII range. - // ((c1 == Literal('_') || c1 == Literal('-')) && - // ((c1 ^ c2) == (Literal('-') ^ Literal('_')))); - - // One of the following would be an illegal comparison between a char and a wchar_t. - // However, placing them behind a constexpr gate results in the elision of the if - // statement and the incorrect branch, with the compiler's SFINAE support suppressing - // any errors in the branch not taken. - if (sizeof(char_t) == sizeof(char)) { - return std::toupper(c1, locale) == std::toupper(c2, locale) || - ((c1 == '_' || c1 == '-') && - ((c1 ^ c2) == ('-' ^ '_'))); - } else { - return std::toupper(c1, locale) == std::toupper(c2, locale) || - ((c1 == L'_' || c1 == L'-') && - ((c1 ^ c2) == (L'-' ^ L'_'))); - } - }; - } - - auto result = - std::search(haystack.begin(), haystack.end(), needle.begin(), needle.end(), icase_eq); - if (result != haystack.end()) { - return result - haystack.begin(); - } - return T::npos; -} +size_t ifind(const wcstring &haystack, const wcstring &needle, bool fuzzy = false); +size_t ifind(const std::string &haystack, const std::string &needle, bool fuzzy = false); /// Split a string by a separator character. wcstring_list_t split_string(const wcstring &val, wchar_t sep); @@ -565,55 +503,40 @@ void debug_safe(int level, const char *msg, const char *param1 = NULL, const cha /// Writes out a long safely. void format_long_safe(char buff[64], long val); void format_long_safe(wchar_t buff[64], long val); +void format_ullong_safe(wchar_t buff[64], unsigned long long val); /// "Narrows" a wide character string. This just grabs any ASCII characters and trunactes. void narrow_string_safe(char buff[64], const wchar_t *s); -template -T from_string(const wcstring &x) { - T result; - std::wstringstream stream(x); - stream >> result; - return result; -} - -template -T from_string(const std::string &x) { - T result = T(); - std::stringstream stream(x); - stream >> result; - return result; -} - -template -wcstring to_string(const T &x) { - std::wstringstream stream; - stream << x; - return stream.str(); -} - -// wstringstream is a huge memory pig. Let's provide some specializations where we can. -template <> -inline wcstring to_string(const long &x) { - wchar_t buff[128]; +inline wcstring to_string(long x) { + wchar_t buff[64]; format_long_safe(buff, x); return wcstring(buff); } -template <> -inline bool from_string(const std::string &x) { - return !x.empty() && strchr("YTyt1", x.at(0)); +inline wcstring to_string(int x) { return to_string(static_cast(x)); } + +inline wcstring to_string(size_t x) { + wchar_t buff[64]; + format_ullong_safe(buff, x); + return wcstring(buff); } -template <> -inline bool from_string(const wcstring &x) { - return !x.empty() && wcschr(L"YTyt1", x.at(0)); +inline bool bool_from_string(const std::string &x) { + if (x.empty()) return false; + switch (x.front()) { + case 'Y': + case 'T': + case 'y': + case 't': + case '1': + return true; + default: + return false; + } } -template <> -inline wcstring to_string(const int &x) { - return to_string(static_cast(x)); -} +inline bool bool_from_string(const wcstring &x) { return !x.empty() && wcschr(L"YTyt1", x.at(0)); } wchar_t **make_null_terminated_array(const wcstring_list_t &lst); char **make_null_terminated_array(const std::vector &lst); @@ -1022,22 +945,6 @@ static const wchar_t *enum_to_str(T enum_val, const enum_map map[]) { return NULL; }; -template -using tuple_list = std::vector>; - -// Given a container mapping one X to many Y, return a list of {X,Y} -template -inline tuple_list flatten(const std::unordered_map> &list) { - tuple_list results(list.size() * 1.5); // just a guess as to the initial size - for (auto &kv : list) { - for (auto &v : kv.second) { - results.emplace_back(std::make_tuple(kv.first, v)); - } - } - - return results; -} - void redirect_tty_output(); // Minimum allowed terminal size and default size if the detected size is not reasonable. @@ -1123,7 +1030,7 @@ struct cleanup_t { const std::function cleanup; public: cleanup_t(std::function exit_actions) - : cleanup{exit_actions} {} + : cleanup{std::move(exit_actions)} {} ~cleanup_t() { cleanup(); } diff --git a/src/complete.cpp b/src/complete.cpp index 8beaade7d..e9e00307e 100644 --- a/src/complete.cpp +++ b/src/complete.cpp @@ -171,6 +171,10 @@ struct equal_to { typedef std::unordered_set completion_entry_set_t; static owning_lock s_completion_set; +/// Completion "wrapper" support. The map goes from wrapping-command to wrapped-command-list. +using wrapper_map_t = std::unordered_map; +static owning_lock wrapper_map; + /// Comparison function to sort completions by their order field. static bool compare_completions_by_order(const completion_entry_t &p1, const completion_entry_t &p2) { @@ -1637,22 +1641,20 @@ wcstring complete_print() { } } - // Append wraps. This is a wonky interface where even values are the commands, and odd values - // are the targets that they wrap. - auto wrap_pairs = complete_get_wrap_pairs(); - for (const auto &entry : wrap_pairs) { - append_format(out, L"complete --command %ls --wraps %ls\n", std::get<0>(entry).c_str(), - std::get<1>(entry).c_str()); + // Append wraps. + auto locked_wrappers = wrapper_map.acquire(); + for (const auto &entry : *locked_wrappers) { + const wcstring &src = entry.first; + for (const wcstring &target : entry.second) { + append_format(out, L"complete --command %ls --wraps %ls\n", src.c_str(), + target.c_str()); + } } return out; } void complete_invalidate_path() { completion_autoloader.invalidate(); } -/// Completion "wrapper" support. The map goes from wrapping-command to wrapped-command-list. -using wrapper_map_t = std::unordered_map; -static owning_lock wrapper_map; - /// Add a new target that wraps a command. Example: __fish_XYZ (function) wraps XYZ (target). bool complete_add_wrapper(const wcstring &command, const wcstring &new_target) { if (command.empty() || new_target.empty()) { @@ -1704,8 +1706,3 @@ wcstring_list_t complete_get_wrap_targets(const wcstring &command) { if (iter == wraps.end()) return {}; return iter->second; } - -tuple_list complete_get_wrap_pairs() { - auto locked_map = wrapper_map.acquire(); - return flatten(*locked_map); -} diff --git a/src/complete.h b/src/complete.h index d73ae1d8c..3b1655f5d 100644 --- a/src/complete.h +++ b/src/complete.h @@ -204,8 +204,6 @@ bool complete_remove_wrapper(const wcstring &command, const wcstring &wrap_targe /// Returns a list of wrap targets for a given command. wcstring_list_t complete_get_wrap_targets(const wcstring &command); -tuple_list complete_get_wrap_pairs(); - // Observes that fish_complete_path has changed. void complete_invalidate_path(); diff --git a/src/env.cpp b/src/env.cpp index aee612cc1..56b6be201 100644 --- a/src/env.cpp +++ b/src/env.cpp @@ -469,7 +469,7 @@ static void update_fish_color_support(const environment_t &vars) { wcstring term = term_var.missing_or_empty() ? L"" : term_var->as_string(); bool support_term256 = false; // default to no support if (!fish_term256.missing_or_empty()) { - support_term256 = from_string(fish_term256->as_string()); + support_term256 = bool_from_string(fish_term256->as_string()); debug(2, L"256 color support determined by 'fish_term256'"); } else if (term.find(L"256color") != wcstring::npos) { // TERM=*256color*: Explicitly supported. @@ -501,7 +501,7 @@ static void update_fish_color_support(const environment_t &vars) { auto fish_term24bit = vars.get(L"fish_term24bit"); bool support_term24bit; if (!fish_term24bit.missing_or_empty()) { - support_term24bit = from_string(fish_term24bit->as_string()); + support_term24bit = bool_from_string(fish_term24bit->as_string()); debug(2, L"'fish_term24bit' preference: 24-bit color %s", support_term24bit ? L"enabled" : L"disabled"); } else { @@ -956,7 +956,7 @@ void env_init(const struct config_paths_t *paths /* or NULL */) { vars.set_one(L"FISH_VERSION", ENV_GLOBAL, version); // Set the $fish_pid variable. - vars.set_one(L"fish_pid", ENV_GLOBAL, to_string(getpid())); + vars.set_one(L"fish_pid", ENV_GLOBAL, to_string(getpid())); // Set the $hostname variable wcstring hostname = L"fish"; @@ -971,7 +971,7 @@ void env_init(const struct config_paths_t *paths /* or NULL */) { // TODO: Figure out how to handle invalid numbers better. Shouldn't we issue a diagnostic? long shlvl_i = fish_wcstol(str2wcstring(shlvl_var).c_str(), &end); if (!errno && shlvl_i >= 0) { - nshlvl_str = to_string(shlvl_i + 1); + nshlvl_str = to_string(shlvl_i + 1); } } vars.set_one(L"SHLVL", ENV_GLOBAL | ENV_EXPORT, nshlvl_str); @@ -1028,7 +1028,7 @@ void env_init(const struct config_paths_t *paths /* or NULL */) { // Set g_use_posix_spawn. Default to true. auto use_posix_spawn = vars.get(L"fish_use_posix_spawn"); g_use_posix_spawn = - use_posix_spawn.missing_or_empty() ? true : from_string(use_posix_spawn->as_string()); + use_posix_spawn.missing_or_empty() ? true : bool_from_string(use_posix_spawn->as_string()); // Set fish_bind_mode to "default". vars.set_one(FISH_BIND_MODE_VAR, ENV_GLOBAL, DEFAULT_BIND_MODE); diff --git a/src/exec.cpp b/src/exec.cpp index eb1a88259..63b10759c 100644 --- a/src/exec.cpp +++ b/src/exec.cpp @@ -335,7 +335,7 @@ void internal_exec(env_stack_t &vars, job_t *j, const io_chain_t &all_ios) { if (shlvl_var) { long shlvl = fish_wcstol(shlvl_var->as_string().c_str()); if (!errno && shlvl > 0) { - shlvl_str = to_string(shlvl - 1); + shlvl_str = to_string(shlvl - 1); } } vars.set_one(L"SHLVL", ENV_GLOBAL | ENV_EXPORT, std::move(shlvl_str)); diff --git a/src/expand.cpp b/src/expand.cpp index 665387499..77ed98c1b 100644 --- a/src/expand.cpp +++ b/src/expand.cpp @@ -787,7 +787,7 @@ static void expand_home_directory(wcstring &input, const environment_t &vars) { /// Expand the %self escape. Note this can only come at the beginning of the string. static void expand_percent_self(wcstring &input) { if (!input.empty() && input.front() == PROCESS_EXPAND_SELF) { - input.replace(0, 1, to_string(getpid())); + input.replace(0, 1, to_string(getpid())); } } diff --git a/src/fallback.h b/src/fallback.h index 62422f923..de6771057 100644 --- a/src/fallback.h +++ b/src/fallback.h @@ -3,10 +3,6 @@ #include "config.h" -// IWYU likes to recommend adding when we want . If we add it breaks -// compiling several modules that include this header because they use symbols which are defined as -// macros in . -// IWYU pragma: no_include #include #include #include diff --git a/src/fish.cpp b/src/fish.cpp index 59ff89f0e..9652f310f 100644 --- a/src/fish.cpp +++ b/src/fish.cpp @@ -442,7 +442,7 @@ int main(int argc, char **argv) { proc_fire_event(L"PROCESS_EXIT", EVENT_EXIT, getpid(), exit_status); // Trigger any exit handlers. - wcstring_list_t event_args = {to_string(exit_status)}; + wcstring_list_t event_args = {to_string(exit_status)}; event_fire_generic(L"fish_exit", &event_args); restore_term_mode(); diff --git a/src/maybe.h b/src/maybe.h index 3281dca02..7f3f96cde 100644 --- a/src/maybe.h +++ b/src/maybe.h @@ -2,7 +2,6 @@ #define FISH_MAYBE_H #include -#include // A none_t is a helper type used to implicitly initialize maybe_t. // Example usage: diff --git a/src/proc.cpp b/src/proc.cpp index a440d1cda..94b5358a6 100644 --- a/src/proc.cpp +++ b/src/proc.cpp @@ -625,8 +625,8 @@ void proc_fire_event(const wchar_t *msg, int type, pid_t pid, int status) { event.param1.pid = pid; event.arguments.push_back(msg); - event.arguments.push_back(to_string(pid)); - event.arguments.push_back(to_string(status)); + event.arguments.push_back(to_string(pid)); + event.arguments.push_back(to_string(status)); event_fire(&event); event.arguments.resize(0); }