Adopt the new function store and rewrite builtin_function

This adopts the new function store, replacing the C++ version.

It also reimplements builtin_function in Rust, as these was too coupled to
the function store to handle in a separate commit.

CMakeLists.txt

@@ -103,7 +103,7 @@ set(FISH_BUILTIN_SRCS
     src/builtins/commandline.cpp src/builtins/complete.cpp
     src/builtins/disown.cpp
     src/builtins/eval.cpp src/builtins/fg.cpp
-    src/builtins/function.cpp src/builtins/functions.cpp src/builtins/history.cpp
+    src/builtins/functions.cpp src/builtins/history.cpp
     src/builtins/jobs.cpp src/builtins/path.cpp
     src/builtins/read.cpp src/builtins/set.cpp
     src/builtins/source.cpp

fish-rust/build.rs

@@ -41,6 +41,7 @@ fn main() {
         "src/abbrs.rs",
         "src/ast.rs",
         "src/builtins/shared.rs",
+        "src/builtins/function.rs",
         "src/common.rs",
         "src/env/env_ffi.rs",
         "src/env_dispatch.rs",
@@ -51,6 +52,7 @@ fn main() {
         "src/ffi_init.rs",
         "src/ffi_tests.rs",
         "src/fish_indent.rs",
+        "src/function.rs",
         "src/future_feature_flags.rs",
         "src/highlight.rs",
         "src/job_group.rs",

fish-rust/src/ast.rs

@@ -4469,6 +4469,11 @@ impl Statement {
     }
 }
 
+unsafe impl ExternType for BlockStatement {
+    type Id = type_id!("BlockStatement");
+    type Kind = cxx::kind::Opaque;
+}
+
 #[derive(Clone)]
 pub enum NodeFfi<'a> {
     None,

fish-rust/src/builtins/function.rs

@@ -0,0 +1,430 @@
+use super::shared::{
+    builtin_missing_argument, builtin_print_error_trailer, builtin_unknown_option, io_streams_t,
+    truncate_args_on_nul, BUILTIN_ERR_VARNAME, STATUS_INVALID_ARGS,
+};
+use crate::ast::BlockStatement;
+use crate::builtins::shared::STATUS_CMD_OK;
+use crate::common::{valid_func_name, valid_var_name};
+use crate::env::environment::Environment;
+use crate::event::{self, EventDescription, EventHandler};
+use crate::ffi::{self, io_streams_t as io_streams_ffi_t, parser_t, Repin};
+use crate::function;
+use crate::global_safety::RelaxedAtomicBool;
+use crate::parse_tree::NodeRef;
+use crate::parse_tree::ParsedSourceRefFFI;
+use crate::parser_keywords::parser_keywords_is_reserved;
+use crate::signal::Signal;
+use crate::wchar::{wstr, WString, L};
+use crate::wchar_ffi::{wcstring_list_ffi_t, WCharFromFFI, WCharToFFI};
+use crate::wgetopt::{wgetopter_t, wopt, woption, woption_argument_t, NONOPTION_CHAR_CODE};
+use crate::wutil::{fish_wcstoi, wgettext_fmt};
+use libc::c_int;
+use std::pin::Pin;
+use std::sync::Arc;
+
+struct FunctionCmdOpts {
+    print_help: bool,
+    shadow_scope: bool,
+    description: WString,
+    events: Vec<EventDescription>,
+    named_arguments: Vec<WString>,
+    inherit_vars: Vec<WString>,
+    wrap_targets: Vec<WString>,
+}
+
+impl Default for FunctionCmdOpts {
+    fn default() -> Self {
+        Self {
+            print_help: false,
+            shadow_scope: true,
+            description: WString::new(),
+            events: Vec::new(),
+            named_arguments: Vec::new(),
+            inherit_vars: Vec::new(),
+            wrap_targets: Vec::new(),
+        }
+    }
+}
+
+// This command is atypical in using the "-" (RETURN_IN_ORDER) option for flag parsing.
+// This is needed due to the semantics of the -a/--argument-names flag.
+const SHORT_OPTIONS: &wstr = L!("-:a:d:e:hj:p:s:v:w:SV:");
+#[rustfmt::skip]
+const LONG_OPTIONS: &[woption] = &[
+    wopt(L!("description"), woption_argument_t::required_argument, 'd'),
+    wopt(L!("on-signal"), woption_argument_t::required_argument, 's'),
+    wopt(L!("on-job-exit"), woption_argument_t::required_argument, 'j'),
+    wopt(L!("on-process-exit"), woption_argument_t::required_argument, 'p'),
+    wopt(L!("on-variable"), woption_argument_t::required_argument, 'v'),
+    wopt(L!("on-event"), woption_argument_t::required_argument, 'e'),
+    wopt(L!("wraps"), woption_argument_t::required_argument, 'w'),
+    wopt(L!("help"), woption_argument_t::no_argument, 'h'),
+    wopt(L!("argument-names"), woption_argument_t::required_argument, 'a'),
+    wopt(L!("no-scope-shadowing"), woption_argument_t::no_argument, 'S'),
+    wopt(L!("inherit-variable"), woption_argument_t::required_argument, 'V'),
+];
+
+/// \return the internal_job_id for a pid, or None if none.
+/// This looks through both active and finished jobs.
+fn job_id_for_pid(pid: i32, parser: &parser_t) -> Option<u64> {
+    if let Some(job) = parser.job_get_from_pid(pid) {
+        Some(job.get_internal_job_id())
+    } else {
+        parser
+            .get_wait_handles()
+            .get_by_pid(pid)
+            .map(|h| h.internal_job_id)
+    }
+}
+
+/// Parses options to builtin function, populating opts.
+/// Returns an exit status.
+fn parse_cmd_opts(
+    opts: &mut FunctionCmdOpts,
+    optind: &mut usize,
+    argv: &mut [&wstr],
+    parser: &mut parser_t,
+    streams: &mut io_streams_t,
+) -> Option<c_int> {
+    let cmd = L!("function");
+    let print_hints = false;
+    let mut handling_named_arguments = false;
+    let mut w = wgetopter_t::new(SHORT_OPTIONS, LONG_OPTIONS, argv);
+    while let Some(opt) = w.wgetopt_long() {
+        // NONOPTION_CHAR_CODE is returned when we reach a non-permuted non-option.
+        if opt != 'a' && opt != NONOPTION_CHAR_CODE {
+            handling_named_arguments = false;
+        }
+        match opt {
+            NONOPTION_CHAR_CODE => {
+                // A positional argument we got because we use RETURN_IN_ORDER.
+                let woptarg = w.woptarg.unwrap().to_owned();
+                if handling_named_arguments {
+                    opts.named_arguments.push(woptarg);
+                } else {
+                    streams.err.append(wgettext_fmt!(
+                        "%ls: %ls: unexpected positional argument",
+                        cmd,
+                        woptarg
+                    ));
+                    return STATUS_INVALID_ARGS;
+                }
+            }
+            'd' => {
+                opts.description = w.woptarg.unwrap().to_owned();
+            }
+            's' => {
+                let Some(signal) = Signal::parse(w.woptarg.unwrap()) else {
+                    streams.err.append(wgettext_fmt!("%ls: Unknown signal '%ls'", cmd, w.woptarg.unwrap()));
+                    return STATUS_INVALID_ARGS;
+                };
+                opts.events.push(EventDescription::Signal { signal });
+            }
+            'v' => {
+                let name = w.woptarg.unwrap().to_owned();
+                if !valid_var_name(&name) {
+                    streams
+                        .err
+                        .append(wgettext_fmt!(BUILTIN_ERR_VARNAME, cmd, name));
+                    return STATUS_INVALID_ARGS;
+                }
+                opts.events.push(EventDescription::Variable { name });
+            }
+            'e' => {
+                let param = w.woptarg.unwrap().to_owned();
+                opts.events.push(EventDescription::Generic { param });
+            }
+            'j' | 'p' => {
+                let woptarg = w.woptarg.unwrap();
+                let e: EventDescription;
+                if opt == 'j' && woptarg == "caller" {
+                    let libdata = parser.ffi_libdata_pod_const();
+                    let caller_id = if libdata.is_subshell {
+                        libdata.caller_id
+                    } else {
+                        0
+                    };
+                    if caller_id == 0 {
+                        streams.err.append(wgettext_fmt!(
+                            "%ls: calling job for event handler not found",
+                            cmd
+                        ));
+                        return STATUS_INVALID_ARGS;
+                    }
+                    e = EventDescription::CallerExit { caller_id };
+                } else if opt == 'p' && woptarg == "%self" {
+                    // Safety: getpid() is always successful.
+                    let pid = unsafe { libc::getpid() };
+                    e = EventDescription::ProcessExit { pid };
+                } else {
+                    let pid = fish_wcstoi(woptarg);
+                    if pid.is_err() || pid.unwrap() < 0 {
+                        streams
+                            .err
+                            .append(wgettext_fmt!("%ls: %ls: invalid process id", cmd));
+                        return STATUS_INVALID_ARGS;
+                    }
+                    let pid = pid.unwrap();
+                    if opt == 'p' {
+                        e = EventDescription::ProcessExit { pid };
+                    } else {
+                        // TODO: rationalize why a default of 0 is sensible.
+                        let internal_job_id = job_id_for_pid(pid, parser).unwrap_or(0);
+                        e = EventDescription::JobExit {
+                            pid,
+                            internal_job_id,
+                        };
+                    }
+                }
+                opts.events.push(e);
+            }
+            'a' => {
+                handling_named_arguments = true;
+                opts.named_arguments.push(w.woptarg.unwrap().to_owned());
+            }
+            'S' => {
+                opts.shadow_scope = false;
+            }
+            'w' => {
+                opts.wrap_targets.push(w.woptarg.unwrap().to_owned());
+            }
+            'V' => {
+                let woptarg = w.woptarg.unwrap();
+                if !valid_var_name(woptarg) {
+                    streams
+                        .err
+                        .append(wgettext_fmt!(BUILTIN_ERR_VARNAME, cmd, woptarg));
+                    return STATUS_INVALID_ARGS;
+                }
+                opts.inherit_vars.push(woptarg.to_owned());
+            }
+            'h' => {
+                opts.print_help = true;
+            }
+            ':' => {
+                builtin_missing_argument(parser, streams, cmd, argv[w.woptind - 1], print_hints);
+                return STATUS_INVALID_ARGS;
+            }
+            '?' => {
+                builtin_unknown_option(parser, streams, cmd, argv[w.woptind - 1], print_hints);
+                return STATUS_INVALID_ARGS;
+            }
+            other => {
+                panic!("Unexpected retval from wgetopt_long: {}", other);
+            }
+        }
+    }
+
+    *optind = w.woptind;
+    STATUS_CMD_OK
+}
+
+fn validate_function_name(
+    argv: &mut [&wstr],
+    function_name: &mut WString,
+    cmd: &wstr,
+    streams: &mut io_streams_t,
+) -> Option<c_int> {
+    if argv.len() < 2 {
+        // This is currently impossible but let's be paranoid.
+        streams
+            .err
+            .append(wgettext_fmt!("%ls: function name required", cmd));
+        return STATUS_INVALID_ARGS;
+    }
+    *function_name = argv[1].to_owned();
+    if !valid_func_name(function_name) {
+        streams.err.append(wgettext_fmt!(
+            "%ls: %ls: invalid function name",
+            cmd,
+            function_name,
+        ));
+        return STATUS_INVALID_ARGS;
+    }
+    if parser_keywords_is_reserved(function_name) {
+        streams.err.append(wgettext_fmt!(
+            "%ls: %ls: cannot use reserved keyword as function name",
+            cmd,
+            function_name
+        ));
+        return STATUS_INVALID_ARGS;
+    }
+    STATUS_CMD_OK
+}
+
+/// Define a function. Calls into `function.rs` to perform the heavy lifting of defining a
+/// function. Note this isn't strictly a "builtin": it is called directly from parse_execution.
+/// That is why its signature is different from the other builtins.
+pub fn function(
+    parser: &mut parser_t,
+    streams: &mut io_streams_t,
+    c_args: &mut [&wstr],
+    func_node: NodeRef<BlockStatement>,
+) -> Option<c_int> {
+    // The wgetopt function expects 'function' as the first argument. Make a new vec with
+    // that property. This is needed because this builtin has a different signature than the other
+    // builtins.
+    let mut args = vec![L!("function")];
+    args.extend_from_slice(c_args);
+    let argv: &mut [&wstr] = &mut args;
+    let cmd = argv[0];
+
+    // A valid function name has to be the first argument.
+    let mut function_name = WString::new();
+    let mut retval = validate_function_name(argv, &mut function_name, cmd, streams);
+    if retval != STATUS_CMD_OK {
+        return retval;
+    }
+    let argv = &mut argv[1..];
+
+    let mut opts = FunctionCmdOpts::default();
+    let mut optind = 0;
+    retval = parse_cmd_opts(&mut opts, &mut optind, argv, parser, streams);
+    if retval != STATUS_CMD_OK {
+        return retval;
+    }
+
+    if opts.print_help {
+        builtin_print_error_trailer(parser, streams, cmd);
+        return STATUS_CMD_OK;
+    }
+
+    if argv.len() != optind {
+        if !opts.named_arguments.is_empty() {
+            // Remaining arguments are named arguments.
+            for &arg in argv[optind..].iter() {
+                if !valid_var_name(arg) {
+                    streams
+                        .err
+                        .append(wgettext_fmt!(BUILTIN_ERR_VARNAME, cmd, arg));
+                    return STATUS_INVALID_ARGS;
+                }
+                opts.named_arguments.push(arg.to_owned());
+            }
+        } else {
+            streams.err.append(wgettext_fmt!(
+                "%ls: %ls: unexpected positional argument",
+                cmd,
+                argv[optind],
+            ));
+            return STATUS_INVALID_ARGS;
+        }
+    }
+
+    // Extract the current filename.
+    let definition_file = unsafe { parser.pin().libdata().get_current_filename().as_ref() }
+        .map(|s| Arc::new(s.from_ffi()));
+
+    // We have what we need to actually define the function.
+    let mut props = function::FunctionProperties {
+        func_node,
+        named_arguments: opts.named_arguments,
+        description: opts.description,
+        inherit_vars: Default::default(),
+        shadow_scope: opts.shadow_scope,
+        is_autoload: RelaxedAtomicBool::new(false),
+        definition_file,
+        is_copy: false,
+        copy_definition_file: None,
+        copy_definition_lineno: 0,
+    };
+
+    // Populate inherit_vars.
+    for name in opts.inherit_vars.into_iter() {
+        if let Some(var) = parser.get_vars().get(&name) {
+            props.inherit_vars.insert(name, var.as_list().to_vec());
+        }
+    }
+
+    // Add the function itself.
+    function::add(function_name.clone(), Arc::new(props));
+
+    // Handle wrap targets by creating the appropriate completions.
+    for wt in opts.wrap_targets.into_iter() {
+        ffi::complete_add_wrapper(&function_name.to_ffi(), &wt.to_ffi());
+    }
+
+    // Add any event handlers.
+    for ed in &opts.events {
+        event::add_handler(EventHandler::new(ed.clone(), Some(function_name.clone())));
+    }
+
+    // If there is an --on-process-exit or --on-job-exit event handler for some pid, and that
+    // process has already exited, run it immediately (#7210).
+    for ed in &opts.events {
+        match *ed {
+            EventDescription::ProcessExit { pid } if pid != event::ANY_PID => {
+                if let Some(status) = parser
+                    .get_wait_handles()
+                    .get_by_pid(pid)
+                    .and_then(|wh| wh.status())
+                {
+                    event::fire(parser, event::Event::process_exit(pid, status));
+                }
+            }
+            EventDescription::JobExit { pid, .. } if pid != event::ANY_PID => {
+                if let Some(wh) = parser.get_wait_handles().get_by_pid(pid) {
+                    if wh.is_completed() {
+                        event::fire(parser, event::Event::job_exit(pid, wh.internal_job_id));
+                    }
+                }
+            }
+            _ => (),
+        }
+    }
+
+    STATUS_CMD_OK
+}
+
+fn builtin_function_ffi(
+    parser: Pin<&mut parser_t>,
+    streams: Pin<&mut io_streams_ffi_t>,
+    c_args: &wcstring_list_ffi_t,
+    source_u8: *const u8, // unowned ParsedSourceRefFFI
+    func_node: &BlockStatement,
+) -> i32 {
+    let storage = c_args.from_ffi();
+    let mut args = truncate_args_on_nul(&storage);
+    let node = unsafe {
+        let source_ref: &ParsedSourceRefFFI = &*(source_u8.cast());
+        NodeRef::from_parts(
+            source_ref
+                .0
+                .as_ref()
+                .expect("Should have parsed source")
+                .clone(),
+            func_node,
+        )
+    };
+    function(
+        parser.unpin(),
+        &mut io_streams_t::new(streams),
+        args.as_mut_slice(),
+        node,
+    )
+    .expect("function builtin should always return a non-None status")
+}
+
+#[cxx::bridge]
+mod builtin_function {
+    extern "C++" {
+        include!("ast.h");
+        include!("parser.h");
+        include!("io.h");
+        type parser_t = crate::ffi::parser_t;
+        type io_streams_t = crate::ffi::io_streams_t;
+        type wcstring_list_ffi_t = crate::ffi::wcstring_list_ffi_t;
+
+        type BlockStatement = crate::ast::BlockStatement;
+    }
+
+    extern "Rust" {
+        fn builtin_function_ffi(
+            parser: Pin<&mut parser_t>,
+            streams: Pin<&mut io_streams_t>,
+            c_args: &wcstring_list_ffi_t,
+            source: *const u8, // unowned ParsedSourceRefFFI
+            func_node: &BlockStatement,
+        ) -> i32;
+    }
+}

fish-rust/src/builtins/mod.rs

@@ -11,6 +11,7 @@ pub mod contains;
 pub mod echo;
 pub mod emit;
 pub mod exit;
+pub mod function;
 pub mod math;
 pub mod printf;
 pub mod pwd;

fish-rust/src/builtins/shared.rs

@@ -3,6 +3,7 @@ use crate::ffi::{self, parser_t, wcstring_list_ffi_t, Repin, RustBuiltin};
 use crate::wchar::{wstr, WString, L};
 use crate::wchar_ffi::{c_str, empty_wstring, ToCppWString, WCharFromFFI};
 use crate::wgetopt::{wgetopter_t, wopt, woption, woption_argument_t};
+use cxx::{type_id, ExternType};
 use libc::c_int;
 use std::os::fd::RawFd;
 use std::pin::Pin;
@@ -31,6 +32,11 @@ mod builtins_ffi {
     }
 }
 
+unsafe impl ExternType for io_streams_t {
+    type Id = type_id!("io_streams_t");
+    type Kind = cxx::kind::Opaque;
+}
+
 /// Error message when too many arguments are supplied to a builtin.
 pub const BUILTIN_ERR_TOO_MANY_ARGUMENTS: &str = "%ls: too many arguments\n";
 
@@ -50,6 +56,9 @@ pub const BUILTIN_ERR_ARG_COUNT2: &str = "%ls: %ls: expected %d arguments; got %
 pub const BUILTIN_ERR_MIN_ARG_COUNT1: &str = "%ls: expected >= %d arguments; got %d\n";
 pub const BUILTIN_ERR_MAX_ARG_COUNT1: &str = "%ls: expected <= %d arguments; got %d\n";
 
+/// Error message for invalid variable name.
+pub const BUILTIN_ERR_VARNAME: &str = "%ls: %ls: invalid variable name. See `help identifiers`\n";
+
 /// Error message on invalid combination of options.
 pub const BUILTIN_ERR_COMBO: &str = "%ls: invalid option combination\n";
 pub const BUILTIN_ERR_COMBO2: &str = "%ls: invalid option combination, %ls\n";
@@ -149,6 +158,19 @@ impl io_streams_t {
     }
 }
 
