Preserve public visibility of Rust fields in the generated C++ bindings.
PiperOrigin-RevId: 537154633
diff --git a/cc_bindings_from_rs/bindings.rs b/cc_bindings_from_rs/bindings.rs
index 2a2f687..984e1f9 100644
--- a/cc_bindings_from_rs/bindings.rs
+++ b/cc_bindings_from_rs/bindings.rs
@@ -1386,7 +1386,7 @@
} else {
// We put the assertions in a method so that they can read private member
// variables.
- quote! { inline static void __crubit_field_offset_assertions(); }
+ quote! { private: inline static void __crubit_field_offset_assertions(); }
};
let mut prereqs = CcPrerequisites::default();
@@ -1404,8 +1404,9 @@
if size > 0 {
let size = Literal::u64_unsuffixed(size);
quote! {
- __COMMENT__ #msg
- unsigned char #cc_name[#size];
+ private:
+ __COMMENT__ #msg
+ unsigned char #cc_name[#size];
}
} else {
// TODO(b/258259459): finalize the approach here.
@@ -1413,8 +1414,9 @@
// field entirely. This also requires removing the field's assertions,
// added above.
quote! {
- __COMMENT__ #msg
- [[no_unique_address]] struct{} #cc_name;
+ private:
+ __COMMENT__ #msg
+ [[no_unique_address]] struct{} #cc_name;
}
}
}
@@ -1425,10 +1427,20 @@
} else {
let padding = Literal::u64_unsuffixed(padding);
let ident = format_ident!("__padding{}", field.index);
- quote! { unsigned char #ident[#padding]; }
+ quote! { private: unsigned char #ident[#padding]; }
+ };
+ let visibility = if field.is_public {
+ quote! { public: }
+ } else {
+ quote! { private: }
};
let cc_type = cc_type.into_tokens(&mut prereqs);
- quote! { #cc_type #cc_name; #padding }
+ // TODO(b/271002281): Preserve doc comments.
+ quote! {
+ #visibility
+ #cc_type #cc_name;
+ #padding
+ }
}
}
})
@@ -1437,10 +1449,8 @@
CcSnippet {
prereqs,
tokens: quote! {
- // TODO(b/271002281): Preserve actual field visibility.
- private: __NEWLINE__
- #fields
- #assertions_method_decl
+ #fields
+ #assertions_method_decl
},
}
};
@@ -3631,9 +3641,9 @@
// In this test there is no custom `Drop`, so C++ can also
// just use the `default` destructor.
~SomeStruct() = default;
+ public: ... std::int32_t x;
+ public: ... std::int32_t y;
private:
- ... std::int32_t x;
- ... std::int32_t y;
inline static void __crubit_field_offset_assertions();
};
}
@@ -3696,9 +3706,9 @@
// In this test there is no custom `Drop`, so C++ can also
// just use the `default` destructor.
~TupleStruct() = default;
+ public: ... std::int32_t __field0;
+ public: ... std::int32_t __field1;
private:
- ... std::int32_t __field0;
- ... std::int32_t __field1;
inline static void __crubit_field_offset_assertions();
};
}
@@ -3750,13 +3760,13 @@
...
struct CRUBIT_INTERNAL_RUST_TYPE(...) alignas(4) SomeStruct final {
...
+ // The particular order below is not guaranteed,
+ // so we may need to adjust this test assertion
+ // (if Rust changes how it lays out the fields).
+ public: ... std::int32_t field2;
+ public: ... std::int16_t field1;
+ public: ... std::int16_t field3;
private:
- // The particular order below is not guaranteed,
- // so we may need to adjust this test assertion
- // (if Rust changes how it lays out the fields).
- ... std::int32_t field2;
- ... std::int16_t field1;
- ... std::int16_t field3;
inline static void __crubit_field_offset_assertions();
};
}
@@ -3810,9 +3820,10 @@
...
struct CRUBIT_INTERNAL_RUST_TYPE(...) alignas(1) __attribute__((packed)) SomeStruct final {
...
- std::uint16_t field1;
- std::uint32_t field2;
- inline static void __crubit_field_offset_assertions();
+ public: ... std::uint16_t field1;
+ public: ... std::uint32_t field2;
+ private:
+ inline static void __crubit_field_offset_assertions();
};
}
);
@@ -3861,10 +3872,11 @@
...
struct CRUBIT_INTERNAL_RUST_TYPE(...) alignas(4) SomeStruct final {
...
- std::uint32_t f2;
- std::uint8_t f1;
- unsigned char __padding0[3];
- inline static void __crubit_field_offset_assertions();
+ public: ... std::uint32_t f2;
+ public: ... std::uint8_t f1;
+ private: unsigned char __padding0[3];
+ private:
+ inline static void __crubit_field_offset_assertions();
};
}
);
@@ -4284,7 +4296,10 @@
private:
__COMMENT__ #broken_field_msg
unsigned char unsupported_field[16];
+ public:
+ ...
std::int32_t successful_field;
+ private:
inline static void __crubit_field_offset_assertions();
};
...
@@ -4415,9 +4430,13 @@
private:
__COMMENT__ #broken_field_msg
[[no_unique_address]] struct{} zst1;
+ private:
__COMMENT__ #broken_field_msg
[[no_unique_address]] struct{} zst2;
+ public:
+ ...
std::int32_t successful_field;
+ private:
inline static void __crubit_field_offset_assertions();
};
...
@@ -4513,6 +4532,7 @@
private:
__COMMENT__ #no_fields_msg
unsigned char __opaque_blob_of_bytes[1];
+ private:
inline static void __crubit_field_offset_assertions();
};
}
@@ -4578,6 +4598,7 @@
private:
__COMMENT__ #no_fields_msg
unsigned char __opaque_blob_of_bytes[12];
+ private:
inline static void __crubit_field_offset_assertions();
};
}
@@ -4656,6 +4677,7 @@
private:
__COMMENT__ #no_fields_msg
unsigned char __opaque_blob_of_bytes[8];
+ private:
inline static void __crubit_field_offset_assertions();
};
}
diff --git a/cc_bindings_from_rs/test/structs/structs_test.cc b/cc_bindings_from_rs/test/structs/structs_test.cc
index b1b9dd0..d22427b 100644
--- a/cc_bindings_from_rs/test/structs/structs_test.cc
+++ b/cc_bindings_from_rs/test/structs/structs_test.cc
@@ -13,16 +13,21 @@
TEST(StructsTest, ReprCPointReturnedOrTakenByValue) {
structs::repr_c::Point p = structs::repr_c::create(123, 456);
+ EXPECT_EQ(123, p.x);
+ EXPECT_EQ(456, p.y);
EXPECT_EQ(123, structs::repr_c::get_x(std::move(p)));
}
TEST(StructsTest, ZstFieldsReturnedOrTakenByValue) {
structs::zst_fields::ZstFields x = structs::zst_fields::create(42);
+ EXPECT_EQ(42, x.value);
EXPECT_EQ(structs::zst_fields::get_value(std::move(x)), 42);
}
TEST(StructsTest, DefaultReprPointReturnedOrTakenByValue) {
structs::default_repr::Point p = structs::default_repr::create(123, 456);
+ EXPECT_EQ(123, p.x);
+ EXPECT_EQ(456, p.y);
EXPECT_EQ(123, structs::default_repr::get_x(std::move(p)));
}
diff --git a/docs/bindings/reference/pointers_and_references.md b/docs/bindings/reference/pointers_and_references.md
index 3c6d4a8..0fc7989 100644
--- a/docs/bindings/reference/pointers_and_references.md
+++ b/docs/bindings/reference/pointers_and_references.md
@@ -96,13 +96,9 @@
Examples of C++ features that may mutate a value that Rust holds a reference to:
-* Using copy assignment operator of C++ value that Rust has a reference to.
-
-TODO(b/258249993): After `cc_bindings_from_rs` generates assignment operators,
-explicitly document them here as an example.
-
-TODO(b/271002281): After `cc_bindings_from_rs` exposes public fields, explicitly
-document them here as an example.
+* Using copy assignment operator of C++ value that Rust has a
+ reference to.
+* Mutating public fields of a C++ struct that Rust has a reference to.
TODO: Try to succintly mention the idea that short-lived / non-retained
references are safe from the mutation risk.