Fix moveit unsoundness

Cargo.toml

@@ -28,7 +28,7 @@ resolver = "2"
 autocxx-macro = { path="macro", version="0.25.0" }
 cxx = "1.0.78" # ... also needed because expansion of type_id refers to ::cxx
 aquamarine = "0.1" # docs
-moveit = { version = "0.5", features = [ "cxx" ] }
+moveit = { version = "0.6", features = [ "cxx" ] }
 
 [workspace]
 members = ["parser", "engine", "gen/cmd", "gen/build", "macro", "demo", "tools/reduce", "tools/mdbook-preprocessor", "integration-tests"]
@@ -39,3 +39,4 @@ exclude = ["examples/s2", "examples/steam-mini", "examples/subclass", "examples/
 #cxx-gen = { path="../cxx/gen/lib" }
 #autocxx-bindgen = { path="../bindgen" }
 #moveit = { path="../moveit" }
+#moveit = { git = "https://github.com/silvanshade/moveit", branch = "fix-deref-move-soundness-hole" }

engine/src/conversion/codegen_rs/mod.rs

@@ -729,7 +729,7 @@ impl<'a> RsCodeGenerator<'a> {
             bindgen_mod_items.push(parse_quote! {
                 impl autocxx::subclass::CppPeerConstructor<#cpp_id> for super::super::super::#id {
                     fn make_peer(&mut self, peer_holder: autocxx::subclass::CppSubclassRustPeerHolder<Self>) -> cxx::UniquePtr<#cpp_path> {
-                        use autocxx::moveit::EmplaceUnpinned;
+                        use autocxx::moveit::Emplace;
                         cxx::UniquePtr::emplace(#cpp_id :: new(peer_holder))
                     }
                 }

integration-tests/Cargo.toml

@@ -33,7 +33,7 @@ autocxx = { path = "..", version = "=0.25.0" }
 autocxx-engine = { version = "=0.25.0", path = "../engine", features = [
     "build",
 ] }
-moveit = { version = "0.5", features = [ "cxx" ] }
+moveit = { version = "0.6", features = [ "cxx" ] }
 link-cplusplus = "1.0"
 tempfile = "3.4"
 indoc = "1.0"

integration-tests/tests/integration_test.rs

@@ -9464,7 +9464,7 @@ fn test_uniqueptr_moveit() {
     };
     "};
     let rs = quote! {
-        use autocxx::moveit::EmplaceUnpinned;
+        use autocxx::moveit::Emplace;
         let mut up_obj = cxx::UniquePtr::emplace(ffi::A::new());
         up_obj.as_mut().unwrap().set(42);
         assert_eq!(up_obj.get(), 42);
@@ -9488,7 +9488,6 @@ fn test_various_emplacement() {
     };
     "};
     let rs = quote! {
-        use autocxx::moveit::EmplaceUnpinned;
         use autocxx::moveit::Emplace;
         let mut up_obj = cxx::UniquePtr::emplace(ffi::A::new());
         up_obj.pin_mut().set(666);
@@ -9552,7 +9551,7 @@ fn test_emplace_uses_overridden_new_and_delete() {
         assert!(ffi::was_delete_called());
         ffi::reset_flags();
         {
-            use autocxx::moveit::EmplaceUnpinned;
+            use autocxx::moveit::Emplace;
             let _ = cxx::UniquePtr::emplace(ffi::A::new());
         }
         assert!(ffi::was_delete_called());

src/lib.rs

@@ -588,8 +588,8 @@ pub trait WithinBox {
 
 use cxx::kind::Trivial;
 use cxx::ExternType;
+use moveit::Emplace;
 use moveit::MakeCppStorage;
-use moveit::{Emplace, EmplaceUnpinned};
 
 impl<N, T> WithinUniquePtr for N
 where
@@ -697,7 +697,6 @@ pub mod prelude {
     pub use moveit::moveit;
     pub use moveit::new::New;
     pub use moveit::Emplace;
-    pub use moveit::EmplaceUnpinned;
 }
 
 /// Re-export moveit for ease of consumers.

src/value_param.rs

@@ -7,7 +7,7 @@
 // except according to those terms.
 
 use cxx::{memory::UniquePtrTarget, UniquePtr};
-use moveit::{CopyNew, DerefMove, MoveNew, New};
+use moveit::{AsMove, CopyNew, MoveNew, New};
 use std::{marker::PhantomPinned, mem::MaybeUninit, ops::Deref, pin::Pin};
 
 /// A trait representing a parameter to a C++ function which is received
@@ -192,22 +192,21 @@ where
 
 /// Explicitly force a value parameter to be taken using any type of [`crate::moveit::new::New`],
 /// i.e. a constructor.
-pub fn as_new<N: New<Output = T>, T>(constructor: N) -> impl ValueParam<T> {
+pub fn as_new<N: New>(constructor: N) -> impl ValueParam<N::Output> {
     ByNew(constructor)
 }
 
 /// Explicitly force a value parameter to be taken by copy.
-pub fn as_copy<P: Deref<Target = T>, T>(ptr: P) -> impl ValueParam<T>
+pub fn as_copy<P: Deref>(ptr: P) -> impl ValueParam<P::Target>
 where
-    T: CopyNew,
+    P::Target: CopyNew,
 {
     ByNew(crate::moveit::new::copy(ptr))
 }
 
 /// Explicitly force a value parameter to be taken using C++ move semantics.
-pub fn as_mov<P: DerefMove + Deref<Target = T>, T>(ptr: P) -> impl ValueParam<T>
+pub fn as_mov<P: AsMove>(ptr: P) -> impl ValueParam<P::Target>
 where
-    P: DerefMove,
     P::Target: MoveNew,
 {
     ByNew(crate::moveit::new::mov(ptr))
@@ -216,23 +215,20 @@ where
 #[doc(hidden)]
 pub struct ByNew<N: New>(N);
 
-unsafe impl<N, T> ValueParam<T> for ByNew<N>
-where
-    N: New<Output = T>,
-{
-    type StackStorage = MaybeUninit<T>;
+unsafe impl<N: New> ValueParam<N::Output> for ByNew<N> {
+    type StackStorage = MaybeUninit<N::Output>;
 
     unsafe fn populate_stack_space(self, mut stack: Pin<&mut Option<Self::StackStorage>>) {
         // Safety: we won't move/swap things within the pin.
         let slot = Pin::into_inner_unchecked(stack.as_mut());
         *slot = Some(MaybeUninit::uninit());
         self.0.new(Pin::new_unchecked(slot.as_mut().unwrap()))
     }
-    fn get_ptr(stack: Pin<&mut Self::StackStorage>) -> *mut T {
-        // Safety: it's OK to (briefly) create a reference to the T because we
+    fn get_ptr(stack: Pin<&mut Self::StackStorage>) -> *mut N::Output {
+        // Safety: it's OK to (briefly) create a reference to the N::Output because we
         // populated it within `populate_stack_space`. It's OK to unpack the pin
         // because we're not going to move the contents.
-        unsafe { Pin::into_inner_unchecked(stack).assume_init_mut() as *mut T }
+        unsafe { Pin::into_inner_unchecked(stack).assume_init_mut() as *mut N::Output }
     }
 
     fn do_drop(stack: Pin<&mut Self::StackStorage>) {