Namespaced output

README.md

@@ -11,20 +11,22 @@ The intention is that it has all the fluent safety from [cxx](https://github.com
 It's intended that eventually this exposes a single procedural macro, something like this:
 
 ```cpp
-class Bob {
-public:
-    Bob(std::string name);
-    ...
-    void do_a_thing();
+namespace base {
+  class Bob {
+  public:
+      Bob(std::string name);
+      ...
+      void do_a_thing();
+  };
 }
 ```
 
 ```rust
-use autocxx::include_cxx;
+use autocxx::include_cpp;
 
-include_cxx!(
-    Header("base/bob.h"),
-    Allow("Bob"),
+include_cpp!(
+    #include "base/bob.h"
+    allow("Bob")
 )
 
 let a = ffi::base::Bob::make_unique("hello".into());
@@ -83,7 +85,7 @@ The project also contains test code which does this end-to-end, for all sorts of
 | Destructors | Works via cxx `UniquePtr` already |
 | Inline functions | Works |
 | Construction of std::unique_ptr<std::string> in Rust | Works |
-| Namespaces | Works, but generated code flat in Rust |
+| Namespaces | Works, but a known limitation |
 | Field access to opaque objects via UniquePtr | - |
 | Plain-old-data structs containing opaque fields | Impossible by design, but may not be ergonomic so may need more thought |
 | Reference counting | - |

engine/src/additional_cpp_generator.rs

@@ -140,7 +140,7 @@ pub(crate) struct ByValueWrapper {
 pub(crate) enum AdditionalNeed {
     MakeStringConstructor,
     MakeUnique(TypeName, Vec<TypeName>),
-    ByValueWrapper(ByValueWrapper),
+    ByValueWrapper(Box<ByValueWrapper>),
 }
 
 #[derive(Ord, PartialOrd, Eq, PartialEq, Clone, Hash)]
@@ -207,7 +207,7 @@ impl AdditionalCppGenerator {
                 AdditionalNeed::MakeStringConstructor => self.generate_string_constructor(),
                 AdditionalNeed::MakeUnique(ty, args) => self.generate_make_unique(&ty, &args),
                 AdditionalNeed::ByValueWrapper(by_value_wrapper) => {
-                    self.generate_by_value_wrapper(by_value_wrapper)
+                    self.generate_by_value_wrapper(*by_value_wrapper)
                 }
             }
         }

engine/src/bridge_converter.rs

@@ -12,7 +12,6 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-use crate::types::TypeName;
 use crate::{
     additional_cpp_generator::{AdditionalNeed, ArgumentConversion, ByValueWrapper},
     known_types::{replace_type_path_without_arguments, should_dereference_in_cpp},
@@ -22,6 +21,10 @@ use crate::{
     types::Namespace,
     unqualify::{unqualify_params, unqualify_ret_type},
 };
+use crate::{
+    namespace_organizer::{NamespaceEntries, Use},
+    types::TypeName,
+};
 use proc_macro2::{Span, TokenStream as TokenStream2, TokenTree};
 use quote::quote;
 use syn::parse::Parser;
@@ -110,6 +113,7 @@ impl BridgeConverter {
                     byvalue_checker: ByValueChecker::new(),
                     pod_requests: &self.pod_requests,
                     include_list: &self.include_list,
+                    final_uses: Vec::new(),
                 };
                 conversion.convert_items(items, exclude_utilities)
             }
@@ -148,6 +152,7 @@ struct BridgeConversion<'a> {
     byvalue_checker: ByValueChecker,
     pod_requests: &'a Vec<TypeName>,
     include_list: &'a Vec<String>,
+    final_uses: Vec<Use>,
 }
 
 impl<'a> BridgeConversion<'a> {
@@ -202,6 +207,7 @@ impl<'a> BridgeConversion<'a> {
                 #(#bindgen_root_items)*
             }
         })];
+        self.generate_final_use_statements();
         self.all_items.push(Item::Mod(self.bindgen_mod));
         let bridge_items = &self.bridge_items;
         self.all_items.push(Item::Mod(parse_quote! {
@@ -400,7 +406,7 @@ impl<'a> BridgeConversion<'a> {
                 type Kind = cxx::kind::#kind_item;
             }
         }));
-        self.add_use(&final_ident);
+        self.add_use(tyname.get_namespace(), &final_ident);
         self.types_found.push(final_ident);
         Ok(())
     }
@@ -421,10 +427,11 @@ impl<'a> BridgeConversion<'a> {
             .collect()
     }
 
-    fn add_use(&mut self, id: &Ident) {
-        self.all_items.push(Item::Use(parse_quote!(
-            pub use cxxbridge:: #id;
-        )));
+    fn add_use(&mut self, ns: &Namespace, id: &Ident) {
+        self.final_uses.push(Use {
+            ns: ns.clone(),
+            id: id.clone(),
+        });
     }
 
     /// Adds items which we always add, cos they're useful.
@@ -437,7 +444,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(&make_ident("make_string"));
+        self.add_use(&Namespace::new(), &make_ident("make_string"));
         self.additional_cpp_needs
             .push(AdditionalNeed::MakeStringConstructor);
     }
@@ -471,7 +478,7 @@ impl<'a> BridgeConversion<'a> {
             &format!("{}_make_unique", ty.to_string()),
             Span::call_site(),
         );
-        self.add_use(&call_name);
+        self.add_use(&ty.get_namespace(), &call_name);
         self.extern_c_mod_items.push(ForeignItem::Fn(parse_quote! {
             pub fn #call_name ( #rs_args ) -> UniquePtr< #self_ty >;
         }));
@@ -584,12 +591,12 @@ impl<'a> BridgeConversion<'a> {
             // First give instructions to generate the additional C++.
             let cpp_construction_ident = cxxbridge_name;
             cxxbridge_name = make_ident(&format!("{}_up_wrapper", rust_name));
-            let a = AdditionalNeed::ByValueWrapper(ByValueWrapper {
+            let a = AdditionalNeed::ByValueWrapper(Box::new(ByValueWrapper {
                 id: cpp_construction_ident,
                 return_conversion: ret_type_conversion.clone(),
                 argument_conversion: param_details.iter().map(|d| d.conversion.clone()).collect(),
                 is_a_method,
-            });
+            }));
             self.additional_cpp_needs.push(a);
             // Now modify the cxx::bridge entry we're going to make.
             if let Some(conversion) = ret_type_conversion {
@@ -648,7 +655,7 @@ impl<'a> BridgeConversion<'a> {
             #vis fn #cxxbridge_name ( #params ) #ret_type;
         )));
         if !is_a_method || wrapper_function_needed {
-            self.add_use(&rust_name_ident);
+            self.add_use(&ns, &rust_name_ident);
         }
         Ok(())
     }
@@ -853,6 +860,35 @@ impl<'a> BridgeConversion<'a> {
         }
         new_pun
     }
+
+    /// Generate lots of 'use' statements to pull cxxbridge items into the output
+    /// mod hierarchy according to C++ namespaces.
+    fn generate_final_use_statements(&mut self) {
+        let ns_entries = NamespaceEntries::new(&self.final_uses);
+        Self::append_child_namespace(&ns_entries, &mut self.all_items);
+    }
+
+    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;
+            )));
+        }
+        for (child_name, child_ns_entries) in ns_entries.children() {
+            let child_id = make_ident(child_name);
+            let mut new_mod: ItemMod = parse_quote!(
+                pub mod #child_id {
+                    use super::cxxbridge;
+                }
+            );
+            Self::append_child_namespace(
+                child_ns_entries,
+                &mut new_mod.content.as_mut().unwrap().1,
+            );
+            output_items.push(Item::Mod(new_mod));
+        }
+    }
 }
 
 struct ArgumentAnalysis {

engine/src/integration_tests.rs

@@ -1855,7 +1855,7 @@ fn test_ns_take_struct() {
         uint32_t take_bob(A::B::Bob a);
     "};
     let rs = quote! {
-        let a = ffi::Bob { a: 12, b: 13 };
+        let a = ffi::A::B::Bob { a: 12, b: 13 };
         assert_eq!(ffi::take_bob(a), 12);
     };
     run_test(cxx, hdr, rs, &["take_bob"], &["A::B::Bob"]);
@@ -1887,7 +1887,7 @@ fn test_ns_func() {
         }
     "};
     let rs = quote! {
-        assert_eq!(ffi::give_bob().b, 4);
+        assert_eq!(ffi::C::give_bob().b, 4);
     };
     run_test(cxx, hdr, rs, &["C::give_bob"], &["A::B::Bob"]);
 }

engine/src/lib.rs

@@ -16,6 +16,7 @@ mod additional_cpp_generator;
 mod bridge_converter;
 mod byvalue_checker;
 mod known_types;
+mod namespace_organizer;
 mod parse;
 mod rust_pretty_printer;
 mod types;
@@ -84,6 +85,7 @@ pub enum CppInclusion {
     Header(String),
 }
 
+#[allow(clippy::large_enum_variant)] // because this is only used once
 enum State {
     NotGenerated,
     ParseOnly,

engine/src/namespace_organizer.rs

@@ -0,0 +1,139 @@
+// 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 proc_macro2::Ident;
+use std::collections::BTreeMap;
+
+use crate::types::Namespace;
+
+pub(crate) struct Use {
+    pub(crate) ns: Namespace,
+    pub(crate) id: Ident,
+}
+
+pub struct NamespaceEntries<'a> {
+    entries: Vec<&'a Use>,
+    children: BTreeMap<&'a String, NamespaceEntries<'a>>,
+}
+
+impl<'a> NamespaceEntries<'a> {
+    pub(crate) fn new(apis: &'a [Use]) -> Self {
+        let api_refs = apis.iter().collect::<Vec<_>>();
+        Self::sort_by_inner_namespace(api_refs, 0)
+    }
+
+    pub(crate) fn entries(&self) -> &[&'a Use] {
+        &self.entries
+    }
+
+    pub(crate) fn children(&self) -> impl Iterator<Item = (&&String, &NamespaceEntries)> {
+        self.children.iter()
+    }
+
+    fn sort_by_inner_namespace(apis: Vec<&'a Use>, depth: usize) -> Self {
+        let mut root = NamespaceEntries {
+            entries: Vec::new(),
+            children: BTreeMap::new(),
+        };
+
+        let mut kids_by_child_ns = BTreeMap::new();
+        for api in apis {
+            let first_ns_elem = api.ns.iter().nth(depth);
+            if let Some(first_ns_elem) = first_ns_elem {
+                let list = kids_by_child_ns
+                    .entry(first_ns_elem)
+                    .or_insert_with(Vec::new);
+                list.push(api);
+                continue;
+            }
+            root.entries.push(api);
+        }
+
+        for (k, v) in kids_by_child_ns.into_iter() {
+            root.children
+                .insert(k, Self::sort_by_inner_namespace(v, depth + 1));
+        }
+
+        root
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::NamespaceEntries;
+    use super::Use;
+    use crate::types::Namespace;
+    use proc_macro2::{Ident, Span};
+
+    #[test]
+    fn test_ns_entries_sort() {
+        let entries = vec![
+            make_api(None, "C"),
+            make_api(None, "A"),
+            make_api(Some("G"), "E"),
+            make_api(Some("D"), "F"),
+            make_api(Some("G"), "H"),
+            make_api(Some("D::K"), "L"),
+            make_api(Some("D::K"), "M"),
+            make_api(None, "B"),
+            make_api(Some("D"), "I"),
+            make_api(Some("D"), "J"),
+        ];
+        let ns = NamespaceEntries::new(&entries);
+        let root_entries = ns.entries();
+        assert_eq!(root_entries.len(), 3);
+        assert_ident(root_entries[0], "C");
+        assert_ident(root_entries[1], "A");
+        assert_ident(root_entries[2], "B");
+        let mut kids = ns.children();
+        let (d_id, d_nse) = kids.next().unwrap();
+        assert_eq!(d_id.to_string(), "D");
+        let (g_id, g_nse) = kids.next().unwrap();
+        assert_eq!(g_id.to_string(), "G");
+        assert!(kids.next().is_none());
+        let d_nse_entries = d_nse.entries();
+        assert_eq!(d_nse_entries.len(), 3);
+        assert_ident(d_nse_entries[0], "F");
+        assert_ident(d_nse_entries[1], "I");
+        assert_ident(d_nse_entries[2], "J");
+        let g_nse_entries = g_nse.entries();
+        assert_eq!(g_nse_entries.len(), 2);
+        assert_ident(g_nse_entries[0], "E");
+        assert_ident(g_nse_entries[1], "H");
+        let mut g_kids = g_nse.children();
+        assert!(g_kids.next().is_none());
+        let mut d_kids = d_nse.children();
+        let (k_id, k_nse) = d_kids.next().unwrap();
+        assert_eq!(k_id.to_string(), "K");
+        let k_nse_entries = k_nse.entries();
+        assert_eq!(k_nse_entries.len(), 2);
+        assert_ident(k_nse_entries[0], "L");
+        assert_ident(k_nse_entries[1], "M");
+    }
+
+    fn assert_ident(api: &Use, expected: &str) {
+        assert_eq!(api.id.to_string(), expected);
+    }
+
+    fn make_api(ns: Option<&str>, id: &str) -> Use {
+        let ns = match ns {
+            Some(st) => Namespace::from_user_input(st),
+            None => Namespace::new(),
+        };
+        Use {
+            ns,
+            id: Ident::new(id, Span::call_site()),
+        }
+    }
+}