From 1adfce18ee27ea27bfe35d727190a95dfe5c1223 Mon Sep 17 00:00:00 2001 From: Neeraj Jaiswal Date: Sat, 18 Feb 2023 21:43:58 +0530 Subject: [PATCH] builtins: port return/exit to rust --- CMakeLists.txt | 4 +- fish-rust/src/builtins/exit.rs | 26 +++++++ fish-rust/src/builtins/mod.rs | 2 + fish-rust/src/builtins/return.rs | 130 +++++++++++++++++++++++++++++++ fish-rust/src/builtins/shared.rs | 12 +++ fish-rust/src/ffi.rs | 2 + src/builtin.cpp | 12 ++- src/builtin.h | 2 + src/builtins/exit.cpp | 94 ---------------------- src/builtins/exit.h | 11 --- src/builtins/return.cpp | 122 ----------------------------- src/builtins/return.h | 11 --- src/parser.cpp | 17 ++++ src/parser.h | 6 ++ 14 files changed, 207 insertions(+), 244 deletions(-) create mode 100644 fish-rust/src/builtins/exit.rs create mode 100644 fish-rust/src/builtins/return.rs delete mode 100644 src/builtins/exit.cpp delete mode 100644 src/builtins/exit.h delete mode 100644 src/builtins/return.cpp delete mode 100644 src/builtins/return.h diff --git a/CMakeLists.txt b/CMakeLists.txt index 020d3b44d..ec2908a4b 100644 --- a/CMakeLists.txt +++ b/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 ) diff --git a/fish-rust/src/builtins/exit.rs b/fish-rust/src/builtins/exit.rs new file mode 100644 index 000000000..b0ffc0f77 --- /dev/null +++ b/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 { + 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); +} diff --git a/fish-rust/src/builtins/mod.rs b/fish-rust/src/builtins/mod.rs index 3e05226b0..6634804b7 100644 --- a/fish-rust/src/builtins/mod.rs +++ b/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; diff --git a/fish-rust/src/builtins/return.rs b/fish-rust/src/builtins/return.rs new file mode 100644 index 000000000..650c73232 --- /dev/null +++ b/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> { + 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 { + 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> { + 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); + } + } + } +} diff --git a/fish-rust/src/builtins/shared.rs b/fish-rust/src/builtins/shared.rs index 6fb7ec1be..2c7c3eaf2 100644 --- a/fish-rust/src/builtins/shared.rs +++ b/fish-rust/src/builtins/shared.rs @@ -31,6 +31,12 @@ mod builtins_ffi { impl Vec {} } +/// 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 = 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, diff --git a/fish-rust/src/ffi.rs b/fish-rust/src/ffi.rs index 4ae4eb053..087841b50 100644 --- a/fish-rust/src/ffi.rs +++ b/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") diff --git a/src/builtin.cpp b/src/builtin.cpp index 1b98fa940..5468b5f9f 100644 --- a/src/builtin.cpp +++ b/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 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(); } diff --git a/src/builtin.h b/src/builtin.h index bce6edb47..e1a61452b 100644 --- a/src/builtin.h +++ b/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 diff --git a/src/builtins/exit.cpp b/src/builtins/exit.cpp deleted file mode 100644 index 47687a644..000000000 --- a/src/builtins/exit.cpp +++ /dev/null @@ -1,94 +0,0 @@ -// Implementation of the exit builtin. -#include "config.h" // IWYU pragma: keep - -#include "exit.h" - -#include - -#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 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; -} diff --git a/src/builtins/exit.h b/src/builtins/exit.h deleted file mode 100644 index cf6bbb6db..000000000 --- a/src/builtins/exit.h +++ /dev/null @@ -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 builtin_exit(parser_t &parser, io_streams_t &streams, const wchar_t **argv); -#endif diff --git a/src/builtins/return.cpp b/src/builtins/return.cpp deleted file mode 100644 index 2289b72a7..000000000 --- a/src/builtins/return.cpp +++ /dev/null @@ -1,122 +0,0 @@ -// Implementation of the return builtin. -#include "config.h" // IWYU pragma: keep - -#include "return.h" - -#include -#include -#include -#include - -#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 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; -} diff --git a/src/builtins/return.h b/src/builtins/return.h deleted file mode 100644 index 243c56e1c..000000000 --- a/src/builtins/return.h +++ /dev/null @@ -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 builtin_return(parser_t &parser, io_streams_t &streams, const wchar_t **argv); -#endif diff --git a/src/parser.cpp b/src/parser.cpp index 452fe496f..d43f16255 100644 --- a/src/parser.cpp +++ b/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 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_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 { diff --git a/src/parser.h b/src/parser.h index cc0683fb0..9381426ab 100644 --- a/src/parser.h +++ b/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 { /// autocxx junk. RustFFIJobList ffi_jobs() const; + /// autocxx junk. + bool ffi_has_funtion_block() const; + ~parser_t(); };