Stop removing functions and completions in autoload

autoloading has a "feature" where functions are removed in an LRU-fashion.
But there's hardly any benefit in removing autoloaded functions. Just stop
doing it.

src/autoload.cpp

@@ -35,16 +35,11 @@ file_access_attempt_t access_file(const wcstring &path, int mode) {
     return result;
 }
 
-autoload_t::autoload_t(wcstring env_var_name_var,
-                       command_removed_function_t cmd_removed_callback)
-    : env_var_name(std::move(env_var_name_var)), command_removed(cmd_removed_callback) {}
+autoload_t::autoload_t(wcstring env_var_name_var) : env_var_name(std::move(env_var_name_var)) {}
 
 void autoload_t::entry_was_evicted(wcstring key, autoload_function_t node) {
     // This should only ever happen on the main thread.
     ASSERT_IS_MAIN_THREAD();
-
-    // Tell ourselves that the command was removed if it was loaded.
-    if (node.is_loaded) this->command_removed(std::move(key));
 }
 
 int autoload_t::unload(const wcstring &cmd) { return this->evict_node(cmd); }
@@ -200,10 +195,8 @@ bool autoload_t::locate_file_and_maybe_load_it(const wcstring &cmd, bool really_
             // Generate the script source.
             script_source = L"source " + escape_string(path, ESCAPE_ALL);
 
-            // Remove any loaded command because we are going to reload it. Note that this
-            // will deadlock if command_removed calls back into us.
-            if (func && func->is_loaded) {
-                command_removed(cmd);
+            // Mark that our function is no longer a placeholder, because we will load it.
+            if (func) {
                 func->is_placeholder = false;
             }
 

src/autoload.h

@@ -55,9 +55,6 @@ class autoload_t : public lru_cache_t<autoload_t, autoload_function_t> {
     /// A table containing all the files that are currently being loaded.
     /// This is here to help prevent recursion.
     std::unordered_set<wcstring> is_loading_set;
-    // Function invoked when a command is removed
-    typedef void (*command_removed_function_t)(const wcstring &);
-    const command_removed_function_t command_removed;
 
     void remove_all_functions() { this->evict_all_nodes(); }
 
@@ -72,11 +69,10 @@ class autoload_t : public lru_cache_t<autoload_t, autoload_function_t> {
     void entry_was_evicted(wcstring key, autoload_function_t node);
 
     // Create an autoload_t for the given environment variable name.
-    autoload_t(wcstring env_var_name_var, command_removed_function_t callback);
+    explicit autoload_t(wcstring env_var_name_var);
 
     /// Autoload the specified file, if it exists in the specified path. Do not load it multiple
     /// times unless its timestamp changes or parse_util_unload is called.
-    /// Autoloading one file may unload another.
     /// @param cmd the filename to search for. The suffix '.fish' is always added to this name
     /// @param reload wheter to recheck file timestamps on already loaded files
     int load(const wcstring &cmd, bool reload);

src/builtin_complete.cpp

@@ -329,7 +329,8 @@ int builtin_complete(parser_t &parser, io_streams_t &streams, wchar_t **argv) {
             recursion_level++;
 
             std::vector<completion_t> comp;
-            complete(do_complete_param, &comp, COMPLETION_REQUEST_DEFAULT | COMPLETION_REQUEST_FUZZY_MATCH, parser.vars());
+            complete(do_complete_param, &comp,
+                     COMPLETION_REQUEST_DEFAULT | COMPLETION_REQUEST_FUZZY_MATCH, parser.vars());
 
             for (size_t i = 0; i < comp.size(); i++) {
                 const completion_t &next = comp.at(i);

src/complete.cpp

@@ -380,13 +380,8 @@ class completer_t {
     std::vector<completion_t> acquire_completions() { return std::move(completions); }
 };
 
-// Callback when an autoloaded completion is removed.
-static void autoloaded_completion_removed(const wcstring &cmd) {
-    complete_remove_all(cmd, false /* not a path */);
-}
-
 // Autoloader for completions
-static autoload_t completion_autoloader(L"fish_complete_path", autoloaded_completion_removed);
+static autoload_t completion_autoloader(L"fish_complete_path");
 
 /// Create a new completion entry.
 void append_completion(std::vector<completion_t> *completions, wcstring comp, wcstring desc,

src/function.cpp

@@ -62,15 +62,8 @@ static std::unordered_set<wcstring> function_tombstones;
 /// Lock for functions.
 static std::recursive_mutex functions_lock;
 
-static bool function_remove_ignore_autoload(const wcstring &name, bool tombstone = true);
-
-/// Callback when an autoloaded function is removed.
-void autoloaded_function_removed(const wcstring &cmd) {
-    function_remove_ignore_autoload(cmd, false);
-}
-
 // Function autoloader
-static autoload_t function_autoloader(L"fish_function_path", autoloaded_function_removed);
+static autoload_t function_autoloader(L"fish_function_path");
 
 /// Kludgy flag set by the load function in order to tell function_add that the function being
 /// defined is autoloaded. There should be a better way to do this...
@@ -211,7 +204,7 @@ int function_exists_no_autoload(const wcstring &cmd, const environment_t &vars)
            function_autoloader.can_load(cmd, vars);
 }
 
-static bool function_remove_ignore_autoload(const wcstring &name, bool tombstone) {
+static bool function_remove_ignore_autoload(const wcstring &name) {
     // Note: the lock may be held at this point, but is recursive.
     scoped_rlock locker(functions_lock);
 
@@ -220,8 +213,8 @@ static bool function_remove_ignore_autoload(const wcstring &name, bool tombstone
     // Not found.  Not erasing.
     if (iter == loaded_functions.end()) return false;
 
-    // Removing an auto-loaded function.  Prevent it from being auto-reloaded.
-    if (iter->second.is_autoload && tombstone) function_tombstones.insert(name);
+    // If we are removing an auto-loaded function, prevent it from being auto-reloaded.
+    if (iter->second.is_autoload) function_tombstones.insert(name);
 
     loaded_functions.erase(iter);
     event_remove_function_handlers(name);