builtins: port return/exit to rust

CMakeLists.txt

@@ -104,11 +104,11 @@ set(FISH_BUILTIN_SRCS
     src/builtins/builtin.cpp src/builtins/cd.cpp src/builtins/command.cpp
     src/builtins/commandline.cpp src/builtins/complete.cpp src/builtins/contains.cpp
     src/builtins/disown.cpp
-    src/builtins/eval.cpp src/builtins/exit.cpp src/builtins/fg.cpp
+    src/builtins/eval.cpp src/builtins/fg.cpp
     src/builtins/function.cpp src/builtins/functions.cpp src/builtins/history.cpp
     src/builtins/jobs.cpp src/builtins/math.cpp src/builtins/printf.cpp src/builtins/path.cpp
     src/builtins/pwd.cpp src/builtins/random.cpp src/builtins/read.cpp
-    src/builtins/realpath.cpp src/builtins/return.cpp src/builtins/set.cpp
+    src/builtins/realpath.cpp src/builtins/set.cpp
     src/builtins/set_color.cpp src/builtins/source.cpp src/builtins/status.cpp
     src/builtins/string.cpp src/builtins/test.cpp src/builtins/type.cpp src/builtins/ulimit.cpp
 )

fish-rust/src/builtins/exit.rs

@@ -0,0 +1,26 @@
+use libc::c_int;
+
+use super::r#return::parse_return_value;
+use super::shared::io_streams_t;
+use crate::ffi::{parser_t, Repin};
+use crate::wchar::wstr;
+
+/// Function for handling the exit builtin.
+pub fn exit(
+    parser: &mut parser_t,
+    streams: &mut io_streams_t,
+    args: &mut [&wstr],
+) -> Option<c_int> {
+    let retval = match parse_return_value(args, parser, streams) {
+        Ok(v) => v,
+        Err(e) => return e,
+    };
+
+    // Mark that we are exiting in the parser.
+    // TODO: in concurrent mode this won't successfully exit a pipeline, as there are other parsers
+    // involved. That is, `exit | sleep 1000` may not exit as hoped. Need to rationalize what
+    // behavior we want here.
+    parser.pin().libdata().set_exit_current_script(true);
+
+    return Some(retval);
+}

fish-rust/src/builtins/mod.rs

@@ -2,4 +2,6 @@ pub mod shared;
 
 pub mod echo;
 pub mod emit;
+pub mod r#return;
 pub mod wait;
+mod exit;

fish-rust/src/builtins/return.rs

