From 23b21afcf9e70d20117479608b42b5612627892a Mon Sep 17 00:00:00 2001 From: Adrian Taylor Date: Mon, 5 Oct 2020 13:18:16 -0700 Subject: [PATCH 1/9] Outdent. --- engine/src/bridge_converter.rs | 224 +++++++++++++++++---------------- 1 file changed, 116 insertions(+), 108 deletions(-) diff --git a/engine/src/bridge_converter.rs b/engine/src/bridge_converter.rs index 6637c54cd..d0a3fc6f4 100644 --- a/engine/src/bridge_converter.rs +++ b/engine/src/bridge_converter.rs @@ -21,9 +21,9 @@ use std::collections::HashSet; use syn::punctuated::Punctuated; use syn::Token; use syn::{ - parse_quote, AngleBracketedGenericArguments, Attribute, FnArg, ForeignItem, ForeignItemFn, - GenericArgument, Ident, Item, ItemForeignMod, ItemMod, PatType, Path, PathArguments, - PathSegment, ReturnType, Type, TypePath, TypePtr, TypeReference, + parse_quote, AngleBracketedGenericArguments, Attribute, FnArg, ForeignItem, + ForeignItemFn, GenericArgument, Ident, Item, ItemForeignMod, ItemMod, PatType, Path, + PathArguments, PathSegment, ReturnType, Type, TypePath, TypePtr, TypeReference, }; #[derive(Debug)] @@ -151,9 +151,7 @@ impl<'a> BridgeConverter { match bindings.content { None => Err(ConvertError::NoContent), Some((brace, items)) => { - self.find_nested_pod_types(&items)?; - let mut all_items: Vec = Vec::new(); - let mut bindgen_mod = ItemMod { + let bindgen_mod = ItemMod { attrs: bindings.attrs, vis: bindings.vis, ident: bindings.ident, @@ -161,116 +159,126 @@ impl<'a> BridgeConverter { content: Some((brace, Vec::new())), semi: bindings.semi, }; - let mut bridge_items = Vec::new(); - let mut extern_c_mod = None; - let mut extern_c_mod_items = self.build_include_foreign_items(extra_inclusion); - let mut additional_cpp_needs = Vec::new(); - let mut types_found = Vec::new(); - let mut bindgen_items = Vec::new(); - for item in items { - match item { - Item::ForeignMod(fm) => { - if extern_c_mod.is_none() { - // We'll use the first 'extern "C"' mod we come - // across for attributes, spans etc. but we'll stuff - // the contents of all bindgen 'extern "C"' mods into this - // one. - extern_c_mod = Some(ItemForeignMod { - attrs: fm.attrs, - abi: fm.abi, - brace_token: fm.brace_token, - items: Vec::new(), - }); - } - extern_c_mod - .as_mut() - .unwrap() - .items - .extend(self.convert_foreign_mod_items(&types_found, fm.items)?); - } - Item::Struct(s) => { - let tyident = s.ident.clone(); - let tyname = TypeName::from_ident(&tyident); - types_found.push(tyname.clone()); - self.class_names_discovered.insert(tyname.clone()); - let should_be_pod = self.byvalue_checker.is_pod(&tyname); - let type_alias = self.generate_type_alias(&tyname, should_be_pod); - bridge_items.push(type_alias.for_bridge); - extern_c_mod_items.push(type_alias.for_extern_c); - bindgen_mod - .content - .as_mut() - .unwrap() - .1 - .push(Item::Struct(self.convert_struct(s))); - all_items.push(type_alias.for_anywhere); - } - Item::Enum(e) => { - let tyident = e.ident.clone(); - let tyname = TypeName::from_ident(&tyident); - types_found.push(tyname.clone()); - let type_alias = self.generate_type_alias(&tyname, true); - bridge_items.push(type_alias.for_bridge); - extern_c_mod_items.push(type_alias.for_extern_c); - bindgen_mod.content.as_mut().unwrap().1.push(Item::Enum(e)); - all_items.push(type_alias.for_anywhere); - } - Item::Impl(i) => { - if let Some(ty) = self.type_to_typename(&i.self_ty) { - for item in i.items.clone() { - match item { - syn::ImplItem::Method(m) if m.sig.ident == "new" => { - if let Some(new_item_impl) = self.convert_new_method( - m, - &mut additional_cpp_needs, - &ty, - &i, - ) { - bindgen_items.push(new_item_impl); - } - } - _ => {} + self.convert_items(bindgen_mod, items, extra_inclusion) + }, + } + } + + fn convert_items( + &mut self, + mut bindgen_mod: ItemMod, + items: Vec, + extra_inclusion: Option<&str>, + ) -> Result { + self.find_nested_pod_types(&items)?; + let mut all_items: Vec = Vec::new(); + let mut bridge_items = Vec::new(); + let mut extern_c_mod = None; + let mut extern_c_mod_items = self.build_include_foreign_items(extra_inclusion); + let mut additional_cpp_needs = Vec::new(); + let mut types_found = Vec::new(); + let mut bindgen_items = Vec::new(); + for item in items { + match item { + Item::ForeignMod(fm) => { + if extern_c_mod.is_none() { + // We'll use the first 'extern "C"' mod we come + // across for attributes, spans etc. but we'll stuff + // the contents of all bindgen 'extern "C"' mods into this + // one. + extern_c_mod = Some(ItemForeignMod { + attrs: fm.attrs, + abi: fm.abi, + brace_token: fm.brace_token, + items: Vec::new(), + }); + } + extern_c_mod + .as_mut() + .unwrap() + .items + .extend(self.convert_foreign_mod_items(&types_found, fm.items)?); + } + Item::Struct(s) => { + let tyident = s.ident.clone(); + let tyname = TypeName::from_ident(&tyident); + types_found.push(tyname.clone()); + self.class_names_discovered.insert(tyname.clone()); + let should_be_pod = self.byvalue_checker.is_pod(&tyname); + let type_alias = self.generate_type_alias(&tyname, should_be_pod); + bridge_items.push(type_alias.for_bridge); + extern_c_mod_items.push(type_alias.for_extern_c); + bindgen_mod + .content + .as_mut() + .unwrap() + .1 + .push(Item::Struct(self.convert_struct(s))); + all_items.push(type_alias.for_anywhere); + } + Item::Enum(e) => { + let tyident = e.ident.clone(); + let tyname = TypeName::from_ident(&tyident); + types_found.push(tyname.clone()); + let type_alias = self.generate_type_alias(&tyname, true); + bridge_items.push(type_alias.for_bridge); + extern_c_mod_items.push(type_alias.for_extern_c); + bindgen_mod.content.as_mut().unwrap().1.push(Item::Enum(e)); + all_items.push(type_alias.for_anywhere); + } + Item::Impl(i) => { + if let Some(ty) = self.type_to_typename(&i.self_ty) { + for item in i.items.clone() { + match item { + syn::ImplItem::Method(m) if m.sig.ident == "new" => { + if let Some(new_item_impl) = self.convert_new_method( + m, + &mut additional_cpp_needs, + &ty, + &i, + ) { + bindgen_items.push(new_item_impl); } } + _ => {} } } - _ => { - all_items.push(item); - } } } - // We will always create an extern "C" mod even if bindgen - // didn't generate one, e.g. because it only generated types. - // We still want cxx to know about those types. - let mut extern_c_mod = - extern_c_mod.unwrap_or_else(|| self.get_blank_extern_c_mod()); - extern_c_mod.items.append(&mut extern_c_mod_items); - bridge_items.push(Item::ForeignMod(extern_c_mod)); - bindgen_mod - .content - .as_mut() - .unwrap() - .1 - .append(&mut bindgen_items); - all_items.push(Item::Mod(bindgen_mod)); - let mut bridge_mod: ItemMod = parse_quote! { - #[cxx::bridge] - pub mod cxxbridge { - } - }; - bridge_mod - .content - .as_mut() - .unwrap() - .1 - .append(&mut bridge_items); - all_items.push(Item::Mod(bridge_mod)); - Ok(BridgeConversion { - items: all_items, - additional_cpp_needs, - }) + _ => { + all_items.push(item); + } } } + // We will always create an extern "C" mod even if bindgen + // didn't generate one, e.g. because it only generated types. + // We still want cxx to know about those types. + let mut extern_c_mod = extern_c_mod.unwrap_or_else(|| self.get_blank_extern_c_mod()); + extern_c_mod.items.append(&mut extern_c_mod_items); + bridge_items.push(Item::ForeignMod(extern_c_mod)); + bindgen_mod + .content + .as_mut() + .unwrap() + .1 + .append(&mut bindgen_items); + all_items.push(Item::Mod(bindgen_mod)); + let mut bridge_mod: ItemMod = parse_quote! { + #[cxx::bridge] + pub mod cxxbridge { + } + }; + bridge_mod + .content + .as_mut() + .unwrap() + .1 + .append(&mut bridge_items); + all_items.push(Item::Mod(bridge_mod)); + Ok(BridgeConversion { + items: all_items, + additional_cpp_needs, + }) } fn convert_new_method( From c105467e0fe9d078e9f64f07200ae0f556e7a727 Mon Sep 17 00:00:00 2001 From: Adrian Taylor Date: Mon, 5 Oct 2020 14:45:32 -0700 Subject: [PATCH 2/9] Slight tidying. --- engine/src/bridge_converter.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/engine/src/bridge_converter.rs b/engine/src/bridge_converter.rs index d0a3fc6f4..97ff44047 100644 --- a/engine/src/bridge_converter.rs +++ b/engine/src/bridge_converter.rs @@ -200,8 +200,7 @@ impl<'a> BridgeConverter { .extend(self.convert_foreign_mod_items(&types_found, fm.items)?); } Item::Struct(s) => { - let tyident = s.ident.clone(); - let tyname = TypeName::from_ident(&tyident); + let tyname = TypeName::from_ident(&s.ident); types_found.push(tyname.clone()); self.class_names_discovered.insert(tyname.clone()); let should_be_pod = self.byvalue_checker.is_pod(&tyname); @@ -217,8 +216,7 @@ impl<'a> BridgeConverter { all_items.push(type_alias.for_anywhere); } Item::Enum(e) => { - let tyident = e.ident.clone(); - let tyname = TypeName::from_ident(&tyident); + let tyname = TypeName::from_ident(&e.ident); types_found.push(tyname.clone()); let type_alias = self.generate_type_alias(&tyname, true); bridge_items.push(type_alias.for_bridge); From ce60f5861bfd786044d432cb78014b0337dc388d Mon Sep 17 00:00:00 2001 From: Adrian Taylor Date: Mon, 5 Oct 2020 14:53:13 -0700 Subject: [PATCH 3/9] Zap a clone. --- engine/src/bridge_converter.rs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/engine/src/bridge_converter.rs b/engine/src/bridge_converter.rs index 97ff44047..62f54f87a 100644 --- a/engine/src/bridge_converter.rs +++ b/engine/src/bridge_converter.rs @@ -127,12 +127,9 @@ impl<'a> BridgeConverter { } fn build_include_foreign_items(&self, extra_inclusion: Option<&str>) -> Vec { - let mut full_include_list = self.include_list.clone(); - if let Some(extra_inclusion) = extra_inclusion { - full_include_list.push(extra_inclusion.to_string()); - } - full_include_list - .iter() + let extra_owned = extra_inclusion.and_then(|x| Some(x.to_owned())); + let chained = self.include_list.iter().chain(extra_owned.iter()); + chained .map(|inc| { ForeignItem::Macro(parse_quote! { include!(#inc); From c5a0ac0beb33b4e2827ba7c8aa91aaf7b05fd47c Mon Sep 17 00:00:00 2001 From: Adrian Taylor Date: Mon, 5 Oct 2020 14:57:32 -0700 Subject: [PATCH 4/9] Rename. --- engine/src/bridge_converter.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/engine/src/bridge_converter.rs b/engine/src/bridge_converter.rs index 62f54f87a..0ddb8a2d6 100644 --- a/engine/src/bridge_converter.rs +++ b/engine/src/bridge_converter.rs @@ -34,7 +34,7 @@ pub enum ConvertError { } /// Results of a conversion. -pub(crate) struct BridgeConversion { +pub(crate) struct BridgeConversionResults { pub items: Vec, pub additional_cpp_needs: Vec, } @@ -144,7 +144,7 @@ impl<'a> BridgeConverter { &mut self, bindings: ItemMod, extra_inclusion: Option<&str>, - ) -> Result { + ) -> Result { match bindings.content { None => Err(ConvertError::NoContent), Some((brace, items)) => { @@ -166,7 +166,7 @@ impl<'a> BridgeConverter { mut bindgen_mod: ItemMod, items: Vec, extra_inclusion: Option<&str>, - ) -> Result { + ) -> Result { self.find_nested_pod_types(&items)?; let mut all_items: Vec = Vec::new(); let mut bridge_items = Vec::new(); @@ -270,7 +270,7 @@ impl<'a> BridgeConverter { .1 .append(&mut bridge_items); all_items.push(Item::Mod(bridge_mod)); - Ok(BridgeConversion { + Ok(BridgeConversionResults { items: all_items, additional_cpp_needs, }) From f57876e86000e552f1020d549d8e442dfe56b440 Mon Sep 17 00:00:00 2001 From: Adrian Taylor Date: Mon, 5 Oct 2020 15:50:44 -0700 Subject: [PATCH 5/9] Fix. --- engine/src/bridge_converter.rs | 206 ++++++++++++++++----------------- 1 file changed, 103 insertions(+), 103 deletions(-) diff --git a/engine/src/bridge_converter.rs b/engine/src/bridge_converter.rs index 0ddb8a2d6..c06181d58 100644 --- a/engine/src/bridge_converter.rs +++ b/engine/src/bridge_converter.rs @@ -17,13 +17,12 @@ use crate::byvalue_checker::ByValueChecker; use crate::known_types::KNOWN_TYPES; use crate::TypeName; use proc_macro2::{Span, TokenStream as TokenStream2, TokenTree}; -use std::collections::HashSet; use syn::punctuated::Punctuated; use syn::Token; use syn::{ - parse_quote, AngleBracketedGenericArguments, Attribute, FnArg, ForeignItem, - ForeignItemFn, GenericArgument, Ident, Item, ItemForeignMod, ItemMod, PatType, Path, - PathArguments, PathSegment, ReturnType, Type, TypePath, TypePtr, TypeReference, + parse_quote, AngleBracketedGenericArguments, Attribute, FnArg, ForeignItem, ForeignItemFn, + GenericArgument, Ident, Item, ItemForeignMod, ItemMod, PatType, Path, PathArguments, + PathSegment, ReturnType, Type, TypePath, TypePtr, TypeReference, }; #[derive(Debug)] @@ -55,26 +54,75 @@ pub(crate) struct BridgeConversionResults { pub(crate) struct BridgeConverter { include_list: Vec, pod_requests: Vec, - class_names_discovered: HashSet, - byvalue_checker: ByValueChecker, -} - -struct TypeAliasResults { - for_extern_c: ForeignItem, - for_bridge: Item, - for_anywhere: Item, } -impl<'a> BridgeConverter { +impl BridgeConverter { pub fn new(include_list: Vec, pod_requests: Vec) -> Self { Self { include_list, - class_names_discovered: HashSet::new(), - byvalue_checker: ByValueChecker::new(), pod_requests, } } + /// Convert a TokenStream of bindgen-generated bindings to a form + /// suitable for cxx. + pub(crate) fn convert( + &mut self, + bindings: ItemMod, + extra_inclusion: Option<&str>, + ) -> Result { + match bindings.content { + None => Err(ConvertError::NoContent), + Some((brace, items)) => { + let bindgen_mod = ItemMod { + attrs: bindings.attrs, + vis: bindings.vis, + ident: bindings.ident, + mod_token: bindings.mod_token, + content: Some((brace, Vec::new())), + semi: bindings.semi, + }; + let conversion = BridgeConversion { + bindgen_mod, + all_items: Vec::new(), + bridge_items: Vec::new(), + extern_c_mod: None, + extern_c_mod_items: Vec::new(), + additional_cpp_needs: Vec::new(), + types_found: Vec::new(), + bindgen_items: Vec::new(), + byvalue_checker: ByValueChecker::new(), + pod_requests: &self.pod_requests, + include_list: &self.include_list, + }; + conversion.convert_items(items, extra_inclusion) + } + } + } +} + +struct BridgeConversion<'a> { + bindgen_mod: ItemMod, + all_items: Vec, + bridge_items: Vec, + extern_c_mod: Option, + extern_c_mod_items: Vec, + additional_cpp_needs: Vec, + types_found: Vec, + bindgen_items: Vec, + byvalue_checker: ByValueChecker, + pod_requests: &'a Vec, + include_list: &'a Vec, +} + +impl<'a> BridgeConversion<'a> { + fn to_results(self) -> BridgeConversionResults { + BridgeConversionResults { + items: self.all_items, + additional_cpp_needs: self.additional_cpp_needs, + } + } + fn find_nested_pod_types(&mut self, items: &[Item]) -> Result<(), ConvertError> { for item in items { if let Item::Struct(s) = item { @@ -86,7 +134,8 @@ impl<'a> BridgeConverter { .map_err(ConvertError::UnsafePODType) } - fn generate_type_alias(&self, tyname: &TypeName, should_be_pod: bool) -> TypeAliasResults { + fn generate_type_alias(&mut self, tyname: &TypeName, should_be_pod: bool) { + self.types_found.push(tyname.clone()); let tyident = tyname.to_ident(); let kind_item: Ident = Ident::new( if should_be_pod { "Trivial" } else { "Opaque" }, @@ -112,18 +161,17 @@ impl<'a> BridgeConverter { ] .to_vec(), ); - TypeAliasResults { - for_extern_c: ForeignItem::Verbatim(for_extern_c_ts), - for_bridge: Item::Impl(parse_quote! { - impl UniquePtr<#tyident> {} - }), - for_anywhere: Item::Impl(parse_quote! { - unsafe impl cxx::ExternType for bindgen::#tyident { - type Id = cxx::type_id!(#tynamestring); - type Kind = cxx::kind::#kind_item; - } - }), - } + self.extern_c_mod_items + .push(ForeignItem::Verbatim(for_extern_c_ts)); + self.bridge_items.push(Item::Impl(parse_quote! { + impl UniquePtr<#tyident> {} + })); + self.all_items.push(Item::Impl(parse_quote! { + unsafe impl cxx::ExternType for bindgen::#tyident { + type Id = cxx::type_id!(#tynamestring); + type Kind = cxx::kind::#kind_item; + } + })); } fn build_include_foreign_items(&self, extra_inclusion: Option<&str>) -> Vec { @@ -138,88 +186,42 @@ impl<'a> BridgeConverter { .collect() } - /// Convert a TokenStream of bindgen-generated bindings to a form - /// suitable for cxx. - pub(crate) fn convert( - &mut self, - bindings: ItemMod, - extra_inclusion: Option<&str>, - ) -> Result { - match bindings.content { - None => Err(ConvertError::NoContent), - Some((brace, items)) => { - let bindgen_mod = ItemMod { - attrs: bindings.attrs, - vis: bindings.vis, - ident: bindings.ident, - mod_token: bindings.mod_token, - content: Some((brace, Vec::new())), - semi: bindings.semi, - }; - self.convert_items(bindgen_mod, items, extra_inclusion) - }, - } - } - fn convert_items( - &mut self, - mut bindgen_mod: ItemMod, + mut self, items: Vec, extra_inclusion: Option<&str>, ) -> Result { self.find_nested_pod_types(&items)?; - let mut all_items: Vec = Vec::new(); - let mut bridge_items = Vec::new(); - let mut extern_c_mod = None; - let mut extern_c_mod_items = self.build_include_foreign_items(extra_inclusion); - let mut additional_cpp_needs = Vec::new(); - let mut types_found = Vec::new(); - let mut bindgen_items = Vec::new(); + self.extern_c_mod_items = self.build_include_foreign_items(extra_inclusion); for item in items { match item { Item::ForeignMod(fm) => { - if extern_c_mod.is_none() { + if self.extern_c_mod.is_none() { // We'll use the first 'extern "C"' mod we come // across for attributes, spans etc. but we'll stuff // the contents of all bindgen 'extern "C"' mods into this // one. - extern_c_mod = Some(ItemForeignMod { + self.extern_c_mod = Some(ItemForeignMod { attrs: fm.attrs, abi: fm.abi, brace_token: fm.brace_token, items: Vec::new(), }); } - extern_c_mod - .as_mut() - .unwrap() - .items - .extend(self.convert_foreign_mod_items(&types_found, fm.items)?); + let fm_items = self.convert_foreign_mod_items(&self.types_found, fm.items)?; + self.extern_c_mod.as_mut().unwrap().items.extend(fm_items); } Item::Struct(s) => { let tyname = TypeName::from_ident(&s.ident); - types_found.push(tyname.clone()); - self.class_names_discovered.insert(tyname.clone()); let should_be_pod = self.byvalue_checker.is_pod(&tyname); - let type_alias = self.generate_type_alias(&tyname, should_be_pod); - bridge_items.push(type_alias.for_bridge); - extern_c_mod_items.push(type_alias.for_extern_c); - bindgen_mod - .content - .as_mut() - .unwrap() - .1 + self.generate_type_alias(&tyname, should_be_pod); + self.bindgen_items .push(Item::Struct(self.convert_struct(s))); - all_items.push(type_alias.for_anywhere); } Item::Enum(e) => { let tyname = TypeName::from_ident(&e.ident); - types_found.push(tyname.clone()); - let type_alias = self.generate_type_alias(&tyname, true); - bridge_items.push(type_alias.for_bridge); - extern_c_mod_items.push(type_alias.for_extern_c); - bindgen_mod.content.as_mut().unwrap().1.push(Item::Enum(e)); - all_items.push(type_alias.for_anywhere); + self.generate_type_alias(&tyname, true); + self.bindgen_items.push(Item::Enum(e)); } Item::Impl(i) => { if let Some(ty) = self.type_to_typename(&i.self_ty) { @@ -228,11 +230,10 @@ impl<'a> BridgeConverter { syn::ImplItem::Method(m) if m.sig.ident == "new" => { if let Some(new_item_impl) = self.convert_new_method( m, - &mut additional_cpp_needs, &ty, &i, ) { - bindgen_items.push(new_item_impl); + self.bindgen_items.push(new_item_impl); } } _ => {} @@ -241,23 +242,26 @@ impl<'a> BridgeConverter { } } _ => { - all_items.push(item); + self.all_items.push(item); } } } // We will always create an extern "C" mod even if bindgen // didn't generate one, e.g. because it only generated types. // We still want cxx to know about those types. - let mut extern_c_mod = extern_c_mod.unwrap_or_else(|| self.get_blank_extern_c_mod()); - extern_c_mod.items.append(&mut extern_c_mod_items); - bridge_items.push(Item::ForeignMod(extern_c_mod)); - bindgen_mod + let mut extern_c_mod = self + .extern_c_mod + .take() + .unwrap_or_else(|| self.get_blank_extern_c_mod()); + extern_c_mod.items.append(&mut self.extern_c_mod_items); + self.bridge_items.push(Item::ForeignMod(extern_c_mod)); + self.bindgen_mod .content .as_mut() .unwrap() .1 - .append(&mut bindgen_items); - all_items.push(Item::Mod(bindgen_mod)); + .append(&mut self.bindgen_items); + self.all_items.push(Item::Mod(self.bindgen_mod.clone())); let mut bridge_mod: ItemMod = parse_quote! { #[cxx::bridge] pub mod cxxbridge { @@ -268,18 +272,14 @@ impl<'a> BridgeConverter { .as_mut() .unwrap() .1 - .append(&mut bridge_items); - all_items.push(Item::Mod(bridge_mod)); - Ok(BridgeConversionResults { - items: all_items, - additional_cpp_needs, - }) + .append(&mut self.bridge_items); + self.all_items.push(Item::Mod(bridge_mod)); + Ok(self.to_results()) } fn convert_new_method( - &self, + &mut self, m: syn::ImplItemMethod, - additional_cpp_needs: &mut Vec, ty: &TypeName, i: &syn::ItemImpl, ) -> Option { @@ -298,7 +298,7 @@ impl<'a> BridgeConverter { FnArg::Receiver(_) => None, }); let (arg_types, arg_names): (Vec<_>, Vec<_>) = constructor_args.unzip(); - additional_cpp_needs.push(AdditionalNeed::MakeUnique(ty.clone(), arg_types)); + self.additional_cpp_needs.push(AdditionalNeed::MakeUnique(ty.clone(), arg_types)); // Create a function which calls Bob_make_unique // from Bob::make_unique. let call_name = Ident::new( @@ -411,7 +411,7 @@ impl<'a> BridgeConverter { // We want to feed cxx methods with just the method name, so let's // strip off the class name. // TODO test with class names containing underscores. It should work. - for cn in &self.class_names_discovered { + for cn in &self.types_found { if old_name.starts_with(&cn.0) { s.ident = Ident::new(&old_name[cn.0.len() + 1..], s.ident.span()); break; From 10de0cf6a95989aa34b3b0c820aff9ba9f56e058 Mon Sep 17 00:00:00 2001 From: Adrian Taylor Date: Mon, 5 Oct 2020 16:12:39 -0700 Subject: [PATCH 6/9] Further tidyings. --- engine/src/bridge_converter.rs | 24 +++++++----------------- 1 file changed, 7 insertions(+), 17 deletions(-) diff --git a/engine/src/bridge_converter.rs b/engine/src/bridge_converter.rs index c06181d58..8259d61c6 100644 --- a/engine/src/bridge_converter.rs +++ b/engine/src/bridge_converter.rs @@ -228,13 +228,7 @@ impl<'a> BridgeConversion<'a> { for item in i.items.clone() { match item { syn::ImplItem::Method(m) if m.sig.ident == "new" => { - if let Some(new_item_impl) = self.convert_new_method( - m, - &ty, - &i, - ) { - self.bindgen_items.push(new_item_impl); - } + self.convert_new_method(m, &ty, &i) } _ => {} } @@ -277,15 +271,10 @@ impl<'a> BridgeConversion<'a> { Ok(self.to_results()) } - fn convert_new_method( - &mut self, - m: syn::ImplItemMethod, - ty: &TypeName, - i: &syn::ItemImpl, - ) -> Option { + fn convert_new_method(&mut self, m: syn::ImplItemMethod, ty: &TypeName, i: &syn::ItemImpl) { let (arrow, oldreturntype) = match &m.sig.output { ReturnType::Type(arrow, ty) => (arrow, ty), - ReturnType::Default => return None, + ReturnType::Default => return, }; let constructor_args = m.sig.inputs.iter().filter_map(|x| match x { FnArg::Typed(pt) => { @@ -298,7 +287,8 @@ impl<'a> BridgeConversion<'a> { FnArg::Receiver(_) => None, }); let (arg_types, arg_names): (Vec<_>, Vec<_>) = constructor_args.unzip(); - self.additional_cpp_needs.push(AdditionalNeed::MakeUnique(ty.clone(), arg_types)); + self.additional_cpp_needs + .push(AdditionalNeed::MakeUnique(ty.clone(), arg_types)); // Create a function which calls Bob_make_unique // from Bob::make_unique. let call_name = Ident::new( @@ -324,7 +314,7 @@ impl<'a> BridgeConversion<'a> { block: new_block, sig: new_sig, }); - Some(Item::Impl(syn::ItemImpl { + self.bindgen_items.push(Item::Impl(syn::ItemImpl { attrs: Vec::new(), defaultness: i.defaultness, generics: i.generics.clone(), @@ -334,7 +324,7 @@ impl<'a> BridgeConversion<'a> { self_ty: i.self_ty.clone(), brace_token: i.brace_token, items: vec![new_impl_method], - })) + })); } fn get_blank_extern_c_mod(&self) -> ItemForeignMod { From c90d7579c2eb93cd56536b1315944c6dd885977d Mon Sep 17 00:00:00 2001 From: Adrian Taylor Date: Mon, 5 Oct 2020 16:21:44 -0700 Subject: [PATCH 7/9] More tidying. --- engine/src/bridge_converter.rs | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/engine/src/bridge_converter.rs b/engine/src/bridge_converter.rs index 8259d61c6..73d26e8db 100644 --- a/engine/src/bridge_converter.rs +++ b/engine/src/bridge_converter.rs @@ -116,12 +116,6 @@ struct BridgeConversion<'a> { } impl<'a> BridgeConversion<'a> { - fn to_results(self) -> BridgeConversionResults { - BridgeConversionResults { - items: self.all_items, - additional_cpp_needs: self.additional_cpp_needs, - } - } fn find_nested_pod_types(&mut self, items: &[Item]) -> Result<(), ConvertError> { for item in items { @@ -134,8 +128,7 @@ impl<'a> BridgeConversion<'a> { .map_err(ConvertError::UnsafePODType) } - fn generate_type_alias(&mut self, tyname: &TypeName, should_be_pod: bool) { - self.types_found.push(tyname.clone()); + fn generate_type_alias(&mut self, tyname: TypeName, should_be_pod: bool) { let tyident = tyname.to_ident(); let kind_item: Ident = Ident::new( if should_be_pod { "Trivial" } else { "Opaque" }, @@ -172,6 +165,7 @@ impl<'a> BridgeConversion<'a> { type Kind = cxx::kind::#kind_item; } })); + self.types_found.push(tyname); } fn build_include_foreign_items(&self, extra_inclusion: Option<&str>) -> Vec { @@ -214,13 +208,13 @@ impl<'a> BridgeConversion<'a> { Item::Struct(s) => { let tyname = TypeName::from_ident(&s.ident); let should_be_pod = self.byvalue_checker.is_pod(&tyname); - self.generate_type_alias(&tyname, should_be_pod); + self.generate_type_alias(tyname, should_be_pod); self.bindgen_items .push(Item::Struct(self.convert_struct(s))); } Item::Enum(e) => { let tyname = TypeName::from_ident(&e.ident); - self.generate_type_alias(&tyname, true); + self.generate_type_alias(tyname, true); self.bindgen_items.push(Item::Enum(e)); } Item::Impl(i) => { @@ -268,7 +262,10 @@ impl<'a> BridgeConversion<'a> { .1 .append(&mut self.bridge_items); self.all_items.push(Item::Mod(bridge_mod)); - Ok(self.to_results()) + Ok(BridgeConversionResults { + items: self.all_items, + additional_cpp_needs: self.additional_cpp_needs, + }) } fn convert_new_method(&mut self, m: syn::ImplItemMethod, ty: &TypeName, i: &syn::ItemImpl) { From eda0437b4a9816055793310cafc1c106e197ed0b Mon Sep 17 00:00:00 2001 From: Adrian Taylor Date: Mon, 5 Oct 2020 16:46:31 -0700 Subject: [PATCH 8/9] Clippyings. --- engine/src/bridge_converter.rs | 24 ++++++++++-------------- engine/src/rust_pretty_printer.rs | 16 ++++++++-------- 2 files changed, 18 insertions(+), 22 deletions(-) diff --git a/engine/src/bridge_converter.rs b/engine/src/bridge_converter.rs index 73d26e8db..74d8b1dfb 100644 --- a/engine/src/bridge_converter.rs +++ b/engine/src/bridge_converter.rs @@ -116,7 +116,6 @@ struct BridgeConversion<'a> { } impl<'a> BridgeConversion<'a> { - fn find_nested_pod_types(&mut self, items: &[Item]) -> Result<(), ConvertError> { for item in items { if let Item::Struct(s) = item { @@ -169,7 +168,7 @@ impl<'a> BridgeConversion<'a> { } fn build_include_foreign_items(&self, extra_inclusion: Option<&str>) -> Vec { - let extra_owned = extra_inclusion.and_then(|x| Some(x.to_owned())); + let extra_owned = extra_inclusion.map(|x| x.to_owned()); let chained = self.include_list.iter().chain(extra_owned.iter()); chained .map(|inc| { @@ -564,21 +563,18 @@ impl<'a> BridgeConversion<'a> { } fn convert_struct_field_type(&self, ty: &mut Type) { - match ty { - Type::Path(ty) => { - // O(n*m) but m is small. - for (type_name, type_details) in KNOWN_TYPES.iter() { - if ty.path.is_ident(&type_name.to_string()) { - if let Some(replacement) = &type_details.cxx_replacement { - let replacement = replacement.to_ident(); - ty.path = parse_quote! { - cxx:: #replacement - }; - } + if let Type::Path(ty) = ty { + // O(n*m) but m is small. + for (type_name, type_details) in KNOWN_TYPES.iter() { + if ty.path.is_ident(&type_name.to_string()) { + if let Some(replacement) = &type_details.cxx_replacement { + let replacement = replacement.to_ident(); + ty.path = parse_quote! { + cxx:: #replacement + }; } } } - _ => {} } } } diff --git a/engine/src/rust_pretty_printer.rs b/engine/src/rust_pretty_printer.rs index 874c63f78..0c7ccee7a 100644 --- a/engine/src/rust_pretty_printer.rs +++ b/engine/src/rust_pretty_printer.rs @@ -17,10 +17,10 @@ use std::io::Write; use std::process::{Command, Stdio}; enum Error { - RunError(std::io::Error), - WriteError(std::io::Error), - Utf8Error(std::string::FromUtf8Error), - WaitError(std::io::Error), + Run(std::io::Error), + Write(std::io::Error), + Utf8(std::string::FromUtf8Error), + Wait(std::io::Error), } pub(crate) fn pretty_print(ts: &TokenStream) -> String { @@ -39,8 +39,8 @@ fn reformat(text: impl std::fmt::Display) -> Result { .stdin(Stdio::piped()) .stdout(Stdio::piped()) .spawn() - .map_err(Error::RunError)?; - write!(rustfmt.stdin.take().unwrap(), "{}", text).map_err(Error::WriteError)?; - let output = rustfmt.wait_with_output().map_err(Error::WaitError)?; - String::from_utf8(output.stdout).map_err(Error::Utf8Error) + .map_err(Error::Run)?; + write!(rustfmt.stdin.take().unwrap(), "{}", text).map_err(Error::Write)?; + let output = rustfmt.wait_with_output().map_err(Error::Wait)?; + String::from_utf8(output.stdout).map_err(Error::Utf8) } From c076e4dcbdb4993a2c710d4197642118419613ed Mon Sep 17 00:00:00 2001 From: Adrian Taylor Date: Fri, 9 Oct 2020 12:10:42 -0700 Subject: [PATCH 9/9] Remove some type database duplication. --- engine/src/byvalue_checker.rs | 41 ++++++++++++----------------------- engine/src/known_types.rs | 11 ++++++---- 2 files changed, 21 insertions(+), 31 deletions(-) diff --git a/engine/src/byvalue_checker.rs b/engine/src/byvalue_checker.rs index be02a507b..d2a6a87b7 100644 --- a/engine/src/byvalue_checker.rs +++ b/engine/src/byvalue_checker.rs @@ -13,6 +13,7 @@ // limitations under the License. use crate::TypeName; +use crate::known_types::KNOWN_TYPES; use std::collections::HashMap; use syn::{ItemStruct, Type}; @@ -49,33 +50,19 @@ pub struct ByValueChecker { impl ByValueChecker { pub fn new() -> Self { let mut results = HashMap::new(); - results.insert( - TypeName::new("CxxString"), - StructDetails::new(PODState::UnsafeToBePOD( - "std::string has a self-referential pointer.".to_string(), - )), - ); - results.insert( - TypeName::new("UniquePtr"), - StructDetails::new(PODState::SafeToBePOD), - ); - results.insert( - TypeName::new("i32"), - StructDetails::new(PODState::SafeToBePOD), - ); - results.insert( - TypeName::new("i64"), - StructDetails::new(PODState::SafeToBePOD), - ); - results.insert( - TypeName::new("u32"), - StructDetails::new(PODState::SafeToBePOD), - ); - results.insert( - TypeName::new("u64"), - StructDetails::new(PODState::SafeToBePOD), - ); - // TODO expand with all primitives, or find a better way. + for (tn, td) in KNOWN_TYPES.iter() { + let canonical_name = if let Some(rust_name) = td.cxx_replacement.as_ref() { + rust_name.clone() + } else { + tn.clone() + }; + let safety = if td.by_value_safe { + PODState::SafeToBePOD + } else { + PODState::UnsafeToBePOD("type is not safe for POD".to_owned()) + }; + results.insert(canonical_name, StructDetails::new(safety)); + } ByValueChecker { results } } diff --git a/engine/src/known_types.rs b/engine/src/known_types.rs index 009197ca8..1ad32f13a 100644 --- a/engine/src/known_types.rs +++ b/engine/src/known_types.rs @@ -23,13 +23,16 @@ pub(crate) struct TypeDetails { pub(crate) cxx_replacement: Option, /// C++ equivalent name for a Rust type. pub(crate) cxx_name: Option, + /// Whether this can be safely represented by value. + pub(crate) by_value_safe: bool, } impl TypeDetails { - fn new(cxx_replacement: Option, cxx_name: Option) -> Self { + fn new(cxx_replacement: Option, cxx_name: Option, by_value_safe: bool) -> Self { TypeDetails { cxx_replacement, cxx_name, + by_value_safe, } } } @@ -39,11 +42,11 @@ lazy_static! { let mut map = HashMap::new(); map.insert( TypeName::new("std_unique_ptr"), - TypeDetails::new(Some(TypeName::new("UniquePtr")), None), + TypeDetails::new(Some(TypeName::new("UniquePtr")), None, true), ); map.insert( TypeName::new("std_string"), - TypeDetails::new(Some(TypeName::new("CxxString")), None), + TypeDetails::new(Some(TypeName::new("CxxString")), None, false), ); for (cpp_type, rust_type) in (3..7) .map(|x| 2i32.pow(x)) @@ -57,7 +60,7 @@ lazy_static! { { map.insert( TypeName::new(&rust_type), - TypeDetails::new(None, Some(cpp_type)), + TypeDetails::new(None, Some(cpp_type), true), ); } map