Eliminate some mutable global variables

Make them const or otherwise get rid of them

build_tools/find_globals.fish

@@ -5,13 +5,24 @@
 # This was written for macOS nm.
 
 set total_globals 0
+set boring_files fish_key_reader.cpp.o
+set whitelist \
+    termsize_lock termsize \
+    initial_fg_process_group \
+    _debug_level \
+    sitm_esc ritm_esc dim_esc \
+    s_main_thread_performer_lock s_main_thread_performer_cond s_main_thread_request_q_lock
 
 for file in ./**.o
   set filename (basename $file)
+  # Skip boring files.
+  contains $filename $boring_files ; and continue
   for line in (nm -p -P -U $file)
     # Look in data (dD) and bss (bB) segments.
     set matches (string match --regex '^([^ ]+) ([dDbB])' -- $line) ; or continue
-    echo $filename (echo $matches[2] | c++filt) $matches[3]
+    set symname (echo $matches[2] | c++filt)
+    contains $symname $whitelist ; and continue
+    echo $filename $symname $matches[3]
     set total_globals (math $total_globals + 1)
   end
 end

src/builtin_set_color.cpp

@@ -62,9 +62,9 @@ static const struct woption long_options[] = {{L"background", required_argument,
                                               {NULL, 0, NULL, 0}};
 
 #if __APPLE__
-char sitm_esc[] = "\x1B[3m";
-char ritm_esc[] = "\x1B[23m";
-char dim_esc[] = "\x1B[2m";
+static char sitm_esc[] = "\x1B[3m";
+static char ritm_esc[] = "\x1B[23m";
+static char dim_esc[] = "\x1B[2m";
 #endif
 
 /// set_color builtin.

src/common.cpp

@@ -64,6 +64,7 @@ int debug_stack_frames = 0;  // default number of stack frames to show on debug(
 static pid_t initial_pid = 0;
 
 /// Be able to restore the term's foreground process group.
+/// This is set during startup and not modified after.
 static pid_t initial_fg_process_group = -1;
 
 /// This struct maintains the current state of the terminal size. It is updated on demand after
@@ -76,23 +77,34 @@ static volatile bool termsize_valid = false;
 static char *wcs2str_internal(const wchar_t *in, char *out);
 static void debug_shared(const wchar_t msg_level, const wcstring &msg);
 
-const wcstring whitespace = L" \t\r\n\v";
-const char *whitespace_narrow = " \t\r\n\v";
-
-bool is_whitespace(const wchar_t input) {
-    for (auto c : whitespace) {
-        if (c == input) {
+bool is_whitespace(wchar_t c) {
+    switch (c) {
+        case ' ':
+        case '\t':
+        case '\r':
+        case '\n':
+        case '\v':
             return true;
-        }
-    }
+        default:
             return false;
     }
+}
 
 bool is_whitespace(const wcstring &input) {
-    return input.find_first_not_of(whitespace) == wcstring::npos;
+    bool (*pred)(wchar_t c) = is_whitespace;
+    return std::all_of(input.begin(), input.end(), pred);
 }
 
-bool has_working_tty_timestamps = true;
+#if defined(OS_IS_CYGWIN) || defined(WSL)
+// MS Windows tty devices do not currently have either a read or write timestamp. Those
+// respective fields of `struct stat` are always the current time. Which means we can't
+// use them. So we assume no external program has written to the terminal behind our
+// back. This makes multiline promptusable. See issue #2859 and
+// https://github.com/Microsoft/BashOnWindows/issues/545
+const bool has_working_tty_timestamps = false;
+#else
+const bool has_working_tty_timestamps = true;
+#endif
 
 /// Convert a character to its integer equivalent if it is a valid character for the requested base.
 /// Return the integer value if it is valid else -1.
@@ -598,7 +610,7 @@ static void debug_shared(const wchar_t level, const wcstring &msg) {
     }
 }
 
-static wchar_t level_char[] = {L'E', L'W', L'2', L'3', L'4', L'5'};
+static const wchar_t level_char[] = {L'E', L'W', L'2', L'3', L'4', L'5'};
 void __attribute__((noinline)) debug_impl(int level, const wchar_t *msg, ...) {
     int errno_old = errno;
     va_list va;
@@ -2065,7 +2077,10 @@ void setup_fork_guards() {
     initial_pid = getpid();
 }
 
-void save_term_foreground_process_group() { initial_fg_process_group = tcgetpgrp(STDIN_FILENO); }
+void save_term_foreground_process_group() {
+    ASSERT_IS_MAIN_THREAD();
+    initial_fg_process_group = tcgetpgrp(STDIN_FILENO);
+}
 
 void restore_term_foreground_process_group() {
     if (initial_fg_process_group == -1) return;

src/common.h

@@ -206,9 +206,8 @@ extern bool g_profiling_active;
 /// Name of the current program. Should be set at startup. Used by the debug function.
 extern const wchar_t *program_name;
 
-/// Set to false at run-time if it's been determined we can't trust the last modified timestamp on
-/// the tty.
-extern bool has_working_tty_timestamps;
+/// Set to false if it's been determined we can't trust the last modified timestamp on the tty.
+extern const bool has_working_tty_timestamps;
 
 /// A list of all whitespace characters
 extern const wcstring whitespace;

src/complete.cpp

@@ -1649,18 +1649,8 @@ wcstring complete_print() {
 void complete_invalidate_path() { completion_autoloader.invalidate(); }
 
 /// Completion "wrapper" support. The map goes from wrapping-command to wrapped-command-list.
-static fish_mutex_t wrapper_lock;
-typedef std::unordered_map<wcstring, wcstring_list_t> wrapper_map_t;
-static wrapper_map_t &wrap_map() {
-    ASSERT_IS_LOCKED(wrapper_lock);
-    // A pointer is a little more efficient than an object as a static because we can elide the
-    // thread-safe initialization.
-    static wrapper_map_t *wrapper_map = NULL;
-    if (wrapper_map == NULL) {
-        wrapper_map = new wrapper_map_t();
-    }
-    return *wrapper_map;
-}
+using wrapper_map_t = std::unordered_map<wcstring, wcstring_list_t>;
+static owning_lock<wrapper_map_t> 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) {
@@ -1668,8 +1658,8 @@ bool complete_add_wrapper(const wcstring &command, const wcstring &new_target) {
         return false;
     }
 
-    scoped_lock locker(wrapper_lock);
-    wrapper_map_t &wraps = wrap_map();
+    auto locked_map = wrapper_map.acquire();
+    wrapper_map_t &wraps = *locked_map;
     wcstring_list_t *targets = &wraps[command];
     // If it's already present, we do nothing.
     if (!contains(*targets, new_target)) {
@@ -1683,8 +1673,8 @@ bool complete_remove_wrapper(const wcstring &command, const wcstring &target_to_
         return false;
     }
 
-    scoped_lock locker(wrapper_lock);
-    wrapper_map_t &wraps = wrap_map();
+    auto locked_map = wrapper_map.acquire();
+    wrapper_map_t &wraps = *locked_map;
     bool result = false;
     wrapper_map_t::iterator current_targets_iter = wraps.find(command);
     if (current_targets_iter != wraps.end()) {
@@ -1702,14 +1692,14 @@ wcstring_list_t complete_get_wrap_targets(const wcstring &command) {
     if (command.empty()) {
         return {};
     }
-    scoped_lock locker(wrapper_lock);
-    const wrapper_map_t &wraps = wrap_map();
+    auto locked_map = wrapper_map.acquire();
+    wrapper_map_t &wraps = *locked_map;
     auto iter = wraps.find(command);
     if (iter == wraps.end()) return {};
     return iter->second;
 }
 
 tuple_list<wcstring, wcstring> complete_get_wrap_pairs() {
-    scoped_lock locker(wrapper_lock);
-    return flatten(wrap_map());
+    auto locked_map = wrapper_map.acquire();
+    return flatten(*locked_map);
 }

src/env.cpp

@@ -736,15 +736,6 @@ void misc_init() {
         fflush(stdout);
         setvbuf(stdout, NULL, _IONBF, 0);
     }
-
-#if defined(OS_IS_CYGWIN) || defined(WSL)
-    // MS Windows tty devices do not currently have either a read or write timestamp. Those
-    // respective fields of `struct stat` are always the current time. Which means we can't
-    // use them. So we assume no external program has written to the terminal behind our
-    // back. This makes multiline promptusable. See issue #2859 and
-    // https://github.com/Microsoft/BashOnWindows/issues/545
-    has_working_tty_timestamps = false;
-#endif
 }
 
 static void env_universal_callbacks(callback_data_list_t &callbacks) {

src/fish.cpp

@@ -241,7 +241,7 @@ int run_command_list(std::vector<std::string> *cmds, const io_chain_t &io) {
 
 /// Parse the argument list, return the index of the first non-flag arguments.
 static int fish_parse_opt(int argc, char **argv, fish_cmd_opts_t *opts) {
-    static const char *short_opts = "+hilnvc:C:p:d:f:D:";
+    static const char * const short_opts = "+hilnvc:C:p:d:f:D:";
     static const struct option long_opts[] = {{"command", required_argument, NULL, 'c'},
                                               {"init-command", required_argument, NULL, 'C'},
                                               {"features", required_argument, NULL, 'f'},
@@ -361,8 +361,8 @@ int main(int argc, char **argv) {
     // struct stat tmp;
     // stat("----------FISH_HIT_MAIN----------", &tmp);
 
+    const char *dummy_argv[2] = {"fish", NULL};
     if (!argv[0]) {
-        static const char *dummy_argv[2] = {"fish", NULL};
         argv = (char **)dummy_argv;  //!OCLINT(parameter reassignment)
         argc = 1;                    //!OCLINT(parameter reassignment)
     }

src/fish_version.cpp

@@ -8,7 +8,7 @@
 // The contents of FISH-BUILD-VERSION-FILE looks like:
 // FISH_BUILD_VERSION="2.7.1-62-gc0480092-dirty"
 // Arrange for it to become a variable.
-static const char *
+static const char * const
 #include "FISH-BUILD-VERSION-FILE"
     ;
 #endif