@@ -0,0 +1,130 @@
+// Implementation of the return builtin.
+
+use libc::c_int;
+use num_traits::abs;
+
+use super::shared::{
+    builtin_missing_argument, builtin_print_error_trailer, builtin_print_help, io_streams_t,
+    BUILTIN_ERR_NOT_NUMBER, STATUS_CMD_OK, STATUS_INVALID_ARGS,
+};
+use crate::builtins::shared::BUILTIN_ERR_TOO_MANY_ARGUMENTS;
+use crate::ffi::{parser_t, Repin};
+use crate::wchar::{wstr, L};
+use crate::wgetopt::{wgetopter_t, wopt, woption, woption_argument_t};
+use crate::wutil::fish_wcstoi;
+use crate::wutil::wgettext_fmt;
+
+#[derive(Debug, Clone, Copy, Default)]
+struct Options {
+    print_help: bool,
+}
+
+fn parse_options(
+    args: &mut [&wstr],
+    parser: &mut parser_t,
+    streams: &mut io_streams_t,
+) -> Result<(Options, usize), Option<c_int>> {
+    let cmd = args[0];
+
+    const SHORT_OPTS: &wstr = L!(":h");
+    const LONG_OPTS: &[woption] = &[wopt(L!("help"), woption_argument_t::no_argument, 'h')];
+
+    let mut opts = Options::default();
+
+    let mut w = wgetopter_t::new(SHORT_OPTS, LONG_OPTS, args);
+
+    while let Some(c) = w.wgetopt_long() {
+        match c {
+            'h' => opts.print_help = true,
+            ':' => {
+                builtin_missing_argument(parser, streams, cmd, args[w.woptind - 1], true);
+                return Err(STATUS_INVALID_ARGS);
+            }
+            '?' => {
+                // We would normally invoke builtin_unknown_option() and return an error.
+                // But for this command we want to let it try and parse the value as a negative
+                // return value.
+                return Ok((opts, w.woptind - 1));
+            }
+            _ => {
+                panic!("unexpected retval from wgetopt_long");
+            }
+        }
+    }
+
+    Ok((opts, w.woptind))
+}
+
+/// Function for handling the return builtin.
+pub fn r#return(
+    parser: &mut parser_t,
+    streams: &mut io_streams_t,
+    args: &mut [&wstr],
+) -> Option<c_int> {
+    let mut retval = match parse_return_value(args, parser, streams) {
+        Ok(v) => v,
+        Err(e) => return e,
+    };
+
+    let has_function_block = parser.ffi_has_funtion_block();
+
+    // *nix does not support negative return values, but our `return` builtin happily accepts being
+    // called with negative literals (e.g. `return -1`).
+    // Map negative values to (256 - their absolute value). This prevents `return -1` from
+    // evaluating to a `$status` of 0 and keeps us from running into undefined behavior by trying to
+    // left shift a negative value in W_EXITCODE().
+    if retval < 0 {
+        retval = 256 - (abs(retval) % 256);
+    }
+
+    // If we're not in a function, exit the current script (but not an interactive shell).
+    if !has_function_block {
+        if !parser.is_interactive() {
+            parser.pin().libdata().set_exit_current_script(true);
+        }
+        return Some(retval);
+    }
+
+    // Mark a return in the libdata.
+    parser.pin().libdata().set_returning(true);
+
+    return Some(retval);
+}
+
+pub fn parse_return_value(
+    args: &mut [&wstr],
+    parser: &mut parser_t,
+    streams: &mut io_streams_t,
+) -> Result<i32, Option<c_int>> {
+    let cmd = args[0];
+    let (opts, optind) = match parse_options(args, parser, streams) {
+        Ok((opts, optind)) => (opts, optind),
+        Err(err @ Some(_)) if err != STATUS_CMD_OK => return Err(err),
+        Err(err) => panic!("Illogical exit code from parse_options(): {err:?}"),
+    };
+    if opts.print_help {
+        builtin_print_help(parser, streams, cmd);
+        return Err(STATUS_CMD_OK);
+    }
+    if optind + 1 < args.len() {
+        streams
+            .err
+            .append(wgettext_fmt!(BUILTIN_ERR_TOO_MANY_ARGUMENTS, cmd));
+        builtin_print_error_trailer(parser, streams, cmd);
+        return Err(STATUS_INVALID_ARGS);
+    }
+    if optind == args.len() {
+        Ok(parser.get_last_status().into())
+    } else {
+        match fish_wcstoi(args[optind].chars()) {
+            Ok(i) => Ok(i),
+            Err(_e) => {
+                streams
+                    .err
+                    .append(wgettext_fmt!(BUILTIN_ERR_NOT_NUMBER, cmd, args[1]));
+                builtin_print_error_trailer(parser, streams, cmd);
+                return Err(STATUS_INVALID_ARGS);
+            }
+        }
+    }
+}

fish-rust/src/builtins/shared.rs

@@ -31,6 +31,12 @@ mod builtins_ffi {
     impl Vec<wcharz_t> {}
 }
 
+/// Error message when too many arguments are supplied to a builtin.
+pub const BUILTIN_ERR_TOO_MANY_ARGUMENTS: &str = "%ls: too many arguments\n";
+
+/// Error message when integer expected
+pub const BUILTIN_ERR_NOT_NUMBER: &str = "%ls: %ls: invalid integer\n";
+
 /// A handy return value for successful builtins.
 pub const STATUS_CMD_OK: Option<c_int> = Some(0);
 
@@ -111,6 +117,8 @@ pub fn run_builtin(
     match builtin {
         RustBuiltin::Echo => super::echo::echo(parser, streams, args),
         RustBuiltin::Emit => super::emit::emit(parser, streams, args),
+        RustBuiltin::Exit => super::exit::exit(parser, streams, args),
+        RustBuiltin::Return => super::r#return::r#return(parser, streams, args),
         RustBuiltin::Wait => wait::wait(parser, streams, args),
     }
 }
@@ -158,6 +166,10 @@ pub fn builtin_print_help(parser: &mut parser_t, streams: &io_streams_t, cmd: &w
     );
 }
 
