Make $status and $pipestatus per-parser

Another step towards allowing multiple parsers to execute in parallel.

src/builtin.cpp

@@ -326,7 +326,7 @@ static int builtin_breakpoint(parser_t &parser, io_streams_t &streams, wchar_t *
     const breakpoint_block_t *bpb = parser.push_block<breakpoint_block_t>();
     reader_read(STDIN_FILENO, streams.io_chain ? *streams.io_chain : io_chain_t());
     parser.pop_block(bpb);
-    return proc_get_last_status();
+    return parser.get_last_status();
 }
 
 int builtin_true(parser_t &parser, io_streams_t &streams, wchar_t **argv) {

src/builtin_eval.cpp

@@ -35,7 +35,7 @@ int builtin_eval(parser_t &parser, io_streams_t &streams, wchar_t **argv) {
             // where we have an argument but nothing is executed.
             status = STATUS_CMD_OK;
         } else {
-            status = proc_get_last_status();
+            status = parser.get_last_status();
         }
     }
 

src/builtin_exit.cpp

@@ -9,6 +9,7 @@
 #include "common.h"
 #include "fallback.h"  // IWYU pragma: keep
 #include "io.h"
+#include "parser.h"
 #include "proc.h"
 #include "reader.h"
 #include "wgetopt.h"
