Merge pull request #161 from google/handle-templated-types

Slightly better templated type support

Author: adetaylor

engine/src/additional_cpp_generator.rs

@@ -19,12 +19,14 @@ use crate::{
 };
 use itertools::Itertools;
 use std::collections::HashSet;
+use syn::Type;
 
 /// Instructions for new C++ which we need to generate.
 pub(crate) enum AdditionalNeed {
     MakeStringConstructor,
     FunctionWrapper(Box<FunctionWrapper>),
     CTypeTypedef(TypeName),
+    ConcreteTemplatedTypeTypedef(TypeName, Box<Type>),
 }
 
 #[derive(Ord, PartialOrd, Eq, PartialEq, Clone, Hash)]
@@ -55,6 +57,7 @@ impl Header {
 }
 
 struct AdditionalFunction {
+    type_definition: String, // are output before main declarations
     declaration: String,
     definition: String,
     headers: Vec<Header>,
@@ -96,7 +99,10 @@ impl AdditionalCppGenerator {
                 AdditionalNeed::FunctionWrapper(by_value_wrapper) => {
                     self.generate_by_value_wrapper(*by_value_wrapper, type_database)
                 }
-                AdditionalNeed::CTypeTypedef(tn) => self.generate_typedef(tn),
+                AdditionalNeed::CTypeTypedef(tn) => self.generate_ctype_typedef(tn),
+                AdditionalNeed::ConcreteTemplatedTypeTypedef(tn, def) => {
+                    self.generate_typedef(tn, type_database.type_to_cpp(&def))
+                }
             }
         }
     }
