Hiding more namespace-body-formatting within `code_gen_utils.rs`.
As pointed out in "A Philosophy of Software Design" moving code up or
down the architectural layers can often bring related code together and
therefore help improve modularity of code. Before this CL
`common/code_gen_utils.rs` would provide
`NamespaceQualifier::format_with_cc_body` and
`cc_bindings_from_rs/bindings.rs` would group consecutive snippets from
the same namespace. After this CL, all of this is hidden as an
implementation detail of a new `format_namespace_bound_cc_tokens` API
offered by `common/code_gen_utils.rs`.
The public API of `common/code_gen_utils.rs` is slightly simpler before
this CL:
fn format_with_cc_body(&self, body: TokenStream) -> Result<TokenStream>
and slightly more complicated after this CL:
pub fn format_namespace_bound_cc_tokens(
iter: impl IntoIterator<Item = (NamespaceQualifier, TokenStream)>,
) -> Result<TokenStream>
Nevertheless, overall the modularity and readability seems improved.
The only behavior change in this CL is to insert an empty line
between different namespaces.
PiperOrigin-RevId: 495980348
diff --git a/cc_bindings_from_rs/bindings.rs b/cc_bindings_from_rs/bindings.rs
index 7526a4c..2b36bf6 100644
--- a/cc_bindings_from_rs/bindings.rs
+++ b/cc_bindings_from_rs/bindings.rs
@@ -4,7 +4,8 @@
use anyhow::{anyhow, bail, ensure, Context, Result};
use code_gen_utils::{
- format_cc_ident, format_cc_includes, make_rs_ident, CcInclude, NamespaceQualifier,
+ format_cc_ident, format_cc_includes, format_namespace_bound_cc_tokens, make_rs_ident,
+ CcInclude, NamespaceQualifier,
};
use itertools::Itertools;
use proc_macro2::{Literal, TokenStream};
@@ -963,6 +964,7 @@
let mut ordered_cc = Vec::new();
let mut rs_body = quote! {};
for local_def_id in ordered_ids.into_iter() {
+ let mod_path = FullyQualifiedName::new(tcx, local_def_id.to_def_id()).mod_path;
let MixedSnippet {
rs: inner_rs,
cc: CcSnippet {
@@ -974,7 +976,7 @@
}
} = bindings.remove(&local_def_id).unwrap();
includes.append(&mut inner_includes);
- ordered_cc.push((local_def_id, cc_tokens));
+ ordered_cc.push((mod_path, cc_tokens));
rs_body.extend(inner_rs);
}
@@ -985,35 +987,17 @@
// unique + ergonomic).
let crate_name = format_cc_ident(tcx.crate_name(LOCAL_CRATE).as_str())?;
- // Neighboring `cc` items that belong to the same namespace are put under a
- // single `namespace foo::bar::baz { #items }`. We don't just translate
- // `mod foo` => `namespace foo` in a top-down fashion, because of the
- // need to reorder the bindings of individual items (see
- // `CcPrerequisites::defs` toposort above).
- let ordered_cc = ordered_cc
- .into_iter()
- .group_by(|(local_def_id, _)| {
- FullyQualifiedName::new(tcx, local_def_id.to_def_id()).mod_path
- })
- .into_iter()
- .map(|(mod_path, items)| {
- let tokens = items.into_iter().map(|(_, tokens)| tokens).collect();
- mod_path.format_with_cc_body(tokens)
- })
- .collect::<Result<Vec<_>>>()?;
-
- // Replace `failed` ids with unsupported-item comments.
+ let includes = format_cc_includes(&includes);
+ let ordered_cc = format_namespace_bound_cc_tokens(ordered_cc)?;
let failed_cc = failed_ids.into_iter().map(|def_id| {
// TODO(b/260725687): Add test coverage for the error condition below.
format_unsupported_def(tcx, def_id, anyhow!("Definition dependency cycle")).cc.tokens
});
-
- let includes = format_cc_includes(&includes);
- let cc = ordered_cc.into_iter().chain(failed_cc).collect::<TokenStream>();
quote! {
#includes __NEWLINE__
namespace #crate_name {
- #cc
+ #ordered_cc
+ #( #failed_cc )*
}
}
};
diff --git a/common/code_gen_utils.rs b/common/code_gen_utils.rs
index aa8fdaf..1322ff6 100644
--- a/common/code_gen_utils.rs
+++ b/common/code_gen_utils.rs
@@ -72,7 +72,7 @@
Ok(quote! { #(#namespace_cc_idents::)* })
}
- pub fn format_with_cc_body(&self, body: TokenStream) -> Result<TokenStream> {
+ fn format_with_cc_body(&self, body: TokenStream) -> Result<TokenStream> {
if self.0.is_empty() {
Ok(body)
} else {
@@ -90,6 +90,58 @@
}
}
+/// `format_namespace_bound_cc_tokens` formats a sequence of namespace-bound
+/// snippets. For example, `[(ns, tokens)]` will be formatted as:
+///
+/// ```
+/// namespace ns {
+/// #tokens
+/// }
+/// ```
+///
+/// `format_namespace_bound_cc_tokens` tries to give a nice-looking output - for
+/// example it combines consecutive items that belong to the same namespace,
+/// when given `[(ns, tokens1), (ns, tokens2)]` as input:
+///
+/// ```
+/// namespace ns {
+/// #tokens1
+/// #tokens2
+/// }
+/// ```
+///
+/// `format_namespace_bound_cc_tokens` also knows that top-level items (e.g.
+/// ones where `NamespaceQualifier` doesn't contain any namespace names) should
+/// be emitted at the top-level (not nesting them under a `namespace` keyword).
+/// For example, `[(toplevel_ns, tokens)]` will be formatted as just:
+///
+/// ```
+/// #tokens
+/// ```
+pub fn format_namespace_bound_cc_tokens(
+ iter: impl IntoIterator<Item = (NamespaceQualifier, TokenStream)>,
+) -> Result<TokenStream> {
+ let mut iter = iter.into_iter().peekable();
+ let mut tokens_in_curr_ns = Vec::with_capacity(iter.size_hint().0);
+ let mut result = TokenStream::default();
+ while let Some((curr_ns, tokens)) = iter.next() {
+ tokens_in_curr_ns.push(tokens);
+
+ // Flush `tokens_in_curr_ns` when at the end of the current namespace.
+ let next_ns = iter.peek().map(|(next_ns, _)| next_ns);
+ if next_ns != Some(&curr_ns) {
+ let tokens_in_curr_ns = tokens_in_curr_ns.drain(..);
+ result.extend(curr_ns.format_with_cc_body(quote! { #(#tokens_in_curr_ns)* })?);
+ }
+
+ // Separate namespaces with a single empty line.
+ if iter.peek().is_some() {
+ result.extend(quote! { __NEWLINE__ __NEWLINE__ });
+ }
+ }
+ Ok(result)
+}
+
/// `CcInclude` represents a single `#include ...` directive in C++.
#[derive(Debug, Eq, Hash, Ord, PartialEq, PartialOrd)]
pub enum CcInclude {
@@ -514,4 +566,44 @@
},
);
}
+
+ #[test]
+ fn test_format_namespace_bound_cc_tokens() {
+ let top_level = create_namespace_qualifier_for_tests(&[]);
+ let m1 = create_namespace_qualifier_for_tests(&["m1"]);
+ let m2 = create_namespace_qualifier_for_tests(&["m2"]);
+ let input = vec![
+ (top_level.clone(), quote! { void f0a(); }),
+ (m1.clone(), quote! { void f1a(); }),
+ (m1.clone(), quote! { void f1b(); }),
+ (top_level.clone(), quote! { void f0b(); }),
+ (top_level.clone(), quote! { void f0c(); }),
+ (m2.clone(), quote! { void f2a(); }),
+ (m1.clone(), quote! { void f1c(); }),
+ (m1.clone(), quote! { void f1d(); }),
+ ];
+ assert_cc_matches!(
+ format_namespace_bound_cc_tokens(input).unwrap(),
+ quote! {
+ void f0a();
+
+ namespace m1 {
+ void f1a();
+ void f1b();
+ } // namespace m1
+
+ void f0b();
+ void f0c();
+
+ namespace m2 {
+ void f2a();
+ }
+
+ namespace m1 {
+ void f1c();
+ void f1d();
+ } // namespace m1
+ },
+ );
+ }
}
