Observe item order in Rust source code generation.
I would also like to offer more examples for the ongoing discussion about what
kind of tests we want to write for source code generation. I have added
essentially the same kind of test written in two different ways:
In `src_code_gen.rs` the unit test `test_item_order` mocks up an `IR` instance
and performs a targeted check that the generated functions and structs appear in
the expected order. To make writing tests like this comfortable we should have
helpers to build mock IR and perform checks against the generated source code.
The test contains very simple helpers that illustrate what I mean. I think
mboehme@ previously suggested something similar. If we want to write more tests
like this, this would need to be made more robust and flexible.
The other option is a golden test in `item_order.h` etc. That test was much
faster to write, but of course it has the usual pros and cons of golden tests.
I would like to suggest that we don't write tests in the style that we have been
using in `src_code_gen.rs` before, for example like `test_simple_struct` or
several others. Writing full IR without any helpers is very tedious, and also
brittle if we change the IR definition. And then, checking the whole output
against a `quote!{...}` invocation is essentially a golden file test that is
difficult to generate/update.
The final option is of course again `FileCheck` which we haven't configured,
yet.
PiperOrigin-RevId: 400670396
diff --git a/rs_bindings_from_cc/ir_testing.rs b/rs_bindings_from_cc/ir_testing.rs
new file mode 100644
index 0000000..d41df83
--- /dev/null
+++ b/rs_bindings_from_cc/ir_testing.rs
@@ -0,0 +1,40 @@
+// Part of the Crubit project, under the Apache License v2.0 with LLVM
+// Exceptions. See /LICENSE for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+
+use ir::{CcType, Func, Identifier, Item, MappedType, Record, RsType};
+
+/// Creates an identifier
+pub fn id(name: &str) -> Identifier {
+ Identifier { identifier: name.to_string() }
+}
+
+/// Creates a simple type instance for `int`/`i32`
+pub fn int() -> MappedType {
+ MappedType {
+ rs_type: RsType { name: "i32".to_string(), type_params: vec![] },
+ cc_type: CcType { name: "int".to_string(), type_params: vec![], is_const: false },
+ }
+}
+
+/// Creates a simple `Item::Func` with a given name
+pub fn func(name: &str) -> Item {
+ Item::Func(Func {
+ identifier: id(name),
+ is_inline: true,
+ mangled_name: name.to_string(),
+ return_type: int(),
+ params: vec![],
+ })
+}
+
+/// Creates a simple `Item::Record` with a given name
+pub fn record(name: &str) -> Item {
+ Item::Record(Record {
+ identifier: id(name),
+ alignment: 0,
+ size: 0,
+ fields: vec![],
+ is_trivial_abi: true,
+ })
+}
diff --git a/rs_bindings_from_cc/src_code_gen.rs b/rs_bindings_from_cc/src_code_gen.rs
index 9881506..1ef7ae9 100644
--- a/rs_bindings_from_cc/src_code_gen.rs
+++ b/rs_bindings_from_cc/src_code_gen.rs
@@ -83,7 +83,44 @@
!func.is_inline
}
-/// Generate Rust source code for a given Record.
+/// Generates Rust source code for a given `Func`.
+///
+/// Returns the generated function and the thunk as a tuple.
+fn generate_func(func: &Func) -> Result<(TokenStream, TokenStream)> {
+ let mangled_name = &func.mangled_name;
+ let ident = make_ident(&func.identifier.identifier);
+ let thunk_ident = format_ident!("__rust_thunk__{}", &func.identifier.identifier);
+ // TODO(hlopko): do not emit `-> ()` when return type is void, it's implicit.
+ let return_type_name = format_rs_type(&func.return_type.rs_type)?;
+
+ let param_idents =
+ func.params.iter().map(|p| make_ident(&p.identifier.identifier)).collect_vec();
+
+ let param_types =
+ func.params.iter().map(|p| format_rs_type(&p.type_.rs_type)).collect::<Result<Vec<_>>>()?;
+
+ let api_func = quote! {
+ #[inline(always)]
+ pub fn #ident( #( #param_idents: #param_types ),* ) -> #return_type_name {
+ unsafe { crate::detail::#thunk_ident( #( #param_idents ),* ) }
+ }
+ };
+
+ let thunk_attr = if can_skip_cc_thunk(func) {
+ quote! {#[link_name = #mangled_name]}
+ } else {
+ quote! {}
+ };
+
+ let thunk = quote! {
+ #thunk_attr
+ pub(crate) fn #thunk_ident( #( #param_idents: #param_types ),* ) -> #return_type_name ;
+ };
+
+ Ok((api_func, thunk))
+}
+
+/// Generates Rust source code for a given `Record`.
fn generate_record(record: &Record) -> Result<TokenStream> {
let ident = make_ident(&record.identifier.identifier);
let field_idents =
@@ -128,45 +165,25 @@
}
fn generate_rs_api(ir: &IR) -> Result<String> {
+ let mut items = vec![];
let mut thunks = vec![];
- let mut api_funcs = vec![];
- for func in ir.functions() {
- let mangled_name = &func.mangled_name;
- let ident = make_ident(&func.identifier.identifier);
- let thunk_ident = format_ident!("__rust_thunk__{}", &func.identifier.identifier);
- // TODO(hlopko): do not emit `-> ()` when return type is void, it's implicit.
- let return_type_name = format_rs_type(&func.return_type.rs_type)?;
- let param_idents =
- func.params.iter().map(|p| make_ident(&p.identifier.identifier)).collect_vec();
+ let mut generate_imports = false;
- let param_types = func
- .params
- .iter()
- .map(|p| format_rs_type(&p.type_.rs_type))
- .collect::<Result<Vec<_>>>()?;
-
- api_funcs.push(quote! {
- #[inline(always)]
- pub fn #ident( #( #param_idents: #param_types ),* ) -> #return_type_name {
- unsafe { crate::detail::#thunk_ident( #( #param_idents ),* ) }
+ for item in &ir.items {
+ match item {
+ Item::Func(func) => {
+ let (api_func, thunk) = generate_func(func)?;
+ items.push(api_func);
+ thunks.push(thunk);
}
- });
-
- let thunk_attr = if can_skip_cc_thunk(&func) {
- quote! {#[link_name = #mangled_name]}
- } else {
- quote! {}
- };
-
- thunks.push(quote! {
- #thunk_attr
- pub(crate) fn #thunk_ident( #( #param_idents: #param_types ),* ) -> #return_type_name ;
- });
+ Item::Record(record) => {
+ items.push(generate_record(record)?);
+ generate_imports = true;
+ }
+ }
}
- let records = ir.records().map(generate_record).collect::<Result<Vec<_>>>()?;
-
let mod_detail = if thunks.is_empty() {
quote! {}
} else {
@@ -179,9 +196,7 @@
}
};
- let imports = if records.is_empty() {
- quote! {}
- } else {
+ let imports = if generate_imports {
// TODO(mboehme): For the time being, we're using unstable features to
// be able to use offset_of!() in static assertions. This is fine for a
// prototype, but longer-term we want to either get those features
@@ -192,13 +207,14 @@
use memoffset_unstable_const::offset_of;
use static_assertions::const_assert_eq;
}
+ } else {
+ quote! {}
};
let result = quote! {
#imports
- #( #api_funcs )*
- #( #records )*
+ #( #items )*
#mod_detail
};
@@ -372,6 +388,7 @@
use super::Result;
use super::{generate_rs_api, generate_rs_api_impl};
+ use anyhow::anyhow;
use ir::*;
use quote::quote;
use token_stream_printer::cc_tokens_to_string;
@@ -691,4 +708,36 @@
);
Ok(())
}
+
+ #[test]
+ fn test_item_order() -> Result<()> {
+ let ir = IR {
+ used_headers: vec![],
+ items: vec![
+ ir_testing::func("first_func"),
+ ir_testing::record("FirstStruct"),
+ ir_testing::func("second_func"),
+ ir_testing::record("SecondStruct"),
+ ],
+ };
+
+ let rs_api = generate_rs_api(&ir)?;
+
+ let idx = |s: &str| rs_api.find(s).ok_or(anyhow!("'{}' missing", s));
+
+ let f1 = idx("fn first_func")?;
+ let f2 = idx("fn second_func")?;
+ let s1 = idx("struct FirstStruct")?;
+ let s2 = idx("struct SecondStruct")?;
+ let t1 = idx("fn __rust_thunk__first_func")?;
+ let t2 = idx("fn __rust_thunk__second_func")?;
+
+ assert!(f1 < s1);
+ assert!(s1 < f2);
+ assert!(f2 < s2);
+ assert!(s2 < t1);
+ assert!(t1 < t2);
+
+ Ok(())
+ }
}
diff --git a/rs_bindings_from_cc/test/golden/item_order.h b/rs_bindings_from_cc/test/golden/item_order.h
new file mode 100644
index 0000000..f8581ee
--- /dev/null
+++ b/rs_bindings_from_cc/test/golden/item_order.h
@@ -0,0 +1,20 @@
+// Part of the Crubit project, under the Apache License v2.0 with LLVM
+// Exceptions. See /LICENSE for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+
+#ifndef CRUBIT_RS_BINDINGS_FROM_CC_TEST_GOLDEN_ITEM_ORDER_H_
+#define CRUBIT_RS_BINDINGS_FROM_CC_TEST_GOLDEN_ITEM_ORDER_H_
+
+struct FirstStruct {
+ int field;
+};
+
+inline int first_func() { return 42; }
+
+struct SecondStruct {
+ int field;
+};
+
+inline int second_func() { return 23; }
+
+#endif // CRUBIT_RS_BINDINGS_FROM_CC_TEST_GOLDEN_ITEM_ORDER_H_
diff --git a/rs_bindings_from_cc/test/golden/item_order_rs_api.rs b/rs_bindings_from_cc/test/golden/item_order_rs_api.rs
new file mode 100644
index 0000000..41b0cf7
--- /dev/null
+++ b/rs_bindings_from_cc/test/golden/item_order_rs_api.rs
@@ -0,0 +1,35 @@
+#![feature(const_ptr_offset_from, const_maybe_uninit_as_ptr, const_raw_ptr_deref)]
+// Part of the Crubit project, under the Apache License v2.0 with LLVM
+// Exceptions. See /LICENSE for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+
+use memoffset_unstable_const::offset_of;
+use static_assertions::const_assert_eq;
+#[repr(C)]
+pub struct FirstStruct {
+ pub field: i32,
+}
+const_assert_eq!(std::mem::size_of::<FirstStruct>(), 4usize);
+const_assert_eq!(std::mem::align_of::<FirstStruct>(), 4usize);
+const_assert_eq!(offset_of!(FirstStruct, field) * 8, 0usize);
+#[inline(always)]
+pub fn first_func() -> i32 {
+ unsafe { crate::detail::__rust_thunk__first_func() }
+}
+#[repr(C)]
+pub struct SecondStruct {
+ pub field: i32,
+}
+const_assert_eq!(std::mem::size_of::<SecondStruct>(), 4usize);
+const_assert_eq!(std::mem::align_of::<SecondStruct>(), 4usize);
+const_assert_eq!(offset_of!(SecondStruct, field) * 8, 0usize);
+#[inline(always)]
+pub fn second_func() -> i32 {
+ unsafe { crate::detail::__rust_thunk__second_func() }
+}
+mod detail {
+ extern "C" {
+ pub(crate) fn __rust_thunk__first_func() -> i32;
+ pub(crate) fn __rust_thunk__second_func() -> i32;
+ }
+}
diff --git a/rs_bindings_from_cc/test/golden/item_order_rs_api_impl.cc b/rs_bindings_from_cc/test/golden/item_order_rs_api_impl.cc
new file mode 100644
index 0000000..35c7237
--- /dev/null
+++ b/rs_bindings_from_cc/test/golden/item_order_rs_api_impl.cc
@@ -0,0 +1,14 @@
+// Part of the Crubit project, under the Apache License v2.0 with LLVM
+// Exceptions. See /LICENSE for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+
+#include <cstddef>
+#include "rs_bindings_from_cc/test/golden/item_order.h"
+extern "C" int __rust_thunk__first_func() { return first_func(); }
+extern "C" int __rust_thunk__second_func() { return second_func(); }
+static_assert(sizeof(FirstStruct) == 4);
+static_assert(alignof(FirstStruct) == 4);
+static_assert(offsetof(FirstStruct, field) * 8 == 0);
+static_assert(sizeof(SecondStruct) == 4);
+static_assert(alignof(SecondStruct) == 4);
+static_assert(offsetof(SecondStruct, field) * 8 == 0);
\ No newline at end of file
