Repository: dtolnay/cxx (rust)

Issue:
CxxVector::push_back
Adding push_back/pop functionality (and other mutation functionality) to `CxxVector` would be helpful for `CxxVector`s containing types that rust can obtain (e.g. `CxxVector<CxxString>` or `CxxVector<SharedPtr<T>>`). Similarly, `CxxVector::new()` or `CxxVector::default()` would be helpful to create them from Rust.

Pull request description:
Add CxxVector::push in Rust
Closes #778.

Patch:
diff --git a/gen/src/builtin.rs b/gen/src/builtin.rs
index eaaa08d1..7ac9209c 100644
--- a/gen/src/builtin.rs
+++ b/gen/src/builtin.rs
@@ -30,6 +30,7 @@ pub struct Builtins<'a> {
     pub relocatable: bool,
     pub friend_impl: bool,
     pub is_complete: bool,
+    pub destroy: bool,
     pub deleter_if: bool,
     pub content: Content<'a>,
 }
@@ -334,6 +335,14 @@ pub(super) fn write(out: &mut OutFile) {
         writeln!(out, "}};");
     }
 
+    if builtin.destroy {
+        out.next_section();
+        writeln!(out, "template <typename T>");
+        writeln!(out, "void destroy(T *ptr) {{");
+        writeln!(out, "  ptr->~T();");
+        writeln!(out, "}}");
+    }
+
     if builtin.deleter_if {
         out.next_section();
         writeln!(out, "template <bool> struct deleter_if {{");
diff --git a/gen/src/write.rs b/gen/src/write.rs
index 7285a64a..f9627d36 100644
--- a/gen/src/write.rs
+++ b/gen/src/write.rs
@@ -1823,6 +1823,8 @@ fn write_cxx_vector(out: &mut OutFile, key: NamedImplKey) {
     let instance = element.to_mangled(out.types);
 
     out.include.cstddef = true;
+    out.include.utility = true;
+    out.builtin.destroy = true;
 
     writeln!(
         out,
@@ -1838,6 +1840,16 @@ fn write_cxx_vector(out: &mut OutFile, key: NamedImplKey) {
     );
     writeln!(out, "  return &(*s)[pos];");
     writeln!(out, "}}");
+    if out.types.is_maybe_trivial(element) {
+        writeln!(
+            out,
+            "void cxxbridge1$std$vector${}$push_back(::std::vector<{}> *v, {} *value) noexcept {{",
+            instance, inner, inner,
+        );
+        writeln!(out, "  v->push_back(::std::move(*value));");
+        writeln!(out, "  ::rust::destroy(value);");
+        writeln!(out, "}}");
+    }
 
     out.include.memory = true;
     write_unique_ptr_common(out, UniquePtr::CxxVector(element));
diff --git a/macro/src/expand.rs b/macro/src/expand.rs
index ee82e267..5ff5f8ef 100644
--- a/macro/src/expand.rs
+++ b/macro/src/expand.rs
@@ -1537,6 +1537,7 @@ fn expand_cxx_vector(
     let prefix = format!("cxxbridge1$std$vector${}$", resolve.name.to_symbol());
     let link_size = format!("{}size", prefix);
     let link_get_unchecked = format!("{}get_unchecked", prefix);
+    let link_push_back = format!("{}push_back", prefix);
     let unique_ptr_prefix = format!(
         "cxxbridge1$unique_ptr$std$vector${}$",
         resolve.name.to_symbol(),
@@ -1553,6 +1554,28 @@ fn expand_cxx_vector(
     let end_span = explicit_impl.map_or(key.end_span, |explicit| explicit.brace_token.span);
     let unsafe_token = format_ident!("unsafe", span = begin_span);
 
+    let can_pass_element_by_value = types.is_maybe_trivial(elem);
+    let push_back_method = if can_pass_element_by_value {
+        Some(quote_spanned! {end_span=>
+            #[doc(hidden)]
+            unsafe fn __push_back(
+                this: ::std::pin::Pin<&mut ::cxx::CxxVector<Self>>,
+                value: &mut ::std::mem::ManuallyDrop<Self>,
+            ) {
+                extern "C" {
+                    #[link_name = #link_push_back]
+                    fn __push_back #impl_generics(
+                        this: ::std::pin::Pin<&mut ::cxx::CxxVector<#elem #ty_generics>>,
+                        value: &mut ::std::mem::ManuallyDrop<#elem #ty_generics>,
+                    );
+                }
+                __push_back(this, value);
+            }
+        })
+    } else {
+        None
+    };
+
     quote_spanned! {end_span=>
         #unsafe_token impl #impl_generics ::cxx::private::VectorElement for #elem #ty_generics {
             #[doc(hidden)]
@@ -1575,6 +1598,7 @@ fn expand_cxx_vector(
                 }
                 __get_unchecked(v, pos)
             }
+            #push_back_method
             #[doc(hidden)]
             fn __unique_ptr_null() -> *mut ::std::ffi::c_void {
                 extern "C" {
diff --git a/src/cxx.cc b/src/cxx.cc
index c0ef7389..f1b8ad81 100644
--- a/src/cxx.cc
+++ b/src/cxx.cc
@@ -427,6 +427,13 @@ using isize_if_unique =
 } // namespace cxxbridge1
 } // namespace rust
 
+namespace {
+template <typename T>
+void destroy(T *ptr) {
+  ptr->~T();
+}
+} // namespace
+
 extern "C" {
 void cxxbridge1$unique_ptr$std$string$null(
     std::unique_ptr<std::string> *ptr) noexcept {
@@ -491,6 +498,13 @@ static_assert(sizeof(std::string) <= kMaxExpectedWordsInString * sizeof(void *),
     ptr->~unique_ptr();                                                        \
   }
 
+#define STD_VECTOR_TRIVIAL_OPS(RUST_TYPE, CXX_TYPE)                            \
+  void cxxbridge1$std$vector$##RUST_TYPE##$push_back(                          \
+      std::vector<CXX_TYPE> *v, CXX_TYPE *value) noexcept {                    \
+    v->push_back(std::move(*value));                                           \
+    destroy(value);                                                            \
+  }
+
 #define RUST_VEC_EXTERNS(RUST_TYPE, CXX_TYPE)                                  \
   void cxxbridge1$rust_vec$##RUST_TYPE##$new(                                  \
       rust::Vec<CXX_TYPE> *ptr) noexcept;                                      \
@@ -603,10 +617,13 @@ static_assert(sizeof(std::string) <= kMaxExpectedWordsInString * sizeof(void *),
   MACRO(f32, float)                                                            \
   MACRO(f64, double)
 
-#define FOR_EACH_STD_VECTOR(MACRO)                                             \
+#define FOR_EACH_TRIVIAL_STD_VECTOR(MACRO)                                     \
   FOR_EACH_NUMERIC(MACRO)                                                      \
   MACRO(usize, std::size_t)                                                    \
-  MACRO(isize, rust::isize)                                                    \
+  MACRO(isize, rust::isize)
+
+#define FOR_EACH_STD_VECTOR(MACRO)                                             \
+  FOR_EACH_TRIVIAL_STD_VECTOR(MACRO)                                           \
   MACRO(string, std::string)
 
 #define FOR_EACH_RUST_VEC(MACRO)                                               \
@@ -627,6 +644,7 @@ static_assert(sizeof(std::string) <= kMaxExpectedWordsInString * sizeof(void *),
 
 extern "C" {
 FOR_EACH_STD_VECTOR(STD_VECTOR_OPS)
+FOR_EACH_TRIVIAL_STD_VECTOR(STD_VECTOR_TRIVIAL_OPS)
 FOR_EACH_RUST_VEC(RUST_VEC_EXTERNS)
 FOR_EACH_SHARED_PTR(SHARED_PTR_OPS)
 } // extern "C"
diff --git a/src/cxx_vector.rs b/src/cxx_vector.rs
index 16433417..23f7621d 100644
--- a/src/cxx_vector.rs
+++ b/src/cxx_vector.rs
@@ -8,7 +8,7 @@ use core::ffi::c_void;
 use core::fmt::{self, Debug};
 use core::iter::FusedIterator;
 use core::marker::{PhantomData, PhantomPinned};
-use core::mem;
+use core::mem::{self, ManuallyDrop};
 use core::pin::Pin;
 use core::ptr;
 use core::slice;
@@ -146,6 +146,22 @@ where
     pub fn iter_mut(self: Pin<&mut Self>) -> IterMut<T> {
         IterMut { v: self, index: 0 }
     }
+
+    /// Appends an element to the back of the vector.
+    ///
+    /// Matches the behavior of C++ [std::vector\<T\>::push_back][push_back].
+    ///
+    /// [push_back]: https://en.cppreference.com/w/cpp/container/vector/push_back
+    pub fn push(self: Pin<&mut Self>, value: T)
+    where
+        T: ExternType<Kind = Trivial>,
+    {
+        let mut value = ManuallyDrop::new(value);
+        unsafe {
+            // C++ calls move constructor followed by destructor on `value`.
+            T::__push_back(self, &mut value);
+        }
+    }
 }
 
 /// Iterator over elements of a `CxxVector` by shared reference.
@@ -296,6 +312,14 @@ pub unsafe trait VectorElement: Sized {
     #[doc(hidden)]
     unsafe fn __get_unchecked(v: *mut CxxVector<Self>, pos: usize) -> *mut Self;
     #[doc(hidden)]
+    unsafe fn __push_back(v: Pin<&mut CxxVector<Self>>, value: &mut ManuallyDrop<Self>) {
+        // Opaque C type vector elements do not get this method because they can
+        // never exist by value on the Rust side of the bridge.
+        let _ = v;
+        let _ = value;
+        unreachable!()
+    }
+    #[doc(hidden)]
     fn __unique_ptr_null() -> *mut c_void;
     #[doc(hidden)]
     unsafe fn __unique_ptr_raw(raw: *mut CxxVector<Self>) -> *mut c_void;
@@ -307,8 +331,24 @@ pub unsafe trait VectorElement: Sized {
     unsafe fn __unique_ptr_drop(repr: *mut c_void);
 }
 
+macro_rules! vector_element_push_back {
+    (opaque, $segment:expr, $ty:ty) => {};
+    (trivial, $segment:expr, $ty:ty) => {
+        #[doc(hidden)]
+        unsafe fn __push_back(v: Pin<&mut CxxVector<$ty>>, value: &mut ManuallyDrop<$ty>) {
+            extern "C" {
+                attr! {
+                    #[link_name = concat!("cxxbridge1$std$vector$", $segment, "$push_back")]
+                    fn __push_back(_: Pin<&mut CxxVector<$ty>>, _: &mut ManuallyDrop<$ty>);
+                }
+            }
+            __push_back(v, value);
+        }
+    };
+}
+
 macro_rules! impl_vector_element {
-    ($segment:expr, $name:expr, $ty:ty) => {
+    ($kind:ident, $segment:expr, $name:expr, $ty:ty) => {
         const_assert_eq!(1, mem::align_of::<CxxVector<$ty>>());
 
         unsafe impl VectorElement for $ty {
@@ -336,6 +376,7 @@ macro_rules! impl_vector_element {
                 }
                 __get_unchecked(v, pos)
             }
+            vector_element_push_back!($kind, $segment, $ty);
             #[doc(hidden)]
             fn __unique_ptr_null() -> *mut c_void {
                 extern "C" {
@@ -396,7 +437,7 @@ macro_rules! impl_vector_element {
 
 macro_rules! impl_vector_element_for_primitive {
     ($ty:ident) => {
-        impl_vector_element!(stringify!($ty), stringify!($ty), $ty);
+        impl_vector_element!(trivial, stringify!($ty), stringify!($ty), $ty);
     };
 }
 
@@ -413,4 +454,4 @@ impl_vector_element_for_primitive!(isize);
 impl_vector_element_for_primitive!(f32);
 impl_vector_element_for_primitive!(f64);
 
-impl_vector_element!("string", "CxxString", CxxString);
+impl_vector_element!(opaque, "string", "CxxString", CxxString);


Test patch:
diff --git a/tests/ffi/tests.cc b/tests/ffi/tests.cc
index 214bfcd6..98069b28 100644
--- a/tests/ffi/tests.cc
+++ b/tests/ffi/tests.cc
@@ -342,7 +342,7 @@ void c_take_unique_ptr_vector_u8(std::unique_ptr<std::vector<uint8_t>> v) {
 }
 
 void c_take_unique_ptr_vector_f64(std::unique_ptr<std::vector<double>> v) {
-  if (v->size() == 4) {
+  if (v->size() == 5) {
     cxx_test_suite_set_correct();
   }
 }
@@ -354,7 +354,7 @@ void c_take_unique_ptr_vector_string(
 }
 
 void c_take_unique_ptr_vector_shared(std::unique_ptr<std::vector<Shared>> v) {
-  if (v->size() == 2) {
+  if (v->size() == 3) {
     cxx_test_suite_set_correct();
   }
 }
diff --git a/tests/test.rs b/tests/test.rs
index c8204ce0..60e943f1 100644
--- a/tests/test.rs
+++ b/tests/test.rs
@@ -153,12 +153,12 @@ fn test_c_take() {
     check!(ffi::c_take_unique_ptr_vector_u8(
         ffi::c_return_unique_ptr_vector_u8()
     ));
-    check!(ffi::c_take_unique_ptr_vector_f64(
-        ffi::c_return_unique_ptr_vector_f64()
-    ));
-    check!(ffi::c_take_unique_ptr_vector_shared(
-        ffi::c_return_unique_ptr_vector_shared()
-    ));
+    let mut vector = ffi::c_return_unique_ptr_vector_f64();
+    vector.pin_mut().push(9.0);
+    check!(ffi::c_take_unique_ptr_vector_f64(vector));
+    let mut vector = ffi::c_return_unique_ptr_vector_shared();
+    vector.pin_mut().push(ffi::Shared { z: 9 });
+    check!(ffi::c_take_unique_ptr_vector_shared(vector));
     check!(ffi::c_take_ref_vector(&ffi::c_return_unique_ptr_vector_u8()));
     let test_vec = [86_u8, 75_u8, 30_u8, 9_u8].to_vec();
     check!(ffi::c_take_rust_vec(test_vec.clone()));