Make the 'time' keyword a fixed property of a job.

The 'time' prefix may come about either because the job itself is marked
with time, or because of the "inside out" weirdness of 'not time...'.
Factor this logic together and precompute it for a job.

src/exec.cpp

@@ -983,7 +983,7 @@ bool exec_job(parser_t &parser, const shared_ptr<job_t> &j, const io_chain_t &bl
         // A false return tells the caller to remove the job from the list.
         return false;
     }
-    cleanup_t timer = push_timer(j->flags().has_time_prefix && !no_exec());
+    cleanup_t timer = push_timer(j->wants_timing() && !no_exec());
 
     // Get the deferred process, if any. We will have to remember its pipes.
     autoclose_pipes_t deferred_pipes;

src/parse_execution.cpp

@@ -998,12 +998,6 @@ end_execution_reason_t parse_execution_context_t::populate_not_process(
     job_t *job, process_t *proc, const ast::not_statement_t &not_statement) {
     auto &flags = job->mut_flags();
     flags.negate = !flags.negate;
-    if (not_statement.time) {
-        flags.has_time_prefix = true;
-        if (job->is_initially_background()) {
-            return this->report_error(STATUS_INVALID_ARGS, not_statement, ERROR_TIME_BACKGROUND);
-        }
-    }
     return this->populate_job_process(job, proc, not_statement.contents, not_statement.variables);
 }
 
@@ -1151,12 +1145,6 @@ end_execution_reason_t parse_execution_context_t::populate_job_from_job_node(
     // Create processes. Each one may fail.
     process_list_t processes;
     processes.emplace_back(new process_t());
-    if (job_node.time) {
-        j->mut_flags().has_time_prefix = true;
-        if (job_node.bg) {
-            return this->report_error(STATUS_INVALID_ARGS, job_node, ERROR_TIME_BACKGROUND);
-        }
-    }
     end_execution_reason_t result = this->populate_job_process(
         j, processes.back().get(), job_node.statement, job_node.variables);
 
@@ -1210,6 +1198,32 @@ static bool remove_job(parser_t &parser, job_t *job) {
     return false;
 }
 
+/// Decide if a job node should be 'time'd.
+/// For historical reasons the 'not' and 'time' prefix are "inside out". That is, it's
+/// 'not time cmd'. Note that a time appearing anywhere in the pipeline affects the whole job.
+/// `sleep 1 | not time true` will time the whole job!
+static bool job_node_wants_timing(const ast::job_t &job_node) {
+    // Does our job have the job-level time prefix?
+    if (job_node.time) return true;
+
+    // Helper to return true if a node is 'not time ...' or 'not not time...' or...
+    auto is_timed_not_statement = [](const ast::statement_t &stat) {
+        const auto *ns = stat.contents->try_as<ast::not_statement_t>();
+        while (ns) {
+            if (ns->time) return true;
+            ns = ns->contents.try_as<ast::not_statement_t>();
+        }
+        return false;
+    };
+
+    // Do we have a 'not time ...' anywhere in our pipeline?
+    if (is_timed_not_statement(job_node.statement)) return true;
+    for (const ast::job_continuation_t &jc : job_node.continuation) {
+        if (is_timed_not_statement(jc.statement)) return true;
+    }
+    return false;
+}
+
 end_execution_reason_t parse_execution_context_t::run_1_job(const ast::job_t &job_node,
                                                             const block_t *associated_block) {
     if (auto ret = check_end_execution()) {
@@ -1306,6 +1320,12 @@ end_execution_reason_t parse_execution_context_t::run_1_job(const ast::job_t &jo
         ld.is_subshell || ld.is_block || ld.is_event || !parser->is_interactive();
     props.from_event_handler = ld.is_event;
     props.job_control = wants_job_control;
+    props.wants_timing = job_node_wants_timing(job_node);
+
+    // It's an error to have 'time' in a background job.
+    if (props.wants_timing && props.initial_background) {
+        return this->report_error(STATUS_INVALID_ARGS, job_node, ERROR_TIME_BACKGROUND);
+    }
 
     shared_ptr<job_t> job = std::make_shared<job_t>(props, get_source(job_node));
 

src/proc.h

@@ -318,6 +318,9 @@ class job_t {
         /// initial state should be.
         bool initial_background{};
 
+        /// Whether the job has the 'time' prefix and so we should print timing for this job.
+        bool wants_timing{};
+
         /// Whether this job was created as part of an event handler.
         bool from_event_handler{};
 
@@ -409,9 +412,6 @@ class job_t {
         /// This job is disowned, and should be removed from the active jobs list.
         bool disown_requested{false};
 
-        /// Whether to print timing for this job.
-        bool has_time_prefix{false};
-
         // Indicates that we are the "group root." Any other jobs using this tree are nested.
         bool is_group_root{false};
 
@@ -423,6 +423,9 @@ class job_t {
     /// Access mutable job flags.
     flags_t &mut_flags() { return job_flags; }
 
+    // \return whether we should print timing information.
+    bool wants_timing() const { return properties.wants_timing; }
+
     /// \return if we want job control.
     bool wants_job_control() const { return properties.job_control; }