diff --git a/engine/src/additional_cpp_generator.rs b/engine/src/additional_cpp_generator.rs index a2b8cd4e2..501444e83 100644 --- a/engine/src/additional_cpp_generator.rs +++ b/engine/src/additional_cpp_generator.rs @@ -239,7 +239,7 @@ impl AdditionalCppGenerator { .iter() .map(field_access) .collect::>() - .join("\n\n"); + .join("\n"); s.push('\n'); s } diff --git a/engine/src/bridge_converter.rs b/engine/src/bridge_converter.rs index 63076e582..4a13b95df 100644 --- a/engine/src/bridge_converter.rs +++ b/engine/src/bridge_converter.rs @@ -30,7 +30,7 @@ use crate::{ }; use proc_macro2::{Span, TokenStream as TokenStream2, TokenTree}; use quote::quote; -use syn::{parse::Parser, ItemType}; +use syn::{parse::Parser, Field, GenericParam, Generics, ItemStruct}; use syn::{ parse_quote, Attribute, FnArg, ForeignItem, ForeignItemFn, GenericArgument, Ident, Item, ItemForeignMod, ItemMod, Pat, PathArguments, PathSegment, ReturnType, Type, TypePath, TypePtr, @@ -262,26 +262,13 @@ impl<'a> BridgeConversion<'a> { Item::Struct(mut s) => { let tyname = TypeName::new(&ns, &s.ident.to_string()); let should_be_pod = self.byvalue_checker.is_pod(&tyname); - self.generate_type_alias(tyname, should_be_pod)?; + if !Self::generics_contentful(&s.generics) { + // cxx::bridge can't cope with type aliases to generic + // types at the moment. + self.generate_type_alias(tyname, should_be_pod)?; + } if !should_be_pod { - // See cxx's opaque::Opaque for rationale for this type... in - // short, it's to avoid being Send/Sync. - s.fields = syn::Fields::Named(parse_quote! { - { - do_not_attempt_to_allocate_nonpod_types: [*const u8; 0], - } - }); - // Thanks to dtolnay@ for this explanation of why the following - // is needed: - // If the real alignment of the C++ type is smaller and a reference - // is returned from C++ to Rust, mere existence of an insufficiently - // aligned reference in Rust causes UB even if never dereferenced - // by Rust code - // (see https://doc.rust-lang.org/1.47.0/reference/behavior-considered-undefined.html). - // Rustc can use least-significant bits of the reference for other storage. - s.attrs = vec![parse_quote!( - #[repr(C, packed)] - )]; + Self::make_non_pod(&mut s); } output_items.push(Item::Struct(s)); } @@ -319,7 +306,7 @@ impl<'a> BridgeConversion<'a> { self.all_items.push(item); } Item::Type(ity) => { - if Self::should_ignore_item_type(&ity) { + if Self::generics_contentful(&ity.generics) { // Ignore this for now. Sometimes bindgen generates such things // without an actual need to do so. continue; @@ -349,10 +336,51 @@ impl<'a> BridgeConversion<'a> { Ok(()) } - fn should_ignore_item_type(ity: &ItemType) -> bool { - ity.generics.lifetimes().next().is_some() - || ity.generics.const_params().next().is_some() - || ity.generics.type_params().next().is_some() + fn make_non_pod(s: &mut ItemStruct) { + // Thanks to dtolnay@ for this explanation of why the following + // is needed: + // If the real alignment of the C++ type is smaller and a reference + // is returned from C++ to Rust, mere existence of an insufficiently + // aligned reference in Rust causes UB even if never dereferenced + // by Rust code + // (see https://doc.rust-lang.org/1.47.0/reference/behavior-considered-undefined.html). + // Rustc can use least-significant bits of the reference for other storage. + s.attrs = vec![parse_quote!( + #[repr(C, packed)] + )]; + // Now fill in fields. Usually, we just want a single field + // but if this is a generic type we need to faff a bit. + let generic_type_fields = + s.generics + .params + .iter() + .enumerate() + .filter_map(|(counter, gp)| match gp { + GenericParam::Type(gpt) => { + let id = &gpt.ident; + let field_name = make_ident(&format!("_phantom_{}", counter)); + let toks = quote! { + #field_name: ::std::marker::PhantomData<::std::cell::UnsafeCell< #id >> + }; + let parser = Field::parse_named; + Some(parser.parse2(toks).unwrap()) + } + _ => None, + }); + // See cxx's opaque::Opaque for rationale for this type... in + // short, it's to avoid being Send/Sync. + s.fields = syn::Fields::Named(parse_quote! { + { + do_not_attempt_to_allocate_nonpod_types: [*const u8; 0], + #(#generic_type_fields),* + } + }); + } + + fn generics_contentful(generics: &Generics) -> bool { + generics.lifetimes().next().is_some() + || generics.const_params().next().is_some() + || generics.type_params().next().is_some() } fn analyze_typedef_target(ty: &Type) -> TypedefTarget { @@ -381,7 +409,7 @@ impl<'a> BridgeConversion<'a> { .byvalue_checker .ingest_pod_type(TypeName::new(&ns, &e.ident.to_string())), Item::Type(ity) => { - if Self::should_ignore_item_type(&ity) { + if Self::generics_contentful(&ity.generics) { // Ignore this for now. Sometimes bindgen generates such things // without an actual need to do so. continue; diff --git a/engine/src/integration_tests.rs b/engine/src/integration_tests.rs index 52ae84e34..1d5c83032 100644 --- a/engine/src/integration_tests.rs +++ b/engine/src/integration_tests.rs @@ -2372,10 +2372,117 @@ fn test_non_pod_constant() { run_test("", hdr, rs, &["BOB"], &[]); } +#[test] +fn test_templated_typedef() { + let hdr = indoc! {" + #include + #include + + template class BasicStringPiece { + public: + const STRING_TYPE* ptr_; + size_t length_; + }; + typedef BasicStringPiece StringPiece; + + struct Origin { + Origin() {} + StringPiece host; + }; + "}; + let rs = quote! { + ffi::Origin::make_unique(); + }; + run_test("", hdr, rs, &["Origin"], &[]); +} + +#[test] +fn test_struct_templated_typedef() { + let hdr = indoc! {" + #include + #include + + struct Concrete { + uint8_t a; + }; + template class BasicStringPiece { + public: + const STRING_TYPE* ptr_; + size_t length_; + }; + typedef BasicStringPiece StringPiece; + + struct Origin { + Origin() {} + StringPiece host; + }; + "}; + let rs = quote! { + ffi::Origin::make_unique(); + }; + run_test("", hdr, rs, &["Origin"], &[]); +} + +#[ignore] // https://github.com/google/autocxx/issues/106 +#[test] +fn test_string_templated_typedef() { + let hdr = indoc! {" + #include + #include + + template class BasicStringPiece { + public: + const STRING_TYPE* ptr_; + size_t length_; + }; + typedef BasicStringPiece StringPiece; + + struct Origin { + Origin() {} + StringPiece host; + }; + "}; + let rs = quote! { + ffi::Origin::make_unique(); + }; + run_test("", hdr, rs, &["Origin"], &[]); +} + +#[ignore] // https://github.com/google/autocxx/issues/106 +#[test] +fn test_string_forward_declared_templated_typedef() { + let hdr = indoc! {" + #include + #include + + template + class BasicStringPiece; + + typedef BasicStringPiece StringPiece; + + template class BasicStringPiece { + public: + typedef size_t size_type; + typedef typename STRING_TYPE::value_type value_type; + const value_type* ptr_; + size_type length_; + }; + + struct Origin { + // void SetHost(StringPiece host); + StringPiece host; + }; + "}; + let rs = quote! { + ffi::Origin::make_unique(); + }; + run_test("", hdr, rs, &["Origin"], &[]); +} + // Yet to test: -// 5. Templated stuff +// 5. Using templated types. // 6. Ifdef -// 7. Out params +// 7. Pointers (including out params) // 10. ExcludeUtilities // Stuff which requires much more thought: // 1. Shared pointers