Add expansion of shared type aliases and unit tests

sbrocket · sbrocket · commit 3ab1a3add361 · 2020-10-02T11:49:29.000-07:00
diff --git a/macro/src/expand.rs b/macro/src/expand.rs
@@ -41,8 +41,8 @@ fn expand(ffi: Module, apis: &[Api], types: &Types) -> TokenStream {
     for api in apis {
         match api {
             Api::Include(_) | Api::RustType(_) => {}
-            Api::Struct(strct) => expanded.extend(expand_struct(strct)),
-            Api::Enum(enm) => expanded.extend(expand_enum(enm)),
+            Api::Struct(strct) => expanded.extend(expand_struct(namespace, strct)),
+            Api::Enum(enm) => expanded.extend(expand_enum(namespace, enm)),
             Api::CxxType(ety) => {
                 let ident = &ety.ident;
                 if !types.structs.contains_key(ident) && !types.enums.contains_key(ident) {
@@ -120,7 +120,7 @@ fn expand(ffi: Module, apis: &[Api], types: &Types) -> TokenStream {
     }
 }
 
-fn expand_struct(strct: &Struct) -> TokenStream {
+fn expand_struct(namespace: &Namespace, strct: &Struct) -> TokenStream {
     let ident = &strct.ident;
     let doc = &strct.doc;
     let derives = &strct.derives;
@@ -130,17 +130,24 @@ fn expand_struct(strct: &Struct) -> TokenStream {
         let vis = Token![pub](field.ident.span());
         quote!(#vis #field)
     });
+    let type_id = type_id(namespace, ident);
+    // TODO: Add ui test for mismatched alias kinds
     quote! {
         #doc
         #[derive(#(#derives),*)]
         #[repr(C)]
         pub struct #ident {
             #(#fields,)*
         }
+
+        unsafe impl ::cxx::ExternType for #ident {
+            type Kind = ::cxx::extern_type::KindShared;
+            type Id = #type_id;
+        }
     }
 }
 
-fn expand_enum(enm: &Enum) -> TokenStream {
+fn expand_enum(namespace: &Namespace, enm: &Enum) -> TokenStream {
     let ident = &enm.ident;
     let doc = &enm.doc;
     let repr = enm.repr;
@@ -151,6 +158,7 @@ fn expand_enum(enm: &Enum) -> TokenStream {
             pub const #variant_ident: Self = #ident { repr: #discriminant };
         })
     });
+    let type_id = type_id(namespace, ident);
     quote! {
         #doc
         #[derive(Copy, Clone, PartialEq, Eq)]
@@ -163,6 +171,11 @@ fn expand_enum(enm: &Enum) -> TokenStream {
         impl #ident {
             #(#variants)*
         }
+
+        unsafe impl ::cxx::ExternType for #ident {
+            type Kind = ::cxx::extern_type::KindShared;
+            type Id = #type_id;
+        }
     }
 }
 
