Stop creating subclasses of block_t

Move all block_t creation methods to static methods, and stop creating
subclasses (all of which are now empty).

src/builtin.cpp

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

src/builtin_source.cpp

@@ -73,7 +73,7 @@ int builtin_source(parser_t &parser, io_streams_t &streams, wchar_t **argv) {
         fn_intern = intern(argv[optind]);
     }
 
-    const source_block_t *sb = parser.push_block<source_block_t>(fn_intern);
+    const block_t *sb = parser.push_block(block_t::source_block(fn_intern));
     reader_push_current_filename(fn_intern);
 
     // This is slightly subtle. If this is a bare `source` with no args then `argv + optind` already

src/event.cpp

@@ -284,7 +284,7 @@ static void event_fire_internal(const event_t &event) {
         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);
+        block_t *b = parser.push_block(block_t::event_block(event));
         parser.eval(buffer, io_chain_t(), TOP);
         parser.pop_block(b);
         proc_pop_interactive();

src/exec.cpp

@@ -822,8 +822,8 @@ static bool exec_block_or_func_process(parser_t &parser, std::shared_ptr<job_t>
         wcstring_list_t argv = p->get_argv_array().to_list();
         // Remove the function name from argv.
         if (!argv.empty()) argv.erase(argv.begin());
-        function_block_t *fb =
-            parser.push_block<function_block_t>(func_name, argv, props->shadow_scope);
+        block_t *fb =
+            parser.push_block(block_t::function_block(func_name, argv, props->shadow_scope));
         function_prepare_environment(parser.vars(), func_name, std::move(argv), inherit_vars);
         parser.forbid_function(func_name);
 

src/parse_execution.cpp

@@ -288,7 +288,7 @@ parse_execution_result_t parse_execution_context_t::run_if_statement(
 
     // Execute any job list we got.
     if (job_list_to_execute) {
-        if_block_t *ib = parser->push_block<if_block_t>();
+        block_t *ib = parser->push_block(block_t::if_block());
         run_job_list(job_list_to_execute, ib);
         if (should_cancel_execution(ib)) {
             result = parse_execution_cancelled;
@@ -311,7 +311,7 @@ parse_execution_result_t parse_execution_context_t::run_if_statement(
 parse_execution_result_t parse_execution_context_t::run_begin_statement(
     tnode_t<g::job_list> contents) {
     // Basic begin/end block. Push a scope block, run jobs, pop it
-    scope_block_t *sb = parser->push_block<scope_block_t>(BEGIN);
+    block_t *sb = parser->push_block(block_t::scope_block(BEGIN));
     parse_execution_result_t ret = run_job_list(contents, sb);
     parser->pop_block(sb);
 
@@ -408,7 +408,7 @@ parse_execution_result_t parse_execution_context_t::run_for_statement(
         return parse_execution_errored;
     }
 
-    for_block_t *fb = parser->push_block<for_block_t>();
+    block_t *fb = parser->push_block(block_t::for_block());
 
     // Now drive the for loop.
     for (const wcstring &val : arguments) {
@@ -486,7 +486,7 @@ parse_execution_result_t parse_execution_context_t::run_switch_statement(
 
     const wcstring &switch_value_expanded = switch_values_expanded.at(0).completion;
 
-    switch_block_t *sb = parser->push_block<switch_block_t>();
+    block_t *sb = parser->push_block(block_t::switch_block());
 
     // Expand case statements.
     tnode_t<g::case_item_list> case_item_list = statement.child<3>();
@@ -585,7 +585,7 @@ parse_execution_result_t parse_execution_context_t::run_while_statement(
         // Push a while block and then check its cancellation reason.
         auto &ld = parser->libdata();
         ld.loop_status = loop_status_t::normals;
-        while_block_t *wb = parser->push_block<while_block_t>();
+        block_t *wb = parser->push_block(block_t::while_block());
         this->run_job_list(contents, wb);
         auto cancel_reason = this->cancellation_reason(wb);
         parser->pop_block(wb);

src/parser.cpp

@@ -119,7 +119,8 @@ void parser_t::skip_all_blocks() {
 }
 
 // Given a new-allocated block, push it onto our block stack, acquiring ownership
-void parser_t::push_block_int(block_t *new_current) {
+block_t *parser_t::push_block(block_t &&block) {
+    std::unique_ptr<block_t> new_current = make_unique<block_t>(std::move(block));
     const enum block_type_t type = new_current->type();
     new_current->src_lineno = parser_t::get_lineno();
 
@@ -138,9 +139,6 @@ void parser_t::push_block_int(block_t *new_current) {
         new_current->skip = false;
     }
 
-    // Push it onto our stack. This acquires ownership because of unique_ptr.
-    this->block_stack.emplace_back(new_current);
-
     // Types TOP and SUBST are not considered blocks for the purposes of `status is-block`.
     if (type != TOP && type != SUBST) {
         libdata().is_block = true;
@@ -154,6 +152,10 @@ void parser_t::push_block_int(block_t *new_current) {
         vars().push(type == FUNCTION_CALL);
         new_current->wants_pop_env = true;
     }
+
+    // Push it onto our stack and return it.
+    this->block_stack.push_back(std::move(new_current));
+    return this->block_stack.back().get();
 }
 
 void parser_t::pop_block(const block_t *expected) {
@@ -666,7 +668,7 @@ int parser_t::eval_node(parsed_source_ref_t ps, tnode_t<T> node, const io_chain_
     job_reap(*this, false);  // not sure why we reap jobs here
 
     // Start it up
-    scope_block_t *scope_block = this->push_block<scope_block_t>(block_type);
+    block_t *scope_block = this->push_block(block_t::scope_block(block_type));
 
     // Create and set a new execution context.
     using exc_ctx_ref_t = std::unique_ptr<parse_execution_context_t>;
@@ -835,6 +837,36 @@ wcstring block_t::description() const {
 
 // Various block constructors.
 
+block_t block_t::if_block() { return block_t(IF); }
+
+block_t block_t::event_block(event_t evt) {
+    block_t b{EVENT};
+    b.event = std::move(evt);
+    return b;
+}
+
+block_t block_t::function_block(wcstring name, wcstring_list_t args, bool shadows) {
+    block_t b{shadows ? FUNCTION_CALL : FUNCTION_CALL_NO_SHADOW};
+    b.function_name = std::move(name);
+    b.function_args = std::move(args);
+    return b;
+}
+
+block_t block_t::source_block(const wchar_t *src) {
+    block_t b{SOURCE};
+    b.sourced_file = src;
+    return b;
+}
+
+block_t block_t::for_block() { return block_t{FOR}; }
+block_t block_t::while_block() { return block_t{WHILE}; }
+block_t block_t::switch_block() { return block_t{SWITCH}; }
+block_t block_t::scope_block(block_type_t type) {
+    assert((type == BEGIN || type == TOP || type == SUBST) && "Invalid scope type");
+    return block_t(type);
+}
+block_t block_t::breakpoint_block() { return block_t(BREAKPOINT); }
+
 if_block_t::if_block_t() : block_t(IF) {}
 
 event_block_t::event_block_t(const event_t &evt) : block_t(EVENT) { this->event = evt; }

src/parser.h

@@ -92,6 +92,17 @@ struct block_t {
     /// Description of the block, for debugging.
     wcstring description() const;
 
+    /// Entry points for creating blocks.
+    static block_t if_block();
+    static block_t event_block(event_t evt);
+    static block_t function_block(wcstring name, wcstring_list_t args, bool shadows);
+    static block_t source_block(const wchar_t *src);
+    static block_t for_block();
+    static block_t while_block();
+    static block_t switch_block();
+    static block_t scope_block(block_type_t type);
+    static block_t breakpoint_block();
+
     /// Destructor
     virtual ~block_t();
 };
@@ -230,9 +241,6 @@ class parser_t : public std::enable_shared_from_this<parser_t> {
     /// Helper for stack_trace().
     void stack_trace_internal(size_t block_idx, wcstring *out) const;
 
-    /// Helper for push_block()
-    void push_block_int(block_t *b);
-
     /// Create a parser.
     parser_t();
 
@@ -314,15 +322,9 @@ class parser_t : public std::enable_shared_from_this<parser_t> {
     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()
-    template <typename BLOCKTYPE, typename... Args>
-    BLOCKTYPE *push_block(Args &&... args) {
-        BLOCKTYPE *ret = new BLOCKTYPE(std::forward<Args>(args)...);
-        this->push_block_int(ret);
-        return ret;
-    }
+    /// Pushes a new block. Returns a pointer to the block, stored in the parser. The pointer is
+    /// valid until the call to pop_block()
+    block_t *push_block(block_t &&b);
 
     /// Remove the outermost block, asserting it's the given one.
     void pop_block(const block_t *b);