From 0681b6b53aa4f99277fe1c20539c1dd38c15e743 Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Sat, 29 Apr 2023 19:58:45 -0700 Subject: [PATCH] Make C++ env_var_t wrap Rust EnvVar This reimplements C++'s env_var_t to reference a Rust EnvVar. The C++ env_var_t is now just a thin wrapper. --- fish-rust/build.rs | 1 + fish-rust/src/env/env_ffi.rs | 119 +++++++++++++++++++++++++++++++++++ fish-rust/src/env/mod.rs | 1 + fish-rust/src/env/var.rs | 11 +++- fish-rust/src/wchar_ffi.rs | 18 ++++++ src/env.cpp | 53 +++++++++++++--- src/env.h | 76 ++++++++++++---------- src/wutil.h | 1 + 8 files changed, 233 insertions(+), 47 deletions(-) create mode 100644 fish-rust/src/env/env_ffi.rs diff --git a/fish-rust/build.rs b/fish-rust/build.rs index fd2383b80..c4b4d1a90 100644 --- a/fish-rust/build.rs +++ b/fish-rust/build.rs @@ -25,6 +25,7 @@ fn main() -> miette::Result<()> { let source_files = vec![ "src/abbrs.rs", "src/ast.rs", + "src/env/env_ffi.rs", "src/event.rs", "src/common.rs", "src/fd_monitor.rs", diff --git a/fish-rust/src/env/env_ffi.rs b/fish-rust/src/env/env_ffi.rs new file mode 100644 index 000000000..a4ca835e2 --- /dev/null +++ b/fish-rust/src/env/env_ffi.rs @@ -0,0 +1,119 @@ +use super::var::{EnvVar, EnvVarFlags}; +use crate::ffi::{wchar_t, wcharz_t, wcstring_list_ffi_t}; +use crate::wchar_ffi::WCharToFFI; +use crate::wchar_ffi::{AsWstr, WCharFromFFI}; +use cxx::{CxxWString, UniquePtr}; +use std::pin::Pin; + +#[allow(clippy::module_inception)] +#[cxx::bridge] +mod env_ffi { + /// Return values for `EnvStack::set()`. + #[repr(u8)] + #[cxx_name = "env_stack_set_result_t"] + enum EnvStackSetResult { + ENV_OK, + ENV_PERM, + ENV_SCOPE, + ENV_INVALID, + ENV_NOT_FOUND, + } + + extern "C++" { + include!("wutil.h"); + type wcstring_list_ffi_t = super::wcstring_list_ffi_t; + type wcharz_t = super::wcharz_t; + } + + extern "Rust" { + type EnvVar; + + fn is_empty(&self) -> bool; + + fn exports(&self) -> bool; + fn is_read_only(&self) -> bool; + fn is_pathvar(&self) -> bool; + + #[cxx_name = "equals"] + fn equals_ffi(&self, rhs: &EnvVar) -> bool; + + #[cxx_name = "as_string"] + fn as_string_ffi(&self) -> UniquePtr; + + #[cxx_name = "as_list"] + fn as_list_ffi(&self) -> UniquePtr; + + #[cxx_name = "to_list"] + fn to_list_ffi(&self, out: Pin<&mut wcstring_list_ffi_t>); + + #[cxx_name = "get_delimiter"] + fn get_delimiter_ffi(&self) -> wchar_t; + + #[cxx_name = "get_flags"] + fn get_flags_ffi(&self) -> u8; + + #[cxx_name = "clone_box"] + fn clone_box_ffi(&self) -> Box; + + #[cxx_name = "env_var_create"] + fn env_var_create_ffi(vals: &wcstring_list_ffi_t, flags: u8) -> Box; + + #[cxx_name = "env_var_create_from_name"] + fn env_var_create_from_name_ffi( + name: wcharz_t, + values: &wcstring_list_ffi_t, + ) -> Box; + } +} +pub use env_ffi::EnvStackSetResult; + +impl Default for EnvStackSetResult { + fn default() -> Self { + EnvStackSetResult::ENV_OK + } +} + +/// FFI bits. +impl EnvVar { + pub fn equals_ffi(&self, rhs: &EnvVar) -> bool { + self == rhs + } + + pub fn as_string_ffi(&self) -> UniquePtr { + self.as_string().to_ffi() + } + + pub fn as_list_ffi(&self) -> UniquePtr { + self.as_list().to_ffi() + } + + pub fn to_list_ffi(&self, mut out: Pin<&mut wcstring_list_ffi_t>) { + out.as_mut().clear(); + for val in self.as_list() { + out.as_mut().push(val); + } + } + + pub fn clone_box_ffi(&self) -> Box { + Box::new(self.clone()) + } + + pub fn get_flags_ffi(&self) -> u8 { + self.get_flags().bits() + } + + pub fn get_delimiter_ffi(self: &EnvVar) -> wchar_t { + self.get_delimiter().into() + } +} + +fn env_var_create_ffi(vals: &wcstring_list_ffi_t, flags: u8) -> Box { + Box::new(EnvVar::new_vec( + vals.from_ffi(), + EnvVarFlags::from_bits(flags).expect("invalid flags"), + )) +} + +pub fn env_var_create_from_name_ffi(name: wcharz_t, values: &wcstring_list_ffi_t) -> Box { + Box::new(EnvVar::new_from_name_vec(name.as_wstr(), values.from_ffi())) +} diff --git a/fish-rust/src/env/mod.rs b/fish-rust/src/env/mod.rs index 3dfe0f725..6f1912b75 100644 --- a/fish-rust/src/env/mod.rs +++ b/fish-rust/src/env/mod.rs @@ -1,3 +1,4 @@ +mod env_ffi; pub mod environment; pub mod var; diff --git a/fish-rust/src/env/var.rs b/fish-rust/src/env/var.rs index 4e8599902..e5bc96c14 100644 --- a/fish-rust/src/env/var.rs +++ b/fish-rust/src/env/var.rs @@ -11,7 +11,6 @@ pub const PATH_ARRAY_SEP: char = ':'; pub const NONPATH_ARRAY_SEP: char = ' '; -// Flags that may be passed as the 'mode' in env_stack_t::set() / environment_t::get(). bitflags! { /// Flags that may be passed as the 'mode' in env_stack_t::set() / environment_t::get(). #[repr(C)] @@ -71,6 +70,12 @@ pub enum EnvStackSetResult { ENV_NOT_FOUND, } +impl Default for EnvStackSetResult { + fn default() -> Self { + EnvStackSetResult::ENV_OK + } +} + /// A struct of configuration directories, determined in main() that fish will optionally pass to /// env_init. pub struct ConfigPaths { @@ -213,7 +218,7 @@ pub fn as_list(&self) -> &[WString] { } /// Returns the delimiter character used when converting from a list to a string. - fn get_delimiter(&self) -> char { + pub fn get_delimiter(&self) -> char { if self.is_pathvar() { PATH_ARRAY_SEP } else { @@ -250,7 +255,7 @@ pub fn setting_pathvar(&mut self, pathvar: bool) -> Self { } /// Returns flags for a variable with the given name. - fn flags_for(name: &wstr) -> EnvVarFlags { + pub fn flags_for(name: &wstr) -> EnvVarFlags { let mut result = EnvVarFlags::empty(); if is_read_only(name) { result.insert(EnvVarFlags::READ_ONLY); diff --git a/fish-rust/src/wchar_ffi.rs b/fish-rust/src/wchar_ffi.rs index b4881c733..f81cac2c0 100644 --- a/fish-rust/src/wchar_ffi.rs +++ b/fish-rust/src/wchar_ffi.rs @@ -104,6 +104,18 @@ fn into_cpp(self) -> cxx::UniquePtr { } } +impl ToCppWString for WString { + fn into_cpp(self) -> cxx::UniquePtr { + self.to_ffi() + } +} + +impl ToCppWString for &WString { + fn into_cpp(self) -> cxx::UniquePtr { + self.to_ffi() + } +} + /// WString may be converted to CxxWString. impl WCharToFFI for WString { type Target = cxx::UniquePtr; @@ -213,6 +225,12 @@ fn as_wstr(&'a self) -> &'a wstr { } } +impl AsWstr<'_> for wcharz_t { + fn as_wstr(&self) -> &wstr { + wstr::from_char_slice(self.chars()) + } +} + use crate::ffi_tests::add_test; add_test!("test_wcstring_list_ffi_t", || { let data: Vec = wcstring_list_ffi_t::get_test_data().from_ffi(); diff --git a/src/env.cpp b/src/env.cpp index ffe631fb8..77f4a9433 100644 --- a/src/env.cpp +++ b/src/env.cpp @@ -50,9 +50,8 @@ /// At init, we read all the environment variables from this array. extern char **environ; -/// The character used to delimit path and non-path variables in exporting and in string expansion. +/// The character used to delimit path variables in exporting and in string expansion. static constexpr wchar_t PATH_ARRAY_SEP = L':'; -static constexpr wchar_t NONPATH_ARRAY_SEP = L' '; bool curses_initialized = false; @@ -151,23 +150,57 @@ static export_generation_t next_export_generation() { return ++*val; } -const std::vector &env_var_t::as_list() const { return *vals_; } - -wchar_t env_var_t::get_delimiter() const { - return is_pathvar() ? PATH_ARRAY_SEP : NONPATH_ARRAY_SEP; +// static +env_var_t env_var_t::new_ffi(EnvVar *ptr) { + assert(ptr != nullptr && "env_var_t::new_ffi called with null pointer"); + return env_var_t(rust::Box::from_raw(ptr)); } -/// Return a string representation of the var. -wcstring env_var_t::as_string() const { return join_strings(*vals_, get_delimiter()); } +wchar_t env_var_t::get_delimiter() const { return impl_->get_delimiter(); } -void env_var_t::to_list(std::vector &out) const { out = *vals_; } +bool env_var_t::empty() const { return impl_->is_empty(); } +bool env_var_t::exports() const { return impl_->exports(); } +bool env_var_t::is_read_only() const { return impl_->is_read_only(); } +bool env_var_t::is_pathvar() const { return impl_->is_pathvar(); } +env_var_t::env_var_flags_t env_var_t::get_flags() const { return impl_->get_flags(); } + +wcstring env_var_t::as_string() const { + wcstring res = std::move(*impl_->as_string()); + return res; +} + +void env_var_t::to_list(std::vector &out) const { + wcstring_list_ffi_t list{}; + impl_->to_list(list); + out = std::move(list.vals); +} + +std::vector env_var_t::as_list() const { + std::vector res = std::move(impl_->as_list()->vals); + return res; +} + +env_var_t &env_var_t::operator=(const env_var_t &rhs) { + this->impl_ = rhs.impl_->clone_box(); + return *this; +} env_var_t::env_var_flags_t env_var_t::flags_for(const wchar_t *name) { env_var_flags_t result = 0; - if (is_read_only(name)) result |= flag_read_only; + if (::is_read_only(name)) result |= flag_read_only; return result; } +env_var_t::env_var_t(const env_var_t &rhs) : impl_(rhs.impl_->clone_box()) {} + +env_var_t::env_var_t(std::vector vals, env_var_flags_t flags) + : impl_(env_var_create(std::move(vals), flags)) {} + +env_var_t::env_var_t(const wchar_t *name, std::vector vals) + : impl_(env_var_create_from_name(name, std::move(vals))) {} + +bool env_var_t::operator==(const env_var_t &rhs) const { return impl_->equals(*rhs.impl_); } + /// \return a singleton empty list, to avoid unnecessary allocations in env_var_t. std::shared_ptr> env_var_t::empty_list() { static const auto s_empty_result = std::make_shared>(); diff --git a/src/env.h b/src/env.h index afb53e2e7..0f24bdcf7 100644 --- a/src/env.h +++ b/src/env.h @@ -15,6 +15,13 @@ #include "common.h" #include "cxx.h" #include "maybe.h" +#include "wutil.h" + +#if INCLUDE_RUST_HEADERS +#include "env/env_ffi.rs.h" +#else +struct EnvVar; +#endif struct owning_null_terminated_array_t; @@ -98,17 +105,6 @@ class env_var_t { public: using env_var_flags_t = uint8_t; - private: - env_var_t(std::shared_ptr> vals, env_var_flags_t flags) - : vals_(std::move(vals)), flags_(flags) {} - - /// The list of values in this variable. - /// shared_ptr allows for cheap copying. - std::shared_ptr> vals_{empty_list()}; - - /// Flag in this variable. - env_var_flags_t flags_{}; - public: enum { flag_export = 1 << 0, // whether the variable is exported @@ -117,69 +113,80 @@ class env_var_t { }; // Constructors. - env_var_t() = default; - env_var_t(const env_var_t &) = default; + env_var_t() : env_var_t{std::vector{}, 0} {} + env_var_t(const env_var_t &); env_var_t(env_var_t &&) = default; - env_var_t(std::vector vals, env_var_flags_t flags) - : env_var_t(std::make_shared>(std::move(vals)), flags) {} - + env_var_t(std::vector vals, env_var_flags_t flags); env_var_t(wcstring val, env_var_flags_t flags) - : env_var_t(std::vector{std::move(val)}, flags) {} + : env_var_t{std::vector{std::move(val)}, flags} {} // Constructors that infer the flags from a name. - env_var_t(const wchar_t *name, std::vector vals) - : env_var_t(std::move(vals), flags_for(name)) {} + env_var_t(const wchar_t *name, std::vector vals); + env_var_t(const wchar_t *name, wcstring val) + : env_var_t{name, std::vector{std::move(val)}} {} - env_var_t(const wchar_t *name, wcstring val) : env_var_t(std::move(val), flags_for(name)) {} + // Construct from FFI. This transfers ownership of the EnvVar, which should originate + // in Box::into_raw(). + static env_var_t new_ffi(EnvVar *ptr); - bool empty() const { return vals_->empty() || (vals_->size() == 1 && vals_->front().empty()); } - bool exports() const { return flags_ & flag_export; } - bool is_pathvar() const { return flags_ & flag_pathvar; } - env_var_flags_t get_flags() const { return flags_; } + // Get the underlying EnvVar pointer. + // Note you may need to mem::transmute this, since autocxx gets confused when going from Rust -> + // C++ -> Rust. + const EnvVar *ffi_ptr() const { return &*this->impl_; } + + bool empty() const; + bool exports() const; + bool is_read_only() const; + bool is_pathvar() const; + env_var_flags_t get_flags() const; wcstring as_string() const; void to_list(std::vector &out) const; - const std::vector &as_list() const; + std::vector as_list() const; + wcstring_list_ffi_t as_list_ffi() const { return as_list(); } /// \return the character used when delimiting quoted expansion. wchar_t get_delimiter() const; /// \return a copy of this variable with new values. env_var_t setting_vals(std::vector vals) const { - return env_var_t{std::move(vals), flags_}; + return env_var_t{std::move(vals), get_flags()}; } env_var_t setting_exports(bool exportv) const { - env_var_flags_t flags = flags_; + env_var_flags_t flags = get_flags(); if (exportv) { flags |= flag_export; } else { flags &= ~flag_export; } - return env_var_t{vals_, flags}; + return env_var_t{as_list(), flags}; } env_var_t setting_pathvar(bool pathvar) const { - env_var_flags_t flags = flags_; + env_var_flags_t flags = get_flags(); if (pathvar) { flags |= flag_pathvar; } else { flags &= ~flag_pathvar; } - return env_var_t{vals_, flags}; + return env_var_t{as_list(), flags}; } static env_var_flags_t flags_for(const wchar_t *name); static std::shared_ptr> empty_list(); - env_var_t &operator=(const env_var_t &) = default; + env_var_t &operator=(const env_var_t &); env_var_t &operator=(env_var_t &&) = default; - bool operator==(const env_var_t &rhs) const { - return *vals_ == *rhs.vals_ && flags_ == rhs.flags_; - } + bool operator==(const env_var_t &rhs) const; bool operator!=(const env_var_t &rhs) const { return !(*this == rhs); } + + private: + env_var_t(rust::Box &&impl) : impl_(std::move(impl)) {} + + rust::Box impl_; }; typedef std::unordered_map var_table_t; @@ -334,4 +341,5 @@ void unsetenv_lock(const char *name); /// Returns the originally inherited variables and their values. /// This is a simple key->value map and not e.g. cut into paths. const std::map &env_get_inherited(); + #endif diff --git a/src/wutil.h b/src/wutil.h index 8d1aef859..7ac99f681 100644 --- a/src/wutil.h +++ b/src/wutil.h @@ -47,6 +47,7 @@ struct wcstring_list_ffi_t { size_t size() const { return vals.size(); } const wcstring &at(size_t idx) const { return vals.at(idx); } + void clear() { vals.clear(); } /// Helper to construct one. static std::unique_ptr create() {