Merge `api` and `cc_impl` fields of `BindingsSnippet`.
Before this CL, (optional) thunk declarations would go into `cc_impl`
and API functions would go into the `api` field of `BindingsSnippet`.
`cc_impl` would be emitted in a separate `__crubit_internal` namespace
that would go before `api` in the generated `...cc_api.h` file. This
scheme aesthetically separates implementation details (thunk
declarations in the `__crubit_internal` namespace) from the real API.
Unfortunately, this scheme will break down in the future - once
`struct`s can be used as function parameters, because `stucts`
definitions are part of the API, but thunk declarations still need to
refer to them. Therefore, after this CL, `cc_impl` and `api` fields of
`BindingsSnippet` are merged into a single field.
This CL also performs additional refactorings in the related code:
- The `api` field is renamed to `cc`
- The `rs_impl` field is renamed to `rs`
- `BindingsSnippet` is renamed to `MixedSnippet` (since now it contains
`cc` and `rs` fields - a mix of C++ and Rust).
- Instead of having a separate `include` and `cc` fields, we A)
remove the `include`, B) change the type of the `cc` field from
`TokenStream` to `CcSnippet` (which internally already contains
`includes` and a `TokenStream`).
PiperOrigin-RevId: 489588797
diff --git a/cc_bindings_from_rs/bindings.rs b/cc_bindings_from_rs/bindings.rs
index f6d940e..fa6d2a0 100644
--- a/cc_bindings_from_rs/bindings.rs
+++ b/cc_bindings_from_rs/bindings.rs
@@ -78,7 +78,7 @@
Ok(query_context.peek_mut().enter(f))
}
-#[derive(Debug)]
+#[derive(Debug, Default)]
struct CcSnippet {
snippet: TokenStream,
@@ -284,42 +284,18 @@
})
}
-#[derive(Debug)]
-struct BindingsSnippet {
- /// `#include`s that go at the top of the generated `..._cc_api.h` file.
- includes: BTreeSet<CcInclude>,
-
- /// Public API of the bindings in the generated `..._cc_api.h` file.
- api: TokenStream,
-
- /// Internal implementation details for `..._cc_api.h` file (e.g.
- /// declarations of Rust thunks).
- cc_impl: Option<TokenStream>,
-
- /// Rust implementation of the bindings:
- /// - Definitions of FFI-friendly `extern "C"` thunks that C++ can call
- /// into.
- /// - Static / `const` assrtions about layout of Rust structs
- rs_impl: Option<TokenStream>,
+#[derive(Debug, Default)]
+struct MixedSnippet {
+ cc: CcSnippet,
+ rs: Option<TokenStream>,
}
-impl BindingsSnippet {
- fn new() -> Self {
- Self { includes: BTreeSet::new(), api: quote! {}, cc_impl: None, rs_impl: None }
- }
-}
-
-impl AddAssign for BindingsSnippet {
+impl AddAssign for MixedSnippet {
fn add_assign(&mut self, rhs: Self) {
- let Self {
- includes: mut rhs_includes,
- api: rhs_api,
- cc_impl: rhs_cc_impl,
- rs_impl: rhs_rs_impl,
- } = rhs;
+ let Self { cc: mut rhs_cc, rs: rhs_rs } = rhs;
- self.includes.append(&mut rhs_includes);
- self.api.extend(rhs_api);
+ self.cc.includes.append(&mut rhs_cc.includes);
+ self.cc.snippet.extend(rhs_cc.snippet);
fn concat_optional_tokens(
lhs: Option<TokenStream>,
@@ -335,13 +311,12 @@
}
}
}
- self.cc_impl = concat_optional_tokens(self.cc_impl.take(), rhs_cc_impl);
- self.rs_impl = concat_optional_tokens(self.rs_impl.take(), rhs_rs_impl);
+ self.rs = concat_optional_tokens(self.rs.take(), rhs_rs);
}
}
-impl Add for BindingsSnippet {
- type Output = BindingsSnippet;
+impl Add for MixedSnippet {
+ type Output = MixedSnippet;
fn add(mut self, rhs: Self) -> Self {
self += rhs;
@@ -349,9 +324,9 @@
}
}
-impl Sum for BindingsSnippet {
+impl Sum for MixedSnippet {
fn sum<I: Iterator<Item = Self>>(iter: I) -> Self {
- iter.fold(BindingsSnippet::new(), Add::add)
+ iter.fold(Default::default(), Add::add)
}
}
@@ -362,7 +337,7 @@
/// - doesn't identify a function,
/// - has generic parameters of any kind - lifetime parameters (see also
/// b/258235219), type parameters, or const parameters.
-fn format_fn(tcx: TyCtxt, def_id: LocalDefId) -> Result<BindingsSnippet> {
+fn format_fn(tcx: TyCtxt, def_id: LocalDefId) -> Result<MixedSnippet> {
let def_id: DefId = def_id.to_def_id(); // Convert LocalDefId to DefId.
let item_name = tcx.item_name(def_id);
@@ -422,9 +397,7 @@
};
let mut includes = BTreeSet::new();
- let api: TokenStream;
- let cc_impl: Option<TokenStream>;
- {
+ let cc_snippet = {
let ret_type = format_ret_ty_for_cc(sig.output())
.context("Error formatting function return type")?
.into_tokens(&mut includes);
@@ -450,30 +423,29 @@
})
.collect::<Result<Vec<_>>>()?;
if item_name.as_str() == symbol_name.name {
- api = quote! {
+ quote! {
extern "C" #ret_type #fn_name (
#( #arg_types #arg_names ),*
);
- };
- cc_impl = None;
+ }
} else {
let exported_name =
format_cc_ident(symbol_name.name).context("Error formatting exported name")?;
- api = quote! {
+ quote! {
+ namespace __crubit_internal {
+ extern "C" #ret_type #exported_name (
+ #( #arg_types #arg_names ),*
+ );
+ } // namespace __crubit_internal
inline #ret_type #fn_name (
#( #arg_types #arg_names ),* ) {
- return :: __crubit_internal :: #exported_name( #( #arg_names ),* );
+ return __crubit_internal :: #exported_name( #( #arg_names ),* );
}
- };
- cc_impl = Some(quote! {
- extern "C" #ret_type #exported_name (
- #( #arg_types #arg_names ),*
- );
- });
+ }
}
- }
+ };
- let rs_impl = if !needs_thunk {
+ let rs_snippet = if !needs_thunk {
None
} else {
let crate_name = make_rs_ident(tcx.crate_name(LOCAL_CRATE).as_str());
@@ -503,14 +475,14 @@
}
})
};
- Ok(BindingsSnippet { includes, api, cc_impl, rs_impl })
+ Ok(MixedSnippet { cc: CcSnippet { includes, snippet: cc_snippet }, rs: rs_snippet })
}
/// Formats a Rust item idenfied by `def_id`.
///
/// Will panic if `def_id` is invalid (i.e. doesn't identify a Rust node or
/// item).
-fn format_def(tcx: TyCtxt, def_id: LocalDefId) -> Result<BindingsSnippet> {
+fn format_def(tcx: TyCtxt, def_id: LocalDefId) -> Result<MixedSnippet> {
match tcx.hir().get_by_def_id(def_id) {
Node::Item(item) => match item {
Item { kind: ItemKind::Fn(_, generics, _) |
@@ -538,7 +510,7 @@
tcx: TyCtxt,
local_def_id: LocalDefId,
err: anyhow::Error,
-) -> BindingsSnippet {
+) -> MixedSnippet {
let span = tcx.sess().source_map().span_to_embeddable_string(tcx.def_span(local_def_id));
let name = tcx.def_path_str(local_def_id.to_def_id());
@@ -547,12 +519,12 @@
let msg = format!("Error generating bindings for `{name}` defined at {span}: {err:#}");
let comment = quote! { __NEWLINE__ __NEWLINE__ __COMMENT__ #msg __NEWLINE__ };
- BindingsSnippet { api: comment, ..BindingsSnippet::new() }
+ MixedSnippet { cc: CcSnippet { snippet: comment, ..Default::default() }, rs: None }
}
/// Formats all public items from the Rust crate being compiled.
fn format_crate(tcx: TyCtxt) -> Result<GeneratedBindings> {
- let snippets: BindingsSnippet = tcx
+ let MixedSnippet{ cc, rs } = tcx
.hir()
.items()
.filter_map(|item_id| {
@@ -568,44 +540,30 @@
})
.sum();
- let includes = format_cc_includes(&snippets.includes);
- let api = {
+ let h_body = {
// TODO(b/254690602): Decide whether using `#crate_name` as the name of the
// top-level namespace is okay (e.g. investigate if this name is globally
// unique + ergonomic).
let crate_name = format_cc_ident(tcx.crate_name(LOCAL_CRATE).as_str())?;
- let api_body = &snippets.api;
+
+ let CcSnippet { includes, snippet } = cc;
+ let includes = format_cc_includes(&includes);
quote! {
+ #includes __NEWLINE__
namespace #crate_name {
- #api_body
+ #snippet
}
}
};
- let cc_impl = {
- match snippets.cc_impl {
- None => quote! {},
- Some(details_body) => quote! {
- namespace __crubit_internal {
- #details_body
- }
- __NEWLINE__
- },
- }
- };
- let h_body = quote! {
- #includes __NEWLINE__
- #cc_impl
- #api
- };
- let rs_body = snippets.rs_impl.unwrap_or_else(|| quote! {});
+ let rs_body = rs.unwrap_or_else(|| quote! {});
Ok(GeneratedBindings { h_body, rs_body })
}
#[cfg(test)]
pub mod tests {
use super::{
- format_def, format_ret_ty_for_cc, format_ty_for_cc, format_ty_for_rs, BindingsSnippet,
- GeneratedBindings,
+ format_def, format_ret_ty_for_cc, format_ty_for_cc, format_ty_for_rs, GeneratedBindings,
+ MixedSnippet,
};
use anyhow::Result;
@@ -730,8 +688,8 @@
#[test]
fn test_generated_bindings_fn_export_name() {
- // Coverage of how `BindingsSnippet::cc_impl` are propagated when there are no
- // `BindingsSnippet::rs_impl` (e.g. no Rust thunks are needed).
+ // Coverage of how `MixedSnippet::cc` are propagated when there is no
+ // `MixedSnippet::rs` (e.g. no Rust thunks are needed).
let test_src = r#"
#[export_name = "export_name"]
pub extern "C" fn public_function(x: f64, y: f64) -> f64 { x + y }
@@ -741,12 +699,12 @@
assert_cc_matches!(
bindings.h_body,
quote! {
- namespace __crubit_internal {
- extern "C" double export_name(double x, double y);
- }
namespace rust_out {
+ namespace __crubit_internal {
+ extern "C" double export_name(double x, double y);
+ }
inline double public_function(double x, double y) {
- return ::__crubit_internal::export_name(x, y);
+ return __crubit_internal::export_name(x, y);
}
}
}
@@ -865,11 +823,10 @@
"#;
test_format_def(test_src, "public_function", |result| {
let result = result.expect("Test expects success here");
- assert!(result.includes.is_empty());
- assert!(result.cc_impl.is_none());
- assert!(result.rs_impl.is_none());
+ assert!(result.cc.includes.is_empty());
+ assert!(result.rs.is_none());
assert_cc_matches!(
- result.api,
+ result.cc.snippet,
quote! {
extern "C" void public_function();
}
@@ -891,11 +848,10 @@
"#;
test_format_def(test_src, "explicit_unit_return_type", |result| {
let result = result.expect("Test expects success here");
- assert!(result.includes.is_empty());
- assert!(result.cc_impl.is_none());
- assert!(result.rs_impl.is_none());
+ assert!(result.cc.includes.is_empty());
+ assert!(result.rs.is_none());
assert_cc_matches!(
- result.api,
+ result.cc.snippet,
quote! {
extern "C" void explicit_unit_return_type();
}
@@ -917,11 +873,10 @@
// TODO(b/254507801): Expect `crubit::Never` instead (see the bug for more
// details).
let result = result.expect("Test expects success here");
- assert!(result.includes.is_empty());
- assert!(result.cc_impl.is_none());
- assert!(result.rs_impl.is_none());
+ assert!(result.cc.includes.is_empty());
+ assert!(result.rs.is_none());
assert_cc_matches!(
- result.api,
+ result.cc.snippet,
quote! {
extern "C" void never_returning_function();
}
@@ -940,22 +895,19 @@
"#;
test_format_def(test_src, "public_function", |result| {
let result = result.expect("Test expects success here");
- assert!(result.includes.is_empty());
- assert!(result.rs_impl.is_none());
+ assert!(result.cc.includes.is_empty());
+ assert!(result.rs.is_none());
assert_cc_matches!(
- result.api,
+ result.cc.snippet,
quote! {
+ namespace __crubit_internal {
+ extern "C" double ...(double x, double y);
+ }
inline double public_function(double x, double y) {
return ...(x, y);
}
}
);
- assert_cc_matches!(
- result.cc_impl.expect("This test expects separate extern-C decl"),
- quote! {
- extern "C" double ...(double x, double y);
- }
- );
});
}
@@ -967,20 +919,17 @@
"#;
test_format_def(test_src, "public_function", |result| {
let result = result.expect("Test expects success here");
- assert!(result.includes.is_empty());
- assert!(result.rs_impl.is_none());
+ assert!(result.cc.includes.is_empty());
+ assert!(result.rs.is_none());
assert_cc_matches!(
- result.api,
+ result.cc.snippet,
quote! {
- inline double public_function(double x, double y) {
- return ::__crubit_internal::export_name(x, y);
+ namespace __crubit_internal {
+ extern "C" double export_name(double x, double y);
}
- }
- );
- assert_cc_matches!(
- result.cc_impl.expect("This test expects separate extern-C decl"),
- quote! {
- extern "C" double export_name(double x, double y);
+ inline double public_function(double x, double y) {
+ return __crubit_internal::export_name(x, y);
+ }
}
);
});
@@ -1017,26 +966,22 @@
// TODO(b/254095787): Update test expectations below once `const fn` from Rust
// is translated into a `consteval` C++ function.
let result = result.expect("Test expects success here");
- assert!(!result.includes.is_empty());
- assert!(result.cc_impl.is_some());
+ assert!(!result.cc.includes.is_empty());
let thunk_name = quote!{ __crubit_thunk__RNvCsgl0yn9Ytt4z_8rust_out3foo };
assert_cc_matches!(
- result.cc_impl.unwrap(),
+ result.cc.snippet,
quote! {
- extern "C" std::int32_t #thunk_name( std::int32_t i);
- }
- );
- assert_cc_matches!(
- result.api,
- quote! {
+ namespace __crubit_internal {
+ extern "C" std::int32_t #thunk_name( std::int32_t i);
+ }
inline std::int32_t foo(std::int32_t i) {
- return ::__crubit_internal::#thunk_name(i);
+ return __crubit_internal::#thunk_name(i);
}
}
);
- assert!(result.rs_impl.is_some());
+ assert!(result.rs.is_some());
assert_rs_matches!(
- result.rs_impl.unwrap(),
+ result.rs.unwrap(),
quote! {
#[no_mangle]
extern "C"
@@ -1059,11 +1004,10 @@
"#;
test_format_def(test_src, "may_throw", |result| {
let result = result.expect("Test expects success here");
- assert!(result.includes.is_empty());
- assert!(result.cc_impl.is_none());
- assert!(result.rs_impl.is_none());
+ assert!(result.cc.includes.is_empty());
+ assert!(result.rs.is_none());
assert_cc_matches!(
- result.api,
+ result.cc.snippet,
quote! {
extern "C" void may_throw();
}
@@ -1086,11 +1030,10 @@
"#;
test_format_def(test_src, "type_aliased_return", |result| {
let result = result.expect("Test expects success here");
- assert!(result.includes.is_empty());
- assert!(result.cc_impl.is_none());
- assert!(result.rs_impl.is_none());
+ assert!(result.cc.includes.is_empty());
+ assert!(result.rs.is_none());
assert_cc_matches!(
- result.api,
+ result.cc.snippet,
quote! {
extern "C" double type_aliased_return();
}
@@ -1232,26 +1175,22 @@
"#;
test_format_def(test_src, "add", |result| {
let result = result.expect("Test expects success here");
- assert!(result.includes.is_empty());
- assert!(result.cc_impl.is_some());
+ assert!(result.cc.includes.is_empty());
let thunk_name = quote! { __crubit_thunk__RNvCsgl0yn9Ytt4z_8rust_out3add };
assert_cc_matches!(
- result.cc_impl.unwrap(),
+ result.cc.snippet,
quote! {
- extern "C" double #thunk_name(double x, double y);
- }
- );
- assert_cc_matches!(
- result.api,
- quote! {
+ namespace __crubit_internal {
+ extern "C" double #thunk_name(double x, double y);
+ }
inline double add(double x, double y) {
- return ::__crubit_internal::#thunk_name(x, y);
+ return __crubit_internal::#thunk_name(x, y);
}
}
);
- assert!(result.rs_impl.is_some());
+ assert!(result.rs.is_some());
assert_rs_matches!(
- result.rs_impl.unwrap(),
+ result.rs.unwrap(),
quote! {
#[no_mangle]
extern "C"
@@ -1285,26 +1224,22 @@
"#;
test_format_def(test_src, "add", |result| {
let result = result.expect("Test expects success here");
- assert!(result.includes.is_empty());
- assert!(result.cc_impl.is_some());
+ assert!(result.cc.includes.is_empty());
let thunk_name = quote! { __crubit_thunk__RNvCsgl0yn9Ytt4z_8rust_out3add };
assert_cc_matches!(
- result.cc_impl.unwrap(),
+ result.cc.snippet,
quote! {
- extern "C" double #thunk_name(double x, double y);
- }
- );
- assert_cc_matches!(
- result.api,
- quote! {
+ namespace __crubit_internal {
+ extern "C" double #thunk_name(double x, double y);
+ }
inline double add(double x, double y) {
- return ::__crubit_internal::#thunk_name(x, y);
+ return __crubit_internal::#thunk_name(x, y);
}
}
);
- assert!(result.rs_impl.is_some());
+ assert!(result.rs.is_some());
assert_rs_matches!(
- result.rs_impl.unwrap(),
+ result.rs.unwrap(),
quote! {
#[no_mangle]
extern "C"
@@ -1340,11 +1275,10 @@
"#;
test_format_def(test_src, "foo", |result| {
let result = result.expect("Test expects success here");
- assert!(result.includes.is_empty());
- assert!(result.cc_impl.is_none());
- assert!(result.rs_impl.is_none());
+ assert!(result.cc.includes.is_empty());
+ assert!(result.rs.is_none());
assert_cc_matches!(
- result.api,
+ result.cc.snippet,
quote! {
extern "C" void foo(bool b, double f);
}
@@ -1361,11 +1295,10 @@
"#;
test_format_def(test_src, "some_function", |result| {
let result = result.expect("Test expects success here");
- assert!(result.includes.is_empty());
- assert!(result.cc_impl.is_none());
- assert!(result.rs_impl.is_none());
+ assert!(result.cc.includes.is_empty());
+ assert!(result.rs.is_none());
assert_cc_matches!(
- result.api,
+ result.cc.snippet,
quote! {
extern "C" void some_function(double __param_0);
}
@@ -1380,28 +1313,24 @@
"#;
test_format_def(test_src, "foo", |result| {
let result = result.expect("Test expects success here");
- assert!(result.includes.is_empty());
- assert!(result.cc_impl.is_some());
+ assert!(result.cc.includes.is_empty());
assert_cc_matches!(
- result.cc_impl.unwrap(),
+ result.cc.snippet,
quote! {
- extern "C" void ...(
- double __param_0, double __param_1);
- }
- );
- assert_cc_matches!(
- result.api,
- quote! {
+ namespace __crubit_internal {
+ extern "C" void ...(
+ double __param_0, double __param_1);
+ }
inline void foo(double __param_0, double __param_1) {
- return ::__crubit_internal
+ return __crubit_internal
::...(
__param_0, __param_1);
}
}
);
- assert!(result.rs_impl.is_some());
+ assert!(result.rs.is_some());
assert_rs_matches!(
- result.rs_impl.unwrap(),
+ result.rs.unwrap(),
quote! {
#[no_mangle]
extern "C" fn ...(__param_0: f64, __param_1: f64) -> () {
@@ -1767,7 +1696,7 @@
/// result from `format_def`.)
fn test_format_def<F, T>(source: &str, name: &str, test_function: F) -> T
where
- F: FnOnce(Result<BindingsSnippet, String>) -> T + Send,
+ F: FnOnce(Result<MixedSnippet, String>) -> T + Send,
T: Send,
{
run_compiler(source, |tcx| {
diff --git a/cc_bindings_from_rs/cc_bindings_from_rs.rs b/cc_bindings_from_rs/cc_bindings_from_rs.rs
index 692260b..91b8c86 100644
--- a/cc_bindings_from_rs/cc_bindings_from_rs.rs
+++ b/cc_bindings_from_rs/cc_bindings_from_rs.rs
@@ -358,13 +358,13 @@
#pragma once
+namespace test_crate {
namespace __crubit_internal {
extern "C" void
__crubit_thunk__RNvCslKXfKXPWofF_10test_crate15public_function();
}
-namespace test_crate {
inline void public_function() {
- return ::__crubit_internal::
+ return __crubit_internal::
__crubit_thunk__RNvCslKXfKXPWofF_10test_crate15public_function();
}
} // namespace test_crate"#
