diff --git a/engine/src/conversion/analysis/fun/mod.rs b/engine/src/conversion/analysis/fun/mod.rs index 415e40a4d..dcd82d33d 100644 --- a/engine/src/conversion/analysis/fun/mod.rs +++ b/engine/src/conversion/analysis/fun/mod.rs @@ -309,7 +309,7 @@ impl<'a> FnAnalyzer<'a> { type_converter: TypeConverter::new(config, &apis), bridge_name_tracker: BridgeNameTracker::new(), config, - overload_trackers_by_mod: HashMap::new(), + overload_trackers_by_mod: Self::build_overload_trackers(&apis), pod_safe_types: Self::build_pod_safe_type_set(&apis), moveit_safe_types: Self::build_correctly_sized_type_set(&apis), subclasses_by_superclass: subclass::subclasses_by_superclass(&apis), @@ -365,6 +365,20 @@ impl<'a> FnAnalyzer<'a> { .collect() } + /// For calculation of subsequent overloads, we want to work out the + /// names that we ideally would use. e.g. if we find func(int) + /// and func(float) we would normally call them func and func1, + /// but if there's already a func1 in the data structure returned + /// by this function then we'll call it something else. + fn build_overload_trackers(apis: &ApiVec) -> HashMap { + let all_namespaces: HashSet<&Namespace> = + apis.iter().map(|api| api.name().get_namespace()).collect(); + all_namespaces + .into_iter() + .map(|ns| (ns.clone(), OverloadTracker::new(apis, ns))) + .collect() + } + /// Return the set of 'moveit safe' types. That must include only types where /// the size is known to be correct. fn build_correctly_sized_type_set(apis: &ApiVec) -> HashSet { @@ -704,6 +718,36 @@ impl<'a> FnAnalyzer<'a> { name.name } + /// Work out the ideal Rust name for a given function, taking account + /// of overloads and Rust idents. + pub(crate) fn ideal_rust_name(name: &ApiName, fun: &FuncToConvert) -> String { + let cpp_name = name.cpp_name_if_present().cloned(); + // bindgen may have mangled the name either because it's invalid Rust + // syntax (e.g. a keyword like 'async') or it's an overload. + // If the former, we respect that mangling. If the latter, we don't, + // because we'll add our own overload counting mangling later. + // Cases: + // function, IRN=foo, CN= output: foo case 1 + // function, IRN=move_, CN=move (keyword problem) output: move_ case 2 + // function, IRN=foo1, CN=foo (overload) output: foo case 3 + // method, IRN=A_foo, CN=foo output: foo case 4 + // method, IRN=A_move, CN=move (keyword problem) output: move_ case 5 + // method, IRN=A_foo1, CN=foo (overload) output: foo case 6 + let initial_rust_name = fun.ident.to_string(); + match &cpp_name { + None => initial_rust_name, // case 1 + Some(cpp_name) => { + if initial_rust_name.ends_with('_') { + initial_rust_name // case 2 + } else if validate_ident_ok_for_rust(cpp_name).is_err() { + format!("{cpp_name}_") // case 5 + } else { + cpp_name.to_string() // cases 3, 4, 6 + } + } + } + } + /// Determine how to materialize a function. /// /// The main job here is to determine whether a function can simply be noted @@ -775,29 +819,9 @@ impl<'a> FnAnalyzer<'a> { // End of parameter processing. // Work out naming, part one. - // bindgen may have mangled the name either because it's invalid Rust - // syntax (e.g. a keyword like 'async') or it's an overload. - // If the former, we respect that mangling. If the latter, we don't, - // because we'll add our own overload counting mangling later. - // Cases: - // function, IRN=foo, CN= output: foo case 1 - // function, IRN=move_, CN=move (keyword problem) output: move_ case 2 - // function, IRN=foo1, CN=foo (overload) output: foo case 3 - // method, IRN=A_foo, CN=foo output: foo case 4 - // method, IRN=A_move, CN=move (keyword problem) output: move_ case 5 - // method, IRN=A_foo1, CN=foo (overload) output: foo case 6 - let ideal_rust_name = match &cpp_name { - None => initial_rust_name, // case 1 - Some(cpp_name) => { - if initial_rust_name.ends_with('_') { - initial_rust_name // case 2 - } else if validate_ident_ok_for_rust(cpp_name).is_err() { - format!("{cpp_name}_") // case 5 - } else { - cpp_name.to_string() // cases 3, 4, 6 - } - } - }; + // See what we'd ideally call this in Rust, which we might + // later need to alter based on overloads. + let ideal_rust_name = Self::ideal_rust_name(&name, fun); // Let's spend some time figuring out the kind of this function (i.e. method, // virtual function, etc.) @@ -1510,7 +1534,7 @@ impl<'a> FnAnalyzer<'a> { } fn get_overload_name(&mut self, ns: &Namespace, type_ident: &str, rust_name: String) -> String { - let overload_tracker = self.overload_trackers_by_mod.entry(ns.clone()).or_default(); + let overload_tracker = self.overload_trackers_by_mod.get_mut(ns).unwrap(); overload_tracker.get_method_real_name(type_ident, rust_name) } @@ -1624,7 +1648,7 @@ impl<'a> FnAnalyzer<'a> { } fn get_function_overload_name(&mut self, ns: &Namespace, ideal_rust_name: String) -> String { - let overload_tracker = self.overload_trackers_by_mod.entry(ns.clone()).or_default(); + let overload_tracker = self.overload_trackers_by_mod.get_mut(ns).unwrap(); overload_tracker.get_function_real_name(ideal_rust_name) } diff --git a/engine/src/conversion/analysis/fun/overload_tracker.rs b/engine/src/conversion/analysis/fun/overload_tracker.rs index ec8a31fcf..4730a15cc 100644 --- a/engine/src/conversion/analysis/fun/overload_tracker.rs +++ b/engine/src/conversion/analysis/fun/overload_tracker.rs @@ -7,6 +7,14 @@ // except according to those terms. use indexmap::IndexMap as HashMap; +use indexmap::IndexSet as HashSet; + +use crate::conversion::analysis::pod::PodPhase; +use crate::conversion::api::Api; +use crate::conversion::apivec::ApiVec; +use crate::types::Namespace; + +use super::FnAnalyzer; type Offsets = HashMap; @@ -20,13 +28,40 @@ type Offsets = HashMap; /// If bindgen adds a suffix it will be included in 'found_name' /// but not 'original_name' which is an annotation added by our autocxx-bindgen /// fork. -#[derive(Default)] pub(crate) struct OverloadTracker { + names_to_avoid: HashSet, + names_to_avoid_by_type: HashMap>, offset_by_name: Offsets, offset_by_type_and_name: HashMap, } impl OverloadTracker { + pub(crate) fn new(apis: &ApiVec, namespace: &Namespace) -> Self { + let mut names_to_avoid = HashSet::new(); + let mut names_to_avoid_by_type: HashMap> = HashMap::new(); + for api in apis + .iter() + .filter(|api| api.name().get_namespace() == namespace) + { + if let Api::Function { name, fun, .. } = api { + let ideal = FnAnalyzer::ideal_rust_name(name, fun); + let set_to_change = match fun.self_ty { + None => &mut names_to_avoid, + Some(ref self_ty) => names_to_avoid_by_type + .entry(self_ty.get_final_item().to_string()) + .or_default(), + }; + set_to_change.insert(ideal); + } + } + Self { + names_to_avoid, + names_to_avoid_by_type, + offset_by_name: Default::default(), + offset_by_type_and_name: Default::default(), + } + } + pub(crate) fn get_function_real_name(&mut self, found_name: String) -> String { self.get_name(None, found_name) } @@ -36,39 +71,57 @@ impl OverloadTracker { } fn get_name(&mut self, type_name: Option<&str>, cpp_method_name: String) -> String { - let registry = match type_name { - Some(type_name) => self - .offset_by_type_and_name - .entry(type_name.to_string()) - .or_default(), - None => &mut self.offset_by_name, + let empty_set = HashSet::new(); + let (registry, to_avoid) = match type_name { + Some(type_name) => ( + self.offset_by_type_and_name + .entry(type_name.to_string()) + .or_default(), + self.names_to_avoid_by_type + .get(&type_name.to_string()) + .unwrap_or(&empty_set), + ), + None => (&mut self.offset_by_name, &self.names_to_avoid), }; + log::info!( + "Considering {:?}, {}, to avoid contains {:?}", + type_name, + cpp_method_name, + to_avoid + ); let offset = registry.entry(cpp_method_name.clone()).or_default(); let this_offset = *offset; *offset += 1; if this_offset == 0 { cpp_method_name } else { - format!("{cpp_method_name}{this_offset}") + let provisional = format!("{cpp_method_name}{this_offset}"); + if to_avoid.contains(&provisional) { + format!("{cpp_method_name}_autocxx_overload{this_offset}") + } else { + provisional + } } } } #[cfg(test)] mod tests { + use crate::{conversion::apivec::ApiVec, types::Namespace}; + use super::OverloadTracker; #[test] fn test_by_function() { - let mut ot = OverloadTracker::default(); - assert_eq!(ot.get_function_real_name("bob".into()), "bob"); + let mut ot = OverloadTracker::new(&ApiVec::new(), &Namespace::new()); + assert_eq!(ot.get_function_real_name("bob".into()), "bob",); assert_eq!(ot.get_function_real_name("bob".into()), "bob1"); assert_eq!(ot.get_function_real_name("bob".into()), "bob2"); } #[test] fn test_by_method() { - let mut ot = OverloadTracker::default(); + let mut ot = OverloadTracker::new(&ApiVec::new(), &Namespace::new()); assert_eq!(ot.get_method_real_name("Ty1", "bob".into()), "bob"); assert_eq!(ot.get_method_real_name("Ty1", "bob".into()), "bob1"); assert_eq!(ot.get_method_real_name("Ty2", "bob".into()), "bob"); diff --git a/integration-tests/tests/integration_test.rs b/integration-tests/tests/integration_test.rs index 7c2cccfe2..f677a65ba 100644 --- a/integration-tests/tests/integration_test.rs +++ b/integration-tests/tests/integration_test.rs @@ -2280,8 +2280,28 @@ fn test_overload_functions() { } #[test] -#[ignore] // At present, bindgen generates two separate 'daft1' - // functions here, and there's not much we can do about that. +fn test_overload_single_numeric_function() { + let cxx = indoc! {" + void daft1(uint32_t) {} + void daft(float) {} + void daft(uint32_t) {} + "}; + let hdr = indoc! {" + #include + void daft1(uint32_t a); + void daft(uint32_t a); + void daft(float a); + "}; + let rs = quote! { + use ffi::ToCppString; + ffi::daft(32); + ffi::daft1(8); + ffi::daft_autocxx_overload1(3.2); + }; + run_test(cxx, hdr, rs, &["daft", "daft1"], &[]); +} + +#[test] fn test_overload_numeric_functions() { // Because bindgen deals with conflicting overloaded functions by // appending a numeric suffix, let's see if we can cope.