diff --git a/fish-rust/Cargo.lock b/fish-rust/Cargo.lock index 3f67137c4..33e1d1a25 100644 --- a/fish-rust/Cargo.lock +++ b/fish-rust/Cargo.lock @@ -379,6 +379,7 @@ dependencies = [ "libc", "lru", "miette", + "moveit", "nix", "num-traits", "once_cell", diff --git a/fish-rust/Cargo.toml b/fish-rust/Cargo.toml index e130eafbd..1f9c0df2a 100644 --- a/fish-rust/Cargo.toml +++ b/fish-rust/Cargo.toml @@ -18,6 +18,7 @@ errno = "0.2.8" inventory = { version = "0.3.3", optional = true} libc = "0.2.137" lru = "0.10.0" +moveit = "0.5.1" nix = { version = "0.25.0", default-features = false, features = [] } num-traits = "0.2.15" once_cell = "1.17.0" diff --git a/fish-rust/src/builtins/shared.rs b/fish-rust/src/builtins/shared.rs index de4f26b9b..2a33cf131 100644 --- a/fish-rust/src/builtins/shared.rs +++ b/fish-rust/src/builtins/shared.rs @@ -1,7 +1,7 @@ use crate::builtins::{printf, wait}; -use crate::ffi::{self, parser_t, wcharz_t, Repin, RustBuiltin}; -use crate::wchar::{self, wstr, L}; -use crate::wchar_ffi::{c_str, empty_wstring}; +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, WCharFromFFI}; use crate::wgetopt::{wgetopter_t, wopt, woption, woption_argument_t}; use libc::c_int; use std::pin::Pin; @@ -14,6 +14,7 @@ mod builtins_ffi { include!("builtin.h"); type wcharz_t = crate::ffi::wcharz_t; + type wcstring_list_ffi_t = crate::ffi::wcstring_list_ffi_t; type parser_t = crate::ffi::parser_t; type io_streams_t = crate::ffi::io_streams_t; type RustBuiltin = crate::ffi::RustBuiltin; @@ -22,7 +23,7 @@ mod builtins_ffi { fn rust_run_builtin( parser: Pin<&mut parser_t>, streams: Pin<&mut io_streams_t>, - cpp_args: &Vec, + cpp_args: &wcstring_list_ffi_t, builtin: RustBuiltin, status_code: &mut i32, ) -> bool; @@ -112,18 +113,12 @@ pub fn ffi_ref(&self) -> &builtins_ffi::io_streams_t { fn rust_run_builtin( parser: Pin<&mut parser_t>, streams: Pin<&mut builtins_ffi::io_streams_t>, - cpp_args: &Vec, + cpp_args: &wcstring_list_ffi_t, builtin: RustBuiltin, status_code: &mut i32, ) -> bool { - let mut storage = Vec::::new(); - for arg in cpp_args { - storage.push(arg.into()); - } - let mut args = Vec::new(); - for arg in &storage { - args.push(arg.as_utfstr()); - } + let storage: Vec = cpp_args.from_ffi(); + let mut args: Vec<&wstr> = storage.iter().map(|s| s.as_utfstr()).collect(); let streams = &mut io_streams_t::new(streams); match run_builtin(parser.unpin(), streams, args.as_mut_slice(), builtin) { diff --git a/fish-rust/src/ffi.rs b/fish-rust/src/ffi.rs index e166296ff..380e8d682 100644 --- a/fish-rust/src/ffi.rs +++ b/fish-rust/src/ffi.rs @@ -41,6 +41,7 @@ safety!(unsafe_ffi) generate_pod!("wcharz_t") + generate!("wcstring_list_ffi_t") generate!("make_fd_nonblocking") generate!("wperror") diff --git a/fish-rust/src/wchar_ffi.rs b/fish-rust/src/wchar_ffi.rs index 92c7f7137..1a1e17215 100644 --- a/fish-rust/src/wchar_ffi.rs +++ b/fish-rust/src/wchar_ffi.rs @@ -6,8 +6,9 @@ //! - wcharz_t: a "newtyped" pointer to a nul-terminated string, implemented in C++. //! This is useful for FFI boundaries, to work around autocxx limitations on pointers. -pub use crate::ffi::{wchar_t, wcharz_t}; +pub use crate::ffi::{wchar_t, wcharz_t, wcstring_list_ffi_t}; use crate::wchar::{wstr, WString}; +use autocxx::WithinUniquePtr; use once_cell::sync::Lazy; pub use widestring::u32cstr; pub use widestring::U32CString as W0String; @@ -93,11 +94,13 @@ fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { /// Convert self to a CxxWString, in preparation for using over FFI. /// We can't use "From" as WString is implemented in an external crate. pub trait WCharToFFI { - fn to_ffi(&self) -> cxx::UniquePtr; + type Target; + fn to_ffi(&self) -> Self::Target; } /// WString may be converted to CxxWString. impl WCharToFFI for WString { + type Target = cxx::UniquePtr; fn to_ffi(&self) -> cxx::UniquePtr { cxx::CxxWString::create(self.as_char_slice()) } @@ -105,6 +108,7 @@ fn to_ffi(&self) -> cxx::UniquePtr { /// wstr (wide string slices) may be converted to CxxWString. impl WCharToFFI for wstr { + type Target = cxx::UniquePtr; fn to_ffi(&self) -> cxx::UniquePtr { cxx::CxxWString::create(self.as_char_slice()) } @@ -112,6 +116,7 @@ fn to_ffi(&self) -> cxx::UniquePtr { /// wcharz_t (wide char) may be converted to CxxWString. impl WCharToFFI for wcharz_t { + type Target = cxx::UniquePtr; fn to_ffi(&self) -> cxx::UniquePtr { cxx::CxxWString::create(self.chars()) } @@ -121,39 +126,57 @@ fn to_ffi(&self) -> cxx::UniquePtr { pub trait WCharFromFFI { /// Convert from a CxxWString for FFI purposes. #[allow(clippy::wrong_self_convention)] - fn from_ffi(&self) -> Target; + fn from_ffi(self) -> Target; } -impl WCharFromFFI for cxx::CxxWString { - fn from_ffi(&self) -> WString { +impl WCharFromFFI for &cxx::CxxWString { + fn from_ffi(self) -> WString { WString::from_chars(self.as_chars()) } } -impl WCharFromFFI for cxx::UniquePtr { - fn from_ffi(&self) -> WString { +impl WCharFromFFI for &cxx::UniquePtr { + fn from_ffi(self) -> WString { WString::from_chars(self.as_chars()) } } -impl WCharFromFFI for cxx::SharedPtr { - fn from_ffi(&self) -> WString { +impl WCharFromFFI for &cxx::SharedPtr { + fn from_ffi(self) -> WString { WString::from_chars(self.as_chars()) } } -impl WCharFromFFI> for cxx::UniquePtr { - fn from_ffi(&self) -> Vec { +impl WCharFromFFI> for &cxx::UniquePtr { + fn from_ffi(self) -> Vec { self.as_bytes().to_vec() } } -impl WCharFromFFI> for cxx::SharedPtr { - fn from_ffi(&self) -> Vec { +impl WCharFromFFI> for &cxx::SharedPtr { + fn from_ffi(self) -> Vec { self.as_bytes().to_vec() } } +/// Convert wcstring_list_ffi_t to Vec. +impl WCharFromFFI> for &wcstring_list_ffi_t { + fn from_ffi(self) -> Vec { + let count: usize = self.size(); + (0..count).map(|i| self.at(i).from_ffi()).collect() + } +} + +/// Convert from the type we get back for C++ functions which return wcstring_list_ffi_t. +impl WCharFromFFI> for T +where + T: autocxx::moveit::new::New, +{ + fn from_ffi(self) -> Vec { + self.within_unique_ptr().as_ref().unwrap().from_ffi() + } +} + /// Convert from FFI types to a reference to a wide string (i.e. a [`wstr`]) without allocating. pub trait AsWstr<'a> { fn as_wstr(&'a self) -> &'a wstr; @@ -170,3 +193,10 @@ fn as_wstr(&'a self) -> &'a wstr { wstr::from_char_slice(self.as_chars()) } } + +use crate::ffi_tests::add_test; +add_test!("test_wcstring_list_ffi_t", || { + use crate::ffi::wcstring_list_ffi_t; + let data: Vec = wcstring_list_ffi_t::get_test_data().from_ffi(); + assert_eq!(data, vec!["foo", "bar", "baz"]); +}); diff --git a/src/builtin.cpp b/src/builtin.cpp index 9c2c0070e..4a1a3a912 100644 --- a/src/builtin.cpp +++ b/src/builtin.cpp @@ -569,7 +569,7 @@ static maybe_t try_get_rust_builtin(const wcstring &cmd) { static maybe_t builtin_run_rust(parser_t &parser, io_streams_t &streams, const wcstring_list_t &argv, RustBuiltin builtin) { int status_code; - bool update_status = rust_run_builtin(parser, streams, to_rust_string_vec(argv), builtin, status_code); + bool update_status = rust_run_builtin(parser, streams, argv, builtin, status_code); if (update_status) { return status_code; } else { diff --git a/src/wutil.cpp b/src/wutil.cpp index e7c5d4c31..cd20e3187 100644 --- a/src/wutil.cpp +++ b/src/wutil.cpp @@ -986,3 +986,8 @@ int file_id_t::compare_file_id(const file_id_t &rhs) const { } bool file_id_t::operator<(const file_id_t &rhs) const { return this->compare_file_id(rhs) < 0; } + +// static +wcstring_list_ffi_t wcstring_list_ffi_t::get_test_data() { + return wcstring_list_t{L"foo", L"bar", L"baz"}; +} diff --git a/src/wutil.h b/src/wutil.h index 1b94250b9..a615d71f3 100644 --- a/src/wutil.h +++ b/src/wutil.h @@ -37,6 +37,20 @@ struct wcharz_t { inline size_t length() const { return size(); } }; +// A helper type for passing vectors of strings back to Rust. +// This hides the vector so that autocxx doesn't complain about templates. +struct wcstring_list_ffi_t { + wcstring_list_t vals; + + /* implicit */ wcstring_list_ffi_t(wcstring_list_t vals) : vals(std::move(vals)) {} + + size_t size() const { return vals.size(); } + const wcstring &at(size_t idx) const { return vals.at(idx); } + + /// Helper function used in tests only. + static wcstring_list_ffi_t get_test_data(); +}; + class autoclose_fd_t; /// Wide character version of opendir(). Note that opendir() is guaranteed to set close-on-exec by