Code review comments on namespace work.

gen/src/namespace_organizer.rs

@@ -2,39 +2,49 @@ use crate::syntax::Api;
 use proc_macro2::Ident;
 use std::collections::BTreeMap;
 
-pub(crate) struct NamespaceEntries<'a> {
-    pub(crate) entries: Vec<&'a Api>,
-    pub(crate) children: BTreeMap<&'a Ident, NamespaceEntries<'a>>,
+pub struct NamespaceEntries<'a> {
+    entries: Vec<&'a Api>,
+    children: BTreeMap<&'a Ident, NamespaceEntries<'a>>,
 }
 
-pub(crate) fn sort_by_namespace(apis: &[Api]) -> NamespaceEntries {
-    let api_refs = apis.iter().collect::<Vec<_>>();
-    sort_by_inner_namespace(api_refs, 0)
-}
+impl<'a> NamespaceEntries<'a> {
+    pub fn new(apis: &'a [Api]) -> Self {
+        let api_refs = apis.iter().collect::<Vec<_>>();
+        Self::sort_by_inner_namespace(api_refs, 0)
+    }
 
-fn sort_by_inner_namespace(apis: Vec<&Api>, depth: usize) -> NamespaceEntries {
-    let mut root = NamespaceEntries {
-        entries: Vec::new(),
-        children: BTreeMap::new(),
-    };
+    pub fn entries(&self) -> &[&'a Api] {
+        &self.entries
+    }
 
-    let mut kids_by_child_ns = BTreeMap::new();
-    for api in apis {
-        if let Some(ns) = api.get_namespace() {
-            let first_ns_elem = 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(Vec::new());
-                list.push(api);
-                continue;
+    pub fn children(&self) -> impl Iterator<Item = (&&Ident, &NamespaceEntries)> {
+        self.children.iter()
+    }
+
+    fn sort_by_inner_namespace(apis: Vec<&'a Api>, 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 {
+            if let Some(ns) = api.get_namespace() {
+                let first_ns_elem = 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(Vec::new());
+                    list.push(api);
+                    continue;
+                }
             }
+            root.entries.push(api);
         }
-        root.entries.push(api);
-    }
 
-    for (k, v) in kids_by_child_ns.into_iter() {
-        root.children
-            .insert(k, sort_by_inner_namespace(v, depth + 1));
-    }
+        for (k, v) in kids_by_child_ns.into_iter() {
+            root.children
+                .insert(k, Self::sort_by_inner_namespace(v, depth + 1));
+        }
 
-    root
+        root
+    }
 }

gen/src/write.rs

@@ -1,4 +1,4 @@
-use crate::gen::namespace_organizer::{sort_by_namespace, NamespaceEntries};
+use crate::gen::namespace_organizer::NamespaceEntries;
 use crate::gen::out::OutFile;
 use crate::gen::{include, Opt};
 use crate::syntax::atom::Atom::{self, *};
@@ -30,7 +30,7 @@ pub(super) fn gen(apis: &[Api], types: &Types, opt: &Opt, header: bool) -> OutFi
 
     out.next_section();
 
-    let apis_by_namespace = sort_by_namespace(apis);
+    let apis_by_namespace = NamespaceEntries::new(apis);
 
     gen_namespace_contents(&apis_by_namespace, types, opt, header, out);
 
@@ -51,10 +51,10 @@ fn gen_namespace_contents(
     header: bool,
     out: &mut OutFile,
 ) {
-    let apis = &ns_entries.entries;
+    let apis = ns_entries.entries();
 
     out.next_section();
-    for api in apis {
+    for api in apis.into_iter() {
         match api {
             Api::Struct(strct) => write_struct_decl(out, &strct.ident.cxx.ident),
             Api::CxxType(ety) => write_struct_using(out, &ety.ident.cxx),
@@ -64,7 +64,7 @@ fn gen_namespace_contents(
     }
 
     let mut methods_for_type = HashMap::new();
-    for api in apis {
+    for api in apis.into_iter() {
         if let Api::RustFunction(efn) = api {
             if let Some(receiver) = &efn.sig.receiver {
                 methods_for_type
@@ -134,7 +134,7 @@ fn gen_namespace_contents(
 
     out.next_section();
 
-    for (child_ns, child_ns_entries) in &ns_entries.children {
+    for (child_ns, child_ns_entries) in ns_entries.children() {
         writeln!(out, "namespace {} {{", child_ns);
         gen_namespace_contents(&child_ns_entries, types, opt, header, out);
         writeln!(out, "}} // namespace {}", child_ns);

syntax/impls.rs

@@ -315,10 +315,6 @@ impl ResolvableName {
         Self { rust: ident }
     }
 
-    pub fn from_pair(pair: Pair) -> Self {
-        Self { rust: pair.rust }
-    }
-
     pub fn make_self(span: Span) -> Self {
         Self {
             rust: Token![Self](span).into(),
@@ -357,9 +353,7 @@ impl CppName {
         Self { ns, ident }
     }
 
-    fn iter_all_segments(
-        &self,
-    ) -> std::iter::Chain<std::slice::Iter<Ident>, std::iter::Once<&Ident>> {
+    fn iter_all_segments(&self) -> impl Iterator<Item = &Ident> {
         self.ns.iter().chain(std::iter::once(&self.ident))
     }
 

syntax/parse.rs

@@ -245,7 +245,7 @@ fn parse_foreign_mod(
             if let Api::CxxFunction(efn) | Api::RustFunction(efn) = item {
                 if let Some(receiver) = &mut efn.receiver {
                     if receiver.ty.is_self() {
-                        receiver.ty = ResolvableName::from_pair(single_type.clone());
+                        receiver.ty = ResolvableName::new(single_type.rust.clone());
                     }
                 }
             }