diff --git a/rs_bindings_from_cc/importer.cc b/rs_bindings_from_cc/importer.cc index 370140de3..531e0cef7 100644 --- a/rs_bindings_from_cc/importer.cc +++ b/rs_bindings_from_cc/importer.cc @@ -476,6 +476,20 @@ const clang::Decl* Importer::CanonicalizeDecl(const clang::Decl* decl) const { CHECK(canonical != nullptr); return canonical; } + if (const auto* function_decl = llvm::dyn_cast(decl)) { + if (llvm::isa(function_decl)) { + return function_decl->getCanonicalDecl(); + } + const auto* canonical = function_decl->getCanonicalDecl(); + if (canonical->getLexicalDeclContext()->isRecord()) { + for (const auto* redecl : function_decl->redecls()) { + if (!redecl->getLexicalDeclContext()->isRecord()) { + return redecl; + } + } + } + return canonical; + } return decl->getCanonicalDecl(); } diff --git a/rs_bindings_from_cc/importers/friend.cc b/rs_bindings_from_cc/importers/friend.cc index 5f484e576..7e714efcd 100644 --- a/rs_bindings_from_cc/importers/friend.cc +++ b/rs_bindings_from_cc/importers/friend.cc @@ -45,12 +45,15 @@ std::optional FriendDeclImporter::Import( "DeclContext was unexpectedly not a CXXRecordDecl")); } - // If `!is_redeclared_outside_of_friend_decl`, then we need to emit a new item - // to support the following case from - // https://en.cppreference.com/w/cpp/language/adl: "ADL can find a friend - // function (typically, an overloaded operator) that is defined entirely - // within a class or class template, even if it was never declared at - // namespace level." + bool redeclared_outside = false; + for (const auto* redecl : named_decl->redecls()) { + if (redecl != named_decl && !redecl->getLexicalDeclContext()->isRecord()) { + redeclared_outside = true; + break; + } + } + if (redeclared_outside) return std::nullopt; + std::optional item = ictx_.ImportDecl(named_decl); if (!item.has_value()) return std::nullopt; if (std::holds_alternative(*item)) return std::nullopt; diff --git a/rs_bindings_from_cc/ir_from_cc_test.rs b/rs_bindings_from_cc/ir_from_cc_test.rs index 6b28f1842..8e520ee8a 100644 --- a/rs_bindings_from_cc/ir_from_cc_test.rs +++ b/rs_bindings_from_cc/ir_from_cc_test.rs @@ -3924,12 +3924,6 @@ fn test_function_redeclared_as_friend() { // The function should appear only once in IR items. (This is a bit redundant // with the assert below, but double-checks that `...` didn't miss a Func // item.) - let functions = ir - .functions() - .filter(|f| f.rs_name == UnqualifiedIdentifier::Identifier(ir_id("bar"))) - .collect_vec(); - assert_eq!(1, functions.len()); - let function_id = functions[0].id; // There should only be a single Func item. // @@ -3951,7 +3945,6 @@ fn test_function_redeclared_as_friend() { ItemId(...), ItemId(...), ItemId(...), - ItemId(#function_id), ] ... enclosing_item_id: None ... }), @@ -3965,14 +3958,32 @@ fn test_function_redeclared_as_friend() { cc_name: "bar", rs_name: "bar" ... enclosing_item_id: None ... - adl_enclosing_record: Some(ItemId(...)) ... + adl_enclosing_record: None ... }), ], - top_level_item_ids: map! { ... BazelLabel(#TESTING_TARGET): [ItemId(...)] ... } + top_level_item_ids: map! { ... BazelLabel(#TESTING_TARGET): [ItemId(...), ItemId(...)] ... } } ); } +#[gtest] +fn test_friend_not_definition_not_redeclared() { + let ir = ir_from_cc( + r#" + class SomeClass final { + friend void some_friend_func(); + }; + "#, + ) + .unwrap(); + + let functions = ir + .functions() + .filter(|f| f.rs_name == UnqualifiedIdentifier::Identifier(ir_id("some_friend_func"))) + .collect_vec(); + assert_eq!(1, functions.len()); +} + #[gtest] fn test_function_redeclared_in_separate_namespace_chunk() { let ir = ir_from_cc( diff --git a/rs_bindings_from_cc/test/golden/friend_functions_rs_api.rs b/rs_bindings_from_cc/test/golden/friend_functions_rs_api.rs index 1c07e7a84..c4c5c2f76 100644 --- a/rs_bindings_from_cc/test/golden/friend_functions_rs_api.rs +++ b/rs_bindings_from_cc/test/golden/friend_functions_rs_api.rs @@ -43,6 +43,15 @@ pub fn visible_val(mut __param_0: crate::SomeClass) { unsafe { crate::detail::__rust_thunk___Z11visible_val9SomeClass(&mut __param_0) } } +/// # Safety +/// +/// The caller must ensure that the following unsafe arguments are not misused by the function: +/// * `__param_0`: raw pointer +#[inline(always)] +pub unsafe fn multiple_declarations(__param_0: *const crate::SomeClass) -> ::ffi_11::c_int { + crate::detail::__rust_thunk___Z21multiple_declarationsRK9SomeClass(__param_0) +} + mod detail { #[allow(unused_imports)] use super::*; @@ -51,6 +60,9 @@ mod detail { pub(crate) unsafe fn __rust_thunk___Z11visible_val9SomeClass( __param_0: &mut crate::SomeClass, ); + pub(crate) unsafe fn __rust_thunk___Z21multiple_declarationsRK9SomeClass( + __param_0: *const crate::SomeClass, + ) -> ::ffi_11::c_int; } } diff --git a/rs_bindings_from_cc/test/golden/friend_functions_rs_api_impl.cc b/rs_bindings_from_cc/test/golden/friend_functions_rs_api_impl.cc index 887bf3640..f7ef8f40d 100644 --- a/rs_bindings_from_cc/test/golden/friend_functions_rs_api_impl.cc +++ b/rs_bindings_from_cc/test/golden/friend_functions_rs_api_impl.cc @@ -30,4 +30,11 @@ extern "C" void __rust_thunk___Z11visible_val9SomeClass( visible_val(std::move(*__param_0)); } +extern "C" int __rust_thunk___Z21multiple_declarationsRK9SomeClass( + class SomeClass const* __param_0) { + return multiple_declarations(*__param_0); +} + +static_assert((int (*)(class SomeClass const&)) & ::multiple_declarations); + #pragma clang diagnostic pop diff --git a/rs_bindings_from_cc/test/golden/unsafe_types_transitive_rs_api.rs b/rs_bindings_from_cc/test/golden/unsafe_types_transitive_rs_api.rs index c320b2bf8..13de68198 100644 --- a/rs_bindings_from_cc/test/golden/unsafe_types_transitive_rs_api.rs +++ b/rs_bindings_from_cc/test/golden/unsafe_types_transitive_rs_api.rs @@ -69,11 +69,6 @@ impl Default for PrivatePointer { } } -#[inline(always)] -pub fn DerefPrivatePointer(mut p: crate::PrivatePointer) -> ::ffi_11::c_int { - unsafe { crate::detail::__rust_thunk___Z19DerefPrivatePointer14PrivatePointer(&mut p) } -} - /// # Safety /// /// To call a function that accepts this type, you must uphold these requirements: @@ -151,6 +146,11 @@ pub unsafe fn DerefPublicPointer(mut p: crate::PublicPointer) -> ::ffi_11::c_int crate::detail::__rust_thunk___Z18DerefPublicPointer13PublicPointer(&mut p) } +#[inline(always)] +pub fn DerefPrivatePointer(mut p: crate::PrivatePointer) -> ::ffi_11::c_int { + unsafe { crate::detail::__rust_thunk___Z19DerefPrivatePointer14PrivatePointer(&mut p) } +} + /// # Safety /// /// The caller must ensure that the following unsafe arguments are not misused by the function: @@ -179,9 +179,6 @@ mod detail { pub(crate) unsafe fn __rust_thunk___ZN14PrivatePointerC1Ev( __this: *mut ::core::ffi::c_void, ); - pub(crate) unsafe fn __rust_thunk___Z19DerefPrivatePointer14PrivatePointer( - p: &mut crate::PrivatePointer, - ) -> ::ffi_11::c_int; pub(crate) unsafe fn __rust_thunk___ZN23TransitivePublicPointerC1Ev( __this: *mut ::core::ffi::c_void, ); @@ -193,6 +190,9 @@ mod detail { pub(crate) unsafe fn __rust_thunk___Z18DerefPublicPointer13PublicPointer( p: &mut crate::PublicPointer, ) -> ::ffi_11::c_int; + pub(crate) unsafe fn __rust_thunk___Z19DerefPrivatePointer14PrivatePointer( + p: &mut crate::PrivatePointer, + ) -> ::ffi_11::c_int; pub(crate) unsafe fn __rust_thunk___Z28DerefTransitivePublicPointer23TransitivePublicPointer( p: &mut crate::TransitivePublicPointer, ) -> ::ffi_11::c_int; diff --git a/rs_bindings_from_cc/test/golden/unsafe_types_transitive_rs_api_impl.cc b/rs_bindings_from_cc/test/golden/unsafe_types_transitive_rs_api_impl.cc index b56faac0f..1fb42c621 100644 --- a/rs_bindings_from_cc/test/golden/unsafe_types_transitive_rs_api_impl.cc +++ b/rs_bindings_from_cc/test/golden/unsafe_types_transitive_rs_api_impl.cc @@ -35,11 +35,6 @@ extern "C" void __rust_thunk___ZN14PrivatePointerC1Ev( crubit::construct_at(__this); } -extern "C" int __rust_thunk___Z19DerefPrivatePointer14PrivatePointer( - class PrivatePointer* p) { - return DerefPrivatePointer(std::move(*p)); -} - static_assert(CRUBIT_SIZEOF(struct TransitivePublicPointer) == 16); static_assert(alignof(struct TransitivePublicPointer) == 8); static_assert(CRUBIT_OFFSET_OF(pub, struct TransitivePublicPointer) == 0); @@ -68,6 +63,13 @@ extern "C" int __rust_thunk___Z18DerefPublicPointer13PublicPointer( static_assert((int (*)(struct PublicPointer)) & ::DerefPublicPointer); +extern "C" int __rust_thunk___Z19DerefPrivatePointer14PrivatePointer( + class PrivatePointer* p) { + return DerefPrivatePointer(std::move(*p)); +} + +static_assert((int (*)(class PrivatePointer)) & ::DerefPrivatePointer); + extern "C" int __rust_thunk___Z28DerefTransitivePublicPointer23TransitivePublicPointer( struct TransitivePublicPointer* p) {