Repository: dtolnay/cxx
Pull request #514 (2020-11-26 03:36:03)

Generalize SliceRef codegen to non-u8 element type


Patch:
diff --git a/README.md b/README.md
index 49bd43ce..746e4661 100644
--- a/README.md
+++ b/README.md
@@ -322,8 +322,8 @@ returns of functions.
 <tr><th>name in Rust</th><th>name in C++</th><th>restrictions</th></tr>
 <tr><td>String</td><td>rust::String</td><td></td></tr>
 <tr><td>&amp;str</td><td>rust::Str</td><td></td></tr>
-<tr><td>&amp;[u8]</td><td>rust::Slice&lt;const uint8_t&gt;</td><td><sup><i>arbitrary &amp;[T] not implemented yet</i></sup></td></tr>
-<tr><td>&amp;mut [u8]</td><td>rust::Slice&lt;uint8_t&gt;</td><td><sup><i>arbitrary &amp;mut [T] not implemented yet</i></sup></td></tr>
+<tr><td>&amp;[T]</td><td>rust::Slice&lt;const T&gt;</td><td><sup><i>cannot hold opaque Rust or C++ type</i></sup></td></tr>
+<tr><td>&amp;mut [T]</td><td>rust::Slice&lt;T&gt;</td><td><sup><i>cannot hold opaque Rust or C++ type</i></sup></td></tr>
 <tr><td><a href="https://docs.rs/cxx/1.0/cxx/struct.CxxString.html">CxxString</a></td><td>std::string</td><td><sup><i>cannot be passed by value</i></sup></td></tr>
 <tr><td>Box&lt;T&gt;</td><td>rust::Box&lt;T&gt;</td><td><sup><i>cannot hold opaque C++ type</i></sup></td></tr>
 <tr><td><a href="https://docs.rs/cxx/1.0/cxx/struct.UniquePtr.html">UniquePtr&lt;T&gt;</a></td><td>std::unique_ptr&lt;T&gt;</td><td><sup><i>cannot hold opaque Rust type</i></sup></td></tr>
diff --git a/book/src/binding/slice.md b/book/src/binding/slice.md
index 4da7d739..106a2580 100644
--- a/book/src/binding/slice.md
+++ b/book/src/binding/slice.md
@@ -33,8 +33,8 @@ public:
 
 ### Restrictions:
 
-Only T=u8 i.e. rust::Slice\<const uint8\_t\> and rust::Slice\<uint8\_t\> are
-currently implemented. Support for arbitrary T is coming.
+T must not be an opaque Rust type or opaque C++ type. Support for opaque Rust
+types in slices is coming.
 
 Allowed as function argument or return value. Not supported in shared structs.
 
diff --git a/gen/src/write.rs b/gen/src/write.rs
index 0757146b..1aa128ff 100644
--- a/gen/src/write.rs
+++ b/gen/src/write.rs
@@ -167,10 +167,7 @@ fn pick_includes_and_builtins(out: &mut OutFile, apis: &[Api]) {
             Type::Str(_) => out.builtin.rust_str = true,
             Type::CxxVector(_) => out.include.vector = true,
             Type::Fn(_) => out.builtin.rust_fn = true,
-            Type::SliceRef(_) => {
-                out.include.cstdint = true;
-                out.builtin.rust_slice = true;
-            }
+            Type::SliceRef(_) => out.builtin.rust_slice = true,
             Type::Array(_) => out.include.array = true,
             Type::Ref(_) | Type::Void(_) => {}
         }
@@ -481,7 +478,8 @@ fn write_cxx_function_shim<'a>(out: &mut OutFile<'a>, efn: &'a ExternFn) {
             if slice.mutability.is_none() {
                 write!(out, "const ");
             }
