From 686b84396ca50514b64f7bcb26eab73618c9f913 Mon Sep 17 00:00:00 2001
From: ridiculousfish <corydoras@ridiculousfish.com>
Date: Sun, 19 May 2019 19:29:25 -0700
Subject: [PATCH] Migrate the return bool outside of block_t

This is a flag that gets set by the return function. But we only need one,
not per-block. Move it into libdata.
---
 src/builtin_return.cpp  |  7 ++-----
 src/exec.cpp            |  3 +++
 src/parse_execution.cpp |  5 +++--
 src/parser.cpp          | 10 ----------
 src/parser.h            |  5 +++--
 5 files changed, 11 insertions(+), 19 deletions(-)

diff --git a/src/builtin_return.cpp b/src/builtin_return.cpp
index b157c04fd..c8873e2bc 100644
--- a/src/builtin_return.cpp
+++ b/src/builtin_return.cpp
@@ -102,11 +102,8 @@ int builtin_return(parser_t &parser, io_streams_t &streams, wchar_t **argv) {
         return STATUS_CMD_ERROR;
     }
 
-    // Skip everything up to and including the function block.
-    for (size_t i = 0; i <= function_block_idx; i++) {
-        block_t *b = parser.block_at_index(i);
-        b->skip = true;
-    }
+    // Mark a return in the libdata.
+    parser.libdata().returning = true;
 
     return retval;
 }
diff --git a/src/exec.cpp b/src/exec.cpp
index 894a83250..68d9fb949 100644
--- a/src/exec.cpp
+++ b/src/exec.cpp
@@ -831,6 +831,9 @@ static bool exec_block_or_func_process(parser_t &parser, std::shared_ptr<job_t>
 
         parser.allow_function();
         parser.pop_block(fb);
+
+        // If we returned due to a return statement, then stop returning now.
+        parser.libdata().returning = false;
     } else {
         assert(p->type == process_type_t::block_node);
         assert(p->block_node_source && p->internal_block_node && "Process is missing node info");
diff --git a/src/parse_execution.cpp b/src/parse_execution.cpp
index c46cf2471..2ca3cb585 100644
--- a/src/parse_execution.cpp
+++ b/src/parse_execution.cpp
@@ -189,10 +189,11 @@ parse_execution_context_t::cancellation_reason(const block_t *block) const {
     if (parser && parser->cancellation_requested) {
         return execution_cancellation_skip;
     }
-    if (block && block->skip) {
+    const auto &ld = parser->libdata();
+    if (ld.returning) {
         return execution_cancellation_skip;
     }
-    if (parser->libdata().loop_status != loop_status_t::normals) {
+    if (ld.loop_status != loop_status_t::normals) {
         return execution_cancellation_loop_control;
     }
     return execution_cancellation_none;
diff --git a/src/parser.cpp b/src/parser.cpp
index afbc3700c..e3def49ed 100644
--- a/src/parser.cpp
+++ b/src/parser.cpp
@@ -133,16 +133,6 @@ block_t *parser_t::push_block(block_t &&block) {
         new_current.src_filename = intern(filename);
     }
 
-    // New blocks should be skipped if the outer block is skipped, except TOP and SUBST block, which
-    // open up new environments.
-    const block_t *old_current = this->current_block();
-    new_current.skip = old_current && old_current->skip;
-
-    // Type TOP and SUBST are never skipped.
-    if (type == TOP || type == SUBST) {
-        new_current.skip = false;
-    }
-
     // Types TOP and SUBST are not considered blocks for the purposes of `status is-block`.
     if (type != TOP && type != SUBST) {
         libdata().is_block = true;
diff --git a/src/parser.h b/src/parser.h
index 0874b6ffb..e2661df47 100644
--- a/src/parser.h
+++ b/src/parser.h
@@ -61,8 +61,6 @@ class block_t {
     const block_type_t block_type;
 
    public:
-    /// Whether execution of the commands in this block should be skipped.
-    bool skip{false};
     /// 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.
@@ -155,6 +153,9 @@ struct library_data_t {
 
     /// Whether we should break or continue the current loop.
     enum loop_status_t loop_status { loop_status_t::normals };
+
+    /// Whether we should return from the current function.
+    bool returning{false};
 };
 
 class parser_t : public std::enable_shared_from_this<parser_t> {