Fix issues related to redirections and block level IO with new parser

exec.cpp

@@ -820,7 +820,6 @@ void exec_job(parser_t &parser, job_t *j)
         {
             pipe_write.reset(new io_pipe_t(p->pipe_write_fd, false));
             process_net_io_chain.push_back(pipe_write);
-
         }
 
         /* The explicit IO redirections associated with the process */
@@ -1156,7 +1155,7 @@ void exec_job(parser_t &parser, job_t *j)
                       No buffer, so we exit directly. This means we
                       have to manually set the exit status.
                     */
-                    if (p->next == 0)
+                    if (p->next == NULL)
                     {
                         proc_set_last_status(job_get_flag(j, JOB_NEGATE)?(!status):status);
                     }

parse_execution.cpp

@@ -16,7 +16,7 @@
 #include "path.h"
 
 
-parse_execution_context_t::parse_execution_context_t(const parse_node_tree_t &t, const wcstring &s, const io_chain_t &io, parser_t *p) : tree(t), src(s), block_io(io), parser(p), eval_level(0)
+parse_execution_context_t::parse_execution_context_t(const parse_node_tree_t &t, const wcstring &s, parser_t *p) : tree(t), src(s), parser(p), eval_level(0)
 {
 }
 
@@ -45,9 +45,9 @@ node_offset_t parse_execution_context_t::get_offset(const parse_node_t &node) co
 }
 
 
-bool parse_execution_context_t::should_cancel() const
+bool parse_execution_context_t::should_cancel_execution(const block_t *block) const
 {
-    return false;
+    return block && block->skip;
 }
 
 int parse_execution_context_t::run_if_statement(const parse_node_t &statement)
@@ -63,11 +63,11 @@ int parse_execution_context_t::run_if_statement(const parse_node_t &statement)
     const parse_node_t *job_list_to_execute = NULL;
     const parse_node_t *if_clause = get_child(statement, 0, symbol_if_clause);
     const parse_node_t *else_clause = get_child(statement, 1, symbol_else_clause);
