Merge branch 'eval_parser'

Implements `eval` in cpp rather than as a fish function.

share/functions/eval.fish

@@ -1,71 +1,3 @@
-function eval -S -d "Evaluate parameters as a command"
+function eval
-    # keep a copy of the previous $status and use restore_status
-    # to preserve the status in case the block that is evaluated
-    # does not modify the status itself.
-    set -l status_copy $status
-    function __fish_restore_status
-        return $argv[1]
-    end
 
-    if not set -q argv[2]
-        # like most builtins, we only check for -h/--help
-        # if we only have a single argument
-        switch "$argv[1]"
-            case -h --help
-                __fish_print_help eval
-                return 0
-        end
-    end
-
-    if not string length -q -- $argv
-        # If the argument is empty, eval should return 0 for compatibility with other shells.
-        # See #5692.
-        return 0
-    end
-
-    # If we are in an interactive shell, eval should enable full
-    # job control since it should behave like the real code was
-    # executed.  If we don't do this, commands that expect to be
-    # used interactively, like less, wont work using eval.
-
-    set -l mode
-    if status --is-interactive-job-control
-        set mode interactive
-    else
-        if status --is-full-job-control
-            set mode full
-        else
-            set mode none
-        end
-    end
-    if status --is-interactive
-        status --job-control full
-    end
-    __fish_restore_status $status_copy
-
-    # To eval 'foo', we construct a block "begin ; foo; end <&3 3<&-"
-    # Note the redirections are also within the quotes.
-    #
-    # We then pipe this to 'source 3<&0’.
-    #
-    # You might expect that the dup2(3, stdin) should overwrite stdin,
-    # and therefore prevent 'source' from reading the piped-in block. This doesn't happen
-    # because when you pipe to a builtin, we don't overwrite stdin with the read end
-    # of the block; instead we set a separate fd in a variable 'builtin_stdin', which is
-    # what it reads from. So builtins are magic in that, in pipes, their stdin
-    # is not fd 0.
-    #
-    # ‘source’ does not apply the redirections to itself. Instead it saves them and passes
-    # them as block-level redirections to parser.eval(). Ultimately the eval’d code sees
-    # the following redirections (in the following order):
-    #    dup2 0 -> 3
-    #    dup2 pipe -> 0
-    #    dup2 3 -> 0
-    # where the pipe is the pipe we get from piping ‘echo’ to ‘source’. Thus the redirection
-    # effectively makes stdin fd0, instead of the thing that was piped to ‘source’
-    echo "begin; $argv "\n" ;end <&3 3<&-" | source 3<&0
-    set -l res $status
-
-    status --job-control $mode
-    return $res
 end

src/complete.cpp

@@ -1465,6 +1465,14 @@ void completer_t::perform() {
                 use_abbr = false;
                 break;
             }