diff --git a/syntax/check.rs b/syntax/check.rs
@@ -64,6 +64,7 @@ fn check_type_ident(cx: &mut Check, ident: &Ident) {
         && !cx.types.enums.contains_key(ident)
         && !cx.types.cxx.contains(ident)
         && !cx.types.rust.contains(ident)
+        && !cx.types.aliases.contains_key(ident)
     {
         cx.error(ident, "unsupported type");
     }
@@ -230,6 +231,8 @@ fn check_api_fn(cx: &mut Check, efn: &ExternFn) {
             && !cx.types.cxx.contains(&receiver.ty)
             && !cx.types.rust.contains(&receiver.ty)
         {
+            // TODO: Add ui test for aliases being disallowed in receiver position since we can't
+            // tell if it's a struct or enum, unique error message
             cx.error(span, "unrecognized receiver type");
         }
 
diff --git a/syntax/types.rs b/syntax/types.rs
@@ -1,7 +1,7 @@
 use crate::syntax::atom::Atom::{self, *};
 use crate::syntax::report::Errors;
 use crate::syntax::set::OrderedSet as Set;
-use crate::syntax::{Api, Derive, Enum, ExternType, Struct, Type, TypeAlias};
+use crate::syntax::{AliasKind, Api, Derive, Enum, ExternType, Struct, Type, TypeAlias};
 use proc_macro2::Ident;
 use quote::ToTokens;
 use std::collections::{BTreeMap as Map, HashSet as UnorderedSet};
@@ -129,7 +129,9 @@ impl<'a> Types<'a> {
                     if !type_names.insert(ident) {
                         duplicate_name(cx, alias, ident);
                     }
-                    cxx.insert(ident);
+                    if let AliasKind::OpaqueCpp = alias.kind {
+                        cxx.insert(ident);
+                    }
                     aliases.insert(ident, alias);
                 }
             }
diff --git a/tests/BUCK b/tests/BUCK
@@ -28,6 +28,7 @@ cxx_library(
     ],
     header_namespace = "cxx-test-suite",
     headers = {
+        "alias.rs.h": ":alias/header",
         "lib.rs.h": ":bridge/header",
         "tests.h": "ffi/tests.h",
     },
diff --git a/tests/BUILD b/tests/BUILD
@@ -32,6 +32,7 @@ cc_library(
     include_prefix = "cxx-test-suite",
     strip_include_prefix = "ffi",
     deps = [
+        ":alias/include",
         ":bridge/include",
         "//:core",
     ],
diff --git a/tests/ffi/alias.rs b/tests/ffi/alias.rs
@@ -3,11 +3,52 @@
 #[rustfmt::skip]
 #[cxx::bridge(namespace = tests)]
 pub mod ffi {
+    type Shared = crate::ffi::Shared;
+    type Enum = crate::ffi::Enum;
+
     extern "C" {
         include!("cxx-test-suite/tests.h");
+        // Need complete C++ type declaration generated by other bridge for aliased shared types.
+        include!("cxx-test-suite/lib.rs.h");
 
         type C = crate::ffi::C;
 
+        // TODO: The alias prefix can be removed once these are in their own namespace.
+        fn alias_c_return_shared() -> Shared;
+        fn alias_c_return_ref(shared: &Shared) -> &usize;
+        fn alias_c_return_mut(shared: &mut Shared) -> &mut usize;
+        fn alias_c_return_enum(n: u16) -> Enum;
+        fn alias_c_return_unique_ptr_vector_shared() -> UniquePtr<CxxVector<Shared>>;
+
+        fn alias_c_take_shared(shared: Shared);
+        // TODO: This don't work yet because both bridges try to emit the rust_vec$tests$Shared
+        // functions. Need the remote bridge to always emit if this is gonna work. Or can we work
+        // around ODR by making the rust_vec functions inline?
+        //fn alias_c_take_rust_vec_shared(v: Vec<Shared>);
+        // TODO: Box<Shared> probably has the same problem, not currently tested
+        // TODO: Same for UniquePtr<Shared>, not currently tested
+        // TODO: Below probably doesn't work if the remote bridge doesn't use CxxVector<Shared>
+        fn alias_c_take_unique_ptr_vector_shared(v: UniquePtr<CxxVector<Shared>>);
+        fn alias_c_take_enum(e: Enum);
+
         fn c_take_unique_ptr(c: UniquePtr<C>);
     }
+
+    extern "Rust" {
+        // TODO: The alias prefix can be removed once these are in their own namespace.
+        fn alias_r_return_shared() -> Shared;
+        fn alias_r_return_ref(shared: &Shared) -> &usize;
+        fn alias_r_return_mut(shared: &mut Shared) -> &mut usize;
+        fn alias_r_return_enum(n: u32) -> Enum;
+
+        fn alias_r_take_shared(shared: Shared);
+        fn alias_r_take_enum(e: Enum);
+    }
 }
+
+use crate::r_return_enum as alias_r_return_enum;
+use crate::r_return_mut as alias_r_return_mut;
+use crate::r_return_ref as alias_r_return_ref;
+use crate::r_return_shared as alias_r_return_shared;
+use crate::r_take_enum as alias_r_take_enum;
+use crate::r_take_shared as alias_r_take_shared;
diff --git a/tests/ffi/lib.rs b/tests/ffi/lib.rs
@@ -193,7 +193,7 @@ fn r_return_primitive() -> usize {
     2020
 }
 
-fn r_return_shared() -> ffi::Shared {
+pub(crate) fn r_return_shared() -> ffi::Shared {
     ffi::Shared { z: 2020 }
 }
 
@@ -208,11 +208,11 @@ fn r_return_unique_ptr() -> UniquePtr<ffi::C> {
     unsafe { UniquePtr::from_raw(cxx_test_suite_get_unique_ptr()) }
 }
 
-fn r_return_ref(shared: &ffi::Shared) -> &usize {
+pub(crate) fn r_return_ref(shared: &ffi::Shared) -> &usize {
     &shared.z
 }
 
-fn r_return_mut(shared: &mut ffi::Shared) -> &mut usize {
+pub(crate) fn r_return_mut(shared: &mut ffi::Shared) -> &mut usize {
     &mut shared.z
 }
 
@@ -258,7 +258,7 @@ fn r_return_sum(n1: usize, n2: usize) -> usize {
     n1 + n2
 }
 
-fn r_return_enum(n: u32) -> ffi::Enum {
+pub(crate) fn r_return_enum(n: u32) -> ffi::Enum {
     if n == 0 {
         ffi::Enum::AVal
     } else if n <= 2020 {
@@ -272,7 +272,7 @@ fn r_take_primitive(n: usize) {
     assert_eq!(n, 2020);
 }
 
-fn r_take_shared(shared: ffi::Shared) {
+pub(crate) fn r_take_shared(shared: ffi::Shared) {
     assert_eq!(shared.z, 2020);
 }
 
@@ -335,7 +335,7 @@ fn r_take_ref_rust_vec_string(v: &Vec<String>) {
     let _ = v;
 }
 
-fn r_take_enum(e: ffi::Enum) {
+pub(crate) fn r_take_enum(e: ffi::Enum) {
     let _ = e;
 }
 
diff --git a/tests/ffi/tests.cc b/tests/ffi/tests.cc
@@ -1,4 +1,5 @@
 #include "cxx-test-suite/tests.h"
+#include "cxx-test-suite/alias.rs.h"
 #include "cxx-test-suite/lib.rs.h"
 #include <cstring>
 #include <iterator>
@@ -357,15 +358,16 @@ const rust::Vec<uint8_t> &c_try_return_ref_rust_vec(const C &c) {
   throw std::runtime_error("unimplemented");
 }
 
-extern "C" C *cxx_test_suite_get_unique_ptr() noexcept {
+extern "C" {
+
+C *cxx_test_suite_get_unique_ptr() noexcept {
   return std::unique_ptr<C>(new C{2020}).release();
 }
 
-extern "C" std::string *cxx_test_suite_get_unique_ptr_string() noexcept {
+std::string *cxx_test_suite_get_unique_ptr_string() noexcept {
   return std::unique_ptr<std::string>(new std::string("2020")).release();
 }
 
-extern "C" const char *cxx_run_test() noexcept {
 #define STRINGIFY(x) #x
 #define TOSTRING(x) STRINGIFY(x)
 #define ASSERT(x)                                                              \
@@ -375,6 +377,7 @@ extern "C" const char *cxx_run_test() noexcept {
     }                                                                          \
   } while (false)
 
+const char *cxx_run_test() noexcept {
   ASSERT(r_return_primitive() == 2020);
   ASSERT(r_return_shared().z == 2020);
   ASSERT(cxx_test_suite_r_is_correct(&*r_return_box()));
@@ -432,4 +435,27 @@ extern "C" const char *cxx_run_test() noexcept {
   return nullptr;
 }
 
+const char *cxx_run_alias_test() noexcept {
+  ASSERT(alias_r_return_shared().z == 2020);
+  ASSERT(alias_r_return_ref(Shared{2020}) == 2020);
+  {
+    Shared shared{2019};
+    size_t &mut_z = alias_r_return_mut(shared);
+    ASSERT(mut_z == 2019);
+    mut_z = 2020;
+    ASSERT(alias_r_return_ref(shared) == 2020);
+  }
+  ASSERT(alias_r_return_enum(0) == Enum::AVal);
+  ASSERT(alias_r_return_enum(1) == Enum::BVal);
+  ASSERT(alias_r_return_enum(2021) == Enum::CVal);
+
+  r_take_shared(Shared{2020});
+  r_take_enum(Enum::AVal);
+
+  cxx_test_suite_set_correct();
+  return nullptr;
+}
+
+} // extern "C"
+
 } // namespace tests
diff --git a/tests/ffi/tests.h b/tests/ffi/tests.h
@@ -101,4 +101,15 @@ rust::Vec<uint8_t> c_try_return_rust_vec();
 rust::Vec<rust::String> c_try_return_rust_vec_string();
 const rust::Vec<uint8_t> &c_try_return_ref_rust_vec(const C &c);
 
+const auto alias_c_return_shared = c_return_shared;
+const auto alias_c_return_ref = c_return_ref;
+const auto alias_c_return_mut = c_return_mut;
+const auto alias_c_return_enum = c_return_enum;
+const auto alias_c_return_unique_ptr_vector_shared =
+    c_return_unique_ptr_vector_shared;
+const auto alias_c_take_unique_ptr_vector_shared =
+    c_take_unique_ptr_vector_shared;
+const auto alias_c_take_shared = c_take_shared;
+const auto alias_c_take_enum = c_take_enum;
+
 } // namespace tests
diff --git a/tests/test.rs b/tests/test.rs
@@ -73,6 +73,26 @@ fn test_c_return() {
     }
 }
 
+#[test]
+fn test_alias_c_return() {
+    let mut shared = ffi::Shared { z: 2020 };
+
+    assert_eq!(2020, alias::ffi::alias_c_return_shared().z);
+    assert_eq!(2020, *alias::ffi::alias_c_return_ref(&shared));
+
+    {
+        shared.z = 2019;
+        let mut_z = alias::ffi::alias_c_return_mut(&mut shared);
+        assert_eq!(2019, *mut_z);
+        *mut_z = 2020;
+    }
+
+    match alias::ffi::alias_c_return_enum(1) {
+        enm @ ffi::Enum::BVal => assert_eq!(2020, enm.repr),
+        _ => assert!(false),
+    }
+}
+
 #[test]
 fn test_c_try_return() {
     assert_eq!((), ffi::c_try_return_void().unwrap());
@@ -129,6 +149,15 @@ fn test_c_take() {
     check!(ffi::c_take_enum(ffi::Enum::AVal));
 }
 
+#[test]
+fn test_alias_c_take() {
+    check!(alias::ffi::alias_c_take_shared(ffi::Shared { z: 2020 }));
+    check!(alias::ffi::alias_c_take_enum(ffi::Enum::AVal));
+    check!(alias::ffi::alias_c_take_unique_ptr_vector_shared(
+        alias::ffi::alias_c_return_unique_ptr_vector_shared()
+    ));
+}
+
 /*
 // https://github.com/dtolnay/cxx/issues/232
 #[test]
@@ -159,6 +188,21 @@ fn test_c_call_r() {
     check!(cxx_run_test());
 }
 
+#[test]
+fn test_alias_c_call_r() {
+    fn cxx_run_alias_test() {
+        extern "C" {
+            fn cxx_run_alias_test() -> *const i8;
+        }
+        let failure = unsafe { cxx_run_alias_test() };
+        if !failure.is_null() {
+            let msg = unsafe { CStr::from_ptr(failure as *mut std::os::raw::c_char) };
+            eprintln!("{}", msg.to_string_lossy());
+        }
+    }
+    check!(cxx_run_alias_test());
+}
+
 #[test]
 fn test_c_method_calls() {
     let mut unique_ptr = ffi::c_return_unique_ptr();

Original file line number	Diff line number	Diff line change
`@@ -64,6 +64,7 @@ fn check_type_ident(cx: &mut Check, ident: &Ident) {`
`64`	`64`	`&& !cx.types.enums.contains_key(ident)`
`65`	`65`	`&& !cx.types.cxx.contains(ident)`
`66`	`66`	`&& !cx.types.rust.contains(ident)`
	`67`	`+ && !cx.types.aliases.contains_key(ident)`
`67`	`68`	`{`
`68`	`69`	`cx.error(ident, "unsupported type");`
`69`	`70`	`}`
`@@ -230,6 +231,8 @@ fn check_api_fn(cx: &mut Check, efn: &ExternFn) {`
`230`	`231`	`&& !cx.types.cxx.contains(&receiver.ty)`
`231`	`232`	`&& !cx.types.rust.contains(&receiver.ty)`
`232`	`233`	`{`
	`234`	`+ // TODO: Add ui test for aliases being disallowed in receiver position since we can't`
	`235`	`+ // tell if it's a struct or enum, unique error message`
`233`	`236`	`cx.error(span, "unrecognized receiver type");`
`234`	`237`	`}`
`235`	`238`
Original file line number	Diff line number	Diff line change
`@@ -1,7 +1,7 @@`
`1`	`1`	`use crate::syntax::atom::Atom::{self, *};`
`2`	`2`	`use crate::syntax::report::Errors;`
`3`	`3`	`use crate::syntax::set::OrderedSet as Set;`
`4`		`-use crate::syntax::{Api, Derive, Enum, ExternType, Struct, Type, TypeAlias};`
	`4`	`+use crate::syntax::{AliasKind, Api, Derive, Enum, ExternType, Struct, Type, TypeAlias};`
`5`	`5`	`use proc_macro2::Ident;`
`6`	`6`	`use quote::ToTokens;`
`7`	`7`	`use std::collections::{BTreeMap as Map, HashSet as UnorderedSet};`
`@@ -129,7 +129,9 @@ impl<'a> Types<'a> {`
`129`	`129`	`if !type_names.insert(ident) {`
`130`	`130`	`duplicate_name(cx, alias, ident);`
`131`	`131`	`}`
`132`		`- cxx.insert(ident);`
	`132`	`+ if let AliasKind::OpaqueCpp = alias.kind {`
	`133`	`+ cxx.insert(ident);`
	`134`	`+ }`
`133`	`135`	`aliases.insert(ident, alias);`
`134`	`136`	`}`
`135`	`137`	`}`
Original file line number	Diff line number	Diff line change
`@@ -193,7 +193,7 @@ fn r_return_primitive() -> usize {`
`193`	`193`	`2020`
`194`	`194`	`}`
`195`	`195`
`196`		`-fn r_return_shared() -> ffi::Shared {`
	`196`	`+pub(crate) fn r_return_shared() -> ffi::Shared {`
`197`	`197`	`ffi::Shared { z: 2020 }`
`198`	`198`	`}`
`199`	`199`
`@@ -208,11 +208,11 @@ fn r_return_unique_ptr() -> UniquePtr<ffi::C> {`
`208`	`208`	`unsafe { UniquePtr::from_raw(cxx_test_suite_get_unique_ptr()) }`
`209`	`209`	`}`
`210`	`210`
`211`		`-fn r_return_ref(shared: &ffi::Shared) -> &usize {`
	`211`	`+pub(crate) fn r_return_ref(shared: &ffi::Shared) -> &usize {`
`212`	`212`	`&shared.z`
`213`	`213`	`}`
`214`	`214`
`215`		`-fn r_return_mut(shared: &mut ffi::Shared) -> &mut usize {`
	`215`	`+pub(crate) fn r_return_mut(shared: &mut ffi::Shared) -> &mut usize {`
`216`	`216`	`&mut shared.z`
`217`	`217`	`}`
`218`	`218`
`@@ -258,7 +258,7 @@ fn r_return_sum(n1: usize, n2: usize) -> usize {`
`258`	`258`	`n1 + n2`
`259`	`259`	`}`
`260`	`260`
`261`		`-fn r_return_enum(n: u32) -> ffi::Enum {`
	`261`	`+pub(crate) fn r_return_enum(n: u32) -> ffi::Enum {`
`262`	`262`	`if n == 0 {`
`263`	`263`	`ffi::Enum::AVal`
`264`	`264`	`} else if n <= 2020 {`
`@@ -272,7 +272,7 @@ fn r_take_primitive(n: usize) {`
`272`	`272`	`assert_eq!(n, 2020);`
`273`	`273`	`}`
`274`	`274`
`275`		`-fn r_take_shared(shared: ffi::Shared) {`
	`275`	`+pub(crate) fn r_take_shared(shared: ffi::Shared) {`
`276`	`276`	`assert_eq!(shared.z, 2020);`
`277`	`277`	`}`
`278`	`278`
`@@ -335,7 +335,7 @@ fn r_take_ref_rust_vec_string(v: &Vec<String>) {`
`335`	`335`	`let _ = v;`
`336`	`336`	`}`
`337`	`337`
`338`		`-fn r_take_enum(e: ffi::Enum) {`
	`338`	`+pub(crate) fn r_take_enum(e: ffi::Enum) {`
`339`	`339`	`let _ = e;`
`340`	`340`	`}`
`341`	`341`