More robust test for definition reordering.
Before this CL definition reordering *was* covered by an end-to-end test
under `cc_bindings_from_rs/test/structs`, but:
1) it was unnecessarily tangled with test of other features and
2) it only required reordering if bindings would be generated in the
original source order, and
3) it didn't force a module/namespace to be split in two chunks (which
is why namespace handling in b/258265044 isn't done top-down).
This CL tweaks to tests to fix these issues.
PiperOrigin-RevId: 495495236
diff --git a/cc_bindings_from_rs/test/structs/structs.rs b/cc_bindings_from_rs/test/structs/structs.rs
index d71cb92..8102b65 100644
--- a/cc_bindings_from_rs/test/structs/structs.rs
+++ b/cc_bindings_from_rs/test/structs/structs.rs
@@ -5,37 +5,91 @@
//! This crate is used as a test input for `cc_bindings_from_rs` and the
//! generated C++ bindings are then tested via `structs_test.cc`.
-pub fn create_repr_c_point_via_free_function(x: i32, y: i32) -> ReprCPoint {
- ReprCPoint { x, y }
+/// Test for a `#[repr(C)` struct.
+pub mod repr_c {
+
+ #[repr(C)]
+ pub struct Point {
+ pub x: i32,
+ pub y: i32,
+ }
+
+ pub fn create(x: i32, y: i32) -> Point {
+ Point { x, y }
+ }
+
+ pub fn get_x(p: Point) -> i32 {
+ p.x
+ }
}
-pub fn get_x_of_repr_c_point_via_free_function(p: ReprCPoint) -> i32 {
- p.x
+/// Test for a struct using default layout (i.e. one without an explicit
+/// `#[repr(C)]` or similar attribute). Among other things, it tests that
+/// building generated `..._cc_api_impl.rs` will not warn about
+/// `improper_ctypes_definitions` (search for this warning name in `bindings.rs`
+/// for a longer explanation of why suppressing this warning is okay).
+pub mod default_repr {
+
+ pub struct Point {
+ pub x: i32,
+ pub y: i32,
+ }
+
+ pub fn create(x: i32, y: i32) -> Point {
+ Point { x, y }
+ }
+
+ pub fn get_x(p: Point) -> i32 {
+ p.x
+ }
}
-pub fn create_default_repr_point_via_free_function(x: i32, y: i32) -> DefaultReprPoint {
- DefaultReprPoint { x, y }
-}
-
-pub fn get_x_of_default_repr_point_via_free_function(p: DefaultReprPoint) -> i32 {
- p.x
-}
-
-// `ReprCPoint` is defined *after* functions that take or return this type,
-// because preserving this order in the generated `..._cc_api.h` file would
-// trigger a C++ error (because in C++ one can't refer to a struct before it has
-// been defined).
-#[repr(C)]
-pub struct ReprCPoint {
- pub x: i32,
- pub y: i32,
-}
-
-// `DefaultReprPoint` is defined *after* functions that take or return this
-// type, because preserving this order in the generated `..._cc_api.h` file
-// would trigger a C++ error (because in C++ one can't refer to a struct before
-// it has been defined).
-pub struct DefaultReprPoint {
- pub x: i32,
- pub y: i32,
+/// This module provides test coverage for reordering the generated bindings in
+/// a way that ensures that C++ structs are defined *before* being referring to
+/// them when (say) declaring a function that returns the struct by value, or
+/// takes it by value as an argument.
+///
+/// This module has been structured in a way that forces at least one submodule
+/// to be broken up into 2 separate chunks. Definition dependencies force
+/// bindings from one of the structs to come first - let's assume that `m1::S1`
+/// comes first (the case where `m2::S2` comes first is symmetrical -
+/// all the same conclusions apply). Before `m1::create_S2` can be declared,
+/// `m1::S2` needs to be defined. This means that the order will be: `m1::S1`,
+/// ..., `m2::S2`, ..., `m1::create_S2` - the `m1` module has to be split into
+/// two non-contiguous chunks (in the generated bindings):
+///
+/// ```cpp
+/// namespace m1 { // <- FIRST CHUNK OF `mod m1`
+/// struct S1 { ... };
+/// }
+///
+/// namespace m2 {
+/// struct S2 { ... };
+/// }
+///
+/// namespace m1 { // <- SECOND CHUNK OF `mod m1`
+/// S2 create_s2();
+/// }
+/// ```
+pub mod reordering_defs {
+ pub mod m1 {
+ use super::m2::S2;
+ pub struct S1(pub i32);
+ pub fn create_s2() -> S2 {
+ S2(123)
+ }
+ pub fn get_int_from_s2(s2: S2) -> i32 {
+ s2.0
+ }
+ }
+ pub mod m2 {
+ use super::m1::S1;
+ pub struct S2(pub i32);
+ pub fn create_s1() -> S1 {
+ S1(456)
+ }
+ pub fn get_int_from_s1(s1: S1) -> i32 {
+ s1.0
+ }
+ }
}
diff --git a/cc_bindings_from_rs/test/structs/structs_test.cc b/cc_bindings_from_rs/test/structs/structs_test.cc
index edd2185..9bca5d7 100644
--- a/cc_bindings_from_rs/test/structs/structs_test.cc
+++ b/cc_bindings_from_rs/test/structs/structs_test.cc
@@ -12,17 +12,24 @@
namespace {
TEST(StructsTest, ReprCPointReturnedOrTakenByValue) {
- structs::ReprCPoint p =
- structs::create_repr_c_point_via_free_function(123, 456);
- EXPECT_EQ(123,
- structs::get_x_of_repr_c_point_via_free_function(std::move(p)));
+ structs::repr_c::Point p = structs::repr_c::create(123, 456);
+ EXPECT_EQ(123, structs::repr_c::get_x(std::move(p)));
}
TEST(StructsTest, DefaultReprPointReturnedOrTakenByValue) {
- structs::DefaultReprPoint p =
- structs::create_default_repr_point_via_free_function(123, 456);
- EXPECT_EQ(123, structs::get_x_of_default_repr_point_via_free_function(
- std::move(p)));
+ structs::default_repr::Point p = structs::default_repr::create(123, 456);
+ EXPECT_EQ(123, structs::default_repr::get_x(std::move(p)));
+}
+
+TEST(StructsTest, ReorderingDefs) {
+ namespace m1 = structs::reordering_defs::m1;
+ namespace m2 = structs::reordering_defs::m2;
+
+ m1::S1 s1 = m2::create_s1();
+ EXPECT_EQ(456, m2::get_int_from_s1(std::move(s1)));
+
+ m2::S2 s2 = m1::create_s2();
+ EXPECT_EQ(123, m1::get_int_from_s2(std::move(s2)));
}
} // namespace
