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 519f5f268..eda83fe72 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>) { cpp_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 fa0c46a11..8978899c2 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; @@ -50,12 +48,12 @@ 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, PodStructAnalysisBody}; +use super::{ + bridge_name_tracker::BridgeNameTracker, + pod::{PodAnalysis, PodStructAnalysisBody}, +}; pub(crate) enum MethodKind { Normal, @@ -240,6 +238,7 @@ impl<'a> FnAnalyzer<'a> { cpp_name, deps: new_deps, detail: api_detail, + rename_to: api.rename_to, })) } @@ -866,7 +865,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 0f68ca781..9f774bfb7 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 1be26e014..2225b7452 100644 --- a/engine/src/conversion/analysis/pod/mod.rs +++ b/engine/src/conversion/analysis/pod/mod.rs @@ -18,7 +18,7 @@ use std::collections::HashSet; use autocxx_parser::IncludeCppConfig; use byvalue_checker::ByValueChecker; -use syn::{ItemStruct, Type}; +use syn::{Ident, ItemStruct, Type}; use crate::{ conversion::{ @@ -28,10 +28,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 PodStructAnalysisBody { pub(crate) kind: TypeKind, @@ -63,8 +63,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. @@ -75,6 +83,7 @@ pub(crate) fn analyze_pod_apis( &byvalue_checker, &mut type_converter, &mut more_extra_apis, + &mut bridge_tracker, ) .map(Some) }); @@ -87,9 +96,13 @@ 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 mut cpp_name = api.cpp_name; let api_detail = match api.detail { // No changes to any of these... ApiDetail::ConcreteType { @@ -99,7 +112,28 @@ 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 { + if cpp_name == None { + cpp_name = Some(ty_id.get_final_item().to_string()); + } + 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 }, @@ -120,7 +154,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, @@ -133,6 +167,24 @@ 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 { + if cpp_name == None { + cpp_name = Some(item.ident.to_string()); + } + Some(item.ident.clone()) + }; ApiDetail::Struct { item, analysis: PodStructAnalysisBody { @@ -143,11 +195,13 @@ fn analyze_pod_api( } ApiDetail::IgnoredItem { err, ctx } => ApiDetail::IgnoredItem { err, ctx }, }; + println!("Rename to {:?}", rename_to); Ok(Api { - name: api.name, - cpp_name: api.cpp_name, + name: new_name, + cpp_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 575625c07..49d7637e7 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 d0b7f2e48..449c7621e 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 cpp_name = api.cpp_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, cpp_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 f6537672a..03fff1186 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 { cpp_name: api.cpp_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 eee894523..ffebbc1cf 100644 --- a/engine/src/conversion/api.rs +++ b/engine/src/conversion/api.rs @@ -21,7 +21,7 @@ use syn::{ use super::{convert_error::ErrorContext, ConvertError}; -#[derive(Copy, Clone, Eq, PartialEq)] +#[derive(Copy, Clone, Eq, PartialEq, Debug)] pub(crate) enum TypeKind { Pod, // trivial. Can be moved and copied in Rust. NonPod, // has destructor or non-trivial move constructors. Can only hold by UniquePtr @@ -153,16 +153,19 @@ 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={:?}, cpp_name={:?})", 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, + self.cpp_name, ) } } diff --git a/engine/src/conversion/codegen_rs/mod.rs b/engine/src/conversion/codegen_rs/mod.rs index c52ec1128..4db4e5077 100644 --- a/engine/src/conversion/codegen_rs/mod.rs +++ b/engine/src/conversion/codegen_rs/mod.rs @@ -549,6 +549,7 @@ impl<'a> RsCodeGenerator<'a> { } fn generate_extern_type_impl(&self, type_kind: TypeKind, tyname: &QualifiedName) -> Vec { + println! ("Generate extern type {:?} {:?}", type_kind, tyname); let tynamestring = namespaced_name_using_original_name_map(tyname, &self.original_name_map); let fulltypath = tyname.get_bindgen_path_idents(); let kind_item = match type_kind { @@ -567,6 +568,7 @@ impl<'a> RsCodeGenerator<'a> { fn generate_cxxbridge_type(&self, name: &QualifiedName) -> TokenStream { let ns = name.get_namespace(); let id = name.get_final_ident(); + let mut final_id = id.clone(); let mut ns_components: Vec<_> = ns.iter().cloned().collect(); let mut cxx_name = None; if let Some(cpp_name) = self.original_name_map.get(name) { @@ -588,19 +590,21 @@ impl<'a> RsCodeGenerator<'a> { for_extern_c_ts.extend(quote! { #[cxx_name = #n] }); + final_id = make_ident(&n); } for_extern_c_ts.extend(quote! { type #id = super::bindgen::root:: }); + for_extern_c_ts.extend(ns.iter().map(make_ident).map(|id| { quote! { #id:: } })); for_extern_c_ts.extend(quote! { - #id; - }); + #final_id; }); + for_extern_c_ts } diff --git a/engine/src/conversion/error_reporter.rs b/engine/src/conversion/error_reporter.rs index 8febd6495..83e7ba464 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 cpp_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 41cfea1d2..18efca28b 100644 --- a/engine/src/conversion/parse/parse_bindgen.rs +++ b/engine/src/conversion/parse/parse_bindgen.rs @@ -224,6 +224,7 @@ impl<'a> ParseBindgen<'a> { }), analysis: (), }, + rename_to: None, }); break; } @@ -243,6 +244,7 @@ impl<'a> ParseBindgen<'a> { cpp_name: get_bindgen_original_name_annotation(&const_item.attrs), deps: HashSet::new(), detail: ApiDetail::Const { const_item }, + rename_to: None, }); Ok(()) } @@ -255,6 +257,7 @@ impl<'a> ParseBindgen<'a> { item: TypedefKind::Type(ity), analysis: (), }, + rename_to: None, }); Ok(()) } @@ -309,6 +312,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 a36a34c5f..a5907b85c 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 cd7dd6862..ce7956a21 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 cpp_name: None, deps: HashSet::new(), detail: super::api::ApiDetail::StringConstructor, + rename_to: None, }); } diff --git a/engine/src/integration_tests.rs b/engine/src/integration_tests.rs index 8694be02d..1017433a8 100644 --- a/engine/src/integration_tests.rs +++ b/engine/src/integration_tests.rs @@ -5586,6 +5586,55 @@ fn test_manual_bridge() { do_run_test_manual("", hdr, rs, &[], None).unwrap(); } +#[test] +fn test_issue486() { + let hdr = indoc! {" + namespace a { + namespace spanner { + class Key; + } + } // namespace a + namespace spanner { + class Key { + public: + bool b(a::spanner::Key &); + }; + } // namespace spanner + "}; + let rs = quote! {}; + run_test("", hdr, rs, &["spanner::Key"], &[]); +} + +#[test] +fn test_issue486_multi_types() { + let hdr = indoc! {" + namespace a { + namespace spanner { + inline void Key() {} + } + } // namespace a + namespace b { + namespace spanner { + typedef int Key; + } + } // namespace a + namespace spanner { + class Key { + public: + bool b(a::spanner::Key &); + }; + } // namespace spanner + "}; + let rs = quote! {}; + run_test( + "", + hdr, + rs, + &["spanner::Key", "a::spanner::Key", "b::spanner::Key"], + &[], + ); +} + // Yet to test: // - Ifdef // - Out param pointers