Repository: dtolnay/cxx (rust)

Issue:
Allow UniquePtr::new on Trivial ExternTypes
Example from autocxx:

```rust
mod ffi {
    unsafe impl cxx::ExternType for bindgen::A {
        type Id = cxx::type_id!("A");
        type Kind = cxx::kind::Trivial;
    }
    mod bindgen {
        #[repr(C)]
        pub struct A {
            pub a: u32,
        }
    }
    #[cxx::bridge]
    pub mod cxxbridge {
        impl UniquePtr<A> {}
        extern "C" {
            include!("input.h");
            type A = super::bindgen::A;
        }
    }
}

fn main() {
    let a = ffi::cxxbridge::A { a: 14 };
    let _ = cxx::UniquePtr::new(a);
}
```

fails because it hits this `unreachable!`:

```rust
// Methods are private; not intended to be implemented outside of cxxbridge
// codebase.
pub unsafe trait UniquePtrTarget {
   //...
    #[doc(hidden)]
    fn __new(value: Self) -> *mut c_void
    where
        Self: Sized,
    {
        // Opaque C types do not get this method because they can never exist by
        // value on the Rust side of the bridge.
        let _ = value;
        unreachable!()
    }
    //...
}
```

The fix is for `expand_unique_ptr` in `expand.rs` to generate a `__new` method under slightly more circumstances, when an extern type is known to be trivial.

Pull request:
Allow creation of UniquePtrs to trivial aliased types.
Fixes #360.

@dtolnay the runtime check is out of character for cxx, but we can't do it at:
* generation time, because we don't know if an `ExternType` has claimed a given type to be trivial;
* build time, because what matters is whether the `UniquePtr::new` method is _called_ rather than whether it exists.