+/// Helper function to convert Vec<WString> to Vec<&wstr>, truncating on NUL.
+/// We truncate on NUL for backwards-compatibility reasons.
+/// This used to be passed as c-strings (`wchar_t *`),
+/// and so we act like it for now.
+pub fn truncate_args_on_nul(args: &[WString]) -> Vec<&wstr> {
+    args.iter()
+        .map(|s| match s.chars().position(|c| c == '\0') {
+            Some(i) => &s[..i],
+            None => &s[..],
+        })
+        .collect()
+}
+
 fn rust_run_builtin(
     parser: Pin<&mut parser_t>,
     streams: Pin<&mut builtins_ffi::io_streams_t>,
@@ -157,16 +179,7 @@ fn rust_run_builtin(
     status_code: &mut i32,
 ) -> bool {
     let storage: Vec<WString> = cpp_args.from_ffi();
-    let mut args: Vec<&wstr> = storage
+    let mut args: Vec<&wstr> = truncate_args_on_nul(&storage);
-        .iter()
-        .map(|s| match s.chars().position(|c| c == '\0') {
-            // We truncate on NUL for backwards-compatibility reasons.
-            // This used to be passed as c-strings (`wchar_t *`),
-            // and so we act like it for now.
-            Some(pos) => &s[..pos],
-            None => &s[..],
-        })
-        .collect();
     let streams = &mut io_streams_t::new(streams);
 
     match run_builtin(parser.unpin(), streams, args.as_mut_slice(), builtin) {

fish-rust/src/builtins/type.rs

@@ -8,12 +8,8 @@ use crate::builtins::shared::{
 };
 use crate::ffi::parser_t;
 use crate::ffi::Repin;
-use crate::ffi::{
+use crate::ffi::{builtin_exists, colorize_shell};
-    builtin_exists, colorize_shell, function_get_annotated_definition,
+use crate::function;
-    function_get_copy_definition_file, function_get_copy_definition_lineno,
-    function_get_definition_file, function_get_definition_lineno, function_get_props_autoload,
-    function_is_copy,
-};
 use crate::path::{path_get_path, path_get_paths};
 use crate::wchar::{wstr, WString, L};
 use crate::wchar_ffi::WCharFromFFI;
@@ -95,8 +91,7 @@ pub fn r#type(
     for arg in argv.iter().take(argc).skip(optind) {
         let mut found = 0;
         if !opts.force_path && !opts.no_functions {
-            let props = function_get_props_autoload(&arg.to_ffi(), parser.pin());
+            if let Some(props) = function::get_props_autoload(arg, parser) {
-            if !props.is_null() {
                 found += 1;
                 res = true;
                 // Early out - query means *any of the args exists*.
@@ -104,7 +99,7 @@ pub fn r#type(
                     return STATUS_CMD_OK;
                 }
                 if !opts.get_type {
-                    let path = function_get_definition_file(&props).from_ffi();
+                    let path = props.definition_file().unwrap_or(L!(""));
                     let mut comment = WString::new();
 
                     if path.is_empty() {
@@ -112,7 +107,7 @@ pub fn r#type(
                     } else if path == "-" {
                         comment.push_utfstr(&wgettext_fmt!("Defined via `source`"));
                     } else {
-                        let lineno: i32 = i32::from(function_get_definition_lineno(&props));
+                        let lineno: i32 = props.definition_lineno();
                         comment.push_utfstr(&wgettext_fmt!(
                             "Defined in %ls @ line %d",
                             path,
@@ -120,15 +115,14 @@ pub fn r#type(
                         ));
                     }
 
-                    if function_is_copy(&props) {
+                    if props.is_copy() {
-                        let path = function_get_copy_definition_file(&props).from_ffi();
+                        let path = props.copy_definition_file().unwrap_or(L!(""));
                         if path.is_empty() {
                             comment.push_utfstr(&wgettext_fmt!(", copied interactively"));
                         } else if path == "-" {
                             comment.push_utfstr(&wgettext_fmt!(", copied via `source`"));
                         } else {
-                            let lineno: i32 =
+                            let lineno: i32 = props.copy_definition_lineno();
-                                i32::from(function_get_copy_definition_lineno(&props));
                             comment.push_utfstr(&wgettext_fmt!(
                                 ", copied in %ls @ line %d",
                                 path,
@@ -137,22 +131,21 @@ pub fn r#type(
                         }
                     }
                     if opts.path {
-                        if function_is_copy(&props) {
+                        if let Some(orig_path) = props.copy_definition_file() {
-                            let path = function_get_copy_definition_file(&props).from_ffi();
+                            streams.out.append(orig_path);
-                            streams.out.append(path);
                         } else {
                             streams.out.append(path);
                         }
-                        streams.out.append(L!("\n"));
+                        streams.out.append1('\n');
                     } else if !opts.short_output {
                         streams.out.append(wgettext_fmt!("%ls is a function", arg));
                         streams.out.append(wgettext_fmt!(" with definition"));
-                        streams.out.append(L!("\n"));
+                        streams.out.append1('\n');
                         let mut def = WString::new();
                         def.push_utfstr(&sprintf!(
                             "# %ls\n%ls",
                             comment,
-                            function_get_annotated_definition(&props, &arg.to_ffi()).from_ffi()
+                            props.annotated_definition(arg)
                         ));
 
                         if !streams.out_is_redirected && unsafe { isatty(STDOUT_FILENO) == 1 } {

fish-rust/src/env/env_ffi.rs

@@ -3,6 +3,7 @@ use super::var::{ElectricVar, EnvVar, EnvVarFlags, Statuses};
 use crate::env::EnvMode;
 use crate::event::Event;
 use crate::ffi::{event_list_ffi_t, wchar_t, wcharz_t, wcstring_list_ffi_t};
+use crate::function::FunctionPropertiesRefFFI;
 use crate::null_terminated_array::OwningNullTerminatedArrayRefFFI;
 use crate::signal::Signal;
 use crate::wchar_ffi::WCharToFFI;
@@ -31,6 +32,7 @@ mod env_ffi {
         type event_list_ffi_t = super::event_list_ffi_t;
         type wcstring_list_ffi_t = super::wcstring_list_ffi_t;
         type wcharz_t = super::wcharz_t;
+        type function_properties_t = super::FunctionPropertiesRefFFI;
 
         type OwningNullTerminatedArrayRefFFI =
             crate::null_terminated_array::OwningNullTerminatedArrayRefFFI;
@@ -138,6 +140,8 @@ mod env_ffi {
         fn env_get_principal_ffi() -> Box<EnvStackRefFFI>;
 
         fn universal_sync(&self, always: bool, out_events: Pin<&mut event_list_ffi_t>);
+
+        fn apply_inherited_ffi(&self, props: &function_properties_t);
     }
 
     extern "Rust" {
@@ -296,6 +300,17 @@ impl EnvStackRefFFI {
             out_events.as_mut().push(Box::into_raw(event).cast());
         }
     }
+
+    fn apply_inherited_ffi(&self, props: &FunctionPropertiesRefFFI) {
+        // Ported from C++:
+        // for (const auto &kv : props.inherit_vars) {
+        //     vars.set(kv.first, ENV_LOCAL | ENV_USER, kv.second);
+        // }
+        for (name, vals) in props.0.inherit_vars() {
+            self.0
+                .set(name, EnvMode::LOCAL | EnvMode::USER, vals.clone());
+        }
+    }
 }
 
 impl Statuses {

fish-rust/src/env_dispatch.rs

@@ -4,6 +4,7 @@ use crate::env::{setenv_lock, unsetenv_lock, EnvMode, EnvStack, Environment};
 use crate::env::{CURSES_INITIALIZED, READ_BYTE_LIMIT, TERM_HAS_XN};
 use crate::ffi::is_interactive_session;
 use crate::flog::FLOGF;
+use crate::function;
 use crate::output::ColorSupport;
 use crate::wchar::L;
 use crate::wchar::{wstr, WString};
@@ -273,7 +274,7 @@ fn handle_autosuggestion_change(vars: &EnvStack) {
 }
 
 fn handle_function_path_change(_: &EnvStack) {
-    crate::ffi::function_invalidate_path();
+    function::invalidate_path();
 }
 
 fn handle_complete_path_change(_: &EnvStack) {

fish-rust/src/event.rs

@@ -102,7 +102,7 @@ mod event_ffi {
 
 pub use event_ffi::{event_description_t, event_type_t};
 
-const ANY_PID: pid_t = 0;
+pub const ANY_PID: pid_t = 0;
 
 #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)]
 pub enum EventDescription {

fish-rust/src/ffi.rs

@@ -126,17 +126,6 @@ include_cpp! {
 
     generate!("exec_subshell_ffi")
 
-    generate!("function_properties_t")
-    generate!("function_properties_ref_t")
-    generate!("function_get_props_autoload")
-    generate!("function_get_definition_file")
-    generate!("function_get_copy_definition_file")
-    generate!("function_get_definition_lineno")
-    generate!("function_get_copy_definition_lineno")
-    generate!("function_get_annotated_definition")
-    generate!("function_is_copy")
-    generate!("function_exists")
-
     generate!("rgb_color_t")
     generate_pod!("color24_t")
     generate!("colorize_shell")
@@ -153,8 +142,8 @@ include_cpp! {
     generate!("history_session_id")
     generate!("reader_change_cursor_selection_mode")
     generate!("reader_set_autosuggestion_enabled_ffi")
-    generate!("function_invalidate_path")
     generate!("complete_invalidate_path")
+    generate!("complete_add_wrapper")
     generate!("update_wait_on_escape_ms_ffi")
     generate!("autoload_t")
     generate!("make_autoload_ffi")
@@ -351,7 +340,6 @@ impl Repin for job_t {}
 impl Repin for output_stream_t {}
 impl Repin for parser_t {}
 impl Repin for process_t {}
-impl Repin for function_properties_ref_t {}
 impl Repin for wcstring_list_ffi_t {}
 
 pub use autocxx::c_int;

fish-rust/src/function.rs

@@ -641,6 +641,7 @@ mod function_ffi {
         type FunctionPropertiesRefFFI;
 
         fn definition_file(&self) -> UniquePtr<CxxWString>;
+        fn definition_lineno(&self) -> i32;
         fn copy_definition_lineno(&self) -> i32;
         fn shadow_scope(&self) -> bool;
         fn named_arguments(&self) -> UniquePtr<wcstring_list_ffi_t>;

fish-rust/src/wait_handle.rs

@@ -155,6 +155,11 @@ impl WaitHandle {
         assert!(!self.is_completed(), "wait handle already completed");
         self.status.set(Some(status));
     }
+
+    /// \return the status, or None if not yet completed.
+    pub fn status(&self) -> Option<i32> {
+        self.status.get()
+    }
 }
 
 impl WaitHandle {

src/ast.h

@@ -61,11 +61,13 @@ using while_header_t = WhileHeader;
 #else
 struct Ast;
 struct NodeFfi;
+struct BlockStatement;
 namespace ast {
 using ast_t = Ast;
 
+using block_statement_t = BlockStatement;
+
 struct argument_t;
-struct block_statement_t;
 struct statement_t;
 struct string_t;
 struct maybe_newlines_t;

src/builtins/function.cpp

@@ -1,333 +0,0 @@
-// Implementation of the function builtin.
-#include "config.h"  // IWYU pragma: keep
-
-#include "function.h"
-
-#include <unistd.h>
-
-#include <cerrno>
-#include <cwchar>
-#include <map>
-#include <memory>
-#include <string>
-#include <utility>
-#include <vector>
-
-#include "../builtin.h"
-#include "../common.h"
-#include "../complete.h"
-#include "../env.h"
-#include "../event.h"
-#include "../fallback.h"  // IWYU pragma: keep
-#include "../function.h"
-#include "../io.h"
-#include "../maybe.h"
-#include "../null_terminated_array.h"
-#include "../parse_tree.h"
-#include "../parser.h"
-#include "../parser_keywords.h"
-#include "../proc.h"
-#include "../signals.h"
-#include "../wgetopt.h"
-#include "../wutil.h"  // IWYU pragma: keep
-#include "cxx.h"
-
-namespace {
-struct function_cmd_opts_t {
-    bool print_help = false;
-    bool shadow_scope = true;
-    wcstring description;
-    std::vector<event_description_t> events;
-    std::vector<wcstring> named_arguments;
-    std::vector<wcstring> inherit_vars;
-    std::vector<wcstring> wrap_targets;
-};
-}  // namespace
-
-// This command is atypical in using the "-" (RETURN_IN_ORDER) option for flag parsing.
-// This is needed due to the semantics of the -a/--argument-names flag.
-static const wchar_t *const short_options = L"-:a:d:e:hj:p:s:v:w:SV:";
-static const struct woption long_options[] = {{L"description", required_argument, 'd'},
-                                              {L"on-signal", required_argument, 's'},
-                                              {L"on-job-exit", required_argument, 'j'},
-                                              {L"on-process-exit", required_argument, 'p'},
-                                              {L"on-variable", required_argument, 'v'},
-                                              {L"on-event", required_argument, 'e'},
-                                              {L"wraps", required_argument, 'w'},
-                                              {L"help", no_argument, 'h'},
-                                              {L"argument-names", required_argument, 'a'},
-                                              {L"no-scope-shadowing", no_argument, 'S'},
-                                              {L"inherit-variable", required_argument, 'V'},
-                                              {}};
-
-/// \return the internal_job_id for a pid, or 0 if none.
-/// This looks through both active and finished jobs.
-static internal_job_id_t job_id_for_pid(pid_t pid, parser_t &parser) {
-    if (const auto *job = parser.job_get_from_pid(pid)) {
-        return job->internal_job_id;
-    }
-    return parser.get_wait_handles_ffi()->get_job_id_by_pid(pid);
-}
-
-static int parse_cmd_opts(function_cmd_opts_t &opts, int *optind,  //!OCLINT(high ncss method)
-                          int argc, const wchar_t **argv, parser_t &parser, io_streams_t &streams) {
-    const wchar_t *cmd = L"function";
-    int opt;
-    wgetopter_t w;
-    bool handling_named_arguments = false;
-    while ((opt = w.wgetopt_long(argc, argv, short_options, long_options, nullptr)) != -1) {
-        if (opt != 'a' && opt != 1) handling_named_arguments = false;
-        switch (opt) {
-            case 1: {
-                if (handling_named_arguments) {
-                    opts.named_arguments.push_back(w.woptarg);
-                    break;
-                } else {
-                    streams.err.append_format(_(L"%ls: %ls: unexpected positional argument"), cmd,
-                                              w.woptarg);
-                    return STATUS_INVALID_ARGS;
-                }
-            }
-            case 'd': {
-                opts.description = w.woptarg;
-                break;
-            }
-            case 's': {
-                int sig = wcs2sig(w.woptarg);
-                if (sig == -1) {
-                    streams.err.append_format(_(L"%ls: Unknown signal '%ls'"), cmd, w.woptarg);
-                    return STATUS_INVALID_ARGS;
-                }
-                event_description_t event_desc;
-                event_desc.typ = event_type_t::signal;
-                event_desc.signal = sig;
-                opts.events.push_back(std::move(event_desc));
-                break;
-            }
-            case 'v': {
-                if (!valid_var_name(w.woptarg)) {
-                    streams.err.append_format(BUILTIN_ERR_VARNAME, cmd, w.woptarg);
-                    return STATUS_INVALID_ARGS;
-                }
-
-                event_description_t event_desc;
-                event_desc.typ = event_type_t::variable;
-                event_desc.str_param1 = std::make_unique<wcstring>(w.woptarg);
-                opts.events.push_back(std::move(event_desc));
-                break;
-            }
-            case 'e': {
-                event_description_t event_desc;
-                event_desc.typ = event_type_t::generic;
-                event_desc.str_param1 = std::make_unique<wcstring>(w.woptarg);
-                opts.events.push_back(std::move(event_desc));
-                break;
-            }
-            case 'j':
-            case 'p': {
-                event_description_t e;
-                e.typ = event_type_t::any;
-
-                if ((opt == 'j') && (wcscasecmp(w.woptarg, L"caller") == 0)) {
-                    internal_job_id_t caller_id =
-                        parser.libdata().is_subshell ? parser.libdata().caller_id : 0;
-                    if (caller_id == 0) {
-                        streams.err.append_format(
-                            _(L"%ls: calling job for event handler not found"), cmd);
-                        return STATUS_INVALID_ARGS;
-                    }
-                    e.typ = event_type_t::caller_exit;
-                    e.caller_id = caller_id;
-                } else if ((opt == 'p') && (wcscasecmp(w.woptarg, L"%self") == 0)) {
-                    e.typ = event_type_t::process_exit;
-                    e.pid = getpid();
-                } else {
-                    pid_t pid = fish_wcstoi(w.woptarg);
-                    if (errno || pid < 0) {
-                        streams.err.append_format(_(L"%ls: %ls: invalid process id"), cmd,
-                                                  w.woptarg);
-                        return STATUS_INVALID_ARGS;
-                    }
-                    if (opt == 'p') {
-                        e.typ = event_type_t::process_exit;
-                        e.pid = pid;
-                    } else {
-                        e.typ = event_type_t::job_exit;
-                        e.pid = pid;
-                        e.internal_job_id = job_id_for_pid(pid, parser);
-                    }
-                }
-                opts.events.push_back(std::move(e));
-                break;
-            }
-            case 'a': {
-                handling_named_arguments = true;
-                opts.named_arguments.push_back(w.woptarg);
-                break;
-            }
-            case 'S': {
-                opts.shadow_scope = false;
-                break;
-            }
-            case 'w': {
-                opts.wrap_targets.push_back(w.woptarg);
-                break;
-            }
-            case 'V': {
-                if (!valid_var_name(w.woptarg)) {
-                    streams.err.append_format(BUILTIN_ERR_VARNAME, cmd, w.woptarg);
-                    return STATUS_INVALID_ARGS;
-                }
-                opts.inherit_vars.push_back(w.woptarg);
-                break;
-            }
-            case 'h': {
-                opts.print_help = true;
-                break;
-            }
-            case ':': {
-                builtin_missing_argument(parser, streams, cmd, argv[w.woptind - 1]);
-                return STATUS_INVALID_ARGS;
-            }
-            case '?': {
-                builtin_unknown_option(parser, streams, cmd, argv[w.woptind - 1]);
-                return STATUS_INVALID_ARGS;
-            }
-            default: {
-                DIE("unexpected retval from wgetopt_long");
-            }
-        }
-    }
-
-    *optind = w.woptind;
-    return STATUS_CMD_OK;
-}
-
-static int validate_function_name(int argc, const wchar_t *const *argv, wcstring &function_name,
-                                  const wchar_t *cmd, io_streams_t &streams) {
-    if (argc < 2) {
-        // This is currently impossible but let's be paranoid.
-        streams.err.append_format(_(L"%ls: function name required"), cmd);
-        return STATUS_INVALID_ARGS;
-    }
-
-    function_name = argv[1];
-    if (!valid_func_name(function_name)) {
-        streams.err.append_format(_(L"%ls: %ls: invalid function name"), cmd,
-                                  function_name.c_str());
-        return STATUS_INVALID_ARGS;
-    }
-
-    if (parser_keywords_is_reserved(function_name)) {
-        streams.err.append_format(_(L"%ls: %ls: cannot use reserved keyword as function name"), cmd,
-                                  function_name.c_str());
-        return STATUS_INVALID_ARGS;
-    }
-
-    return STATUS_CMD_OK;
-}
-
-/// Define a function. Calls into `function.cpp` to perform the heavy lifting of defining a
-/// function.
-int builtin_function(parser_t &parser, io_streams_t &streams, const std::vector<wcstring> &c_args,
-                     const parsed_source_ref_t &source, const ast::block_statement_t &func_node) {
-    assert(source.has_value() && "Missing source in builtin_function");
-    // The wgetopt function expects 'function' as the first argument. Make a new wcstring_list with
-    // that property. This is needed because this builtin has a different signature than the other
-    // builtins.
-    std::vector<wcstring> args = {L"function"};
-    args.insert(args.end(), c_args.begin(), c_args.end());
-
-    null_terminated_array_t<wchar_t> argv_array(args);
-    const wchar_t **argv = argv_array.get();
-    const wchar_t *cmd = argv[0];
-    int argc = builtin_count_args(argv);
-
-    // A valid function name has to be the first argument.
-    wcstring function_name;
-    int retval = validate_function_name(argc, argv, function_name, cmd, streams);
-    if (retval != STATUS_CMD_OK) return retval;
-    argv++;
-    argc--;
-
-    function_cmd_opts_t opts;
-    int optind;
-    retval = parse_cmd_opts(opts, &optind, argc, argv, parser, streams);
-    if (retval != STATUS_CMD_OK) return retval;
-
-    if (opts.print_help) {
-        builtin_print_error_trailer(parser, streams.err, cmd);
-        return STATUS_CMD_OK;
-    }
-
-    if (argc != optind) {
-        if (!opts.named_arguments.empty()) {
-            for (int i = optind; i < argc; i++) {
-                if (!valid_var_name(argv[i])) {
-                    streams.err.append_format(BUILTIN_ERR_VARNAME, cmd, argv[i]);
-                    return STATUS_INVALID_ARGS;
-                }
-                opts.named_arguments.push_back(argv[i]);
-            }
-        } else {
-            streams.err.append_format(_(L"%ls: %ls: unexpected positional argument"), cmd,
-                                      argv[optind]);
-            return STATUS_INVALID_ARGS;
-        }
-    }
-
-    // We have what we need to actually define the function.
-    auto props = std::make_shared<function_properties_t>();
-    props->shadow_scope = opts.shadow_scope;
-    props->named_arguments = std::move(opts.named_arguments);
-    props->parsed_source = source.clone();
-    props->func_node = &func_node;
-    props->description = opts.description;
-    props->definition_file = parser.libdata().current_filename;
-
-    // Populate inherit_vars.
-    for (const wcstring &name : opts.inherit_vars) {
-        if (auto var = parser.vars().get(name)) {
-            props->inherit_vars[name] = var->as_list();
-        }
-    }
-
-    // Add the function itself.
-    function_add(function_name, props);
-
-    // Handle wrap targets by creating the appropriate completions.
-    for (const wcstring &wt : opts.wrap_targets) {
-        complete_add_wrapper(function_name, wt);
-    }
-
-    // Add any event handlers.
-    for (const event_description_t &ed : opts.events) {
-        event_add_handler(ed, function_name);
-    }
-
-    // If there is an --on-process-exit or --on-job-exit event handler for some pid, and that
-    // process has already exited, run it immediately (#7210).
-    for (const event_description_t &ed : opts.events) {
-        if (ed.typ == event_type_t::process_exit) {
-            pid_t pid = ed.pid;
-            if (pid == EVENT_ANY_PID) continue;
-            int status{};
-            uint64_t internal_job_id{};
-            if (parser.get_wait_handles_ffi()->try_get_status_and_job_id(pid, true, status,
-                                                                         internal_job_id)) {
-                event_fire(parser, *new_event_process_exit(pid, status));
-            }
-        } else if (ed.typ == event_type_t::job_exit) {
-            pid_t pid = ed.pid;
-            if (pid == EVENT_ANY_PID) continue;
-            int status{};
-            uint64_t internal_job_id{};
-            if (parser.get_wait_handles_ffi()->try_get_status_and_job_id(pid, true, status,
-                                                                         internal_job_id)) {
-                event_fire(parser, *new_event_job_exit(pid, internal_job_id));
-            }
-        }
-    }
-
-    return STATUS_CMD_OK;
-}

src/builtins/function.h

@@ -1,14 +0,0 @@
-// Prototypes for executing builtin_function function.
-#ifndef FISH_BUILTIN_FUNCTION_H
-#define FISH_BUILTIN_FUNCTION_H
-
-#include "../ast.h"
-#include "../common.h"
-#include "../parse_tree.h"
-
-class parser_t;
-struct io_streams_t;
-
-int builtin_function(parser_t &parser, io_streams_t &streams, const std::vector<wcstring> &c_args,
-                     const parsed_source_ref_t &source, const ast::block_statement_t &func_node);
-#endif

src/builtins/functions.cpp

@@ -139,26 +139,29 @@ static int report_function_metadata(const wcstring &funcname, bool verbose, io_s
     wcstring copy_path = L"n/a";
     int copy_line_number = 0;
 
-    if (auto props = function_get_props_autoload(funcname, parser)) {
+    if (auto mprops = function_get_props_autoload(funcname, parser)) {
-        if (props->definition_file) {
+        const auto &props = *mprops;
-            path = *props->definition_file;
+        if (auto def_file = props->definition_file()) {
-            autoloaded = props->is_autoload ? L"autoloaded" : L"not-autoloaded";
+            path = std::move(*def_file);
+            autoloaded = props->is_autoload() ? L"autoloaded" : L"not-autoloaded";
             line_number = props->definition_lineno();
         } else {
             path = L"stdin";
         }
 
-        is_copy = props->is_copy;
+        is_copy = props->is_copy();
 
-        if (props->copy_definition_file) {
+        auto definition_file = props->copy_definition_file();
-            copy_path = *props->copy_definition_file;
+        if (definition_file) {
-            copy_line_number = props->copy_definition_lineno;
+            copy_path = *definition_file;
+            copy_line_number = props->copy_definition_lineno();
         } else {
             copy_path = L"stdin";
         }
 
-        shadows_scope = props->shadow_scope ? L"scope-shadowing" : L"no-scope-shadowing";
+        shadows_scope = props->shadow_scope() ? L"scope-shadowing" : L"no-scope-shadowing";
-        description = escape_string(props->description, ESCAPE_NO_PRINTABLES | ESCAPE_NO_QUOTED);
+        description =
+            escape_string(*props->get_description(), ESCAPE_NO_PRINTABLES | ESCAPE_NO_QUOTED);
     }
 
     if (metadata_as_comments) {
@@ -298,7 +301,9 @@ maybe_t<int> builtin_functions(parser_t &parser, io_streams_t &streams, const wc
     }
 
     if (opts.list || argc == optind) {
-        std::vector<wcstring> names = function_get_names(opts.show_hidden);
+        wcstring_list_ffi_t names_ffi{};
+        function_get_names(opts.show_hidden, names_ffi);
+        std::vector<wcstring> names = std::move(names_ffi.vals);
         std::sort(names.begin(), names.end());
         bool is_screen = !streams.out_is_redirected && isatty(STDOUT_FILENO);
         if (is_screen) {
@@ -375,7 +380,8 @@ maybe_t<int> builtin_functions(parser_t &parser, io_streams_t &streams, const wc
                 if (!opts.no_metadata) {
                     report_function_metadata(funcname, opts.verbose, streams, parser, true);
                 }
-                wcstring def = func->annotated_definition(funcname);
+                std::unique_ptr<wcstring> def_ptr = (*func)->annotated_definition(funcname);
+                wcstring def = std::move(*def_ptr);
 
                 if (!streams.out_is_redirected && isatty(STDOUT_FILENO)) {
                     std::vector<highlight_spec_t> colors;

src/complete.cpp

@@ -622,7 +622,8 @@ void completer_t::complete_cmd_desc(const wcstring &str) {
 /// Returns a description for the specified function, or an empty string if none.
 static wcstring complete_function_desc(const wcstring &fn) {
     if (auto props = function_get_props(fn)) {
-        return props->description;
+        std::unique_ptr<wcstring> desc = (*props)->get_description();
+        return std::move(*desc);
     }
     return wcstring{};
 }
@@ -659,8 +660,9 @@ void completer_t::complete_cmd(const wcstring &str_cmd) {
 
     if (str_cmd.empty() || (str_cmd.find(L'/') == wcstring::npos && str_cmd.at(0) != L'~')) {
         bool include_hidden = !str_cmd.empty() && str_cmd.at(0) == L'_';
-        std::vector<wcstring> names = function_get_names(include_hidden);
+        wcstring_list_ffi_t names{};
-        for (wcstring &name : names) {
+        function_get_names(include_hidden, names);
+        for (wcstring &name : names.vals) {
             // Append all known matching functions
             append_completion(&possible_comp, std::move(name));
         }

src/env.cpp

@@ -385,6 +385,10 @@ std::vector<rust::Box<Event>> env_stack_t::universal_sync(bool always) {
     return std::move(result.events);
 }
 
+void env_stack_t::apply_inherited_ffi(const function_properties_t &props) {
+    impl_->apply_inherited_ffi(props);
+}
+
 statuses_t env_stack_t::get_last_statuses() const {
     auto statuses_ffi = impl_->get_last_statuses();
     statuses_t res{};

src/env.h

@@ -18,6 +18,7 @@
 #include "wutil.h"
 
 struct event_list_ffi_t;
+struct function_properties_t;
 
 #if INCLUDE_RUST_HEADERS
 #include "env/env_ffi.rs.h"
@@ -293,6 +294,9 @@ class env_stack_t final : public environment_t {
     /// \return a list of events for changed variables.
     std::vector<rust::Box<Event>> universal_sync(bool always);
 
+    /// Applies inherited variables in preparation for executing a function.
+    void apply_inherited_ffi(const function_properties_t &props);
+
     // Compatibility hack; access the "environment stack" from back when there was just one.
     static const std::shared_ptr<env_stack_t> &principal_ref();
     static env_stack_t &principal() { return *principal_ref(); }

src/exec.cpp

@@ -583,7 +583,7 @@ static block_t *function_prepare_environment(parser_t &parser, std::vector<wcstr
         func_name = std::move(*argv.begin());
         argv.erase(argv.begin());
     }
-    block_t *fb = parser.push_block(block_t::function_block(func_name, argv, props.shadow_scope));
+    block_t *fb = parser.push_block(block_t::function_block(func_name, argv, props.shadow_scope()));
     auto &vars = parser.vars();
 
     // Setup the environment for the function. There are three components of the environment:
@@ -592,7 +592,8 @@ static block_t *function_prepare_environment(parser_t &parser, std::vector<wcstr
     // 3. argv
 
     size_t idx = 0;
-    for (const wcstring &named_arg : props.named_arguments) {
+    auto args = props.named_arguments();
+    for (const wcstring &named_arg : args->vals) {
         if (idx < argv.size()) {
             vars.set_one(named_arg, ENV_LOCAL | ENV_USER, argv.at(idx));
         } else {
@@ -601,9 +602,10 @@ static block_t *function_prepare_environment(parser_t &parser, std::vector<wcstr
         idx++;
     }
 
-    for (const auto &kv : props.inherit_vars) {
+    vars.apply_inherited_ffi(props);
-        vars.set(kv.first, ENV_LOCAL | ENV_USER, kv.second);
+    // for (const auto &kv : props.inherit_vars) {
-    }
+    //     vars.set(kv.first, ENV_LOCAL | ENV_USER, kv.second);
+    // }
 
     vars.set_argv(std::move(argv));
     return fb;
@@ -651,12 +653,18 @@ static proc_performer_t get_performer_for_process(process_t *p, job_t *job,
             FLOGF(error, _(L"Unknown function '%ls'"), p->argv0());
             return proc_performer_t{};
         }
+        auto props_box = std::make_shared<rust::Box<function_properties_t>>(props.acquire());
         const std::vector<wcstring> &argv = p->argv();
         return [=](parser_t &parser) {
             // Pull out the job list from the function.
-            const ast::job_list_t &body = props->func_node->jobs();
+            const auto *func = reinterpret_cast<const ast::block_statement_t *>(
-            const block_t *fb = function_prepare_environment(parser, argv, *props);
+                (*props_box)->get_block_statement_node());
-            auto res = parser.eval_node(*props->parsed_source, body, io_chain, job_group);
+            const ast::job_list_t &body = func->jobs();
+            const block_t *fb = function_prepare_environment(parser, argv, **props_box);
+            const auto parsed_source_raw = (*props_box)->parsed_source();
+            const auto parsed_source_box = rust::Box<ParsedSourceRefFFI>::from_raw(
+                reinterpret_cast<ParsedSourceRefFFI *>(parsed_source_raw));
+            auto res = parser.eval_node(*parsed_source_box, body, io_chain, job_group);
             function_restore_environment(parser, fb);
 
             // If the function did not execute anything, treat it as success.

src/fish_tests.cpp

@@ -2444,23 +2444,6 @@ static void test_autoload() {
     autoload_tester_t::run_test();
 }
 
-// Construct function properties for testing.
-static std::shared_ptr<function_properties_t> make_test_func_props() {
-    auto ret = std::make_shared<function_properties_t>();
-    ret->parsed_source = parse_source(L"function stuff; end", parse_flag_none, nullptr);
-    assert(ret->parsed_source->has_value() && "Failed to parse");
-    for (auto ast_traversal = new_ast_traversal(*ret->parsed_source->ast().top());;) {
-        auto node = ast_traversal->next();
-        if (!node->has_value()) break;
-        if (const auto *s = node->try_as_block_statement()) {
-            ret->func_node = s;
-            break;
-        }
-    }
-    assert(ret->func_node && "Unable to find block statement");
-    return ret;
-}
-
 static void test_wildcards() {
     say(L"Testing wildcards");
     do_test(!wildcard_has(L""));
@@ -2486,7 +2469,6 @@ static void test_wildcards() {
 static void test_complete() {
     say(L"Testing complete");
 
-    auto func_props = make_test_func_props();
     struct test_complete_vars_t : environment_t {
         std::vector<wcstring> get_names(env_mode_flags_t flags) const override {
             UNUSED(flags);
@@ -2616,7 +2598,7 @@ static void test_complete() {
 #endif
 
     // Add a function and test completing it in various ways.
-    function_add(L"scuttlebutt", func_props);
+    parser->eval(L"function scuttlebutt; end", {});
 
     // Complete a function name.
     completions = do_complete(L"echo (scuttlebut", {});
@@ -2705,7 +2687,7 @@ static void test_complete() {
     completions.clear();
 
     // Test abbreviations.
-    function_add(L"testabbrsonetwothreefour", func_props);
+    parser->eval(L"function testabbrsonetwothreefour; end", {});
     abbrs_get_set()->add(L"somename", L"testabbrsonetwothreezero", L"expansion",
                          abbrs_position_t::command, false);
     completions = complete(L"testabbrsonetwothree", {}, parser->context());

src/function.cpp

@@ -6,438 +6,19 @@
 
 #include "function.h"
 
-#include <algorithm>
-#include <cstdint>
-#include <cwchar>
-#include <map>
-#include <memory>
-#include <string>
-#include <unordered_map>
-#include <unordered_set>
-#include <utility>
-#include <vector>
-
-#include "ast.h"
-#include "autoload.h"
 #include "common.h"
-#include "complete.h"
-#include "env.h"
-#include "event.h"
-#include "fallback.h"  // IWYU pragma: keep
-#include "maybe.h"
-#include "parse_constants.h"
-#include "parser.h"
-#include "parser_keywords.h"
-#include "signals.h"
-#include "wcstringutil.h"
-#include "wutil.h"  // IWYU pragma: keep
 
-namespace {
+maybe_t<rust::Box<function_properties_t>> function_get_props(const wcstring &name) {
-
+    if (auto *ptr = function_get_props_raw(name)) {
-/// Type wrapping up the set of all functions.
+        return rust::Box<function_properties_t>::from_raw(ptr);
-/// There's only one of these; it's managed by a lock.
+    }
-struct function_set_t {
+    return none();
-    /// The map of all functions by name.
-    std::unordered_map<wcstring, function_properties_ref_t> funcs;
-
-    /// Tombstones for functions that should no longer be autoloaded.
-    std::unordered_set<wcstring> autoload_tombstones;
-
-    /// The autoloader for our functions.
-    autoload_t autoloader{L"fish_function_path"};
-
-    /// Remove a function.
-    /// \return true if successful, false if it doesn't exist.
-    bool remove(const wcstring &name);
-
-    /// Get the properties for a function, or nullptr if none.
-    function_properties_ref_t get_props(const wcstring &name) const {
-        auto iter = funcs.find(name);
-        return iter == funcs.end() ? nullptr : iter->second;
 }
 
-    /// \return true if we should allow autoloading a given function.
+maybe_t<rust::Box<function_properties_t>> function_get_props_autoload(const wcstring &name,
-    bool allow_autoload(const wcstring &name) const;
+                                                                      parser_t &parser) {
-
+    if (auto *ptr = function_get_props_autoload_raw(name, parser)) {
-    function_set_t() = default;
+        return rust::Box<function_properties_t>::from_raw(ptr);
-};
-
-/// The big set of all functions.
-static owning_lock<function_set_t> function_set;
-
-bool function_set_t::allow_autoload(const wcstring &name) const {
-    // Prohibit autoloading if we have a non-autoload (explicit) function, or if the function is
-    // tombstoned.
-    auto props = get_props(name);
-    bool has_explicit_func = props && !props->is_autoload;
-    bool is_tombstoned = autoload_tombstones.count(name) > 0;
-    return !has_explicit_func && !is_tombstoned;
     }
-}  // namespace
+    return none();
-
-/// \return a copy of some function props, in a new shared_ptr.
-static std::shared_ptr<function_properties_t> copy_props(const function_properties_ref_t &props) {
-    assert(props && "Null props");
-    return std::make_shared<function_properties_t>(*props);
-}
-
-/// Make sure that if the specified function is a dynamically loaded function, it has been fully
-/// loaded.
-/// Note this executes fish script code.
-bool function_load(const wcstring &name, parser_t &parser) {
-    parser.assert_can_execute();
-    maybe_t<wcstring> path_to_autoload;
-    // Note we can't autoload while holding the funcset lock.
-    // Lock around a local region.
-    {
-        auto funcset = function_set.acquire();
-        if (funcset->allow_autoload(name)) {
-            path_to_autoload = funcset->autoloader.resolve_command(name, env_stack_t::globals());
-        }
-    }
-
-    // Release the lock and perform any autoload, then reacquire the lock and clean up.
-    if (path_to_autoload) {
-        // Crucially, the lock is acquired after perform_autoload().
-        autoload_t::perform_autoload(*path_to_autoload, parser);
-        function_set.acquire()->autoloader.mark_autoload_finished(name);
-    }
-    return path_to_autoload.has_value();
-}
-
-/// Insert a list of all dynamically loaded functions into the specified list.
-static void autoload_names(std::unordered_set<wcstring> &names, bool get_hidden) {
-    size_t i;
-
-    // TODO: justify this.
-    auto &vars = env_stack_t::principal();
-    const auto path_var = vars.get_unless_empty(L"fish_function_path");
-    if (!path_var) return;
-
-    const std::vector<wcstring> &path_list = path_var->as_list();
-
-    for (i = 0; i < path_list.size(); i++) {
-        const wcstring &ndir_str = path_list.at(i);
-        dir_iter_t dir(ndir_str);
-        if (!dir.valid()) continue;
-
-        while (const auto *entry = dir.next()) {
-            const wchar_t *fn = entry->name.c_str();
-            const wchar_t *suffix;
-            if (!get_hidden && fn[0] == L'_') continue;
-
-            suffix = std::wcsrchr(fn, L'.');
-            // We need a ".fish" *suffix*, it can't be the entire name.
-            if (suffix && suffix != fn && (std::wcscmp(suffix, L".fish") == 0)) {
-                // Also ignore directories.
-                if (!entry->is_dir()) {
-                    wcstring name(fn, suffix - fn);
-                    names.insert(name);
-                }
-            }
-        }
-    }
-}
-
-void function_add(wcstring name, std::shared_ptr<function_properties_t> props) {
-    assert(props && "Null props");
-    auto funcset = function_set.acquire();
-
-    // Historical check. TODO: rationalize this.
-    if (name.empty()) {
-        return;
-    }
-
-    // Remove the old function.
-    funcset->remove(name);
-
-    // Check if this is a function that we are autoloading.
-    props->is_autoload = funcset->autoloader.autoload_in_progress(name);
-
-    // Create and store a new function.
-    auto ins = funcset->funcs.emplace(std::move(name), std::move(props));
-    assert(ins.second && "Function should not already be present in the table");
-    (void)ins;
-}
-
-function_properties_ref_t function_get_props(const wcstring &name) {
-    if (parser_keywords_is_reserved(name)) return nullptr;
-    return function_set.acquire()->get_props(name);
-}
-
-wcstring function_get_definition_file(const function_properties_t &props) {
-    return props.definition_file ? *props.definition_file : L"";
-}
-
-wcstring function_get_copy_definition_file(const function_properties_t &props) {
-    return props.copy_definition_file ? *props.copy_definition_file : L"";
-}
-bool function_is_copy(const function_properties_t &props) { return props.is_copy; }
-int function_get_definition_lineno(const function_properties_t &props) {
-    return props.definition_lineno();
-}
-int function_get_copy_definition_lineno(const function_properties_t &props) {
-    return props.copy_definition_lineno;
-}
-
-wcstring function_get_annotated_definition(const function_properties_t &props,
-                                           const wcstring &name) {
-    return props.annotated_definition(name);
-}
-
-function_properties_ref_t function_get_props_autoload(const wcstring &name, parser_t &parser) {
-    parser.assert_can_execute();
-    if (parser_keywords_is_reserved(name)) return nullptr;
-    function_load(name, parser);
-    return function_get_props(name);
-}
-
-bool function_exists(const wcstring &cmd, parser_t &parser) {
-    parser.assert_can_execute();
-    if (!valid_func_name(cmd)) return false;
-    return function_get_props_autoload(cmd, parser) != nullptr;
-}
-
-bool function_exists_no_autoload(const wcstring &cmd) {
-    if (!valid_func_name(cmd)) return false;
-    if (parser_keywords_is_reserved(cmd)) return false;
-    auto funcset = function_set.acquire();
-
-    // Check if we either have the function, or it could be autoloaded.
-    return funcset->get_props(cmd) || funcset->autoloader.can_autoload(cmd);
-}
-
-bool function_set_t::remove(const wcstring &name) {
-    size_t amt = funcs.erase(name);
-    if (amt > 0) {
-        event_remove_function_handlers(name);
-    }
-    return amt > 0;
-}
-
-void function_remove(const wcstring &name) {
-    auto funcset = function_set.acquire();
-    funcset->remove(name);
-    // Prevent (re-)autoloading this function.
-    funcset->autoload_tombstones.insert(name);
-}
-
-// \return the body of a function (everything after the header, up to but not including the 'end').
-static wcstring get_function_body_source(const function_properties_t &props) {
-    // We want to preserve comments that the AST attaches to the header (#5285).
-    // Take everything from the end of the header to the 'end' keyword.
-    if (props.func_node->header().ptr()->try_source_range() &&
-        props.func_node->end().try_source_range()) {
-        auto header_src = props.func_node->header().ptr()->source_range();
-        auto end_kw_src = props.func_node->end().range();
-        uint32_t body_start = header_src.start + header_src.length;
-        uint32_t body_end = end_kw_src.start;
-        assert(body_start <= body_end && "end keyword should come after header");
-        return wcstring(props.parsed_source->src(), body_start, body_end - body_start);
-    }
-    return wcstring{};
-}
-
-void function_set_desc(const wcstring &name, const wcstring &desc, parser_t &parser) {
-    parser.assert_can_execute();
-    function_load(name, parser);
-    auto funcset = function_set.acquire();
-    auto iter = funcset->funcs.find(name);
-    if (iter != funcset->funcs.end()) {
-        // Note the description is immutable, as it may be accessed on another thread, so we copy
-        // the properties to modify it.
-        auto new_props = copy_props(iter->second);
-        new_props->description = desc;
-        iter->second = new_props;
-    }
-}
-
-bool function_copy(const wcstring &name, const wcstring &new_name, parser_t &parser) {
-    auto filename = parser.current_filename();
-    auto lineno = parser.get_lineno();
-
-    auto funcset = function_set.acquire();
-    auto props = funcset->get_props(name);
-    if (!props) {
-        // No such function.
-        return false;
-    }
-    // Copy the function's props.
-    auto new_props = copy_props(props);
-    new_props->is_autoload = false;
-    new_props->is_copy = true;
-    new_props->copy_definition_file = filename;
-    new_props->copy_definition_lineno = lineno;
-
-    // Note this will NOT overwrite an existing function with the new name.
-    // TODO: rationalize if this behavior is desired.
-    funcset->funcs.emplace(new_name, std::move(new_props));
-    return true;
-}
-
-std::vector<wcstring> function_get_names(bool get_hidden) {
-    std::unordered_set<wcstring> names;
-    auto funcset = function_set.acquire();
-    autoload_names(names, get_hidden);
-    for (const auto &func : funcset->funcs) {
-        const wcstring &name = func.first;
-
-        // Maybe skip hidden.
-        if (!get_hidden && (name.empty() || name.at(0) == L'_')) {
-            continue;
-        }
-        names.insert(name);
-    }
-    return std::vector<wcstring>(names.begin(), names.end());
-}
-
-void function_invalidate_path() {
-    // Remove all autoloaded functions and update the autoload path.
-    // Note we don't want to risk removal during iteration; we expect this to be called
-    // infrequently.
-    auto funcset = function_set.acquire();
-    std::vector<wcstring> autoloadees;
-    for (const auto &kv : funcset->funcs) {
-        if (kv.second->is_autoload) {
-            autoloadees.push_back(kv.first);
-        }
-    }
-    for (const wcstring &name : autoloadees) {
-        funcset->remove(name);
-    }
-    funcset->autoloader.clear();
-}
-
-function_properties_t::function_properties_t() : parsed_source(empty_parsed_source_ref()) {}
-
-function_properties_t::function_properties_t(const function_properties_t &other)
-    : parsed_source(empty_parsed_source_ref()) {
-    *this = other;
-}
-
-function_properties_t &function_properties_t::operator=(const function_properties_t &other) {
-    parsed_source = other.parsed_source->clone();
-    func_node = other.func_node;
-    named_arguments = other.named_arguments;
-    description = other.description;
-    inherit_vars = other.inherit_vars;
-    shadow_scope = other.shadow_scope;
-    is_autoload = other.is_autoload;
-    definition_file = other.definition_file;
-    return *this;
-}
-
-wcstring function_properties_t::annotated_definition(const wcstring &name) const {
-    wcstring out;
-    wcstring desc = this->localized_description();
-    wcstring def = get_function_body_source(*this);
-    auto handlers = event_get_function_handler_descs(name);
-
-    out.append(L"function ");
-
-    // Typically we prefer to specify the function name first, e.g. "function foo --description bar"
-    // But if the function name starts with a -, we'll need to output it after all the options.
-    bool defer_function_name = (name.at(0) == L'-');
-    if (!defer_function_name) {
-        out.append(escape_string(name));
-    }
-
-    // Output wrap targets.
-    for (const wcstring &wrap : complete_get_wrap_targets(name)) {
-        out.append(L" --wraps=");
-        out.append(escape_string(wrap));
-    }
-
-    if (!desc.empty()) {
-        out.append(L" --description ");
-        out.append(escape_string(desc));
-    }
-
-    if (!this->shadow_scope) {
-        out.append(L" --no-scope-shadowing");
-    }
-
-    for (const auto &d : handlers) {
-        switch (d.typ) {
-            case event_type_t::signal: {
-                append_format(out, L" --on-signal %ls", sig2wcs(d.signal)->c_str());
-                break;
-            }
-            case event_type_t::variable: {
-                append_format(out, L" --on-variable %ls", d.str_param1->c_str());
-                break;
-            }
-            case event_type_t::process_exit: {
-                append_format(out, L" --on-process-exit %d", d.pid);
-                break;
-            }
-            case event_type_t::job_exit: {
-                append_format(out, L" --on-job-exit %d", d.pid);
-                break;
-            }
-            case event_type_t::caller_exit: {
-                append_format(out, L" --on-job-exit caller");
-                break;
-            }
-            case event_type_t::generic: {
-                append_format(out, L" --on-event %ls", d.str_param1->c_str());
-                break;
-            }
-            case event_type_t::any:
-            default: {
-                DIE("unexpected next->typ");
-            }
-        }
-    }
-
-    const std::vector<wcstring> &named = this->named_arguments;
-    if (!named.empty()) {
-        append_format(out, L" --argument");
-        for (const auto &name : named) {
-            append_format(out, L" %ls", name.c_str());
-        }
-    }
-
-    // Output the function name if we deferred it.
-    if (defer_function_name) {
-        out.append(L" -- ");
-        out.append(escape_string(name));
-    }
-
-    // Output any inherited variables as `set -l` lines.
-    for (const auto &kv : this->inherit_vars) {
-        // We don't know what indentation style the function uses,
-        // so we do what fish_indent would.
-        append_format(out, L"\n    set -l %ls", kv.first.c_str());
-        for (const auto &arg : kv.second) {
-            out.push_back(L' ');
-            out.append(escape_string(arg));
-        }
-    }
-    out.push_back('\n');
-    out.append(def);
-
-    // Append a newline before the 'end', unless there already is one there.
-    if (!string_suffixes_string(L"\n", def)) {
-        out.push_back(L'\n');
-    }
-    out.append(L"end\n");
-    return out;
-}
-
-const wchar_t *function_properties_t::localized_description() const {
-    if (description.empty()) return L"";
-    return _(description.c_str());
-}
-
-int function_properties_t::definition_lineno() const {
-    // return one plus the number of newlines at offsets less than the start of our function's
-    // statement (which includes the header).
-    // TODO: merge with line_offset_of_character_at_offset?
-    assert(func_node->try_source_range() && "Function has no source range");
-    auto source_range = func_node->source_range();
-    uint32_t func_start = source_range.start;
-    const wcstring &source = parsed_source->src();
-    assert(func_start <= source.size() && "function start out of bounds");
-    return 1 + std::count(source.begin(), source.begin() + func_start, L'\n');
 }

src/function.h

@@ -4,120 +4,18 @@
 #ifndef FISH_FUNCTION_H
 #define FISH_FUNCTION_H
 
-#include <map>
+#include "cxx.h"
-#include <memory>
+#include "maybe.h"
-#include <string>
-
-#include "ast.h"
-#include "common.h"
-#include "parse_tree.h"
 
+struct function_properties_t;
 class parser_t;
 
-/// A function's constant properties. These do not change once initialized.
+#if INCLUDE_RUST_HEADERS
-struct function_properties_t {
+#include "function.rs.h"
-    function_properties_t();
+#endif
-    function_properties_t(const function_properties_t &other);
-    function_properties_t &operator=(const function_properties_t &other);
 
-    /// Parsed source containing the function.
+maybe_t<rust::Box<function_properties_t>> function_get_props(const wcstring &name);
-    rust::Box<parsed_source_ref_t> parsed_source;
+maybe_t<rust::Box<function_properties_t>> function_get_props_autoload(const wcstring &name,
-
+                                                                      parser_t &parser);
-    /// Node containing the function statement, pointing into parsed_source.
-    /// We store block_statement, not job_list, so that comments attached to the header are
-    /// preserved.
-    const ast::block_statement_t *func_node;
-
-    /// List of all named arguments for this function.
-    std::vector<wcstring> named_arguments;
-
-    /// Description of the function.
-    wcstring description;
-
-    /// Mapping of all variables that were inherited from the function definition scope to their
-    /// values.
-    std::map<wcstring, std::vector<wcstring>> inherit_vars;
-
-    /// Set to true if invoking this function shadows the variables of the underlying function.
-    bool shadow_scope{true};
-
-    /// Whether the function was autoloaded.
-    bool is_autoload{false};
-
-    /// The file from which the function was created, or nullptr if not from a file.
-    filename_ref_t definition_file{};
-
-    /// Whether the function was copied.
-    bool is_copy{false};
-
-    /// The file from which the function was copied, or nullptr if not from a file.
-    filename_ref_t copy_definition_file{};
-
-    /// The line number where the specified function was copied.
-    int copy_definition_lineno{};
-
-    /// \return the description, localized via _.
-    const wchar_t *localized_description() const;
-
-    /// \return the line number where the definition of the specified function started.
-    int definition_lineno() const;
-
-    /// \return a definition of the function, annotated with properties like event handlers and wrap
-    /// targets. This is to support the 'functions' builtin.
-    /// Note callers must provide the function name, since the function does not know its own name.
-    wcstring annotated_definition(const wcstring &name) const;
-};
-
-// FIXME: Morally, this is const, but cxx doesn't get it
-using function_properties_ref_t = std::shared_ptr<function_properties_t>;
-
-/// Add a function. This may mutate \p props to set is_autoload.
-void function_add(wcstring name, std::shared_ptr<function_properties_t> props);
-
-/// Remove the function with the specified name.
-void function_remove(const wcstring &name);
-
-/// \return the properties for a function, or nullptr if none. This does not trigger autoloading.
-function_properties_ref_t function_get_props(const wcstring &name);
-
-/// Guff to work around cxx not getting function_properties_t.
-wcstring function_get_definition_file(const function_properties_t &props);
-wcstring function_get_copy_definition_file(const function_properties_t &props);
-bool function_is_copy(const function_properties_t &props);
-int function_get_definition_lineno(const function_properties_t &props);
-int function_get_copy_definition_lineno(const function_properties_t &props);
-wcstring function_get_annotated_definition(const function_properties_t &props,
-                                           const wcstring &name);
-
-/// \return the properties for a function, or nullptr if none, perhaps triggering autoloading.
-function_properties_ref_t function_get_props_autoload(const wcstring &name, parser_t &parser);
-
-/// Try autoloading a function.
-/// \return true if something new was autoloaded, false if it was already loaded or did not exist.
-bool function_load(const wcstring &name, parser_t &parser);
-
-/// Sets the description of the function with the name \c name.
-/// This triggers autoloading.
-void function_set_desc(const wcstring &name, const wcstring &desc, parser_t &parser);
-
-/// Returns true if the function named \p cmd exists.
-/// This may autoload.
-bool function_exists(const wcstring &cmd, parser_t &parser);
-
-/// Returns true if the function \p cmd either is loaded, or exists on disk in an autoload
-/// directory.
-bool function_exists_no_autoload(const wcstring &cmd);
-
-/// Returns all function names.
-///
-/// \param get_hidden whether to include hidden functions, i.e. ones starting with an underscore.
-std::vector<wcstring> function_get_names(bool get_hidden);
-
-/// Creates a new function using the same definition as the specified function. Returns true if copy
-/// is successful.
-bool function_copy(const wcstring &name, const wcstring &new_name, parser_t &parser);
-
-/// Observes that fish_function_path has changed.
-void function_invalidate_path();
 
 #endif

src/parse_execution.cpp

@@ -20,7 +20,7 @@
 
 #include "ast.h"
 #include "builtin.h"
-#include "builtins/function.h"
+#include "builtins/function.rs.h"
 #include "common.h"
 #include "complete.h"
 #include "env.h"
@@ -388,21 +388,23 @@ end_execution_reason_t parse_execution_context_t::run_function_statement(
     const ast::block_statement_t &statement, const ast::function_header_t &header) {
     using namespace ast;
     // Get arguments.
-    std::vector<wcstring> arguments;
+    wcstring_list_ffi_t arguments;
     ast_args_list_t arg_nodes = get_argument_nodes(header.args());
     arg_nodes.insert(arg_nodes.begin(), &header.first_arg());
     end_execution_reason_t result =
-        this->expand_arguments_from_nodes(arg_nodes, &arguments, failglob);
+        this->expand_arguments_from_nodes(arg_nodes, &arguments.vals, failglob);
 
     if (result != end_execution_reason_t::ok) {
         return result;
     }
 
-    trace_if_enabled(*parser, L"function", arguments);
+    trace_if_enabled(*parser, L"function", arguments.vals);
     null_output_stream_t outs;
     string_output_stream_t errs;
     io_streams_t streams(outs, errs);
-    int err_code = builtin_function(*parser, streams, arguments, *pstree, statement);
+    int err_code = builtin_function_ffi(*parser, streams, arguments,
+                                        reinterpret_cast<const uint8_t *>(&*pstree), statement);
+
     parser->libdata().status_count++;
     parser->set_last_statuses(statuses_t::just(err_code));
 

src/parser.cpp

@@ -408,7 +408,7 @@ filename_ref_t parser_t::current_filename() const {
     for (const auto &b : block_list) {
         if (b.is_function_call()) {
             auto props = function_get_props(b.function_name);
-            return props ? props->definition_file : nullptr;
+            return props ? (*props)->definition_file() : nullptr;
         } else if (b.type() == block_type_t::source) {
             return b.sourced_file;
         }
@@ -537,6 +537,14 @@ job_t *parser_t::job_get_from_pid(int pid, size_t &job_pos) const {
     return nullptr;
 }
 
+const wcstring *library_data_t::get_current_filename() const {
+    if (current_filename) {
+        return &*current_filename;
+    } else {
+        return nullptr;
+    }
+}
+
 library_data_pod_t *parser_t::ffi_libdata_pod() { return &library_data; }
 
 job_t *parser_t::ffi_job_get_from_pid(int pid) const { return job_get_from_pid(pid); }

src/parser.h

@@ -228,6 +228,7 @@ struct library_data_t : public library_data_pod_t {
    public:
     wcstring get_status_vars_command() const { return status_vars.command; }
     wcstring get_status_vars_commandline() const { return status_vars.commandline; }
+    const wcstring *get_current_filename() const;  // may return nullptr if None
 };
 
 /// The result of parser_t::eval family.