From b914c94cc1f755e412949e453510a57f1763543f Mon Sep 17 00:00:00 2001
From: ridiculousfish <rf@fishshell.com>
Date: Wed, 28 Jul 2021 12:55:01 -0700
Subject: [PATCH] Stop storing 'is_block' inside the parser

is_block is a field which supports 'status is-block', and also controls
whether notifications get posted. However there is no reason to store
this as a distinct field since it is trivially computed from the block
list. Stop storing it. No functional changes in this commit.
---
 src/builtin_status.cpp  |  2 +-
 src/parse_execution.cpp |  2 +-
 src/parser.cpp          | 25 ++++++++++---------------
 src/parser.h            |  7 ++++---
 4 files changed, 16 insertions(+), 20 deletions(-)

diff --git a/src/builtin_status.cpp b/src/builtin_status.cpp
index 62693143b..876c6d0a4 100644
--- a/src/builtin_status.cpp
+++ b/src/builtin_status.cpp
@@ -409,7 +409,7 @@ maybe_t<int> builtin_status(parser_t &parser, io_streams_t &streams, const wchar
         }
         case STATUS_IS_BLOCK: {
             CHECK_FOR_UNEXPECTED_STATUS_ARGS(opts.status_cmd)
-            retval = parser.libdata().is_block ? 0 : 1;
+            retval = parser.is_block() ? 0 : 1;
             break;
         }
         case STATUS_IS_BREAKPOINT: {
diff --git a/src/parse_execution.cpp b/src/parse_execution.cpp
index cd16345fa..0b27f12fe 100644
--- a/src/parse_execution.cpp
+++ b/src/parse_execution.cpp
@@ -1350,7 +1350,7 @@ end_execution_reason_t parse_execution_context_t::run_1_job(const ast::job_t &jo
     job_t::properties_t props{};
     props.initial_background = job_node.bg.has_value();
     props.skip_notification =
-        ld.is_subshell || ld.is_block || ld.is_event || !parser->is_interactive();
+        ld.is_subshell || parser->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);
diff --git a/src/parser.cpp b/src/parser.cpp
index 0f49622b0..a43722c5e 100644
--- a/src/parser.cpp
+++ b/src/parser.cpp
@@ -136,11 +136,6 @@ block_t *parser_t::push_block(block_t &&block) {
         new_current.src_filename = intern(filename);
     }
 
-    // Types top and subst are not considered blocks for the purposes of `status is-block`.
-    if (type != block_type_t::top && type != block_type_t::subst) {
-        libdata().is_block = true;
-    }
-
     if (type == block_type_t::breakpoint) {
         libdata().is_breakpoint = true;
     }
@@ -167,16 +162,6 @@ void parser_t::pop_block(const block_t *expected) {
 
     if (old.wants_pop_env) vars().pop();
 
-    // Figure out if `status is-block` should consider us to be in a block now.
-    bool new_is_block = false;
-    for (const auto &b : block_list) {
-        if (b.type() != block_type_t::top && b.type() != block_type_t::subst) {
-            new_is_block = true;
-            break;
-        }
-    }
-    libdata().is_block = new_is_block;
-
     // Are we still in a breakpoint?
     bool new_is_breakpoint = false;
     for (const auto &b : block_list) {
@@ -424,6 +409,16 @@ bool parser_t::is_function() const {
     return false;
 }
 
+bool parser_t::is_block() const {
+    // Note historically this has descended into 'source', unlike 'is_function'.
+    for (const auto &b : block_list) {
+        if (b.type() != block_type_t::top && b.type() != block_type_t::subst) {
+            return true;
+        }
+    }
+    return false;
+}
+
 maybe_t<wcstring> parser_t::get_function_name(int level) {
     if (level == 0) {
         // Return the function name for the level preceding the most recent breakpoint. If there
diff --git a/src/parser.h b/src/parser.h
index f15e5283d..a48981a6e 100644
--- a/src/parser.h
+++ b/src/parser.h
@@ -169,9 +169,6 @@ struct library_data_t {
     /// Whether we are running a subshell command.
     bool is_subshell{false};
 
-    /// Whether we are running a block of commands.
-    bool is_block{false};
-
     /// Whether we are running due to a `breakpoint` command.
     bool is_breakpoint{false};
 
@@ -340,6 +337,10 @@ class parser_t : public std::enable_shared_from_this<parser_t> {
     /// Returns the current line number.
     int get_lineno() const;
 
+    /// \return whether we are currently evaluating a "block" such as an if statement.
+    /// This supports 'status is-block'.
+    bool is_block() const;
+
     /// Returns the block at the given index. 0 corresponds to the innermost block. Returns nullptr
     /// when idx is at or equal to the number of blocks.
     const block_t *block_at_index(size_t idx) const;