+            case parse_statement_decoration_eval: {
+                use_command = true;
+                use_function = true;
+                use_builtin = true;
+                use_implicit_cd = true;
+                use_abbr = true;
+                break;
+            }
         }
 
         if (cmd_node.location_in_or_at_end_of_source_range(pos)) {

src/exec.cpp

@@ -6,6 +6,7 @@
 
 #include <errno.h>
 #include <fcntl.h>
+#include <numeric>
 #ifdef HAVE_SIGINFO_H
 #include <siginfo.h>
 #endif
@@ -903,6 +904,7 @@ static bool exec_process_in_job(parser_t &parser, process_t *p, std::shared_ptr<
 
     // The IO chain for this process.
     io_chain_t process_net_io_chain = j->block_io_chain();
+    auto cached_status = proc_get_last_status();
 
     if (pipes.write.valid()) {
         process_net_io_chain.push_back(std::make_shared<io_pipe_t>(
@@ -981,6 +983,29 @@ static bool exec_process_in_job(parser_t &parser, process_t *p, std::shared_ptr<
                 "Aborting.");
             break;
         }
+
+        case process_type_t::eval: {
+            // int eval(const wcstring &cmd, const io_chain_t &io, enum block_type_t block_type);
+            bool has_args = false;
+            wcstring new_cmd;
+            for (const wchar_t * const* arg = p->get_argv() + 1; *arg != nullptr; ++arg) {
+                has_args = true;
+                new_cmd += L' ';
+                new_cmd += *arg;
+            }
+
+            // `eval` is not supposed to error or do anything at all if no arguments are provided,
+            // or if it is used to execute a function that wouldn't have changed the status code
+            // (e.g. an empty function) if it were executed normally.
+            j->processes[0]->completed = true;
+            p->status = proc_status_t::from_exit_code(cached_status);
+
+            if (has_args) {
+                parser.eval(new_cmd.c_str(), process_net_io_chain, block_type_t::TOP);
+                p->status = proc_status_t::from_exit_code(proc_get_last_status());
+            }
+            break;
+        }
     }
     return true;
 }

src/parse_constants.h

@@ -97,7 +97,6 @@ const enum_map<parse_token_type_t> token_enum_map[] = {
 //
 // IMPORTANT: These enums must start at zero.
 enum parse_keyword_t {
-    parse_keyword_none = 0,
     parse_keyword_and,
     parse_keyword_begin,
     parse_keyword_builtin,
@@ -105,13 +104,15 @@ enum parse_keyword_t {
     parse_keyword_command,
     parse_keyword_else,
     parse_keyword_end,
+    parse_keyword_eval,
+    parse_keyword_exclam,
     parse_keyword_exec,
     parse_keyword_for,
     parse_keyword_function,
     parse_keyword_if,
     parse_keyword_in,
+    parse_keyword_none,
     parse_keyword_not,
-    parse_keyword_exclam,
     parse_keyword_or,
     parse_keyword_switch,
     parse_keyword_while,
@@ -125,6 +126,7 @@ const enum_map<parse_keyword_t> keyword_enum_map[] = {{parse_keyword_exclam, L"!
                                                       {parse_keyword_command, L"command"},
                                                       {parse_keyword_else, L"else"},
                                                       {parse_keyword_end, L"end"},
+                                                      {parse_keyword_eval, L"eval"},
                                                       {parse_keyword_exec, L"exec"},
                                                       {parse_keyword_for, L"for"},
                                                       {parse_keyword_function, L"function"},
@@ -144,7 +146,8 @@ enum parse_statement_decoration_t {
     parse_statement_decoration_none,
     parse_statement_decoration_command,
     parse_statement_decoration_builtin,
-    parse_statement_decoration_exec
+    parse_statement_decoration_exec,
+    parse_statement_decoration_eval,
 };
 
 // Boolean statement types, stored in node tag.

src/parse_execution.cpp

@@ -151,22 +151,30 @@ process_type_t parse_execution_context_t::process_type_for_command(
     // etc).
     enum parse_statement_decoration_t decoration = get_decoration(statement);
 
-    if (decoration == parse_statement_decoration_exec) {
+    switch (decoration) {
-        // Always exec.
+        case parse_statement_decoration_exec:
             process_type = process_type_t::exec;
-    } else if (decoration == parse_statement_decoration_command) {
+            break;
-        // Always a command.
+        case parse_statement_decoration_command:
             process_type = process_type_t::external;
-    } else if (decoration == parse_statement_decoration_builtin) {
+            break;
-        // What happens if this builtin is not valid?
+        case parse_statement_decoration_builtin:
             process_type = process_type_t::builtin;
-    } else if (function_exists(cmd)) {
+            break;
+        case parse_statement_decoration_eval:
+            process_type = process_type_t::eval;
+            break;
+        case parse_statement_decoration_none:
+            if (function_exists(cmd)) {
                 process_type = process_type_t::function;
             } else if (builtin_exists(cmd)) {
                 process_type = process_type_t::builtin;
             } else {
                 process_type = process_type_t::external;
             }
+            break;
+    }
+
     return process_type;
 }
 

src/parse_grammar.h

@@ -334,7 +334,16 @@ DEF_ALT(decorated_statement) {
     using cmds = seq<keyword<parse_keyword_command>, plain_statement>;
     using builtins = seq<keyword<parse_keyword_builtin>, plain_statement>;
     using execs = seq<keyword<parse_keyword_exec>, plain_statement>;
-    ALT_BODY(decorated_statement, plains, cmds, builtins, execs);
+    // Ideally, `evals` should be defined as `seq<keyword<parse_keyword_eval>,
+    // arguments_or_redirections_list`, but other parts of the code have the logic hard coded to
+    // search for a process at the head of a statement, and bug out if we do that.
+    // We also can't define `evals` as a `seq<keyword<parse_keyword_eval>, plain_statement>` because
+    // `expand.cpp` hard-codes its "command substitution at the head of a statement is not allowed"
+    // check without any way of telling it to perform the substitution anyway. Our solution is to
+    // create an empty function called `eval` that never actually gets executed and convert a
+    // decorated statement `eval ...` into a plain statement `eval ...`
+    using evals = seq<plain_statement>;
+    ALT_BODY(decorated_statement, plains, cmds, builtins, execs, evals);
 };
 
 DEF(plain_statement)

src/parse_productions.cpp

@@ -314,6 +314,10 @@ RESOLVE(decorated_statement) {
             *out_tag = parse_statement_decoration_exec;
             return production_for<execs>();
         }
+        case parse_keyword_eval: {
+            *out_tag = parse_statement_decoration_eval;
+            return production_for<evals>();
+        }
         default: {
             *out_tag = parse_statement_decoration_none;
             return production_for<plains>();

src/proc.cpp

@@ -477,6 +477,9 @@ static bool process_clean_after_marking(bool allow_interactive) {
     static std::vector<shared_ptr<job_t>> erase_list;
     const bool only_one_job = jobs().size() == 1;
     for (const auto &j : jobs()) {
+        if (!j->is_constructed()) {
+            continue;
+        }
         // If we are reaping only jobs who do not need status messages sent to the console, do not
         // consider reaping jobs that need status messages.
         if ((!j->get_flag(job_flag_t::SKIP_NOTIFICATION)) && (!interactive) &&

src/proc.h

@@ -36,6 +36,8 @@ enum class process_type_t {
     block_node,
     /// The exec builtin.
     exec,
+    /// A literal evaluation
+    eval,
 };
 
 enum class job_control_t {

tests/eval_scope.in

@@ -0,0 +1,3 @@
+# Regression test for issue #4443
+eval set -l previously_undefined foo
+echo $previously_undefined

tests/eval_scope.out

@@ -0,0 +1 @@
+foo