Suffix field names with underscore if their names conflict with other member function names.
In Rust, the following is allowed:
```
struct X {
a: i32
}
impl X {
fn a(self: &Self) -> i32 {
self.a
}
}
```
But C++ doesn't allow the fields and the member functions to have the same names in Class/Struct. When the conflicts happen, we rename the fields assuming that the member functions serve as getters/setters.
PiperOrigin-RevId: 671035288
Change-Id: I72bfaa607971bd64aa8db3a02068dcd5a8257a9c
diff --git a/cc_bindings_from_rs/bindings.rs b/cc_bindings_from_rs/bindings.rs
index aecb12e..f7a77f7 100644
--- a/cc_bindings_from_rs/bindings.rs
+++ b/cc_bindings_from_rs/bindings.rs
@@ -2037,6 +2037,7 @@
fn format_fields<'tcx>(
db: &dyn BindingsGenerator<'tcx>,
core: &AdtCoreBindings<'tcx>,
+ member_function_names: &HashSet<String>,
) -> ApiSnippets {
let tcx = db.tcx();
@@ -2115,8 +2116,15 @@
cpp_type: db.format_ty_for_cc(ty, TypeLocation::Other)?,
})
});
- let name = field_def.ident(tcx);
- let cc_name = format_cc_ident(name.as_str())
+ let name = field_def.ident(tcx).to_string();
+ let cc_name = if member_function_names.contains(&name) {
+ // TODO: Handle the case of name_ itself also being taken? e.g. the Rust struct
+ // struct S {a: i32, a_: i32} impl S { fn a() {} fn a_() {} fn a__(){}.
+ format!("{name}_")
+ } else {
+ name.clone()
+ };
+ let cc_name = format_cc_ident(cc_name.as_str())
.unwrap_or_else(|_err| format_ident!("__field{index}").into_token_stream());
let rs_name = {
let name_starts_with_digit = name
@@ -2833,6 +2841,7 @@
let move_ctor_and_assignment_snippets =
db.format_move_ctor_and_assignment_operator(core.clone()).unwrap_or_else(|err| err);
+ let mut member_function_names = HashSet::<String>::new();
let impl_items_snippets = tcx
.inherent_impls(core.def_id)
.into_iter()
@@ -2852,7 +2861,17 @@
return None;
}
let result = match impl_item_ref.kind {
- AssocItemKind::Fn { .. } => db.format_fn(def_id).map(Some),
+ AssocItemKind::Fn { .. } => {
+ let result = db.format_fn(def_id);
+ if result.is_ok() {
+ let cpp_name = FullyQualifiedName::new(tcx, def_id.into())
+ .cpp_name
+ .unwrap()
+ .to_string();
+ member_function_names.insert(cpp_name);
+ }
+ result.map(Some)
+ }
AssocItemKind::Const => format_const(db, def_id).map(Some),
other => Err(anyhow!("Unsupported `impl` item kind: {other:?}")),
};
@@ -2878,7 +2897,7 @@
main_api: fields_main_api,
cc_details: fields_cc_details,
rs_details: fields_rs_details,
- } = format_fields(db, &core);
+ } = format_fields(db, &core, &member_function_names);
let alignment = Literal::u64_unsuffixed(core.alignment_in_bytes);
let size = Literal::u64_unsuffixed(core.size_in_bytes);
@@ -8533,6 +8552,58 @@
}
#[test]
+ fn test_format_item_rename_field_with_conflicting_name() {
+ let test_src = r#"
+ pub struct X {
+ pub a: i32,
+ b: i32,
+ #[allow(dead_code)]
+ c: i32,
+ }
+
+ impl X {
+ pub fn a(&self) -> i32 {
+ self.a
+ }
+ pub fn b(&self) -> i32 {
+ self.b
+ }
+ }
+ "#;
+
+ test_format_item(test_src, "X", |result| {
+ let result = result.unwrap().unwrap();
+ let main_api = &result.main_api;
+ assert!(!main_api.prereqs.is_empty());
+ assert_cc_matches!(
+ main_api.tokens,
+ quote! {
+ std::int32_t a_;
+ }
+ );
+ assert_cc_matches!(
+ main_api.tokens,
+ quote! {
+ std::int32_t b_;
+ }
+ );
+ assert_cc_matches!(
+ main_api.tokens,
+ quote! {
+ std::int32_t c;
+ }
+ );
+ // Check that the fields are not renamed in the Rust side.
+ assert_rs_matches!(
+ result.rs_details,
+ quote! {
+ ::core::mem::offset_of!(::rust_out::X, a) == 0
+ }
+ );
+ })
+ }
+
+ #[test]
fn test_must_use_attr_for_struct_msg() {
let test_src = r#"
#[must_use = "foo"]
diff --git a/cc_bindings_from_rs/test/golden/struct_with_conflicting_fields_and_member_functions.rs b/cc_bindings_from_rs/test/golden/struct_with_conflicting_fields_and_member_functions.rs
new file mode 100644
index 0000000..03bc992
--- /dev/null
+++ b/cc_bindings_from_rs/test/golden/struct_with_conflicting_fields_and_member_functions.rs
@@ -0,0 +1,19 @@
+// 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
+
+#![allow(dead_code)]
+pub struct X {
+ pub a: i32,
+ b: i32,
+ c: i32,
+}
+
+impl X {
+ pub fn a(&self) -> i32 {
+ self.a
+ }
+ pub fn b(&self) -> i32 {
+ self.b
+ }
+}
diff --git a/cc_bindings_from_rs/test/golden/struct_with_conflicting_fields_and_member_functions_cc_api.h b/cc_bindings_from_rs/test/golden/struct_with_conflicting_fields_and_member_functions_cc_api.h
new file mode 100644
index 0000000..1e4cf89
--- /dev/null
+++ b/cc_bindings_from_rs/test/golden/struct_with_conflicting_fields_and_member_functions_cc_api.h
@@ -0,0 +1,104 @@
+// 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
+
+// Automatically @generated C++ bindings for the following Rust crate:
+// struct_with_conflicting_fields_and_member_functions_rust
+
+// clang-format off
+#pragma once
+
+#include "support/internal/attribute_macros.h"
+
+#include <cstddef>
+#include <cstdint>
+#include <type_traits>
+
+namespace struct_with_conflicting_fields_and_member_functions_rust {
+
+// Generated from:
+// cc_bindings_from_rs/test/golden/struct_with_conflicting_fields_and_member_functions.rs;l=6
+struct
+ CRUBIT_INTERNAL_RUST_TYPE(
+ ":: struct_with_conflicting_fields_and_member_functions_rust :: "
+ "X") alignas(4) [[clang::trivial_abi]] X final {
+ public:
+ // `X` doesn't implement the `Default` trait
+ X() = delete;
+
+ // No custom `Drop` impl and no custom \"drop glue\" required
+ ~X() = default;
+ X(X&&) = default;
+ X& operator=(X&&) = default;
+
+ // `X` doesn't implement the `Clone` trait
+ X(const X&) = delete;
+ X& operator=(const X&) = delete;
+
+ // Generated from:
+ // cc_bindings_from_rs/test/golden/struct_with_conflicting_fields_and_member_functions.rs;l=13
+ std::int32_t a() const [[clang::annotate_type("lifetime", "__anon1")]];
+
+ // Generated from:
+ // cc_bindings_from_rs/test/golden/struct_with_conflicting_fields_and_member_functions.rs;l=16
+ std::int32_t b() const [[clang::annotate_type("lifetime", "__anon1")]];
+
+ public:
+ union {
+ // Generated from:
+ // cc_bindings_from_rs/test/golden/struct_with_conflicting_fields_and_member_functions.rs;l=7
+ std::int32_t a_;
+ };
+
+ private:
+ union {
+ // Generated from:
+ // cc_bindings_from_rs/test/golden/struct_with_conflicting_fields_and_member_functions.rs;l=8
+ std::int32_t b_;
+ };
+
+ private:
+ union {
+ // Generated from:
+ // cc_bindings_from_rs/test/golden/struct_with_conflicting_fields_and_member_functions.rs;l=9
+ std::int32_t c;
+ };
+
+ private:
+ static void __crubit_field_offset_assertions();
+};
+
+static_assert(
+ sizeof(X) == 12,
+ "Verify that ADT layout didn't change since this header got generated");
+static_assert(
+ alignof(X) == 4,
+ "Verify that ADT layout didn't change since this header got generated");
+static_assert(std::is_trivially_destructible_v<X>);
+static_assert(std::is_trivially_move_constructible_v<X>);
+static_assert(std::is_trivially_move_assignable_v<X>);
+namespace __crubit_internal {
+extern "C" std::int32_t __crubit_thunk_a(
+ ::struct_with_conflicting_fields_and_member_functions_rust::
+ X const& [[clang::annotate_type("lifetime", "__anon1")]]);
+}
+inline std::int32_t X::a() const
+ [[clang::annotate_type("lifetime", "__anon1")]] {
+ return __crubit_internal::__crubit_thunk_a(*this);
+}
+
+namespace __crubit_internal {
+extern "C" std::int32_t __crubit_thunk_b(
+ ::struct_with_conflicting_fields_and_member_functions_rust::
+ X const& [[clang::annotate_type("lifetime", "__anon1")]]);
+}
+inline std::int32_t X::b() const
+ [[clang::annotate_type("lifetime", "__anon1")]] {
+ return __crubit_internal::__crubit_thunk_b(*this);
+}
+inline void X::__crubit_field_offset_assertions() {
+ static_assert(0 == offsetof(X, a_));
+ static_assert(4 == offsetof(X, b_));
+ static_assert(8 == offsetof(X, c));
+}
+} // namespace struct_with_conflicting_fields_and_member_functions_rust
diff --git a/cc_bindings_from_rs/test/golden/struct_with_conflicting_fields_and_member_functions_cc_api_impl.rs b/cc_bindings_from_rs/test/golden/struct_with_conflicting_fields_and_member_functions_cc_api_impl.rs
new file mode 100644
index 0000000..7ec0563
--- /dev/null
+++ b/cc_bindings_from_rs/test/golden/struct_with_conflicting_fields_and_member_functions_cc_api_impl.rs
@@ -0,0 +1,30 @@
+// 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
+
+// Automatically @generated C++ bindings for the following Rust crate:
+// struct_with_conflicting_fields_and_member_functions_rust
+
+#![allow(improper_ctypes_definitions)]
+
+const _: () = assert!(
+ ::std::mem::size_of::<::struct_with_conflicting_fields_and_member_functions_rust::X>() == 12
+);
+const _: () = assert!(
+ ::std::mem::align_of::<::struct_with_conflicting_fields_and_member_functions_rust::X>() == 4
+);
+#[no_mangle]
+extern "C" fn __crubit_thunk_a<'__anon1>(
+ __self: &'__anon1 ::struct_with_conflicting_fields_and_member_functions_rust::X,
+) -> i32 {
+ ::struct_with_conflicting_fields_and_member_functions_rust::X::a(__self)
+}
+#[no_mangle]
+extern "C" fn __crubit_thunk_b<'__anon1>(
+ __self: &'__anon1 ::struct_with_conflicting_fields_and_member_functions_rust::X,
+) -> i32 {
+ ::struct_with_conflicting_fields_and_member_functions_rust::X::b(__self)
+}
+const _: () = assert!(
+ ::core::mem::offset_of!(::struct_with_conflicting_fields_and_member_functions_rust::X, a) == 0
+);
