Remove the process from function_block_t

Prior to this fix, a function_block stored a process_t, which was only used
when printing backtraces. Switch this to an array of arguments, and make
various other cleanups around null terminated argument arrays.

src/builtin_source.cpp

@@ -78,7 +78,9 @@ int builtin_source(parser_t &parser, io_streams_t &streams, wchar_t **argv) {
 
     // This is slightly subtle. If this is a bare `source` with no args then `argv + optind` already
     // points to the end of argv. Otherwise we want to skip the file name to get to the args if any.
-    parser.vars().set_argv(argv + optind + (argc == optind ? 0 : 1));
+    wcstring_list_t argv_list =
+        null_terminated_array_t<wchar_t>::to_list(argv + optind + (argc == optind ? 0 : 1));
+    parser.vars().set_argv(std::move(argv_list));
 
     retval = reader_read(fd, streams.io_chain ? *streams.io_chain : io_chain_t());
 

src/common.h

@@ -584,6 +584,18 @@ class null_terminated_array_t {
         this->array = make_null_terminated_array(argv);
     }
 
+    /// Convert from a null terminated list to a vector of strings.
+    static string_list_t to_list(const CharType_t *const *arr) {
+        string_list_t result;
+        for (const auto *cursor = arr; cursor && *cursor; cursor++) {
+            result.push_back(*cursor);
+        }
+        return result;
+    }
+
+    /// Instance method.
+    string_list_t to_list() const { return to_list(array); }
+
     const CharType_t *const *get() const { return array; }
     CharType_t **get() { return array; }
 

src/env.cpp

@@ -1257,17 +1257,7 @@ std::shared_ptr<const null_terminated_array_t<char>> env_stack_t::export_arr() {
 
 std::shared_ptr<environment_t> env_stack_t::snapshot() const { return acquire_impl()->snapshot(); }
 
-void env_stack_t::set_argv(const wchar_t *const *argv) {
+void env_stack_t::set_argv(wcstring_list_t argv) { set(L"argv", ENV_LOCAL, std::move(argv)); }
-    if (argv && *argv) {
-        wcstring_list_t list;
-        for (auto arg = argv; *arg; arg++) {
-            list.emplace_back(*arg);
-        }
-        set(L"argv", ENV_LOCAL, std::move(list));
-    } else {
-        set_empty(L"argv", ENV_LOCAL);
-    }
-}
 
 void env_stack_t::push(bool new_scope) {
     auto impl = acquire_impl();

src/env.h

@@ -284,8 +284,8 @@ class env_stack_t final : public environment_t {
     /// Update the termsize variable.
     void set_termsize();
 
-    /// Sets up argv as the given null terminated array of strings.
+    /// Sets up argv as the given list of strings.
-    void set_argv(const wchar_t *const *argv);
+    void set_argv(wcstring_list_t argv);
 
     /// Mark that exported variables have changed.
     void mark_changed_exported();

src/exec.cpp

@@ -817,9 +817,14 @@ static bool exec_block_or_func_process(parser_t &parser, std::shared_ptr<job_t>
 
         const std::map<wcstring, env_var_t> inherit_vars = function_get_inherit_vars(func_name);
 
+        // TODO: we want to store the args in both the function block and the environment.
+        // Find a way to share memory here?
+        wcstring_list_t argv = p->get_argv_array().to_list();
+        // Remove the function name from argv.
+        if (!argv.empty()) argv.erase(argv.begin());
         function_block_t *fb =
-            parser.push_block<function_block_t>(p, func_name, props->shadow_scope);
+            parser.push_block<function_block_t>(func_name, argv, props->shadow_scope);
-        function_prepare_environment(parser.vars(), func_name, p->get_argv() + 1, inherit_vars);
+        function_prepare_environment(parser.vars(), func_name, std::move(argv), inherit_vars);
         parser.forbid_function(func_name);
 
         internal_exec_helper(parser, props->parsed_source, props->body_node, io_chain, j);

src/function.cpp

@@ -371,17 +371,16 @@ void function_invalidate_path() {
 // 1. argv
 // 2. named arguments
 // 3. inherited variables
-void function_prepare_environment(env_stack_t &vars, const wcstring &name,
+void function_prepare_environment(env_stack_t &vars, const wcstring &name, wcstring_list_t argv,
-                                  const wchar_t *const *argv,
                                   const std::map<wcstring, env_var_t> &inherited_vars) {
     vars.set_argv(argv);
     auto props = function_get_properties(name);
     if (props && !props->named_arguments.empty()) {
-        const wchar_t *const *arg = argv;
+        auto argv_iter = argv.cbegin();
         for (const wcstring &named_arg : props->named_arguments) {
-            if (*arg) {
+            if (argv_iter != argv.cend()) {
-                vars.set_one(named_arg, ENV_LOCAL | ENV_USER, *arg);
+                vars.set_one(named_arg, ENV_LOCAL | ENV_USER, std::move(*argv_iter));
-                arg++;
+                ++argv_iter;
             } else {
                 vars.set_empty(named_arg, ENV_LOCAL | ENV_USER);
             }

src/function.h

@@ -109,8 +109,7 @@ std::map<wcstring, env_var_t> function_get_inherit_vars(const wcstring &name);
 bool function_copy(const wcstring &name, const wcstring &new_name);
 
 /// Prepares the environment for executing a function.
-void function_prepare_environment(env_stack_t &vars, const wcstring &name,
+void function_prepare_environment(env_stack_t &vars, const wcstring &name, wcstring_list_t argv,
-                                  const wchar_t *const *argv,
                                   const std::map<wcstring, env_var_t> &inherited_vars);
 
 /// Observes that fish_function_path has changed.

src/parser.cpp

@@ -368,8 +368,6 @@ void parser_t::stack_trace_internal(size_t block_idx, wcstring *buff) const {
     if (b->type() == FUNCTION_CALL || b->type() == FUNCTION_CALL_NO_SHADOW || b->type() == SOURCE ||
         b->type() == SUBST) {
         // These types of blocks should be printed.
-        int i;
-
         switch (b->type()) {
             case SOURCE: {
                 const source_block_t *sb = static_cast<const source_block_t *>(b);
@@ -382,27 +380,23 @@ void parser_t::stack_trace_internal(size_t block_idx, wcstring *buff) const {
             case FUNCTION_CALL_NO_SHADOW: {
                 const function_block_t *fb = static_cast<const function_block_t *>(b);
                 append_format(*buff, _(L"in function '%ls'"), fb->name.c_str());
-                const process_t *const process = fb->process;
                 // Print arguments on the same line.
-                if (process->argv(1)) {
+                wcstring args_str;
-                    wcstring tmp;
+                for (const wcstring &arg : fb->args) {
-
+                    if (!args_str.empty()) args_str.push_back(L' ');
-                    for (i = 1; process->argv(i); i++) {
+                    // We can't quote the arguments because we print this in quotes.
-                        if (i > 1) tmp.push_back(L' ');
+                    // As a special-case, add the empty argument as "".
-                        // We can't quote the arguments because we print this in quotes.
+                    if (!arg.empty()) {
-                        // As a special-case, add the empty argument as "".
+                        args_str.append(escape_string(arg, ESCAPE_ALL | ESCAPE_NO_QUOTED));
-                        if (process->argv(i)[0]) {
+                    } else {
-                            tmp.append(
+                        args_str.append(L"\"\"");
-                                escape_string(process->argv(i), ESCAPE_ALL | ESCAPE_NO_QUOTED));
-                        } else {
-                            tmp.append(L"\"\"");
-                        }
                     }
-                    // TODO: Escape these.
-                    append_format(*buff, _(L" with arguments '%ls'\n"), tmp.c_str());
-                } else {
-                    buff->append(L"\n");
                 }
+                if (!args_str.empty()) {
+                    // TODO: Escape these.
+                    append_format(*buff, _(L" with arguments '%ls'"), args_str.c_str());
+                }
+                buff->push_back('\n');
                 break;
             }
             case SUBST: {
@@ -846,8 +840,10 @@ if_block_t::if_block_t() : block_t(IF) {}
 
 event_block_t::event_block_t(const event_t &evt) : block_t(EVENT), event(evt) {}
 
-function_block_t::function_block_t(const process_t *p, wcstring n, bool shadows)
+function_block_t::function_block_t(wcstring name, wcstring_list_t args, bool shadows)
-    : block_t(shadows ? FUNCTION_CALL : FUNCTION_CALL_NO_SHADOW), process(p), name(std::move(n)) {}
+    : block_t(shadows ? FUNCTION_CALL : FUNCTION_CALL_NO_SHADOW),
+      name(std::move(name)),
+      args(std::move(args)) {}
 
 source_block_t::source_block_t(const wchar_t *src) : block_t(SOURCE), source_file(src) {}
 

src/parser.h

@@ -96,9 +96,9 @@ struct event_block_t : public block_t {
 };
 
 struct function_block_t : public block_t {
-    const process_t *process;
     wcstring name;
-    function_block_t(const process_t *p, wcstring n, bool shadows);
+    wcstring_list_t args;
+    function_block_t(wcstring name, wcstring_list_t args, bool shadows);
 };
 
 struct source_block_t : public block_t {