Static methods, and deconflict various namespaces

engine/src/additional_cpp_generator.rs

@@ -200,6 +200,14 @@ impl AdditionalCppGenerator {
                     format!("{}({})", underlying_function_call, arg_list)
                 }
             },
+            FunctionWrapperPayload::StaticMethodCall(ns, ty_id, fn_id) => {
+                let underlying_function_call = ns
+                    .into_iter()
+                    .cloned()
+                    .chain([ty_id.to_string(), fn_id.to_string()].iter().cloned())
+                    .join("::");
+                format!("{}({})", underlying_function_call, arg_list)
+            }
         };
         if let Some(ret) = details.return_conversion {
             underlying_function_call =

engine/src/bridge_converter.rs

@@ -16,9 +16,11 @@ use std::fmt::Display;
 
 use crate::{
     additional_cpp_generator::AdditionalNeed,
+    bridge_name_tracker::BridgeNameTracker,
     byvalue_checker::ByValueChecker,
     foreign_mod_converter::{ForeignModConversionCallbacks, ForeignModConverter},
     namespace_organizer::{NamespaceEntries, Use},
+    rust_name_tracker::RustNameTracker,
     type_converter::TypeConverter,
     typedef_analyzer::analyze_typedef_target,
     typedef_analyzer::TypedefTarget,
@@ -132,6 +134,8 @@ impl BridgeConverter {
                     pod_requests: &self.pod_requests,
                     include_list: &self.include_list,
                     final_uses: Vec::new(),
+                    bridge_name_tracker: BridgeNameTracker::new(),
+                    rust_name_tracker: RustNameTracker::new(),
                 };
                 conversion.convert_items(items, exclude_utilities)
             }
@@ -161,6 +165,8 @@ struct BridgeConversion<'a> {
     pod_requests: &'a Vec<TypeName>,
     include_list: &'a Vec<String>,
     final_uses: Vec<Use>,
+    bridge_name_tracker: BridgeNameTracker,
+    rust_name_tracker: RustNameTracker,
 }
 
 impl<'a> BridgeConversion<'a> {
@@ -259,7 +265,7 @@ impl<'a> BridgeConversion<'a> {
                         // the contents of all bindgen 'extern "C"' mods into this
                         // one.
                     }
-                    mod_converter.convert_foreign_mod_items(items, self)?;
+                    mod_converter.convert_foreign_mod_items(items)?;
                 }
                 Item::Struct(mut s) => {
                     let tyname = TypeName::new(&ns, &s.ident.to_string());
@@ -279,11 +285,15 @@ impl<'a> BridgeConversion<'a> {
                     self.generate_type_alias(tyname, true)?;
                     output_items.push(Item::Enum(e));
                 }
-                Item::Impl(_) => {
-                    // We ignore all impl blocks generated by bindgen.
+                Item::Impl(imp) => {
+                    // We *mostly* ignore all impl blocks generated by bindgen.
                     // Methods also appear in 'extern "C"' blocks which
                     // we will convert instead. At that time we'll also construct
                     // synthetic impl blocks.
+                    // We do however record which methods were spotted, since
+                    // we have no other way of working out which functions are
+                    // static methods vs plain functions.
+                    mod_converter.convert_impl_items(imp);
                 }
                 Item::Mod(itm) => {
                     let mut new_itm = itm.clone();
@@ -314,11 +324,8 @@ impl<'a> BridgeConversion<'a> {
                 _ => return Err(ConvertError::UnexpectedItemInMod),
             }
         }
-        output_items.extend(
-            mod_converter
-                .get_impl_blocks()
-                .map(|(_, v)| Item::Impl(v)),
-        );
+        mod_converter.finished(self)?;
+        output_items.extend(mod_converter.get_impl_blocks().map(|(_, v)| Item::Impl(v)));
         let supers = std::iter::repeat(make_ident("super")).take(ns.depth() + 2);
         output_items.push(Item::Use(parse_quote! {
             #[allow(unused_imports)]
@@ -503,7 +510,7 @@ impl<'a> BridgeConversion<'a> {
                 type Kind = cxx::kind::#kind_item;
             }
         }));
-        self.add_use(tyname.get_namespace(), &final_ident);
+        self.add_use(tyname.get_namespace(), &final_ident, None);
         self.type_converter.push(tyname);
         self.idents_found.push(final_ident);
         Ok(())
@@ -525,10 +532,11 @@ impl<'a> BridgeConversion<'a> {
             .collect()
     }
 
-    fn add_use(&mut self, ns: &Namespace, id: &Ident) {
+    fn add_use(&mut self, ns: &Namespace, id: &Ident, alias: Option<Ident>) {
         self.final_uses.push(Use {
             ns: ns.clone(),
             id: id.clone(),
+            alias,
         });
     }
 
@@ -542,7 +550,7 @@ impl<'a> BridgeConversion<'a> {
         self.extern_c_mod_items.push(ForeignItem::Fn(parse_quote!(
             fn make_string(str_: &str) -> UniquePtr<CxxString>;
         )));
-        self.add_use(&Namespace::new(), &make_ident("make_string"));
+        self.add_use(&Namespace::new(), &make_ident("make_string"), None);
         self.additional_cpp_needs
             .push(AdditionalNeed::MakeStringConstructor);
     }
@@ -557,9 +565,15 @@ impl<'a> BridgeConversion<'a> {
     fn append_child_namespace(ns_entries: &NamespaceEntries, output_items: &mut Vec<Item>) {
         for item in ns_entries.entries() {
             let id = &item.id;
-            output_items.push(Item::Use(parse_quote!(
-                pub use cxxbridge :: #id;
-            )));
+            let stmt = match &item.alias {
+                Some(alias) => parse_quote!(
+                    pub use cxxbridge :: #id as #alias;
+                ),
+                None => parse_quote!(
+                    pub use cxxbridge :: #id;
+                ),
+            };
+            output_items.push(Item::Use(stmt));
         }
         for (child_name, child_ns_entries) in ns_entries.children() {
             let child_id = make_ident(child_name);
@@ -590,11 +604,25 @@ impl<'a> ForeignModConversionCallbacks for BridgeConversion<'a> {
         self.byvalue_checker.is_pod(ty)
     }
 
-    fn add_use(&mut self, ns: &Namespace, id: &Ident) {
-        self.add_use(ns, id);
+    fn add_use(&mut self, ns: &Namespace, id: &Ident, alias: Option<Ident>) {
+        self.add_use(ns, id, alias);
     }
 
     fn push_extern_c_mod_item(&mut self, item: ForeignItem) {
         self.extern_c_mod_items.push(item);
     }
+
+    fn get_cxx_bridge_name(
+        &mut self,
+        type_name: Option<&str>,
+        found_name: &str,
+        ns: &Namespace,
+    ) -> String {
+        self.bridge_name_tracker
+            .get_unique_cxx_bridge_name(type_name, found_name, ns)
+    }
+
+    fn ok_to_use_rust_name(&mut self, rust_name: &str) -> bool {
+        self.rust_name_tracker.ok_to_use_rust_name(rust_name)
+    }
 }

engine/src/bridge_name_tracker.rs

@@ -0,0 +1,159 @@
+// 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::Namespace;
+use itertools::Itertools;
+use std::collections::HashMap;
+
+/// Type to generate unique names for entries in the [cxx::bridge]
+/// mod which is flat.
+///
+/// # All about the names involved in autocxx
+///
+/// A given function may have many, many names. From C++ to Rust...
+///
+/// 1. The actual C++ name. Fixed, obviously.
+/// 2. The name reported by bindgen. Not always the same, as
+///    bindgen may generate a different name for different overloads.
+///    See `overload_tracker` for the conversion here.
+/// 3. The name in the cxx::bridge mod. This is a flat namespace,
+///    and it's the responsibility of this type to generate a
+///    suitable name here.
+///    If this is different from the C++ name in (1), we'll
+///    add a #[cxx_name] attribute to the cxx::bridge declaration.
+/// 4. The name we wish to present to Rust users. Again, we have
+///    to take stock of the fact Rust doesn't support overloading
+///    so two different functions called 'get' may end up being
+///    'get' and 'get1'. Yet, this may not be the same as the
+///    bindgen name in (2) because we wish to generate such number
+///    sequences on a per-type basis, whilst bindgen does it globally.
+///
+/// This fourth name, the final Rust user name, may be finagled
+/// into place in three different ways:
+/// 1. For methods, we generate an 'impl' block where the
+///    method name is the intended name, but it calls the
+///    cxxbridge name.
+/// 2. For simple functions, we use the #[rust_name] attribute.
+/// 3. Occasionally, there can be conflicts in the rust_name
+///    namespace (if there are two identically named functions
+///    in different C++ namespaces). That's detected by
+///    rust_name_tracker.rs. In such a case, we'll omit the
+///    #[rust_name] attribute and instead generate a 'use A = B;'
+///    declaration in the mod which we generate for the output
+///    namespace.
+#[derive(Default)]
+pub(crate) struct BridgeNameTracker {
+    next_cxx_bridge_name_for_prefix: HashMap<String, usize>,
+}
+
+impl BridgeNameTracker {
+    pub(crate) fn new() -> Self {
+        Self::default()
+    }
+
+    /// Figure out the least confusing unique name for this function 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
+    /// better Rust-side names so it's really just a matter of how it shows up
+    /// in stack traces and for our own sanity as maintainers of autocxx.
+    /// This may become unnecessary if and when cxx supports hierarchic
+    /// namespace mods.
+    /// There is a slight advantage in using the same name as either the
+    /// Rust or C++ symbols as it reduces the amount of rebinding required
+    /// by means of cxx_name or rust_name attributes. In extreme cases it
+    /// may even allow us to remove whole impl blocks. So we may wish to try
+    /// harder to find better names in future instead of always prepending
+    /// the namespace.
+    pub(crate) fn get_unique_cxx_bridge_name(
+        &mut self,
+        type_name: Option<&str>,
+        found_name: &str,
+        ns: &Namespace,
+    ) -> String {
+        let count = self
+            .next_cxx_bridge_name_for_prefix
+            .entry(found_name.to_string())
+            .or_default();
+        if *count == 0 {
+            // Oh, good, we can use this function name as-is.
+            *count += 1;
+            return found_name.to_string();
+        }
+        let prefix = ns
+            .iter()
+            .cloned()
+            .chain(type_name.iter().map(|x| x.to_string()))
+            .chain(std::iter::once(found_name.to_string()))
+            .join("_");
+        let count = self
+            .next_cxx_bridge_name_for_prefix
+            .entry(prefix.clone())
+            .or_default();
+        if *count == 0 {
+            *count += 1;
+            prefix
+        } else {
+            let r = format!("{}_autocxx{}", prefix, count);
+            *count += 1;
+            r
+        }
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use crate::types::Namespace;
+
+    use super::BridgeNameTracker;
+
+    #[test]
+    fn test() {
+        let mut bnt = BridgeNameTracker::new();
+        let ns_root = Namespace::new();
+        let ns_a = Namespace::from_user_input("A");
+        let ns_b = Namespace::from_user_input("B");
+        let ns_ab = Namespace::from_user_input("A::B");
+        assert_eq!(bnt.get_unique_cxx_bridge_name(None, "do", &ns_root), "do");
+        assert_eq!(
+            bnt.get_unique_cxx_bridge_name(None, "do", &ns_root),
+            "do_autocxx1"
+        );
+        assert_eq!(bnt.get_unique_cxx_bridge_name(None, "did", &ns_root), "did");
+        assert_eq!(
+            bnt.get_unique_cxx_bridge_name(Some("ty1"), "do", &ns_root),
+            "ty1_do"
+        );
+        assert_eq!(
+            bnt.get_unique_cxx_bridge_name(Some("ty1"), "do", &ns_root),
+            "ty1_do_autocxx1"
+        );
+        assert_eq!(
+            bnt.get_unique_cxx_bridge_name(Some("ty2"), "do", &ns_root),
+            "ty2_do"
+        );
+        assert_eq!(
+            bnt.get_unique_cxx_bridge_name(Some("ty"), "do", &ns_a),
+            "A_ty_do"
+        );
+        assert_eq!(
+            bnt.get_unique_cxx_bridge_name(Some("ty"), "do", &ns_b),
+            "B_ty_do"
+        );
+        assert_eq!(
+            bnt.get_unique_cxx_bridge_name(Some("ty"), "do", &ns_ab),
+            "A_B_ty_do"
+        );
+    }
+}