@@ -112,8 +118,12 @@ impl AdditionalCppGenerator {
                 .flatten()
                 .collect();
             let headers = headers.iter().map(|x| x.include_stmt()).join("\n");
+            let type_definitions = self.concat_additional_items(|x| &x.type_definition);
             let declarations = self.concat_additional_items(|x| &x.declaration);
-            let declarations = format!("{}\n{}\n{}", headers, self.inclusions, declarations);
+            let declarations = format!(
+                "{}\n{}\n{}\n{}",
+                headers, self.inclusions, type_definitions, declarations
+            );
             let definitions = self.concat_additional_items(|x| &x.definition);
             let definitions = format!("#include \"autocxxgen.h\"\n{}", definitions);
             Some(AdditionalCpp {
@@ -145,6 +155,7 @@ impl AdditionalCppGenerator {
         );
         let declaration = format!("{};", declaration);
         self.additional_functions.push(AdditionalFunction {
+            type_definition: "".into(),
             declaration,
             definition,
             headers: vec![
@@ -238,17 +249,23 @@ impl AdditionalCppGenerator {
         let definition = format!("{} {{ {}; }}", declaration, underlying_function_call,);
         let declaration = format!("{};", declaration);
         self.additional_functions.push(AdditionalFunction {
+            type_definition: "".into(),
             declaration,
             definition,
             headers: vec![Header::system("memory")],
         })
     }
 
-    fn generate_typedef(&mut self, tn: TypeName) {
-        let our_name = tn.get_final_ident();
+    fn generate_ctype_typedef(&mut self, tn: TypeName) {
         let cpp_name = tn.to_cpp_name();
+        self.generate_typedef(tn, cpp_name)
+    }
+
+    fn generate_typedef(&mut self, tn: TypeName, definition: String) {
+        let our_name = tn.get_final_ident();
         self.additional_functions.push(AdditionalFunction {
-            declaration: format!("typedef {} {};", cpp_name, our_name),
+            type_definition: format!("typedef {} {};", definition, our_name),
+            declaration: "".into(),
             definition: "".into(),
             headers: Vec::new(),
         })

engine/src/byvalue_checker.rs

@@ -148,6 +148,10 @@ impl ByValueChecker {
     }
 
     pub fn is_pod(&self, ty_id: &TypeName) -> bool {
+        if !ty_id.has_namespace() && ty_id.get_final_ident().starts_with("AutocxxConcrete") {
+            // Type we created at conversion time.
+            return false;
+        }
         matches!(self
         .results
         .get(ty_id)

engine/src/conversion/codegen.rs

@@ -146,7 +146,7 @@ impl<'a> CodeGenerator<'a> {
         additional_cpp_needs: &mut Vec<AdditionalNeed>,
     ) {
         let ctypes: HashSet<_> = deps
-            .into_iter()
+            .iter()
             .flatten()
             .filter(|ty| KNOWN_TYPES.is_ctype(ty))
             .collect();

engine/src/conversion/parse/mod.rs

@@ -13,6 +13,7 @@
 // limitations under the License.
 
 mod bridge_name_tracker;
+mod non_pod_struct;
 mod overload_tracker;
 pub(crate) mod parse_bindgen;
 mod parse_foreign_mod;

engine/src/conversion/parse/non_pod_struct.rs

@@ -0,0 +1,69 @@
+// Copyright 2020 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+use crate::types::make_ident;
+use proc_macro2::Ident;
+use quote::quote;
+use syn::parse::Parser;
+use syn::{parse_quote, Field, GenericParam, ItemStruct};
+
+pub(crate) fn new_non_pod_struct(id: Ident) -> ItemStruct {
+    let mut s = parse_quote! {
+        pub struct #id {
+        }
+    };
+    make_non_pod(&mut s);
+    s
+}
+
+pub(crate) 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),*
+        }
+    });
+}

engine/src/conversion/parse/parse_bindgen.rs

@@ -22,18 +22,17 @@ use crate::{
     types::Namespace,
     types::TypeName,
 };
-use proc_macro2::{TokenStream as TokenStream2, TokenTree};
+use proc_macro2::TokenStream as TokenStream2;
 use quote::quote;
-use syn::{
-    parse::Parser, parse_quote, Field, Fields, ForeignItem, GenericParam, Item, ItemStruct, Type,
-};
+use syn::{parse_quote, Fields, ForeignItem, Item, ItemStruct, Type};
 
 use super::{
     super::{
         api::{Api, Use},
         utilities::generate_utilities,
     },
     bridge_name_tracker::BridgeNameTracker,
+    non_pod_struct::make_non_pod,
     rust_name_tracker::RustNameTracker,
     type_converter::TypeConverter,
 };
@@ -49,7 +48,7 @@ enum TypeKind {
 
 /// Parses a bindgen mod in order to understand the APIs within it.
 pub(crate) struct ParseBindgen<'a> {
-    type_converter: TypeConverter,
+    type_converter: TypeConverter<'a>,
     byvalue_checker: ByValueChecker,
     type_database: &'a TypeDatabase,
     bridge_name_tracker: BridgeNameTracker,
@@ -61,7 +60,7 @@ pub(crate) struct ParseBindgen<'a> {
 impl<'a> ParseBindgen<'a> {
     pub(crate) fn new(byvalue_checker: ByValueChecker, type_database: &'a TypeDatabase) -> Self {
         ParseBindgen {
-            type_converter: TypeConverter::new(),
+            type_converter: TypeConverter::new(type_database),
             byvalue_checker,
             bridge_name_tracker: BridgeNameTracker::new(),
             rust_name_tracker: RustNameTracker::new(),
@@ -138,7 +137,7 @@ impl<'a> ParseBindgen<'a> {
                     let field_types = match type_kind {
                         TypeKind::POD => self.get_struct_field_types(&ns, &s)?,
                         _ => {
-                            Self::make_non_pod(&mut s);
+                            make_non_pod(&mut s);
                             HashSet::new()
                         }
                     };
@@ -234,13 +233,14 @@ impl<'a> ParseBindgen<'a> {
     }
 
     fn get_struct_field_types(
-        &self,
+        &mut self,
         ns: &Namespace,
         s: &ItemStruct,
     ) -> Result<HashSet<TypeName>, ConvertError> {
         let mut results = HashSet::new();
         for f in &s.fields {
             let annotated = self.type_converter.convert_type(f.ty.clone(), ns)?;
+            self.results.apis.extend(annotated.extra_apis);
             results.extend(annotated.types_encountered);
         }
         Ok(results)
@@ -252,47 +252,6 @@ impl<'a> ParseBindgen<'a> {
             .any(|id| id == "_unused")
     }
 
-    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),*
-            }
-        });
-    }
-
     /// Record the Api for a type, e.g. enum or struct.
     /// Code generated includes the bindgen entry itself,
     /// various entries for the cxx::bridge to ensure cxx
@@ -333,42 +292,20 @@ impl<'a> ParseBindgen<'a> {
             TokenStream2::new()
         };
 
-        let mut fulltypath = Vec::new();
-        // We can't use parse_quote! here because it doesn't support type aliases
-        // at the moment.
-        let colon = TokenTree::Punct(proc_macro2::Punct::new(':', proc_macro2::Spacing::Joint));
-        for_extern_c_ts.extend(
-            [
-                TokenTree::Ident(make_ident("type")),
-                TokenTree::Ident(final_ident.clone()),
-                TokenTree::Punct(proc_macro2::Punct::new('=', proc_macro2::Spacing::Alone)),
-                TokenTree::Ident(make_ident("super")),
-                colon.clone(),
-                colon.clone(),
-                TokenTree::Ident(make_ident("bindgen")),
-                colon.clone(),
-                colon.clone(),
-                TokenTree::Ident(make_ident("root")),
-                colon.clone(),
-                colon.clone(),
-            ]
-            .to_vec(),
-        );
-        fulltypath.push(make_ident("bindgen"));
-        fulltypath.push(make_ident("root"));
+        let mut fulltypath: Vec<_> = ["bindgen", "root"].iter().map(|x| make_ident(x)).collect();
+        for_extern_c_ts.extend(quote! {
+            type #final_ident = super::bindgen::root::
+        });
         for segment in tyname.ns_segment_iter() {
             let id = make_ident(segment);
-            for_extern_c_ts
-                .extend([TokenTree::Ident(id.clone()), colon.clone(), colon.clone()].to_vec());
+            for_extern_c_ts.extend(quote! {
+                #id::
+            });
             fulltypath.push(id);
         }
-        for_extern_c_ts.extend(
-            [
-                TokenTree::Ident(final_ident.clone()),
-                TokenTree::Punct(proc_macro2::Punct::new(';', proc_macro2::Spacing::Alone)),
-            ]
-            .to_vec(),
-        );
+        for_extern_c_ts.extend(quote! {
+            #final_ident;
+        });
         let bridge_item = match type_nature {
             TypeKind::ForwardDeclaration => None,
             _ => Some(Item::Impl(parse_quote! {
@@ -402,11 +339,12 @@ impl<'a> ParseBindgen<'a> {
 
 impl<'a> ForeignModParseCallbacks for ParseBindgen<'a> {
     fn convert_boxed_type(
-        &self,
+        &mut self,
         ty: Box<Type>,
         ns: &Namespace,
     ) -> Result<(Box<Type>, HashSet<TypeName>), ConvertError> {
         let annotated = self.type_converter.convert_boxed_type(ty, ns)?;
+        self.results.apis.extend(annotated.extra_apis);
         Ok((annotated.ty, annotated.types_encountered))
     }