-            write!(out, "uint8_t *>({0}.ptr), {0}.len)", arg.ident);
+            write_type_space(out, &slice.inner);
+            write!(out, "*>({0}.ptr), {0}.len)", arg.ident);
         } else if out.types.needs_indirect_abi(&arg.ty) {
             out.include.utility = true;
             write!(out, "::std::move(*{})", arg.ident);
@@ -891,12 +889,13 @@ fn write_type(out: &mut OutFile, ty: &Type) {
         Type::Str(_) => {
             write!(out, "::rust::Str");
         }
-        Type::SliceRef(ty) => {
+        Type::SliceRef(slice) => {
             write!(out, "::rust::Slice<");
-            if ty.mutability.is_none() {
+            if slice.mutability.is_none() {
                 write!(out, "const ");
             }
-            write!(out, "uint8_t>");
+            write_type(out, &slice.inner);
+            write!(out, ">");
         }
         Type::Fn(f) => {
             write!(out, "::rust::{}<", if f.throws { "TryFn" } else { "Fn" });
diff --git a/include/cxx.h b/include/cxx.h
index 064d51fd..1e939a5f 100644
--- a/include/cxx.h
+++ b/include/cxx.h
@@ -121,7 +121,7 @@ public:
 private:
   friend impl<Slice>;
   // Not necessarily ABI compatible with &[T]. Codegen will translate to
-  // cxx::rust_sliceu8::RustSliceU8 which matches this layout.
+  // cxx::rust_slice::RustSlice which matches this layout.
   T *ptr;
   size_t len;
 };
diff --git a/macro/src/expand.rs b/macro/src/expand.rs
index ccca8b54..c735483f 100644
--- a/macro/src/expand.rs
+++ b/macro/src/expand.rs
@@ -331,8 +331,8 @@ fn expand_cxx_function_shim(efn: &ExternFn, types: &Types) -> TokenStream {
             },
             Type::Str(_) => quote!(::cxx::private::RustStr::from(#var)),
             Type::SliceRef(ty) => match ty.mutable {
-                false => quote!(::cxx::private::RustSliceU8::from_ref(#var)),
-                true => quote!(::cxx::private::RustSliceU8::from_mut(#var)),
+                false => quote!(::cxx::private::RustSlice::from_ref(#var)),
+                true => quote!(::cxx::private::RustSlice::from_mut(#var)),
             },
             ty if types.needs_indirect_abi(ty) => quote!(#var.as_mut_ptr()),
             _ => quote!(#var),
@@ -426,10 +426,13 @@ fn expand_cxx_function_shim(efn: &ExternFn, types: &Types) -> TokenStream {
                     _ => call,
                 },
                 Type::Str(_) => quote!(#call.as_str()),
-                Type::SliceRef(ty) => match ty.mutable {
-                    false => quote!(#call.as_slice()),
-                    true => quote!(#call.as_mut_slice()),
-                },
+                Type::SliceRef(slice) => {
+                    let inner = &slice.inner;
+                    match slice.mutable {
+                        false => quote!(#call.as_slice::<#inner>()),
+                        true => quote!(#call.as_mut_slice::<#inner>()),
+                    }
+                }
                 _ => call,
             },
         };
@@ -616,10 +619,13 @@ fn expand_rust_function_shim_impl(
                 _ => quote!(#ident),
             },
             Type::Str(_) => quote!(#ident.as_str()),
-            Type::SliceRef(ty) => match ty.mutable {
-                false => quote!(#ident.as_slice()),
-                true => quote!(#ident.as_mut_slice()),
-            },
+            Type::SliceRef(slice) => {
+                let inner = &slice.inner;
+                match slice.mutable {
+                    false => quote!(#ident.as_slice::<#inner>()),
+                    true => quote!(#ident.as_mut_slice::<#inner>()),
+                }
+            }
             ty if types.needs_indirect_abi(ty) => quote!(::std::ptr::read(#ident)),
             _ => quote!(#ident),
         }
@@ -664,8 +670,8 @@ fn expand_rust_function_shim_impl(
         },
         Type::Str(_) => Some(quote!(::cxx::private::RustStr::from)),
         Type::SliceRef(ty) => match ty.mutable {
-            false => Some(quote!(::cxx::private::RustSliceU8::from_ref)),
-            true => Some(quote!(::cxx::private::RustSliceU8::from_mut)),
+            false => Some(quote!(::cxx::private::RustSlice::from_ref)),
+            true => Some(quote!(::cxx::private::RustSlice::from_mut)),
         },
         _ => None,
     });
@@ -1107,7 +1113,7 @@ fn expand_extern_type(ty: &Type, types: &Types, proper: bool) -> TokenStream {
             }
         }
         Type::Str(_) => quote!(::cxx::private::RustStr),
-        Type::SliceRef(_) => quote!(::cxx::private::RustSliceU8),
+        Type::SliceRef(_) => quote!(::cxx::private::RustSlice),
         _ => quote!(#ty),
     }
 }
diff --git a/src/lib.rs b/src/lib.rs
index e971b50f..a803d55b 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -331,8 +331,8 @@
 //! <tr><th>name in Rust</th><th>name in C++</th><th>restrictions</th></tr>
 //! <tr><td>String</td><td>rust::String</td><td></td></tr>
 //! <tr><td>&amp;str</td><td>rust::Str</td><td></td></tr>
-//! <tr><td>&amp;[u8]</td><td>rust::Slice&lt;const uint8_t&gt;</td><td><sup><i>arbitrary &amp;[T] not implemented yet</i></sup></td></tr>
-//! <tr><td>&amp;mut [u8]</td><td>rust::Slice&lt;uint8_t&gt;</td><td><sup><i>arbitrary &amp;mut [T] not implemented yet</i></sup></td></tr>
+//! <tr><td>&amp;[T]</td><td>rust::Slice&lt;const T&gt;</td><td><sup><i>cannot hold opaque Rust or C++ type</i></sup></td></tr>
+//! <tr><td>&amp;mut [T]</td><td>rust::Slice&lt;T&gt;</td><td><sup><i>cannot hold opaque Rust or C++ type</i></sup></td></tr>
 //! <tr><td><a href="struct.CxxString.html">CxxString</a></td><td>std::string</td><td><sup><i>cannot be passed by value</i></sup></td></tr>
 //! <tr><td>Box&lt;T&gt;</td><td>rust::Box&lt;T&gt;</td><td><sup><i>cannot hold opaque C++ type</i></sup></td></tr>
 //! <tr><td><a href="struct.UniquePtr.html">UniquePtr&lt;T&gt;</a></td><td>std::unique_ptr&lt;T&gt;</td><td><sup><i>cannot hold opaque Rust type</i></sup></td></tr>
@@ -398,7 +398,7 @@ mod extern_type;
 mod function;
 mod opaque;
 mod result;
-mod rust_sliceu8;
+mod rust_slice;
 mod rust_str;
 mod rust_string;
 mod rust_type;
@@ -442,7 +442,7 @@ pub mod private {
     pub use crate::function::FatFunction;
     pub use crate::opaque::Opaque;
     pub use crate::result::{r#try, Result};
-    pub use crate::rust_sliceu8::RustSliceU8;
+    pub use crate::rust_slice::RustSlice;
     pub use crate::rust_str::RustStr;
     pub use crate::rust_string::RustString;
     pub use crate::rust_type::RustType;
diff --git a/src/rust_slice.rs b/src/rust_slice.rs
new file mode 100644
index 00000000..8a5e74b3
--- /dev/null
+++ b/src/rust_slice.rs
@@ -0,0 +1,40 @@
+use core::mem;
+use core::ptr::NonNull;
+use core::slice;
+
+// Not necessarily ABI compatible with &[T]. Codegen performs the translation.
+#[repr(C)]
+#[derive(Copy, Clone)]
+pub struct RustSlice {
+    pub(crate) ptr: NonNull<()>,
+    pub(crate) len: usize,
+}
+
+impl RustSlice {
+    pub fn from_ref<T>(s: &[T]) -> Self {
+        RustSlice {
+            ptr: NonNull::from(s).cast::<()>(),
+            len: s.len(),
+        }
+    }
+
+    pub fn from_mut<T>(s: &mut [T]) -> Self {
+        RustSlice {
+            len: s.len(),
+            ptr: NonNull::from(s).cast::<()>(),
+        }
+    }
+
+    pub unsafe fn as_slice<'a, T>(self) -> &'a [T] {
+        slice::from_raw_parts(self.ptr.as_ptr().cast::<T>(), self.len)
+    }
+
+    pub unsafe fn as_mut_slice<'a, T>(self) -> &'a mut [T] {
+        slice::from_raw_parts_mut(self.ptr.as_ptr().cast::<T>(), self.len)
+    }
+}
+
+const_assert_eq!(
+    mem::size_of::<Option<RustSlice>>(),
+    mem::size_of::<RustSlice>(),
+);
diff --git a/src/rust_sliceu8.rs b/src/rust_sliceu8.rs
deleted file mode 100644
index e14f9885..00000000
--- a/src/rust_sliceu8.rs
+++ /dev/null
@@ -1,40 +0,0 @@
-use core::mem;
-use core::ptr::NonNull;
-use core::slice;
-
-// Not necessarily ABI compatible with &[u8]. Codegen performs the translation.
-#[repr(C)]
-#[derive(Copy, Clone)]
-pub struct RustSliceU8 {
-    pub(crate) ptr: NonNull<u8>,
-    pub(crate) len: usize,
-}
-
-impl RustSliceU8 {
-    pub fn from_ref(s: &[u8]) -> Self {
-        RustSliceU8 {
-            ptr: NonNull::from(s).cast::<u8>(),
-            len: s.len(),
-        }
-    }
-
-    pub fn from_mut(s: &mut [u8]) -> Self {
-        RustSliceU8 {
-            len: s.len(),
-            ptr: NonNull::from(s).cast::<u8>(),
-        }
-    }
-
-    pub unsafe fn as_slice<'a>(self) -> &'a [u8] {
-        slice::from_raw_parts(self.ptr.as_ptr(), self.len)
-    }
-
-    pub unsafe fn as_mut_slice<'a>(self) -> &'a mut [u8] {
-        slice::from_raw_parts_mut(self.ptr.as_ptr(), self.len)
-    }
-}
-
-const_assert_eq!(
-    mem::size_of::<Option<RustSliceU8>>(),
-    mem::size_of::<RustSliceU8>(),
-);
diff --git a/syntax/check.rs b/syntax/check.rs
index 960c5695..f5e8e00c 100644
--- a/syntax/check.rs
+++ b/syntax/check.rs
@@ -179,16 +179,22 @@ fn check_type_ref(cx: &mut Check, ty: &Ref) {
 }
 
 fn check_type_slice_ref(cx: &mut Check, ty: &SliceRef) {
-    match &ty.inner {
-        Type::Ident(ident) if ident.rust == U8 => {}
-        _ => {
-            let mutable = if ty.mutable { "mut " } else { "" };
-            let msg = format!(
-                "only &{}[u8] is supported so far, not other slice types",
-                mutable,
-            );
-            cx.error(ty, msg);
+    let supported = match &ty.inner {
+        Type::Str(_) | Type::SliceRef(_) => false,
+        element => !is_unsized(cx, element),
+    };
+
+    if !supported {
+        let mutable = if ty.mutable { "mut " } else { "" };
+        let mut msg = format!("unsupported &{}[T] element type", mutable);
+        if let Type::Ident(ident) = &ty.inner {
+            if cx.types.rust.contains(&ident.rust) {
+                msg += ": opaque Rust type is not supported yet";
+            } else if is_opaque_cxx(cx, &ident.rust) {
+                msg += ": opaque C++ type is not supported yet";
+            }
         }
+        cx.error(ty, msg);
     }
 }
 
@@ -507,10 +513,7 @@ fn describe(cx: &mut Check, ty: &Type) -> String {
         Type::Ref(_) => "reference".to_owned(),
         Type::Str(_) => "&str".to_owned(),
         Type::CxxVector(_) => "C++ vector".to_owned(),
-        Type::SliceRef(ty) => match ty.mutable {
-            false => "&[u8]".to_owned(),
-            true => "&mut [u8]".to_owned(),
-        },
+        Type::SliceRef(_) => "slice".to_owned(),
         Type::Fn(_) => "function pointer".to_owned(),
         Type::Void(_) => "()".to_owned(),
         Type::Array(_) => "array".to_owned(),
diff --git a/syntax/types.rs b/syntax/types.rs
index 89832733..8c628b38 100644
--- a/syntax/types.rs
+++ b/syntax/types.rs
@@ -42,13 +42,14 @@ impl<'a> Types<'a> {
         fn visit<'a>(all: &mut Set<&'a Type>, ty: &'a Type) {
             all.insert(ty);
             match ty {
-                Type::Ident(_) | Type::Str(_) | Type::Void(_) | Type::SliceRef(_) => {}
+                Type::Ident(_) | Type::Str(_) | Type::Void(_) => {}
                 Type::RustBox(ty)
                 | Type::UniquePtr(ty)
                 | Type::CxxVector(ty)
                 | Type::RustVec(ty) => visit(all, &ty.inner),
                 Type::Ref(r) => visit(all, &r.inner),
                 Type::Array(a) => visit(all, &a.inner),
+                Type::SliceRef(s) => visit(all, &s.inner),
                 Type::Fn(f) => {
                     if let Some(ret) = &f.ret {
                         visit(all, ret);


Test patch:
diff --git a/tests/ffi/lib.rs b/tests/ffi/lib.rs
index 2d7544ae..5ea7584b 100644
--- a/tests/ffi/lib.rs
+++ b/tests/ffi/lib.rs
@@ -111,6 +111,7 @@ pub mod ffi {
         fn c_take_ref_c(c: &C);
         fn c_take_str(s: &str);
         fn c_take_sliceu8(s: &[u8]);
+        fn c_take_slice_shared(s: &[Shared]);
         fn c_take_rust_string(s: String);
         fn c_take_unique_ptr_string(s: UniquePtr<CxxString>);
         fn c_take_unique_ptr_vector_u8(v: UniquePtr<CxxVector<u8>>);
diff --git a/tests/ffi/tests.cc b/tests/ffi/tests.cc
index 38942e18..4ba48bf5 100644
--- a/tests/ffi/tests.cc
+++ b/tests/ffi/tests.cc
@@ -250,6 +250,12 @@ void c_take_sliceu8(rust::Slice<const uint8_t> s) {
   }
 }
 
+void c_take_slice_shared(rust::Slice<const Shared> s) {
+  if (s.size() == 2 && s.data()->z == 2020 && (s.data() + 1)->z == 2021) {
+    cxx_test_suite_set_correct();
+  }
+}
+
 void c_take_rust_string(rust::String s) {
   if (std::string(s) == "2020") {
     cxx_test_suite_set_correct();
diff --git a/tests/ffi/tests.h b/tests/ffi/tests.h
index 6c288d72..3a4c3cbe 100644
--- a/tests/ffi/tests.h
+++ b/tests/ffi/tests.h
@@ -115,6 +115,7 @@ void c_take_ref_c(const C &c);
 void c_take_ref_ns_c(const ::H::H &h);
 void c_take_str(rust::Str s);
 void c_take_sliceu8(rust::Slice<const uint8_t> s);
+void c_take_slice_shared(rust::Slice<const Shared> s);
 void c_take_rust_string(rust::String s);
 void c_take_unique_ptr_string(std::unique_ptr<std::string> s);
 void c_take_unique_ptr_vector_u8(std::unique_ptr<std::vector<uint8_t>> v);
diff --git a/tests/test.rs b/tests/test.rs
index 3bd41d17..98fbf459 100644
--- a/tests/test.rs
+++ b/tests/test.rs
@@ -117,6 +117,10 @@ fn test_c_take() {
     check!(cxx_test_suite::module::ffi::c_take_unique_ptr(unique_ptr));
     check!(ffi::c_take_str("2020"));
     check!(ffi::c_take_sliceu8(b"2020"));
+    check!(ffi::c_take_slice_shared(&[
+        ffi::Shared { z: 2020 },
+        ffi::Shared { z: 2021 },
+    ]));
     check!(ffi::c_take_rust_string("2020".to_owned()));
     check!(ffi::c_take_unique_ptr_string(
         ffi::c_return_unique_ptr_string()
diff --git a/tests/ui/slice_unsupported.rs b/tests/ui/slice_unsupported.rs
new file mode 100644
index 00000000..7a148dd7
--- /dev/null
+++ b/tests/ui/slice_unsupported.rs
@@ -0,0 +1,10 @@
+#[cxx::bridge]
+mod ffi {
+    unsafe extern "C++" {
+        type Opaque;
+
+        fn f(_: &mut [Opaque]);
+    }
+}
+
+fn main() {}
diff --git a/tests/ui/slice_unsupported.stderr b/tests/ui/slice_unsupported.stderr
new file mode 100644
index 00000000..787076fb
--- /dev/null
+++ b/tests/ui/slice_unsupported.stderr
@@ -0,0 +1,5 @@
+error: unsupported &mut [T] element type: opaque C++ type is not supported yet
+ --> $DIR/slice_unsupported.rs:6:17
+  |
+6 |         fn f(_: &mut [Opaque]);
+  |                 ^^^^^^^^^^^^^