I think it would be possible to make the check at build-time by splitting `UniquePtrTarget` into `UniquePtrTarget` and `CreatableUniquePtrTarget` (such that the `new` method simply doesn't exist for some subset of `UniquePtrs`) but that's significantly beyond my bravery level.

Patch:
diff --git a/gen/src/write.rs b/gen/src/write.rs
index 5d47c631..9b4e120e 100644
--- a/gen/src/write.rs
+++ b/gen/src/write.rs
@@ -1245,7 +1245,12 @@ fn write_unique_ptr_common(out: &mut OutFile, ty: &Type, types: &Types) {
     let instance = to_mangled(&out.namespace, ty);
 
     let can_construct_from_value = match ty {
-        Type::Ident(ident) => types.structs.contains_key(ident),
+        // Some aliases are to opaque types; some are to trivial types.
+        // We can't know at code generation time, so we generate both C++
+        // and Rust side bindings for a "new" method anyway. But the Rust
+        // code can't be called for Opaque types because the 'new'
+        // method is not implemented.
+        Type::Ident(ident) => types.structs.contains_key(ident) || types.aliases.contains_key(ident),
         _ => false,
     };
 
diff --git a/macro/src/expand.rs b/macro/src/expand.rs
index 6fa7ea7a..4a4ec642 100644
--- a/macro/src/expand.rs
+++ b/macro/src/expand.rs
@@ -814,7 +814,7 @@ fn expand_unique_ptr(
     let link_release = format!("{}release", prefix);
     let link_drop = format!("{}drop", prefix);
 
-    let new_method = if types.structs.contains_key(ident) {
+    let new_method = if types.structs.contains_key(ident) || types.aliases.contains_key(ident) {
         Some(quote! {
             fn __new(mut value: Self) -> *mut ::std::ffi::c_void {
                 extern "C" {
diff --git a/src/unique_ptr.rs b/src/unique_ptr.rs
index 477c7169..4bb5d67b 100644
--- a/src/unique_ptr.rs
+++ b/src/unique_ptr.rs
@@ -1,5 +1,7 @@
 use crate::cxx_string::CxxString;
 use crate::cxx_vector::{self, CxxVector, VectorElement};
+use crate::ExternType;
+use crate::kind::Trivial;
 use core::ffi::c_void;
 use core::fmt::{self, Debug, Display};
 use core::marker::PhantomData;
@@ -32,7 +34,9 @@ where
     }
 
     /// Allocates memory on the heap and makes a UniquePtr pointing to it.
-    pub fn new(value: T) -> Self {
+    pub fn new(value: T) -> Self
+    where
+    T: ExternType<Kind = Trivial> {
         UniquePtr {
             repr: T::__new(value),
             ty: PhantomData,


Test patch:
diff --git a/tests/BUCK b/tests/BUCK
index 0ddf1a84..5bbe5009 100644
--- a/tests/BUCK
+++ b/tests/BUCK
@@ -3,7 +3,10 @@ load("//tools/buck:rust_cxx_bridge.bzl", "rust_cxx_bridge")
 rust_test(
     name = "test",
     srcs = ["test.rs"],
-    deps = [":ffi"],
+    deps = [
+        ":ffi",
+        "//:cxx",
+    ],
 )
 
 rust_library(
diff --git a/tests/BUILD b/tests/BUILD
index 233926b1..57ffab99 100644
--- a/tests/BUILD
+++ b/tests/BUILD
@@ -6,7 +6,10 @@ rust_test(
     name = "test",
     size = "small",
     srcs = ["test.rs"],
-    deps = [":cxx_test_suite"],
+    deps = [
+        ":cxx_test_suite",
+        "//:cxx",
+    ],
 )
 
 rust_library(
diff --git a/tests/ffi/lib.rs b/tests/ffi/lib.rs
index 7cffdedd..b3c23306 100644
--- a/tests/ffi/lib.rs
+++ b/tests/ffi/lib.rs
@@ -16,13 +16,13 @@ mod other {
 
     #[repr(C)]
     pub struct D {
-        d: u64,
+        pub d: u64,
     }
 
     #[repr(C)]
     pub struct E {
-        e: u64,
         e_str: CxxString,
+        e: u64,
     }
 
     unsafe impl ExternType for D {
diff --git a/tests/test.rs b/tests/test.rs
index a9cd8e16..b92eebfb 100644
--- a/tests/test.rs
+++ b/tests/test.rs
@@ -199,6 +199,7 @@ fn test_extern_trivial() {
     check!(ffi2::c_take_trivial(d));
     let d = ffi2::c_return_trivial_ptr();
     check!(ffi2::c_take_trivial_ptr(d));
+    cxx::UniquePtr::new(ffi2::D { d: 42 });
 }
 
 #[test]
diff --git a/tests/ui/unique_ptr_to_opaque.rs b/tests/ui/unique_ptr_to_opaque.rs
new file mode 100644
index 00000000..6f3bf8cf
--- /dev/null
+++ b/tests/ui/unique_ptr_to_opaque.rs
@@ -0,0 +1,26 @@
+mod outside {
+    #[repr(C)]
+    pub struct C {
+        pub a: u8,
+    }
+    unsafe impl cxx::ExternType for C {
+        type Id = cxx::type_id!("C");
+        type Kind = cxx::kind::Opaque;
+    }
+}
+
+
+#[cxx::bridge]
+mod ffi {
+    impl UniquePtr<C> {}
+
+    extern "C" {
+        type C = crate::outside::C;
+    }
+
+    impl UniquePtr<C> {}
+}
+
+fn main() {
+    cxx::UniquePtr::new(outside::C { a: 4 } );
+}
diff --git a/tests/ui/unique_ptr_to_opaque.stderr b/tests/ui/unique_ptr_to_opaque.stderr
new file mode 100644
index 00000000..5525a052
--- /dev/null
+++ b/tests/ui/unique_ptr_to_opaque.stderr
@@ -0,0 +1,7 @@
+error[E0271]: type mismatch resolving `<outside::C as ExternType>::Kind == Trivial`
+  --> $DIR/unique_ptr_to_opaque.rs:25:5
+   |
+25 |     cxx::UniquePtr::new(outside::C { a: 4 } );
+   |     ^^^^^^^^^^^^^^^^^^^ expected enum `Trivial`, found enum `cxx::kind::Opaque`
+   |
+   = note: required by `UniquePtr::<T>::new`