Merge pull request #107 from google/solve-template-problems

Allow simple templated types in bindgen generation

Author: adetaylor

engine/src/additional_cpp_generator.rs

@@ -239,7 +239,7 @@ impl AdditionalCppGenerator {
             .iter()
             .map(field_access)
             .collect::<Vec<&str>>()
-            .join("\n\n");
+            .join("\n");
         s.push('\n');
         s
     }

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;

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 <string>
+        #include <cstdint>
+
+        template <typename STRING_TYPE> class BasicStringPiece {
+        public:
+            const STRING_TYPE* ptr_;
+            size_t length_;
+        };
+        typedef BasicStringPiece<uint8_t> 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 <string>
+        #include <cstdint>
+
+        struct Concrete {
+            uint8_t a;
+        };
+        template <typename STRING_TYPE> class BasicStringPiece {
+        public:
+            const STRING_TYPE* ptr_;
+            size_t length_;
+        };
+        typedef BasicStringPiece<Concrete> 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 <string>
+        #include <cstdint>
+
+        template <typename STRING_TYPE> class BasicStringPiece {
+        public:
+            const STRING_TYPE* ptr_;
+            size_t length_;
+        };
+        typedef BasicStringPiece<std::string> 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 <string>
+        #include <cstdint>
+        
+        template <typename STRING_TYPE>
+        class BasicStringPiece;
+        
+        typedef BasicStringPiece<std::string> StringPiece;
+        
+        template <typename STRING_TYPE> 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