@@ -78,7 +79,7 @@ int builtin_exit(parser_t &parser, io_streams_t &streams, wchar_t **argv) {
     }
 
     if (optind == argc) {
-        retval = proc_get_last_status();
+        retval = parser.get_last_status();
     } else {
         retval = fish_wcstoi(argv[optind]);
         if (errno) {

src/builtin_return.cpp

@@ -78,7 +78,7 @@ int builtin_return(parser_t &parser, io_streams_t &streams, wchar_t **argv) {
     }
 
     if (optind == argc) {
-        retval = proc_get_last_status();
+        retval = parser.get_last_status();
     } else {
         retval = fish_wcstoi(argv[1]);
         if (errno) {

src/builtin_set.cpp

@@ -778,7 +778,7 @@ static int builtin_set_set(const wchar_t *cmd, set_cmd_opts_t &opts, int argc, w
 
 /// The set builtin creates, updates, and erases (removes, deletes) variables.
 int builtin_set(parser_t &parser, io_streams_t &streams, wchar_t **argv) {
-    const int incoming_exit_status = proc_get_last_status();
+    const int incoming_exit_status = parser.get_last_status();
     wchar_t *cmd = argv[0];
     int argc = builtin_count_args(argv);
     set_cmd_opts_t opts;

src/builtin_source.cpp

@@ -88,7 +88,7 @@ int builtin_source(parser_t &parser, io_streams_t &streams, wchar_t **argv) {
         streams.err.append_format(_(L"%ls: Error while reading file '%ls'\n"), cmd,
                                   fn_intern == intern_static(L"-") ? L"<stdin>" : fn_intern);
     } else {
-        retval = proc_get_last_status();
+        retval = parser.get_last_status();
     }
 
     // Do not close fd after calling reader_read. reader_read automatically closes it before calling

src/env.cpp

@@ -496,7 +496,8 @@ class env_scoped_impl_t : public environment_t {
     /// A struct wrapping up parser-local variables. These are conceptually variables that differ in
     /// different fish internal processes.
     struct perproc_data_t {
-        wcstring pwd;
+        wcstring pwd{};
+        statuses_t statuses{statuses_t::just(0)};
     };
 
    public:
@@ -647,7 +648,7 @@ maybe_t<env_var_t> env_scoped_impl_t::try_get_computed(const wcstring &key,
         if (history) history->get_history(result);
         return env_var_t(L"history", std::move(result));
     } else if (key == L"pipestatus") {
-        const auto js = proc_get_last_statuses();
+        const auto &js = perproc_data().statuses;
         wcstring_list_t result;
         result.reserve(js.pipestatus.size());
         for (int i : js.pipestatus) {
@@ -655,7 +656,8 @@ maybe_t<env_var_t> env_scoped_impl_t::try_get_computed(const wcstring &key,
         }
         return env_var_t(L"pipestatus", std::move(result));
     } else if (key == L"status") {
-        return env_var_t(L"status", to_string(proc_get_last_status()));
+        const auto &js = perproc_data().statuses;
+        return env_var_t(L"status", to_string(js.status));
     } else if (key == L"umask") {
         // note umask() is an absurd API: you call it to set the value and it returns the old
         // value. Thus we have to call it twice, to reset the value. The env_lock protects
@@ -1165,6 +1167,16 @@ bool env_stack_t::universal_barrier() {
     return changed || !callbacks.empty();
 }
 
+statuses_t env_stack_t::get_last_statuses() const {
+    return acquire_impl()->perproc_data().statuses;
+}
+
+int env_stack_t::get_last_status() const { return acquire_impl()->perproc_data().statuses.status; }
+
+void env_stack_t::set_last_statuses(statuses_t s) {
+    acquire_impl()->perproc_data().statuses = std::move(s);
+}
+
 /// If they don't already exist initialize the `COLUMNS` and `LINES` env vars to reasonable
 /// defaults. They will be updated later by the `get_current_winsize()` function if they need to be
 /// adjusted.

src/env.h

@@ -59,6 +59,23 @@ struct config_paths_t {
     wcstring bin;      // e.g., /usr/local/bin
 };
 
+/// A collection of status and pipestatus.
+struct statuses_t {
+    /// Status of the last job to exit.
+    int status{0};
+
+    /// Pipestatus value.
+    std::vector<int> pipestatus{};
+
+    /// Return a statuses for a single process status.
+    static statuses_t just(int s) {
+        statuses_t result{};
+        result.status = s;
+        result.pipestatus.push_back(s);
+        return result;
+    }
+};
+
 /// Initialize environment variable data.
 void env_init(const struct config_paths_t *paths = NULL);
 
@@ -258,6 +275,12 @@ class env_stack_t final : public environment_t {
     /// you want to read from another thread.
     std::shared_ptr<environment_t> snapshot() const;
 
+    /// Helpers to get and set the proc statuses.
+    /// These correspond to $status and $pipestatus.
+    statuses_t get_last_statuses() const;
+    int get_last_status() const;
+    void set_last_statuses(statuses_t s);
+
     /// Update the termsize variable.
     void set_termsize();
 

src/event.cpp

@@ -279,14 +279,14 @@ static void event_fire_internal(const event_t &event) {
         // Event handlers are not part of the main flow of code, so they are marked as
         // non-interactive.
         proc_push_interactive(0);
-        auto prev_statuses = proc_get_last_statuses();
         parser_t &parser = parser_t::principal_parser();
+        auto prev_statuses = parser.get_last_statuses();
 
         event_block_t *b = parser.push_block<event_block_t>(event);
         parser.eval(buffer, io_chain_t(), TOP);
         parser.pop_block(b);
         proc_pop_interactive();
-        proc_set_last_statuses(std::move(prev_statuses));
+        parser.set_last_statuses(std::move(prev_statuses));
     }
 }
 

src/exec.cpp

@@ -279,7 +279,7 @@ void internal_exec_helper(parser_t &parser, parsed_source_ref_t parsed_source, t
 
     // Did the transmogrification fail - if so, set error status and return.
     if (!transmorgrified) {
-        proc_set_last_statuses(statuses_t::just(STATUS_EXEC_FAIL));
+        parser.set_last_statuses(statuses_t::just(STATUS_EXEC_FAIL));
         return;
     }
 
@@ -589,7 +589,7 @@ static bool exec_internal_builtin_proc(parser_t &parser, const std::shared_ptr<j
 
 /// Handle output from a builtin, by printing the contents of builtin_io_streams to the redirections
 /// given in io_chain.
-static bool handle_builtin_output(const std::shared_ptr<job_t> &j, process_t *p,
+static bool handle_builtin_output(parser_t &parser, const std::shared_ptr<job_t> &j, process_t *p,
                                   io_chain_t *io_chain, const io_streams_t &builtin_io_streams) {
     assert(p->type == process_type_t::builtin && "Process is not a builtin");
 
@@ -663,7 +663,7 @@ static bool handle_builtin_output(const std::shared_ptr<job_t> &j, process_t *p,
         if (p->is_last_in_job) {
             debug(4, L"Set status of job %d (%ls) to %d using short circuit", j->job_id,
                   j->preview().c_str(), p->status);
-            proc_set_last_statuses(j->get_statuses());
+            parser.set_last_statuses(j->get_statuses());
         }
         return true;
     } else {
@@ -832,7 +832,7 @@ static bool exec_block_or_func_process(parser_t &parser, std::shared_ptr<job_t>
         internal_exec_helper(parser, p->block_node_source, p->internal_block_node, io_chain, j);
     }
 
-    int status = proc_get_last_status();
+    int status = parser.get_last_status();
     // FIXME: setting the status this way is dangerous nonsense, we need to decode the status
     // properly if it was a signal.
     p->status = proc_status_t::from_exit_code(status);
@@ -843,7 +843,7 @@ static bool exec_block_or_func_process(parser_t &parser, std::shared_ptr<job_t>
         // status.
         p->completed = 1;
         if (p->is_last_in_job) {
-            proc_set_last_statuses(j->get_statuses());
+            parser.set_last_statuses(j->get_statuses());
         }
         return true;
     }
@@ -859,7 +859,7 @@ static bool exec_block_or_func_process(parser_t &parser, std::shared_ptr<job_t>
         return run_internal_process(p, std::move(buffer_contents), {} /*errdata*/, io_chain);
     } else {
         if (p->is_last_in_job) {
-            proc_set_last_statuses(j->get_statuses());
+            parser.set_last_statuses(j->get_statuses());
         }
         p->completed = 1;
     }
@@ -964,7 +964,7 @@ static bool exec_process_in_job(parser_t &parser, process_t *p, std::shared_ptr<
                                             builtin_io_streams)) {
                 return false;
             }
-            if (!handle_builtin_output(j, p, &process_net_io_chain, builtin_io_streams)) {
+            if (!handle_builtin_output(parser, j, p, &process_net_io_chain, builtin_io_streams)) {
                 return false;
             }
             break;
@@ -1051,7 +1051,7 @@ bool exec_job(parser_t &parser, shared_ptr<job_t> j) {
         // internal_exec only returns if it failed to set up redirections.
         // In case of an successful exec, this code is not reached.
         bool status = j->get_flag(job_flag_t::NEGATE) ? 0 : 1;
-        proc_set_last_statuses(statuses_t::just(status));
+        parser.set_last_statuses(statuses_t::just(status));
         return false;
     }
 
@@ -1131,7 +1131,7 @@ static int exec_subshell_internal(const wcstring &cmd, parser_t &parser, wcstrin
                                   bool apply_exit_status, bool is_subcmd) {
     ASSERT_IS_MAIN_THREAD();
     bool prev_subshell = is_subshell;
-    auto prev_statuses = proc_get_last_statuses();
+    auto prev_statuses = parser.get_last_statuses();
     bool split_output = false;
 
     const auto ifs = parser.vars().get(L"IFS");
@@ -1148,7 +1148,7 @@ static int exec_subshell_internal(const wcstring &cmd, parser_t &parser, wcstrin
     std::shared_ptr<io_buffer_t> buffer;
     if (auto bufferfill = io_bufferfill_t::create(io_chain_t{}, read_limit)) {
         if (parser.eval(cmd, io_chain_t{bufferfill}, SUBST) == 0) {
-            subcommand_statuses = proc_get_last_statuses();
+            subcommand_statuses = parser.get_last_statuses();
         }
         buffer = io_bufferfill_t::finish(std::move(bufferfill));
     }
@@ -1160,9 +1160,9 @@ static int exec_subshell_internal(const wcstring &cmd, parser_t &parser, wcstrin
     // If the caller asked us to preserve the exit status, restore the old status. Otherwise set the
     // status of the subcommand.
     if (apply_exit_status) {
-        proc_set_last_statuses(subcommand_statuses);
+        parser.set_last_statuses(subcommand_statuses);
     } else {
-        proc_set_last_statuses(std::move(prev_statuses));
+        parser.set_last_statuses(std::move(prev_statuses));
     }
 
     is_subshell = prev_subshell;

src/expand.cpp

@@ -624,7 +624,7 @@ static bool expand_cmdsubst(wcstring input, parser_t &parser, std::vector<comple
         return false;
     }
 
-    if (proc_get_last_status() == STATUS_READ_TOO_MUCH) {
+    if (parser.get_last_status() == STATUS_READ_TOO_MUCH) {
         append_cmdsub_error(
             errors, in - paren_begin,
             _(L"Too much data emitted by command substitution so it was discarded\n"));
@@ -698,7 +698,7 @@ static bool expand_cmdsubst(wcstring input, parser_t &parser, std::vector<comple
         }
     }
 
-    if (proc_get_last_status() == STATUS_READ_TOO_MUCH) return false;
+    if (parser.get_last_status() == STATUS_READ_TOO_MUCH) return false;
     return true;
 }
 

src/fish.cpp

@@ -422,7 +422,7 @@ int main(int argc, char **argv) {
 
     if (read_init(paths)) {
         // Stomp the exit status of any initialization commands (issue #635).
-        proc_set_last_statuses(statuses_t::just(STATUS_CMD_OK));
+        parser.set_last_statuses(statuses_t::just(STATUS_CMD_OK));
 
         // Run post-config commands specified as arguments, if any.
         if (!opts.postconfig_cmds.empty()) {
@@ -472,7 +472,7 @@ int main(int argc, char **argv) {
         }
     }
 
-    int exit_status = res ? STATUS_CMD_UNKNOWN : proc_get_last_status();
+    int exit_status = res ? STATUS_CMD_UNKNOWN : parser.get_last_status();
 
     // TODO: The generic process-exit event is useless and unused.
     // Remove this in future.

src/fish_tests.cpp

@@ -5014,7 +5014,7 @@ static void test_illegal_command_exit_code() {
     for (const auto &test : tests) {
         res = parser.eval(test.txt, empty_ios, TOP);
 
-        int exit_status = proc_get_last_status();
+        int exit_status = parser.get_last_status();
         if (exit_status != test.result) {
             err(L"command '%ls': expected exit code %d, got %d", test.txt, test.result,
                 exit_status);

src/input.cpp

@@ -372,11 +372,12 @@ static void input_mapping_execute(const input_mapping_t &m, bool allow_commands)
         //
         // FIXME(snnw): if commands add stuff to input queue (e.g. commandline -f execute), we won't
         // see that until all other commands have also been run.
-        auto last_statuses = proc_get_last_statuses();
+        auto &parser = parser_t::principal_parser();
+        auto last_statuses = parser.get_last_statuses();
         for (const wcstring &cmd : m.commands) {
-            parser_t::principal_parser().eval(cmd, io_chain_t(), TOP);
+            parser.eval(cmd, io_chain_t(), TOP);
         }
-        proc_set_last_statuses(std::move(last_statuses));
+        parser.set_last_statuses(std::move(last_statuses));
         input_common_next_ch(char_event_type_t::check_exit);
     } else {
         // Invalid binding, mixed commands and functions.  We would need to execute these one by

src/parse_execution.cpp

@@ -258,7 +258,7 @@ parse_execution_result_t parse_execution_context_t::run_if_statement(
             cond_ret = run_job_list(condition_boolean_tail, associated_block);
         }
         const bool take_branch =
-            (cond_ret == parse_execution_success) && proc_get_last_status() == EXIT_SUCCESS;
+            (cond_ret == parse_execution_success) && parser->get_last_status() == EXIT_SUCCESS;
 
         if (take_branch) {
             // Condition succeeded.
@@ -268,7 +268,7 @@ parse_execution_result_t parse_execution_context_t::run_if_statement(
         auto else_cont = else_clause.try_get_child<g::else_continuation, 1>();
         if (!else_cont) {
             // 'if' condition failed, no else clause, return 0, we're done.
-            proc_set_last_statuses(statuses_t::just(STATUS_CMD_OK));
+            parser->set_last_statuses(statuses_t::just(STATUS_CMD_OK));
             break;
         } else {
             // We have an 'else continuation' (either else-if or else).
@@ -296,7 +296,7 @@ parse_execution_result_t parse_execution_context_t::run_if_statement(
         parser->pop_block(ib);
     } else {
         // No job list means no sucessful conditions, so return 0 (issue #1443).
-        proc_set_last_statuses(statuses_t::just(STATUS_CMD_OK));
+        parser->set_last_statuses(statuses_t::just(STATUS_CMD_OK));
     }
 
     // It's possible there's a last-minute cancellation (issue #1297).
@@ -332,7 +332,7 @@ parse_execution_result_t parse_execution_context_t::run_function_statement(
     }
     io_streams_t streams(0);  // no limit on the amount of output from builtin_function()
     int err = builtin_function(*parser, streams, arguments, pstree, body);
-    proc_set_last_statuses(statuses_t::just(err));
+    parser->set_last_statuses(statuses_t::just(err));
 
     if (!streams.err.empty()) {
         this->report_error(header, L"%ls", streams.err.contents().c_str());
@@ -557,7 +557,7 @@ parse_execution_result_t parse_execution_context_t::run_while_statement(
         // Save off the exit status if it came from the loop body. We'll restore it if the condition
         // is false.
         auto cond_saved_status =
-            first_cond_check ? statuses_t::just(EXIT_SUCCESS) : proc_get_last_statuses();
+            first_cond_check ? statuses_t::just(EXIT_SUCCESS) : parser->get_last_statuses();
         first_cond_check = false;
 
         // Check the condition.
@@ -572,8 +572,8 @@ parse_execution_result_t parse_execution_context_t::run_while_statement(
         // exit the loop.
         if (cond_ret != parse_execution_success) {
             break;
-        } else if (proc_get_last_status() != EXIT_SUCCESS) {
+        } else if (parser->get_last_status() != EXIT_SUCCESS) {
-            proc_set_last_statuses(cond_saved_status);
+            parser->set_last_statuses(cond_saved_status);
             break;
         }
 
@@ -650,7 +650,7 @@ parse_execution_result_t parse_execution_context_t::report_errors(
 /// Reports an unmatched wildcard error and returns parse_execution_errored.
 parse_execution_result_t parse_execution_context_t::report_unmatched_wildcard_error(
     const parse_node_t &unmatched_wildcard) const {
-    proc_set_last_statuses(statuses_t::just(STATUS_UNMATCHED_WILDCARD));
+    parser->set_last_statuses(statuses_t::just(STATUS_UNMATCHED_WILDCARD));
     report_error(unmatched_wildcard, WILDCARD_ERR_MSG, get_source(unmatched_wildcard).c_str());
     return parse_execution_errored;
 }
@@ -732,7 +732,7 @@ parse_execution_result_t parse_execution_context_t::handle_command_not_found(
 
     // Set the last proc status appropriately.
     int status = err_code == ENOENT ? STATUS_CMD_UNKNOWN : STATUS_NOT_EXECUTABLE;
-    proc_set_last_statuses(statuses_t::just(status));
+    parser->set_last_statuses(statuses_t::just(status));
 
     return parse_execution_errored;
 }
@@ -752,7 +752,7 @@ parse_execution_result_t parse_execution_context_t::expand_command(
     expand_result_t expand_err =
         expand_to_command_and_args(unexp_cmd, parser->vars(), out_cmd, out_args, &errors);
     if (expand_err == expand_result_t::error) {
-        proc_set_last_statuses(statuses_t::just(STATUS_ILLEGAL_CMD));
+        parser->set_last_statuses(statuses_t::just(STATUS_ILLEGAL_CMD));
         return report_errors(errors);
     } else if (expand_err == expand_result_t::wildcard_no_match) {
         return report_unmatched_wildcard_error(statement);
@@ -1327,10 +1327,10 @@ bool parse_execution_context_t::should_skip(parse_bool_statement_type_t type) co
     switch (type) {
         case parse_bool_and:
             // AND. Skip if the last job failed.
-            return proc_get_last_status() != 0;
+            return parser->get_last_status() != 0;
         case parse_bool_or:
             // OR. Skip if the last job succeeded.
-            return proc_get_last_status() == 0;
+            return parser->get_last_status() == 0;
         default:
             return false;
     }

src/parser.h

@@ -285,6 +285,11 @@ class parser_t : public std::enable_shared_from_this<parser_t> {
     library_data_t &libdata() { return library_data; }
     const library_data_t &libdata() const { return library_data; }
 
+    /// Get and set the last proc statuses.
+    int get_last_status() const { return vars().get_last_status(); }
+    statuses_t get_last_statuses() const { return vars().get_last_statuses(); }
+    void set_last_statuses(statuses_t s) { vars().set_last_statuses(std::move(s)); }
+
     /// Pushes a new block created with the given arguments
     /// Returns a pointer to the block. The pointer is valid
     /// until the call to pop_block()

src/proc.cpp

@@ -49,9 +49,6 @@
 #include "signal.h"
 #include "wutil.h"  // IWYU pragma: keep
 
-/// Statuses of last job's processes to exit - ensure we start off with one entry of 0.
-static owning_lock<statuses_t> last_statuses{statuses_t::just(0)};
-
 /// The signals that signify crashes to us.
 static const int crashsignals[] = {SIGABRT, SIGBUS, SIGFPE, SIGILL, SIGSEGV, SIGSYS};
 
@@ -86,15 +83,6 @@ static std::vector<int> interactive_stack;
 
 void proc_init() { proc_push_interactive(0); }
 
-void proc_set_last_statuses(statuses_t s) {
-    ASSERT_IS_MAIN_THREAD();
-    *last_statuses.acquire() = std::move(s);
-}
-
-int proc_get_last_status() { return last_statuses.acquire()->status; }
-
-statuses_t proc_get_last_statuses() { return *last_statuses.acquire(); }
-
 // Basic thread safe job IDs. The vector consumed_job_ids has a true value wherever the job ID
 // corresponding to that slot is in use. The job ID corresponding to slot 0 is 1.
 static owning_lock<std::vector<bool>> locked_consumed_job_ids;
@@ -623,12 +611,12 @@ bool job_reap(parser_t &parser, bool allow_interactive) {
     process_mark_finished_children(parser, false);
 
     // Preserve the exit status.
-    auto saved_statuses = proc_get_last_statuses();
+    auto saved_statuses = parser.get_last_statuses();
 
     bool printed = process_clean_after_marking(parser, allow_interactive);
 
     // Restore the exit status.
-    proc_set_last_statuses(std::move(saved_statuses));
+    parser.set_last_statuses(std::move(saved_statuses));
 
     return printed;
 }
@@ -898,7 +886,7 @@ void job_t::continue_job(parser_t &parser, bool reclaim_foreground_pgrp, bool se
         // finished and is not a short-circuited builtin.
         auto &p = processes.back();
         if (p->status.normal_exited() || p->status.signal_exited()) {
-            proc_set_last_statuses(get_statuses());
+            parser.set_last_statuses(get_statuses());
         }
     }
 }

src/proc.h

@@ -262,23 +262,6 @@ enum class job_flag_t {
     JOB_FLAG_COUNT
 };
 
-/// A collection of status and pipestatus.
-struct statuses_t {
-    /// Status of the last job to exit.
-    int status{0};
-
-    /// Pipestatus value.
-    std::vector<int> pipestatus{};
-
-    /// Return a statuses for a single process status.
-    static statuses_t just(int s) {
-        statuses_t result{};
-        result.status = s;
-        result.pipestatus.push_back(s);
-        return result;
-    }
-};
-
 template <>
 struct enum_info_t<job_flag_t> {
     static constexpr auto count = job_flag_t::JOB_FLAG_COUNT;
@@ -467,13 +450,6 @@ void set_job_control_mode(job_control_t mode);
 /// anything.
 extern int no_exec;
 
-/// Sets the status of the last process to exit.
-void proc_set_last_statuses(statuses_t s);
-
-/// Returns the status of the last process to exit.
-int proc_get_last_status();
-statuses_t proc_get_last_statuses();
-
 /// Notify the user about stopped or terminated jobs, and delete completed jobs from the job list.
 /// If \p interactive is set, allow removing interactive jobs; otherwise skip them.
 /// \return whether text was printed to stdout.