Merge branch 'completion_move'

Some awkward optimizations that improve the time spent in a particular
expansion-heavy benchmark by ~30%.

src/complete.cpp

@@ -381,8 +381,9 @@ static autoload_t completion_autoloader(L"fish_complete_path", autoloaded_comple
 /// Create a new completion entry.
 void append_completion(std::vector<completion_t> *completions, wcstring comp, wcstring desc,
                        complete_flags_t flags, string_fuzzy_match_t match) {
-    completions->emplace_back(std::move(comp), std::move(desc), match,
+    complete_flags_t resolved_flags = resolve_auto_space(comp, flags);
-                              resolve_auto_space(comp, flags));
+    completion_t completion{std::move(comp), std::move(desc), match, resolved_flags};
+    completions->push_back(std::move(completion));
 }
 
 /// Test if the specified script returns zero. The result is cached, so that if multiple completions
@@ -677,9 +678,9 @@ void completer_t::complete_cmd(const wcstring &str_cmd, bool use_function, bool
     if (str_cmd.find(L'/') == wcstring::npos && str_cmd.at(0) != L'~') {
         if (use_function) {
             wcstring_list_t names = function_get_names(str_cmd.at(0) == L'_');
-            for (size_t i = 0; i < names.size(); i++) {
+            for (wcstring &name : names) {
                 // Append all known matching functions
-                append_completion(&possible_comp, names.at(i));
+                append_completion(&possible_comp, std::move(name));
             }
 
             this->complete_strings(str_cmd, 0, &complete_function_desc, possible_comp, 0);
@@ -992,9 +993,9 @@ bool completer_t::complete_param(const wcstring &scmd_orig, const wcstring &spop
             // Check if the short style option matches.
             if (short_ok(str, &o, options)) {
                 // It's a match.
-                const wcstring desc = o.localized_desc();
+                wcstring desc = o.localized_desc();
                 // Append a short-style option
-                append_completion(&this->completions, o.option, desc, 0);
+                append_completion(&this->completions, o.option, std::move(desc), 0);
             }
 
             // Check if the long style option matches.
@@ -1035,11 +1036,11 @@ bool completer_t::complete_param(const wcstring &scmd_orig, const wcstring &spop
                 // functions.
                 wcstring completion = format_string(L"%ls=", whole_opt.c_str() + offset);
                 // Append a long-style option with a mandatory trailing equal sign
-                append_completion(&this->completions, completion, C_(o.desc), flags);
+                append_completion(&this->completions, std::move(completion), C_(o.desc), flags);
             }
 
             // Append a long-style option
-            append_completion(&this->completions, whole_opt.c_str() + offset, C_(o.desc), flags);
+            append_completion(&this->completions, whole_opt.substr(offset), C_(o.desc), flags);
         }
     }
 
@@ -1147,7 +1148,7 @@ bool completer_t::complete_variable(const wcstring &str, size_t start_offset) {
         }
 
         // Append matching environment variables
-        append_completion(&this->completions, comp, desc, flags, match);
+        append_completion(&this->completions, std::move(comp), desc, flags, match);
 
         res = true;
     }

src/complete.h

@@ -78,8 +78,9 @@ class completion_t {
     completion_t(const completion_t &);
     completion_t &operator=(const completion_t &);
 
-    completion_t(completion_t &&);
+    // noexcepts are required for push_back to use the move ctor.
-    completion_t &operator=(completion_t &&);
+    completion_t(completion_t &&) noexcept;
+    completion_t &operator=(completion_t &&) noexcept;
 
     // Compare two completions. No operating overlaoding to make this always explicit (there's
     // potentially multiple ways to compare completions).

src/expand.cpp

@@ -290,14 +290,14 @@ static size_t parse_slice(const wchar_t *in, wchar_t **end_ptr, std::vector<long
 /// Note: last_idx is considered to be where it previously finished procesisng. This means it
 /// actually starts operating on last_idx-1. As such, to process a string fully, pass string.size()
 /// as last_idx instead of string.size()-1.
-static bool expand_variables(const wcstring &instr, std::vector<completion_t> *out, size_t last_idx,
+static bool expand_variables(wcstring instr, std::vector<completion_t> *out, size_t last_idx,
                              parse_error_list_t *errors) {
     const size_t insize = instr.size();
 
     // last_idx may be 1 past the end of the string, but no further.
     assert(last_idx <= insize && "Invalid last_idx");
     if (last_idx == 0) {
-        append_completion(out, instr);
+        append_completion(out, std::move(instr));
         return true;
     }
 
@@ -313,7 +313,7 @@ static bool expand_variables(const wcstring &instr, std::vector<completion_t> *o
     }
     if (varexp_char_idx >= instr.size()) {
         // No variable expand char, we're done.
-        append_completion(out, instr);
+        append_completion(out, std::move(instr));
         return true;
     }
 
@@ -405,7 +405,7 @@ static bool expand_variables(const wcstring &instr, std::vector<completion_t> *o
                 res.push_back(VARIABLE_EXPAND_EMPTY);
             }
             res.append(instr, var_name_and_slice_stop, wcstring::npos);
-            return expand_variables(res, out, varexp_char_idx, errors);
+            return expand_variables(std::move(res), out, varexp_char_idx, errors);
         }
     }
 
@@ -466,7 +466,7 @@ static bool expand_variables(const wcstring &instr, std::vector<completion_t> *o
             res.pop_back();
         }
         res.append(instr, var_name_and_slice_stop, wcstring::npos);
-        return expand_variables(res, out, varexp_char_idx, errors);
+        return expand_variables(std::move(res), out, varexp_char_idx, errors);
     } else {
         // Normal cartesian-product expansion.
         for (const wcstring &item : var_item_list) {
@@ -483,7 +483,7 @@ static bool expand_variables(const wcstring &instr, std::vector<completion_t> *o
                 }
                 new_in.append(item);
                 new_in.append(instr, var_name_and_slice_stop, wcstring::npos);
-                if (!expand_variables(new_in, out, varexp_char_idx, errors)) {
+                if (!expand_variables(std::move(new_in), out, varexp_char_idx, errors)) {
                     return false;
                 }
             }
@@ -881,31 +881,31 @@ static void remove_internal_separator(wcstring *str, bool conv) {
 /// A stage in string expansion is represented as a function that takes an input and returns a list
 /// of output (by reference). We get flags and errors. It may return an error; if so expansion
 /// halts.
-typedef expand_error_t (*expand_stage_t)(const wcstring &input,           //!OCLINT(unused param)
+typedef expand_error_t (*expand_stage_t)(wcstring input,                  //!OCLINT(unused param)
                                          std::vector<completion_t> *out,  //!OCLINT(unused param)
                                          expand_flags_t flags,            //!OCLINT(unused param)
                                          parse_error_list_t *errors);     //!OCLINT(unused param)
 
-static expand_error_t expand_stage_cmdsubst(const wcstring &input, std::vector<completion_t> *out,
+static expand_error_t expand_stage_cmdsubst(wcstring input, std::vector<completion_t> *out,
                                             expand_flags_t flags, parse_error_list_t *errors) {
     if (EXPAND_SKIP_CMDSUBST & flags) {
         wchar_t *begin, *end;
         if (parse_util_locate_cmdsubst(input.c_str(), &begin, &end, true) == 0) {
-            append_completion(out, input);
+            append_completion(out, std::move(input));
         } else {
             append_cmdsub_error(errors, SOURCE_LOCATION_UNKNOWN,
                                 L"Command substitutions not allowed");
             return EXPAND_ERROR;
         }
     } else {
-        bool cmdsubst_ok = expand_cmdsubst(input, out, errors);
+        bool cmdsubst_ok = expand_cmdsubst(std::move(input), out, errors);
         if (!cmdsubst_ok) return EXPAND_ERROR;
     }
 
     return EXPAND_OK;
 }
 
-static expand_error_t expand_stage_variables(const wcstring &input, std::vector<completion_t> *out,
+static expand_error_t expand_stage_variables(wcstring input, std::vector<completion_t> *out,
                                              expand_flags_t flags, parse_error_list_t *errors) {
     // We accept incomplete strings here, since complete uses expand_string to expand incomplete
     // strings from the commandline.
@@ -918,37 +918,36 @@ static expand_error_t expand_stage_variables(const wcstring &input, std::vector<
                 next[i] = L'$';
             }
         }
-        append_completion(out, next);
+        append_completion(out, std::move(next));
     } else {
-        if (!expand_variables(next, out, next.size(), errors)) {
+        size_t size = next.size();
+        if (!expand_variables(std::move(next), out, size, errors)) {
             return EXPAND_ERROR;
         }
     }
     return EXPAND_OK;
 }
 
-static expand_error_t expand_stage_braces(const wcstring &input, std::vector<completion_t> *out,
+static expand_error_t expand_stage_braces(wcstring input, std::vector<completion_t> *out,
                                           expand_flags_t flags, parse_error_list_t *errors) {
     return expand_braces(input, flags, out, errors);
 }
 
-static expand_error_t expand_stage_home(const wcstring &input, std::vector<completion_t> *out,
+static expand_error_t expand_stage_home(wcstring input, std::vector<completion_t> *out,
                                         expand_flags_t flags, parse_error_list_t *errors) {
     (void)errors;
-    wcstring next = input;
-
     if (!(EXPAND_SKIP_HOME_DIRECTORIES & flags)) {
-        expand_home_directory(next);
+        expand_home_directory(input);
     }
-    append_completion(out, next);
+    append_completion(out, std::move(input));
     return EXPAND_OK;
 }
 
-static expand_error_t expand_stage_wildcards(const wcstring &input, std::vector<completion_t> *out,
+static expand_error_t expand_stage_wildcards(wcstring path_to_expand,
-                                             expand_flags_t flags, parse_error_list_t *errors) {
+                                             std::vector<completion_t> *out, expand_flags_t flags,
+                                             parse_error_list_t *errors) {
     UNUSED(errors);
     expand_error_t result = EXPAND_OK;
-    wcstring path_to_expand = input;
 
     remove_internal_separator(&path_to_expand, flags & EXPAND_SKIP_WILDCARDS);
     const bool has_wildcard = wildcard_has(path_to_expand, true /* internal, i.e. ANY_STRING */);
@@ -1031,17 +1030,17 @@ static expand_error_t expand_stage_wildcards(const wcstring &input, std::vector<
         // to mean don't do file expansions, so if we're not doing file expansions, just drop this
         // completion on the floor.
         if (!(flags & EXPAND_FOR_COMPLETIONS)) {
-            append_completion(out, path_to_expand);
+            append_completion(out, std::move(path_to_expand));
         }
     }
     return result;
 }
 
-expand_error_t expand_string(const wcstring &input, std::vector<completion_t> *out_completions,
+expand_error_t expand_string(wcstring input, std::vector<completion_t> *out_completions,
                              expand_flags_t flags, parse_error_list_t *errors) {
     // Early out. If we're not completing, and there's no magic in the input, we're done.
     if (!(flags & EXPAND_FOR_COMPLETIONS) && expand_is_clean(input)) {
-        append_completion(out_completions, input);
+        append_completion(out_completions, std::move(input));
         return EXPAND_OK;
     }
 
@@ -1058,8 +1057,9 @@ expand_error_t expand_string(const wcstring &input, std::vector<completion_t> *o
     for (size_t stage_idx = 0;
          total_result != EXPAND_ERROR && stage_idx < sizeof stages / sizeof *stages; stage_idx++) {
         for (size_t i = 0; total_result != EXPAND_ERROR && i < completions.size(); i++) {
-            const wcstring &next = completions.at(i).completion;
+            wcstring &next = completions.at(i).completion;
-            expand_error_t this_result = stages[stage_idx](next, &output_storage, flags, errors);
+            expand_error_t this_result =
+                stages[stage_idx](std::move(next), &output_storage, flags, errors);
             // If this_result was no match, but total_result is that we have a match, then don't
             // change it.
             if (!(this_result == EXPAND_WILDCARD_NO_MATCH &&
@@ -1078,7 +1078,9 @@ expand_error_t expand_string(const wcstring &input, std::vector<completion_t> *o
         if (!(flags & EXPAND_SKIP_HOME_DIRECTORIES)) {
             unexpand_tildes(input, &completions);
         }
-        out_completions->insert(out_completions->end(), completions.begin(), completions.end());
+        out_completions->insert(out_completions->end(),
+                                std::make_move_iterator(completions.begin()),
+                                std::make_move_iterator(completions.end()));
     }
     return total_result;
 }

src/expand.h

@@ -110,7 +110,7 @@ enum expand_error_t {
 /// \return One of EXPAND_OK, EXPAND_ERROR, EXPAND_WILDCARD_MATCH and EXPAND_WILDCARD_NO_MATCH.
 /// EXPAND_WILDCARD_NO_MATCH and EXPAND_WILDCARD_MATCH are normal exit conditions used only on
 /// strings containing wildcards to tell if the wildcard produced any matches.
-__warn_unused expand_error_t expand_string(const wcstring &input, std::vector<completion_t> *output,
+__warn_unused expand_error_t expand_string(wcstring input, std::vector<completion_t> *output,
                                            expand_flags_t flags, parse_error_list_t *errors);
 
 /// expand_one is identical to expand_string, except it will fail if in expands to more than one