+pub fn builtin_print_error_trailer(parser: &mut parser_t, streams: &mut io_streams_t, cmd: &wstr) {
+    ffi::builtin_print_error_trailer(parser.pin(), streams.err.ffi(), c_str!(cmd));
+}
+
 pub struct HelpOnlyCmdOpts {
     pub print_help: bool,
     pub optind: usize,

fish-rust/src/ffi.rs

@@ -48,6 +48,7 @@ include_cpp! {
     generate!("parser_t")
     generate!("job_t")
     generate!("process_t")
+    generate!("library_data_t")
 
     generate!("proc_wait_any")
 
@@ -61,6 +62,7 @@ include_cpp! {
     generate!("builtin_missing_argument")
     generate!("builtin_unknown_option")
     generate!("builtin_print_help")
+    generate!("builtin_print_error_trailer")
 
     generate!("wait_handle_t")
     generate!("wait_handle_store_t")

src/builtin.cpp

@@ -42,7 +42,6 @@
 #include "builtins/contains.h"
 #include "builtins/disown.h"
 #include "builtins/eval.h"
-#include "builtins/exit.h"
 #include "builtins/fg.h"
 #include "builtins/functions.h"
 #include "builtins/history.h"
@@ -54,7 +53,6 @@
 #include "builtins/random.h"
 #include "builtins/read.h"
 #include "builtins/realpath.h"
-#include "builtins/return.h"
 #include "builtins/set.h"
 #include "builtins/set_color.h"
 #include "builtins/shared.rs.h"
@@ -388,7 +386,7 @@ static constexpr builtin_data_t builtin_datas[] = {
     {L"end", &builtin_generic, N_(L"End a block of commands")},
     {L"eval", &builtin_eval, N_(L"Evaluate a string as a statement")},
     {L"exec", &builtin_generic, N_(L"Run command in current process")},
-    {L"exit", &builtin_exit, N_(L"Exit the shell")},
+    {L"exit", &implemented_in_rust, N_(L"Exit the shell")},
     {L"false", &builtin_false, N_(L"Return an unsuccessful result")},
     {L"fg", &builtin_fg, N_(L"Send job to foreground")},
     {L"for", &builtin_generic, N_(L"Perform a set of commands multiple times")},
@@ -406,7 +404,7 @@ static constexpr builtin_data_t builtin_datas[] = {
     {L"random", &builtin_random, N_(L"Generate random number")},
     {L"read", &builtin_read, N_(L"Read a line of input into variables")},
     {L"realpath", &builtin_realpath, N_(L"Show absolute path sans symlinks")},
-    {L"return", &builtin_return, N_(L"Stop the currently evaluated function")},
+    {L"return", &implemented_in_rust, N_(L"Stop the currently evaluated function")},
     {L"set", &builtin_set, N_(L"Handle environment variables")},
     {L"set_color", &builtin_set_color, N_(L"Set the terminal color")},
     {L"source", &builtin_source, N_(L"Evaluate contents of file")},
@@ -533,9 +531,15 @@ static maybe_t<RustBuiltin> try_get_rust_builtin(const wcstring &cmd) {
     if (cmd == L"emit") {
         return RustBuiltin::Emit;
     }
+    if (cmd == L"exit") {
+        return RustBuiltin::Exit;
+    }
     if (cmd == L"wait") {
         return RustBuiltin::Wait;
     }
+    if (cmd == L"return") {
+        return RustBuiltin::Return;
+    }
     return none();
 }
 

src/builtin.h

@@ -111,6 +111,8 @@ int parse_help_only_cmd_opts(help_only_cmd_opts_t &opts, int *optind, int argc,
 enum RustBuiltin : int32_t {
     Echo,
     Emit,
+    Exit,
     Wait,
+    Return,
 };
 #endif

src/builtins/exit.cpp

@@ -1,94 +0,0 @@
-// Implementation of the exit builtin.
-#include "config.h"  // IWYU pragma: keep
-
-#include "exit.h"
-
-#include <cerrno>
-
-#include "../builtin.h"
-#include "../common.h"
-#include "../fallback.h"  // IWYU pragma: keep
-#include "../io.h"
-#include "../maybe.h"
-#include "../parser.h"
-#include "../wgetopt.h"
-#include "../wutil.h"  // IWYU pragma: keep
-
-struct exit_cmd_opts_t {
-    bool print_help = false;
-};
-static const wchar_t *const short_options = L":h";
-static const struct woption long_options[] = {{L"help", no_argument, 'h'}, {}};
-
-static int parse_cmd_opts(exit_cmd_opts_t &opts, int *optind,  //!OCLINT(high ncss method)
-                          int argc, const wchar_t **argv, parser_t &parser, io_streams_t &streams) {
-    UNUSED(parser);
-    UNUSED(streams);
-    const wchar_t *cmd = argv[0];
-    int opt;
-    wgetopter_t w;
-    while ((opt = w.wgetopt_long(argc, argv, short_options, long_options, nullptr)) != -1) {
-        switch (opt) {  //!OCLINT(too few branches)
-            case 'h': {
-                opts.print_help = true;
-                break;
-            }
-            case ':': {
-                builtin_missing_argument(parser, streams, cmd, argv[w.woptind - 1]);
-                return STATUS_INVALID_ARGS;
-            }
-            case '?': {
-                // We would normally invoke builtin_unknown_option() and return an error.
-                // But for this command we want to let it try and parse the value as a negative
-                // return value.
-                *optind = w.woptind - 1;
-                return STATUS_CMD_OK;
-            }
-            default: {
-                DIE("unexpected retval from wgetopt_long");
-            }
-        }
-    }
-
-    *optind = w.woptind;
-    return STATUS_CMD_OK;
-}
-
-/// The exit builtin. Calls reader_exit to exit and returns the value specified.
-maybe_t<int> builtin_exit(parser_t &parser, io_streams_t &streams, const wchar_t **argv) {
-    const wchar_t *cmd = argv[0];
-    int argc = builtin_count_args(argv);
-    exit_cmd_opts_t opts;
-
-    int optind;
-    int retval = parse_cmd_opts(opts, &optind, argc, argv, parser, streams);
-    if (retval != STATUS_CMD_OK) return retval;
-
-    if (opts.print_help) {
-        builtin_print_help(parser, streams, cmd);
-        return STATUS_CMD_OK;
-    }
-
-    if (optind + 1 < argc) {
-        streams.err.append_format(BUILTIN_ERR_TOO_MANY_ARGUMENTS, cmd);
-        builtin_print_error_trailer(parser, streams.err, cmd);
-        return STATUS_INVALID_ARGS;
-    }
-
-    if (optind == argc) {
-        retval = parser.get_last_status();
-    } else {
-        retval = fish_wcstoi(argv[optind]);
-        if (errno) {
-            streams.err.append_format(BUILTIN_ERR_NOT_NUMBER, cmd, argv[optind]);
-            builtin_print_error_trailer(parser, streams.err, cmd);
-            return STATUS_INVALID_ARGS;
-        }
-    }
-    // Mark that we are exiting in the parser.
-    // TODO: in concurrent mode this won't successfully exit a pipeline, as there are other parsers
-    // involved. That is, `exit | sleep 1000` may not exit as hoped. Need to rationalize what
-    // behavior we want here.
-    parser.libdata().exit_current_script = true;
-    return retval;
-}

src/builtins/exit.h

@@ -1,11 +0,0 @@
-// Prototypes for executing builtin_exit function.
-#ifndef FISH_BUILTIN_EXIT_H
-#define FISH_BUILTIN_EXIT_H
-
-#include "../maybe.h"
-
-class parser_t;
-struct io_streams_t;
-
-maybe_t<int> builtin_exit(parser_t &parser, io_streams_t &streams, const wchar_t **argv);
-#endif

src/builtins/return.cpp

@@ -1,122 +0,0 @@
-// Implementation of the return builtin.
-#include "config.h"  // IWYU pragma: keep
-
-#include "return.h"
-
-#include <cerrno>
-#include <cmath>
-#include <cstdlib>
-#include <deque>
-
-#include "../builtin.h"
-#include "../common.h"
-#include "../fallback.h"  // IWYU pragma: keep
-#include "../io.h"
-#include "../maybe.h"
-#include "../parser.h"
-#include "../wgetopt.h"
-#include "../wutil.h"  // IWYU pragma: keep
-
-struct return_cmd_opts_t {
-    bool print_help = false;
-};
-static const wchar_t *const short_options = L":h";
-static const struct woption long_options[] = {{L"help", no_argument, 'h'}, {}};
-
-static int parse_cmd_opts(return_cmd_opts_t &opts, int *optind,  //!OCLINT(high ncss method)
-                          int argc, const wchar_t **argv, parser_t &parser, io_streams_t &streams) {
-    UNUSED(parser);
-    UNUSED(streams);
-    const wchar_t *cmd = argv[0];
-    int opt;
-    wgetopter_t w;
-    while ((opt = w.wgetopt_long(argc, argv, short_options, long_options, nullptr)) != -1) {
-        switch (opt) {  //!OCLINT(too few branches)
-            case 'h': {
-                opts.print_help = true;
-                break;
-            }
-            case ':': {
-                builtin_missing_argument(parser, streams, cmd, argv[w.woptind - 1]);
-                return STATUS_INVALID_ARGS;
-            }
-            case '?': {
-                // We would normally invoke builtin_unknown_option() and return an error.
-                // But for this command we want to let it try and parse the value as a negative
-                // return value.
-                *optind = w.woptind - 1;
-                return STATUS_CMD_OK;
-            }
-            default: {
-                DIE("unexpected retval from wgetopt_long");
-            }
-        }
-    }
-
-    *optind = w.woptind;
-    return STATUS_CMD_OK;
-}
-
-/// Function for handling the return builtin.
-maybe_t<int> builtin_return(parser_t &parser, io_streams_t &streams, const wchar_t **argv) {
-    const wchar_t *cmd = argv[0];
-    int argc = builtin_count_args(argv);
-    return_cmd_opts_t opts;
-
-    int optind;
-    int retval = parse_cmd_opts(opts, &optind, argc, argv, parser, streams);
-    if (retval != STATUS_CMD_OK) return retval;
-
-    if (opts.print_help) {
-        builtin_print_help(parser, streams, cmd);
-        return STATUS_CMD_OK;
-    }
-
-    if (optind + 1 < argc) {
-        streams.err.append_format(BUILTIN_ERR_TOO_MANY_ARGUMENTS, cmd);
-        builtin_print_error_trailer(parser, streams.err, cmd);
-        return STATUS_INVALID_ARGS;
-    }
-
-    if (optind == argc) {
-        retval = parser.get_last_status();
-    } else {
-        retval = fish_wcstoi(argv[1]);
-        if (errno) {
-            streams.err.append_format(BUILTIN_ERR_NOT_NUMBER, cmd, argv[1]);
-            builtin_print_error_trailer(parser, streams.err, cmd);
-            return STATUS_INVALID_ARGS;
-        }
-    }
-
-    // Find the function block.
-    bool has_function_block = false;
-    for (const auto &b : parser.blocks()) {
-        if (b.is_function_call()) {
-            has_function_block = true;
-            break;
-        }
-    }
-
-    // *nix does not support negative return values, but our `return` builtin happily accepts being
-    // called with negative literals (e.g. `return -1`).
-    // Map negative values to (256 - their absolute value). This prevents `return -1` from
-    // evaluating to a `$status` of 0 and keeps us from running into undefined behavior by trying to
-    // left shift a negative value in W_EXITCODE().
-    if (retval < 0) {
-        retval = 256 - (std::abs(retval) % 256);
-    }
-
-    // If we're not in a function, exit the current script (but not an interactive shell).
-    if (!has_function_block) {
-        if (!parser.libdata().is_interactive) {
-            parser.libdata().exit_current_script = true;
-        }
-        return retval;
-    }
-
-    // Mark a return in the libdata.
-    parser.libdata().returning = true;
-
-    return retval;
-}

src/builtins/return.h

@@ -1,11 +0,0 @@
-// Prototypes for executing builtin_return function.
-#ifndef FISH_BUILTIN_RETURN_H
-#define FISH_BUILTIN_RETURN_H
-
-#include "../maybe.h"
-
-class parser_t;
-struct io_streams_t;
-
-maybe_t<int> builtin_return(parser_t &parser, io_streams_t &streams, const wchar_t **argv);
-#endif

src/parser.cpp

@@ -39,6 +39,14 @@ static wcstring user_presentable_path(const wcstring &path, const environment_t
     return replace_home_directory_with_tilde(path, vars);
 }
 
+void library_data_t::set_exit_current_script(bool val) {
+    exit_current_script = val;
+};
+
+void library_data_t::set_returning(bool val) {
+    returning = val;
+};
+
 parser_t::parser_t(std::shared_ptr<env_stack_t> vars, bool is_principal)
     : variables(std::move(vars)), is_principal_(is_principal) {
     assert(variables.get() && "Null variables in parser initializer");
@@ -669,6 +677,15 @@ RustFFIJobList parser_t::ffi_jobs() const {
     return RustFFIJobList{const_cast<job_ref_t *>(job_list.data()), job_list.size()};
 }
 
+bool parser_t::ffi_has_funtion_block() const {
+    for (const auto &b : blocks()) {
+        if (b.is_function_call()) {
+             return true;
+        }
+    }
+    return false;
+}
+
 block_t::block_t(block_type_t t) : block_type(t) {}
 
 wcstring block_t::description() const {

src/parser.h

@@ -231,6 +231,9 @@ struct library_data_t {
         /// Used to get the full text of the current job for `status current-commandline`.
         wcstring commandline;
     } status_vars;
+
+    void set_exit_current_script(bool val);
+    void set_returning(bool val);
 };
 
 /// The result of parser_t::eval family.
@@ -484,6 +487,9 @@ class parser_t : public std::enable_shared_from_this<parser_t> {
     /// autocxx junk.
     RustFFIJobList ffi_jobs() const;
 
+    /// autocxx junk.
+    bool ffi_has_funtion_block() const;
+
     ~parser_t();
 };