From d524f2ded06de68d88c238ac55d75a97908c081a Mon Sep 17 00:00:00 2001 From: Claire Taylor Date: Thu, 27 May 2021 23:29:43 -0700 Subject: [PATCH] Almost working... --- .../analysis/{fun => }/bridge_name_tracker.rs | 3 +- engine/src/conversion/analysis/ctypes.rs | 1 + engine/src/conversion/analysis/fun/mod.rs | 16 ++--- engine/src/conversion/analysis/mod.rs | 1 + engine/src/conversion/analysis/pod/mod.rs | 59 +++++++++++++++++-- .../src/conversion/analysis/remove_ignored.rs | 1 + engine/src/conversion/analysis/tdef.rs | 47 ++++++++++----- .../src/conversion/analysis/type_converter.rs | 2 + engine/src/conversion/api.rs | 6 +- engine/src/conversion/error_reporter.rs | 1 + engine/src/conversion/parse/parse_bindgen.rs | 4 ++ .../src/conversion/parse/parse_foreign_mod.rs | 1 + engine/src/conversion/utilities.rs | 1 + 13 files changed, 112 insertions(+), 31 deletions(-) rename engine/src/conversion/analysis/{fun => }/bridge_name_tracker.rs (97%) diff --git a/engine/src/conversion/analysis/fun/bridge_name_tracker.rs b/engine/src/conversion/analysis/bridge_name_tracker.rs similarity index 97% rename from engine/src/conversion/analysis/fun/bridge_name_tracker.rs rename to engine/src/conversion/analysis/bridge_name_tracker.rs index 9658e288b..e352aeba5 100644 --- a/engine/src/conversion/analysis/fun/bridge_name_tracker.rs +++ b/engine/src/conversion/analysis/bridge_name_tracker.rs @@ -62,7 +62,7 @@ impl BridgeNameTracker { Self::default() } - /// Figure out the least confusing unique name for this function in the + /// Figure out the least confusing unique name for this thing in the /// cxx::bridge section, which has a flat namespace. /// We mostly just qualify the name with the namespace_with_underscores. /// It doesn't really matter; we'll rebind these things to @@ -82,6 +82,7 @@ impl BridgeNameTracker { found_name: &str, ns: &Namespace, ) -> String { + println!("===Found name getting unique: {}", found_name); let count = self .next_cxx_bridge_name_for_prefix .entry(found_name.to_string()) diff --git a/engine/src/conversion/analysis/ctypes.rs b/engine/src/conversion/analysis/ctypes.rs index ea82df079..2a4cc7cc1 100644 --- a/engine/src/conversion/analysis/ctypes.rs +++ b/engine/src/conversion/analysis/ctypes.rs @@ -37,6 +37,7 @@ pub(crate) fn append_ctype_information(apis: &mut Vec>) { original_name: None, deps: HashSet::new(), detail: ApiDetail::CType { typename: tn }, + rename_to: None, }); } } diff --git a/engine/src/conversion/analysis/fun/mod.rs b/engine/src/conversion/analysis/fun/mod.rs index 33583b4c3..72159ef4c 100644 --- a/engine/src/conversion/analysis/fun/mod.rs +++ b/engine/src/conversion/analysis/fun/mod.rs @@ -11,8 +11,6 @@ // 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. - -mod bridge_name_tracker; pub(crate) mod function_wrapper; mod overload_tracker; mod rust_name_tracker; @@ -49,12 +47,9 @@ use crate::{ types::{make_ident, validate_ident_ok_for_cxx, Namespace, QualifiedName}, }; -use self::{ - bridge_name_tracker::BridgeNameTracker, overload_tracker::OverloadTracker, - rust_name_tracker::RustNameTracker, -}; +use self::{overload_tracker::OverloadTracker, rust_name_tracker::RustNameTracker}; -use super::pod::PodAnalysis; +use super::{bridge_name_tracker::BridgeNameTracker, pod::PodAnalysis}; pub(crate) enum MethodKind { Normal, @@ -235,6 +230,7 @@ impl<'a> FnAnalyzer<'a> { original_name: api.original_name, deps: new_deps, detail: api_detail, + rename_to: api.rename_to, })) } @@ -840,7 +836,11 @@ impl Api { QualifiedName::new(&self.name.get_namespace(), make_ident(&analysis.rust_name)) } }, - _ => self.name(), + _ => self + .rename_to + .as_ref() + .map(|id| QualifiedName::new(self.name().get_namespace(), id.clone())) + .unwrap_or(self.name()), } } diff --git a/engine/src/conversion/analysis/mod.rs b/engine/src/conversion/analysis/mod.rs index c748829d9..a09b02308 100644 --- a/engine/src/conversion/analysis/mod.rs +++ b/engine/src/conversion/analysis/mod.rs @@ -17,6 +17,7 @@ use syn::Attribute; use super::ConvertError; pub(crate) mod abstract_types; +mod bridge_name_tracker; pub(crate) mod ctypes; pub(crate) mod fun; pub(crate) mod gc; diff --git a/engine/src/conversion/analysis/pod/mod.rs b/engine/src/conversion/analysis/pod/mod.rs index 16328488d..2ab0093b0 100644 --- a/engine/src/conversion/analysis/pod/mod.rs +++ b/engine/src/conversion/analysis/pod/mod.rs @@ -18,6 +18,7 @@ use std::collections::HashSet; use autocxx_parser::IncludeCppConfig; use byvalue_checker::ByValueChecker; +use proc_macro2::Ident; use syn::ItemStruct; use crate::{ @@ -28,10 +29,10 @@ use crate::{ error_reporter::convert_item_apis, ConvertError, }, - types::{Namespace, QualifiedName}, + types::{make_ident, Namespace, QualifiedName}, }; -use super::tdef::TypedefAnalysis; +use super::{bridge_name_tracker::BridgeNameTracker, tdef::TypedefAnalysis}; pub(crate) struct PodAnalysis; @@ -58,8 +59,16 @@ pub(crate) fn analyze_pod_apis( let mut extra_apis = Vec::new(); let mut type_converter = TypeConverter::new(config, &apis); let mut results = Vec::new(); + let mut bridge_tracker = BridgeNameTracker::new(); convert_item_apis(apis, &mut results, |api| { - analyze_pod_api(api, &byvalue_checker, &mut type_converter, &mut extra_apis).map(Some) + analyze_pod_api( + api, + &byvalue_checker, + &mut type_converter, + &mut extra_apis, + &mut bridge_tracker, + ) + .map(Some) }); // Conceivably, the process of POD-analysing the first set of APIs could result // in us creating new APIs to concretize generic types. @@ -70,6 +79,7 @@ pub(crate) fn analyze_pod_apis( &byvalue_checker, &mut type_converter, &mut more_extra_apis, + &mut bridge_tracker, ) .map(Some) }); @@ -82,9 +92,12 @@ fn analyze_pod_api( byvalue_checker: &ByValueChecker, type_converter: &mut TypeConverter, extra_apis: &mut Vec, + bridge_name_tracker: &mut BridgeNameTracker, ) -> Result, ConvertError> { let ty_id = api.name(); let mut new_deps = api.deps; + let mut new_name = api.name; + let mut rename_to: Option = None; let api_detail = match api.detail { // No changes to any of these... ApiDetail::ConcreteType { @@ -94,7 +107,24 @@ fn analyze_pod_api( rs_definition, cpp_definition, }, - ApiDetail::ForwardDeclaration => ApiDetail::ForwardDeclaration, + ApiDetail::ForwardDeclaration => { + let replacement_cxx_bridge_name = bridge_name_tracker.get_unique_cxx_bridge_name( + None, + &ty_id.get_final_item(), + ty_id.get_namespace(), + ); + new_name = QualifiedName::new( + ty_id.get_namespace(), + make_ident(&replacement_cxx_bridge_name), + ); + rename_to = if replacement_cxx_bridge_name == ty_id.get_final_item() { + None + } else { + Some(ty_id.get_final_ident().clone()) + }; + + ApiDetail::ForwardDeclaration + } ApiDetail::StringConstructor => ApiDetail::StringConstructor, ApiDetail::Function { fun, analysis } => ApiDetail::Function { fun, analysis }, ApiDetail::Const { const_item } => ApiDetail::Const { const_item }, @@ -122,7 +152,7 @@ fn analyze_pod_api( // It's POD so let's mark dependencies on things in its field get_struct_field_types( type_converter, - &api.name.get_namespace(), + &new_name.get_namespace(), &item, &mut new_deps, extra_apis, @@ -135,6 +165,21 @@ fn analyze_pod_api( new_deps.clear(); TypeKind::NonPod }; + let replacement_cxx_bridge_name = bridge_name_tracker.get_unique_cxx_bridge_name( + None, + &item.ident.to_string(), + ty_id.get_namespace(), + ); + new_name = QualifiedName::new( + ty_id.get_namespace(), + make_ident(&replacement_cxx_bridge_name), + ); + + rename_to = if replacement_cxx_bridge_name == item.ident.to_string() { + None + } else { + Some(item.ident.clone()) + }; ApiDetail::Struct { item, analysis: type_kind, @@ -142,11 +187,13 @@ fn analyze_pod_api( } ApiDetail::IgnoredItem { err, ctx } => ApiDetail::IgnoredItem { err, ctx }, }; + println!("Rename to {:?}", rename_to); Ok(Api { - name: api.name, + name: new_name, original_name: api.original_name, deps: new_deps, detail: api_detail, + rename_to, }) } diff --git a/engine/src/conversion/analysis/remove_ignored.rs b/engine/src/conversion/analysis/remove_ignored.rs index aaf532920..23e13497d 100644 --- a/engine/src/conversion/analysis/remove_ignored.rs +++ b/engine/src/conversion/analysis/remove_ignored.rs @@ -89,5 +89,6 @@ fn create_ignore_item(api: Api, err: ConvertError) -> Api ErrorContext::Item(id), }, }, + rename_to: api.rename_to, } } diff --git a/engine/src/conversion/analysis/tdef.rs b/engine/src/conversion/analysis/tdef.rs index 770b6595b..fe77c247b 100644 --- a/engine/src/conversion/analysis/tdef.rs +++ b/engine/src/conversion/analysis/tdef.rs @@ -15,7 +15,7 @@ use std::collections::HashSet; use autocxx_parser::IncludeCppConfig; -use syn::ItemType; +use syn::{Ident, ItemType}; use crate::{ conversion::{ @@ -25,11 +25,10 @@ use crate::{ error_reporter::report_any_error, ConvertError, }, - types::QualifiedName, + types::{make_ident, QualifiedName}, }; -use super::remove_bindgen_attrs; - +use super::{bridge_name_tracker::BridgeNameTracker, remove_bindgen_attrs}; /// Analysis phase where typedef analysis has been performed but no other /// analyses just yet. pub(crate) struct TypedefAnalysis; @@ -48,6 +47,7 @@ pub(crate) fn convert_typedef_targets( let mut type_converter = TypeConverter::new(config, &apis); let mut extra_apis = Vec::new(); let mut problem_apis = Vec::new(); + let mut bridge_tracker = BridgeNameTracker::new(); let new_apis = apis .into_iter() .filter_map(|api| { @@ -55,6 +55,8 @@ pub(crate) fn convert_typedef_targets( let original_name = api.original_name; let ns = name.get_namespace(); let mut newdeps = api.deps; + let mut new_name = api.name; + let mut rename_to: Option = None; let detail = match api.detail { ApiDetail::ForwardDeclaration => Some(ApiDetail::ForwardDeclaration), ApiDetail::ConcreteType { @@ -72,15 +74,31 @@ pub(crate) fn convert_typedef_targets( ApiDetail::Typedef { item: TypedefKind::Type(ity), analysis: _, - } => report_any_error(ns, &mut problem_apis, || { - get_replacement_typedef( - &name, - ity, - &mut type_converter, - &mut extra_apis, - &mut newdeps, - ) - }), + } => { + let replacement_cxx_bridge_name = bridge_tracker.get_unique_cxx_bridge_name( + None, + &ity.ident.to_string(), + name.get_namespace(), + ); + new_name = QualifiedName::new( + name.get_namespace(), + make_ident(&replacement_cxx_bridge_name), + ); + rename_to = if replacement_cxx_bridge_name == ity.ident.to_string() { + None + } else { + Some(ity.ident.clone()) + }; + report_any_error(ns, &mut problem_apis, || { + get_replacement_typedef( + &name, + ity, + &mut type_converter, + &mut extra_apis, + &mut newdeps, + ) + }) + } ApiDetail::Typedef { item, analysis: _ } => Some(ApiDetail::Typedef { item: item.clone(), analysis: item, @@ -92,9 +110,10 @@ pub(crate) fn convert_typedef_targets( }; detail.map(|detail| Api { detail, - name, + name: new_name, original_name, deps: newdeps, + rename_to, }) }) .collect::>(); diff --git a/engine/src/conversion/analysis/type_converter.rs b/engine/src/conversion/analysis/type_converter.rs index 6705a0b28..7024b9f6a 100644 --- a/engine/src/conversion/analysis/type_converter.rs +++ b/engine/src/conversion/analysis/type_converter.rs @@ -399,6 +399,7 @@ impl<'a> TypeConverter<'a> { rs_definition: Box::new(rs_definition.clone()), cpp_definition, }, + rename_to: None, }; Ok((name, Some(api))) } @@ -526,6 +527,7 @@ pub(crate) fn add_analysis(api: UnanalyzedApi) -> Api { original_name: api.original_name, deps: api.deps, detail: new_detail, + rename_to: api.rename_to, } } pub(crate) trait TypedefTarget { diff --git a/engine/src/conversion/api.rs b/engine/src/conversion/api.rs index 7df206057..53f307b35 100644 --- a/engine/src/conversion/api.rs +++ b/engine/src/conversion/api.rs @@ -150,16 +150,18 @@ pub(crate) struct Api { pub(crate) deps: HashSet, /// Details of this specific API kind. pub(crate) detail: ApiDetail, + pub(crate) rename_to: Option, } impl std::fmt::Debug for Api { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { write!( f, - "{} (kind={}, deps={})", + "{} (kind={}, deps={}, rename_to={:?})", self.name.to_cpp_name(), self.detail, - self.deps.iter().map(|d| d.to_cpp_name()).join(", ") + self.deps.iter().map(|d| d.to_cpp_name()).join(", "), + self.rename_to ) } } diff --git a/engine/src/conversion/error_reporter.rs b/engine/src/conversion/error_reporter.rs index e88020d80..2ca6ce53e 100644 --- a/engine/src/conversion/error_reporter.rs +++ b/engine/src/conversion/error_reporter.rs @@ -95,6 +95,7 @@ fn ignored_item(ns: &Namespace, ctx: ErrorContext, err: Conver original_name: None, deps: HashSet::new(), detail: ApiDetail::IgnoredItem { err, ctx }, + rename_to: None, } } diff --git a/engine/src/conversion/parse/parse_bindgen.rs b/engine/src/conversion/parse/parse_bindgen.rs index 02f1cbc1d..618540344 100644 --- a/engine/src/conversion/parse/parse_bindgen.rs +++ b/engine/src/conversion/parse/parse_bindgen.rs @@ -233,6 +233,7 @@ impl<'a> ParseBindgen<'a> { }), analysis: (), }, + rename_to: None, }); break; } @@ -252,6 +253,7 @@ impl<'a> ParseBindgen<'a> { original_name: get_bindgen_original_name_annotation(&const_item.attrs), deps: HashSet::new(), detail: ApiDetail::Const { const_item }, + rename_to: None, }); Ok(()) } @@ -264,6 +266,7 @@ impl<'a> ParseBindgen<'a> { item: TypedefKind::Type(ity), analysis: (), }, + rename_to: None, }); Ok(()) } @@ -318,6 +321,7 @@ impl<'a> ParseBindgen<'a> { } else { api_make(bindgen_mod_item) }, + rename_to: None, }; self.apis.push(api); } diff --git a/engine/src/conversion/parse/parse_foreign_mod.rs b/engine/src/conversion/parse/parse_foreign_mod.rs index 13236e4c9..cce13b645 100644 --- a/engine/src/conversion/parse/parse_foreign_mod.rs +++ b/engine/src/conversion/parse/parse_foreign_mod.rs @@ -137,6 +137,7 @@ impl ParseForeignMod { fun: Box::new(fun), analysis: (), }, + rename_to: None, }) } } diff --git a/engine/src/conversion/utilities.rs b/engine/src/conversion/utilities.rs index 06e68941c..85978fff6 100644 --- a/engine/src/conversion/utilities.rs +++ b/engine/src/conversion/utilities.rs @@ -33,5 +33,6 @@ pub(crate) fn generate_utilities(apis: &mut Vec, config: &Include original_name: None, deps: HashSet::new(), detail: super::api::ApiDetail::StringConstructor, + rename_to: None, }); }