-    for (;;)
+    while (! should_cancel_execution(ib))
     {
         assert(if_clause != NULL && else_clause != NULL);
         const parse_node_t &condition = *get_child(*if_clause, 1, symbol_job);
-        if (run_1_job(condition) == EXIT_SUCCESS)
+        if (run_1_job(condition, ib) == EXIT_SUCCESS)
         {
             /* condition succeeded */
             job_list_to_execute = get_child(*if_clause, 3, symbol_job_list);
@@ -103,7 +103,7 @@ int parse_execution_context_t::run_if_statement(const parse_node_t &statement)
     /* Execute any job list we got */
     if (job_list_to_execute != NULL)
     {
-        run_job_list(*job_list_to_execute);
+        run_job_list(*job_list_to_execute, ib);
     }
 
     /* Done */
@@ -122,7 +122,7 @@ int parse_execution_context_t::run_begin_statement(const parse_node_t &header, c
     parser->push_block(sb);
     
     /* Run the job list */
-    run_job_list(contents);
+    run_job_list(contents, sb);
     
     /* Pop the block */
     parser->pop_block(sb);
@@ -208,7 +208,6 @@ int parse_execution_context_t::run_for_statement(const parse_node_t &header, con
     
     /* Get the contents to iterate over. Here we could do something with unmatched_wildcard. However it seems nicer to not make for loops complain about this, i.e. just iterate over a potentially empty list
     */
-    const parse_node_t *unmatched_wildcard = NULL;
     wcstring_list_t argument_list = this->determine_arguments(header, NULL);
     
     for_block_t *fb = new for_block_t(for_var_name);
@@ -219,7 +218,7 @@ int parse_execution_context_t::run_for_statement(const parse_node_t &header, con
     fb->sequence = argument_list;
 
     /* Now drive the for loop. TODO: handle break, etc. */
-    while (! fb->sequence.empty())
+    while (! fb->sequence.empty() && ! should_cancel_execution(fb))
     {
         const wcstring &for_variable = fb->variable;
         const wcstring &val = fb->sequence.back();
@@ -228,7 +227,7 @@ int parse_execution_context_t::run_for_statement(const parse_node_t &header, con
         fb->loop_status = LOOP_NORMAL;
         fb->skip = 0;
         
-        this->run_job_list(block_contents);
+        this->run_job_list(block_contents, fb);
     }
     
     return proc_get_last_status();
@@ -280,11 +279,14 @@ int parse_execution_context_t::run_switch_statement(const parse_node_t &statemen
     }
     const wcstring &switch_value_expanded = switch_values_expanded.at(0).completion;
     
+    switch_block_t *sb = new switch_block_t(switch_value_expanded);
+    parser->push_block(sb);
+    
     if (! errored)
     {
         /* Expand case statements */
         const parse_node_t *case_item_list = get_child(statement, 3, symbol_case_item_list);
-        while (matching_case_item == NULL && case_item_list->child_count > 0)
+        while (matching_case_item == NULL && case_item_list->child_count > 0 && ! should_cancel_execution(sb))
         {
             if (case_item_list->production_idx == 2)
             {
@@ -328,34 +330,35 @@ int parse_execution_context_t::run_switch_statement(const parse_node_t &statemen
     {
         /* Success, evaluate the job list */
         const parse_node_t *job_list = get_child(*matching_case_item, 3, symbol_job_list);
-        this->run_job_list(*job_list);
+        this->run_job_list(*job_list, sb);
     }
 
+    parser->pop_block(sb);
+
     // Oops, this is stomping STATUS_WILDCARD_ERROR. TODO: Don't!
     if (errored)
         proc_set_last_status(STATUS_BUILTIN_ERROR);
     return proc_get_last_status();
 }
 
-int parse_execution_context_t::run_while_statement(const parse_node_t &header, const parse_node_t &statement)
+int parse_execution_context_t::run_while_statement(const parse_node_t &header, const parse_node_t &block_contents)
 {
     assert(header.type == symbol_while_header);
-    assert(statement.type == symbol_block_statement);
+    assert(block_contents.type == symbol_job_list);
     
     /* Push a while block */
     while_block_t *wb = new while_block_t();
     wb->status = WHILE_TEST_FIRST;
-    wb->node_offset = this->get_offset(statement);
+    wb->node_offset = this->get_offset(header);
     parser->push_block(wb);
     
     /* The condition and contents of the while loop, as a job and job list respectively */
     const parse_node_t &while_condition = *get_child(header, 1, symbol_job);
-    const parse_node_t &block_contents = *get_child(statement, 2, symbol_job_list);
     
     /* A while loop is a while loop! */
-    while (! this->should_cancel() && this->run_1_job(while_condition) == EXIT_SUCCESS)
+    while (! this->should_cancel_execution(wb) && this->run_1_job(while_condition, wb) == EXIT_SUCCESS)
     {
-        this->run_job_list(block_contents);
+        this->run_job_list(block_contents, wb);
     }
     
     /* Done */
@@ -388,6 +391,8 @@ bool parse_execution_context_t::append_unmatched_wildcard_error(const parse_node
     return append_error(unmatched_wildcard, WILDCARD_ERR_MSG, get_source(unmatched_wildcard).c_str());
 }
 
+
+
 /* Creates a 'normal' (non-block) process */
 process_t *parse_execution_context_t::create_plain_process(job_t *job, const parse_node_t &statement)
 {
@@ -565,13 +570,16 @@ wcstring_list_t parse_execution_context_t::determine_arguments(const parse_node_
     return argument_list;
 }
 
-bool parse_execution_context_t::determine_io_chain(const parse_node_t &statement, io_chain_t *out_chain)
+bool parse_execution_context_t::determine_io_chain(const parse_node_t &statement_node, io_chain_t *out_chain)
 {
     io_chain_t result;
     bool errored = false;
     
+    /* We are called with a statement of varying types. We require that the statement have an arguments_or_redirections_list child. */
+    const parse_node_t &args_and_redirections_list = tree.find_child(statement_node, symbol_arguments_or_redirections_list);
+    
     /* Get all redirection nodes underneath the statement */
-    const parse_node_tree_t::parse_node_list_t redirect_nodes = tree.find_nodes(statement, symbol_redirection);
+    const parse_node_tree_t::parse_node_list_t redirect_nodes = tree.find_nodes(args_and_redirections_list, symbol_redirection);
     for (size_t i=0; i < redirect_nodes.size(); i++)
     {
         const parse_node_t &redirect_node = *redirect_nodes.at(i);
@@ -699,9 +707,17 @@ process_t *parse_execution_context_t::create_block_process(job_t *job, const par
 {
     /* We handle block statements by creating INTERNAL_BLOCK_NODE, that will bounce back to us when it's time to execute them */
     assert(statement_node.type == symbol_block_statement || statement_node.type == symbol_if_statement || statement_node.type == symbol_switch_statement);
+    
+    /* The set of IO redirections that we construct for the process */
+    io_chain_t process_io_chain;
+    bool errored = ! this->determine_io_chain(statement_node, &process_io_chain);
+    if (errored)
+        return NULL;
+
     process_t *result = new process_t();
     result->type = INTERNAL_BLOCK_NODE;
     result->internal_block_node = this->get_offset(statement_node);
+    result->set_io_chain(process_io_chain);
     return result;
 }
 
@@ -777,6 +793,10 @@ bool parse_execution_context_t::populate_job_from_job_node(job_t *j, const parse
     {
         assert(job_cont->type == symbol_job_continuation);
         
+        /* Handle the pipe */
+        const parse_node_t &pipe_node = *get_child(*job_cont, 0, parse_token_type_pipe);
+        last_process->pipe_write_fd = fd_redirected_by_pipe(get_source(pipe_node));
+        
         /* Get the statement node and make a process from it */
         const parse_node_t *statement_node = get_child(*job_cont, 1, symbol_statement);
         assert(statement_node != NULL);
@@ -795,8 +815,20 @@ bool parse_execution_context_t::populate_job_from_job_node(job_t *j, const parse
     return ! process_errored;
 }
 
-int parse_execution_context_t::run_1_job(const parse_node_t &job_node)
+int parse_execution_context_t::run_1_job(const parse_node_t &job_node, const block_t *associated_block)
 {
+    bool log_it = false;
+    if (log_it)
+    {
+        fprintf(stderr, "%s: %ls\n", __FUNCTION__, get_source(job_node).c_str());
+    }
+
+
+    if (should_cancel_execution(associated_block))
+    {
+        return 1;
+    }
+    
     // Get terminal modes
     struct termios tmodes = {};
     if (get_is_interactive())
@@ -913,13 +945,13 @@ int parse_execution_context_t::run_1_job(const parse_node_t &job_node)
     return ret;
 }
 
-int parse_execution_context_t::run_job_list(const parse_node_t &job_list_node)
+int parse_execution_context_t::run_job_list(const parse_node_t &job_list_node, const block_t *associated_block)
 {
     assert(job_list_node.type == symbol_job_list);
     
     int result = 1;
     const parse_node_t *job_list = &job_list_node;
-    while (job_list != NULL)
+    while (job_list != NULL && ! should_cancel_execution(associated_block))
     {
         assert(job_list->type == symbol_job_list);
         
@@ -949,7 +981,7 @@ int parse_execution_context_t::run_job_list(const parse_node_t &job_list_node)
         
         if (job != NULL)
         {
-            result = this->run_1_job(*job);
+            result = this->run_1_job(*job, associated_block);
         }
     }
     
@@ -957,7 +989,7 @@ int parse_execution_context_t::run_job_list(const parse_node_t &job_list_node)
     return result;
 }
 
-int parse_execution_context_t::eval_node_at_offset(node_offset_t offset)
+int parse_execution_context_t::eval_node_at_offset(node_offset_t offset, const block_t *associated_block, const io_chain_t &io)
 {
     bool log_it = false;
     
@@ -965,6 +997,9 @@ int parse_execution_context_t::eval_node_at_offset(node_offset_t offset)
     assert(! tree.empty());
     assert(offset < tree.size());
     
+    /* Apply this block IO for the duration of this function */
+    scoped_push<io_chain_t> block_io_push(&block_io, io);
+    
     const parse_node_t &node = tree.at(offset);
     
     if (log_it)
@@ -982,9 +1017,9 @@ int parse_execution_context_t::eval_node_at_offset(node_offset_t offset)
     switch (node.type)
     {
         case symbol_job_list:
-            /* We should only get a job list if it's top level. This is because this is the entry point for both top-level execution (the first node) and INTERNAL_BLOCK_NODE execution (which does block statements, but never job lists) */
+            /* We should only get a job list if it's the very first node. This is because this is the entry point for both top-level execution (the first node) and INTERNAL_BLOCK_NODE execution (which does block statements, but never job lists) */
             assert(offset == 0);
-            ret = this->run_job_list(node);
+            ret = this->run_job_list(node, associated_block);
             break;
         
         case symbol_block_statement:

parse_execution.h

@@ -13,13 +13,14 @@
 
 class job_t;
 struct profile_item_t;
+struct block_t;
 
 class parse_execution_context_t
 {
     private:
     const parse_node_tree_t tree;
     const wcstring src;
-    const io_chain_t block_io;
+    io_chain_t block_io;
     parser_t * const parser;
     parse_error_list_t errors;
     
@@ -30,8 +31,8 @@ class parse_execution_context_t
     parse_execution_context_t(const parse_execution_context_t&);
     parse_execution_context_t& operator=(const parse_execution_context_t&);
     
-    /* Should I cancel */
+    /* Should I cancel? */
-    bool should_cancel() const;
+    bool should_cancel_execution(const block_t *block) const;
     
     /* Report an error. Always returns true. */
     bool append_error(const parse_node_t &node, const wchar_t *fmt, ...);
@@ -63,15 +64,15 @@ class parse_execution_context_t
     /* Determines the IO chain. Returns true on success, false on error */
     bool determine_io_chain(const parse_node_t &statement, io_chain_t *out_chain);
     
-    int run_1_job(const parse_node_t &job_node);
+    int run_1_job(const parse_node_t &job_node, const block_t *associated_block);
-    int run_job_list(const parse_node_t &job_list_node);
+    int run_job_list(const parse_node_t &job_list_node, const block_t *associated_block);
     bool populate_job_from_job_node(job_t *j, const parse_node_t &job_node);
     
     public:
-    parse_execution_context_t(const parse_node_tree_t &t, const wcstring &s, const io_chain_t &io, parser_t *p);
+    parse_execution_context_t(const parse_node_tree_t &t, const wcstring &s, parser_t *p);
     
     /* Start executing at the given node offset, returning the exit status of the last process. */
-    int eval_node_at_offset(node_offset_t offset);
+    int eval_node_at_offset(node_offset_t offset, const block_t *associated_block, const io_chain_t &io);
     
 };
 

parse_tree.h

@@ -259,7 +259,6 @@ public:
     boolean_statement = AND statement | OR statement | NOT statement
 
 # A decorated_statement is a command with a list of arguments_or_redirections, possibly with "builtin" or "command"
-# TODO: we should be able to construct plain_statements out of e.g. 'command --help' or even just 'command'
 
     decorated_statement = plain_statement | COMMAND plain_statement | BUILTIN plain_statement
     plain_statement = <TOK_STRING> arguments_or_redirections_list optional_background

parser.cpp

@@ -2610,14 +2610,14 @@ int parser_t::eval_new_parser(const wcstring &cmd, const io_chain_t &io, enum bl
     }
     
     /* Append to the execution context stack */
-    parse_execution_context_t *ctx = new parse_execution_context_t(tree, cmd, io, this);
+    parse_execution_context_t *ctx = new parse_execution_context_t(tree, cmd, this);
     execution_contexts.push_back(ctx);
     
     /* Execute the first node */
     int result = 1;
     if (! tree.empty())
     {
-        result = this->eval_block_node(0, io_chain_t(), block_type);
+        result = this->eval_block_node(0, io, block_type);
     }
     
     /* Clean up the execution context stack */
@@ -2652,8 +2652,9 @@ int parser_t::eval_block_node(node_offset_t node_idx, const io_chain_t &io, enum
     
     /* Start it up */
     const block_t * const start_current_block = current_block();
-    this->push_block(new scope_block_t(block_type));
+    block_t *scope_block = new scope_block_t(block_type);
-    int result = ctx->eval_node_at_offset(node_idx);
+    this->push_block(scope_block);
+    int result = ctx->eval_node_at_offset(node_idx, scope_block, io);
     
     /* Clean up the block stack */
     this->pop_block();
@@ -3138,7 +3139,7 @@ bool parser_use_ast(void)
     env_var_t var = env_get_string(L"fish_new_parser");
     if (var.missing_or_empty())
     {
-        return false;
+        return 10;
     }
     else
     {

tokenizer.cpp

@@ -535,6 +535,23 @@ enum token_type redirection_type_for_string(const wcstring &str, int *out_fd)
     return mode;
 }
 
+int fd_redirected_by_pipe(const wcstring &str)
+{
+    /* Hack for the common case */
+    if (str == L"|")
+    {
+        return STDOUT_FILENO;
+    }
+    
+    enum token_type mode = TOK_NONE;
+    int fd = 0;
+    read_redirection_or_fd_pipe(str.c_str(), &mode, &fd);
+    /* Pipes only */
+    if (mode != TOK_PIPE || fd < 0)
+        fd = -1;
+    return fd;
+}
+
 int oflags_for_redirection_type(enum token_type type)
 {
     switch (type)

tokenizer.h

@@ -190,6 +190,9 @@ int tok_get_error(tokenizer_t *tok);
 /* Helper function to determine redirection type from a string, or TOK_NONE if the redirection is invalid. Also returns the fd by reference. */
 enum token_type redirection_type_for_string(const wcstring &str, int *out_fd = NULL);
 
+/* Helper function to determine which fd is redirected by a pipe */
+int fd_redirected_by_pipe(const wcstring &str);
+
 /* Helper function to return oflags (as in open(2)) for a redirection type */
 int oflags_for_redirection_type(enum token_type type);