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

@@ -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",

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<CxxWString>;
+
+        #[cxx_name = "as_list"]
+        fn as_list_ffi(&self) -> UniquePtr<wcstring_list_ffi_t>;
+
+        #[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<EnvVar>;
+
+        #[cxx_name = "env_var_create"]
+        fn env_var_create_ffi(vals: &wcstring_list_ffi_t, flags: u8) -> Box<EnvVar>;
+
+        #[cxx_name = "env_var_create_from_name"]
+        fn env_var_create_from_name_ffi(
+            name: wcharz_t,
+            values: &wcstring_list_ffi_t,
+        ) -> Box<EnvVar>;
+    }
+}
+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<CxxWString> {
+        self.as_string().to_ffi()
+    }
+
+    pub fn as_list_ffi(&self) -> UniquePtr<wcstring_list_ffi_t> {
+        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<Self> {
+        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<EnvVar> {
+    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<EnvVar> {
+    Box::new(EnvVar::new_from_name_vec(name.as_wstr(), values.from_ffi()))
+}

fish-rust/src/env/mod.rs

@@ -1,3 +1,4 @@
+mod env_ffi;
 pub mod environment;
 pub mod var;
 

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);

fish-rust/src/wchar_ffi.rs

@@ -104,6 +104,18 @@ fn into_cpp(self) -> cxx::UniquePtr<cxx::CxxWString> {
     }
 }
 
+impl ToCppWString for WString {
+    fn into_cpp(self) -> cxx::UniquePtr<cxx::CxxWString> {
+        self.to_ffi()
+    }
+}
+
+impl ToCppWString for &WString {
+    fn into_cpp(self) -> cxx::UniquePtr<cxx::CxxWString> {
+        self.to_ffi()
+    }
+}
+
 /// WString may be converted to CxxWString.
 impl WCharToFFI for WString {
     type Target = cxx::UniquePtr<cxx::CxxWString>;
@@ -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<WString> = wcstring_list_ffi_t::get_test_data().from_ffi();

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<wcstring> &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<EnvVar>::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<wcstring> &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<wcstring> &out) const {
+    wcstring_list_ffi_t list{};
+    impl_->to_list(list);
+    out = std::move(list.vals);
+}
+
+std::vector<wcstring> env_var_t::as_list() const {
+    std::vector<wcstring> 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<wcstring> 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<wcstring> 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<const std::vector<wcstring>> env_var_t::empty_list() {
     static const auto s_empty_result = std::make_shared<const std::vector<wcstring>>();

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<const std::vector<wcstring>> 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<const std::vector<wcstring>> 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<wcstring>{}, 0} {}
+    env_var_t(const env_var_t &);
     env_var_t(env_var_t &&) = default;
 
-    env_var_t(std::vector<wcstring> vals, env_var_flags_t flags)
-        : env_var_t(std::make_shared<std::vector<wcstring>>(std::move(vals)), flags) {}
-
+    env_var_t(std::vector<wcstring> vals, env_var_flags_t flags);
     env_var_t(wcstring val, env_var_flags_t flags)
-        : env_var_t(std::vector<wcstring>{std::move(val)}, flags) {}
+        : env_var_t{std::vector<wcstring>{std::move(val)}, flags} {}
 
     // Constructors that infer the flags from a name.
-    env_var_t(const wchar_t *name, std::vector<wcstring> vals)
-        : env_var_t(std::move(vals), flags_for(name)) {}
+    env_var_t(const wchar_t *name, std::vector<wcstring> vals);
+    env_var_t(const wchar_t *name, wcstring val)
+        : env_var_t{name, std::vector<wcstring>{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<wcstring> &out) const;
-    const std::vector<wcstring> &as_list() const;
+    std::vector<wcstring> 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<wcstring> 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<const std::vector<wcstring>> 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<EnvVar> &&impl) : impl_(std::move(impl)) {}
+
+    rust::Box<EnvVar> impl_;
 };
 typedef std::unordered_map<wcstring, env_var_t> 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<wcstring, wcstring> &env_get_inherited();
+
 #endif

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<wcstring_list_ffi_t> create() {