From 5158ee812b360869c347f8a15ffed604ee0edb4d Mon Sep 17 00:00:00 2001
From: ridiculousfish <corydoras@ridiculousfish.com>
Date: Sun, 12 May 2019 14:42:18 -0700
Subject: [PATCH] Eliminate the job from block_t

This exists only to support the '--on-job-exit caller' feature.
Just store the calling job ID directly in the parser's libdata.
---
 src/builtin_function.cpp | 14 +-------------
 src/parse_execution.cpp  | 19 ++++++++++---------
 src/parser.cpp           |  1 -
 src/parser.h             |  6 ++++--
 4 files changed, 15 insertions(+), 25 deletions(-)

diff --git a/src/builtin_function.cpp b/src/builtin_function.cpp
index db9777f32..9bc3a2f5f 100644
--- a/src/builtin_function.cpp
+++ b/src/builtin_function.cpp
@@ -93,19 +93,7 @@ static int parse_cmd_opts(function_cmd_opts_t &opts, int *optind,  //!OCLINT(hig
                     job_id_t job_id = -1;
 
                     if (is_subshell) {
-                        size_t block_idx = 0;
-
-                        // Find the outermost substitution block.
-                        for (block_idx = 0;; block_idx++) {
-                            const block_t *b = parser.block_at_index(block_idx);
-                            if (b == NULL || b->type() == SUBST) break;
-                        }
-
-                        // Go one step beyond that, to get to the caller.
-                        const block_t *caller_block = parser.block_at_index(block_idx + 1);
-                        if (caller_block != NULL && caller_block->job != NULL) {
-                            job_id = caller_block->job->job_id;
-                        }
+                        job_id = parser.libdata().caller_job_id;
                     }
 
                     if (job_id == -1) {
diff --git a/src/parse_execution.cpp b/src/parse_execution.cpp
index b03b49525..185012d3b 100644
--- a/src/parse_execution.cpp
+++ b/src/parse_execution.cpp
@@ -1240,27 +1240,28 @@ parse_execution_result_t parse_execution_context_t::run_1_job(tnode_t<g::job> jo
     job->set_flag(job_flag_t::SKIP_NOTIFICATION,
                   is_subshell || is_block || is_event || !shell_is_interactive());
 
-    // Tell the current block what its job is. This has to happen before we populate it (#1394).
-    parser->current_block()->job = job;
+    // We are about to populate a job. One possible argument to the job is a command substitution
+    // which may be interested in the job that's populating it, via '--on-job-exit caller'. Record
+    // the job ID here.
+    auto &libdata = parser->libdata();
+    const auto saved_caller_jid = libdata.caller_job_id;
+    libdata.caller_job_id = job->job_id;
 
     // Populate the job. This may fail for reasons like command_not_found. If this fails, an error
     // will have been printed.
     parse_execution_result_t pop_result =
         this->populate_job_from_job_node(job.get(), job_node, associated_block);
 
-    // Clean up the job on failure or cancellation.
-    bool populated_job = (pop_result == parse_execution_success);
-    if (!populated_job || this->should_cancel_execution(associated_block)) {
-        assert(parser->current_block()->job == job);
-        parser->current_block()->job = NULL;
-        populated_job = false;
-    }
+    assert(libdata.caller_job_id == job->job_id && "Caller job ID unexpectedly changed");
+    parser->libdata().caller_job_id = saved_caller_jid;
 
     // Store time it took to 'parse' the command.
     if (profile_item != NULL) {
         parse_time = get_time();
     }
 
+    // Clean up the job on failure or cancellation.
+    bool populated_job = (pop_result == parse_execution_success);
     if (populated_job) {
         // Success. Give the job to the parser - it will clean it up.
         parser->job_add(job);
diff --git a/src/parser.cpp b/src/parser.cpp
index ec3a0665b..c3fe487ff 100644
--- a/src/parser.cpp
+++ b/src/parser.cpp
@@ -138,7 +138,6 @@ void parser_t::push_block_int(block_t *new_current) {
         new_current->skip = false;
     }
 
-    new_current->job = nullptr;
     new_current->loop_status = LOOP_NORMAL;
 
     // Push it onto our stack. This acquires ownership because of unique_ptr.
diff --git a/src/parser.h b/src/parser.h
index 4917eef64..ee1e1d79e 100644
--- a/src/parser.h
+++ b/src/parser.h
@@ -67,8 +67,6 @@ struct block_t {
     bool skip{false};
     /// Status for the current loop block. Can be any of the values from the loop_status enum.
     enum loop_status_t loop_status { LOOP_NORMAL };
-    /// The job that is currently evaluated in the specified block.
-    shared_ptr<job_t> job{};
     /// Name of file that created this block. This string is intern'd.
     const wchar_t *src_filename{nullptr};
     /// Line number where this block was created.
@@ -159,6 +157,10 @@ struct library_data_t {
 
     /// Whether we are currently cleaning processes.
     bool is_cleaning_procs{false};
+
+    /// The job id of the job being populated.
+    /// This supports the '--on-job-exit caller' feature.
+    job_id_t caller_job_id{-1};
 };
 
 class parser_t : public std::enable_shared_from_this<parser_t> {