Support unique ptrs for types with virtual destructors or overloaded delete

rs_bindings_from_cc/generate_bindings/database/code_snippet.rs

@@ -1326,6 +1326,8 @@ pub enum Thunk {
         param_types: Vec<TokenStream>,
         return_type_fragment: Option<TokenStream>,
     },
+    /// Generates a thunk for `delete ptr`.
+    OperatorDelete { thunk_ident: Ident, type_name: TokenStream },
 }
 
 impl ToTokens for Thunk {
@@ -1364,6 +1366,12 @@ impl ToTokens for Thunk {
                 }
                 .to_tokens(tokens);
             }
+            Thunk::OperatorDelete { thunk_ident, type_name } => {
+                quote! {
+                    pub(crate) unsafe fn #thunk_ident(ptr: *mut #type_name);
+                }
+                .to_tokens(tokens);
+            }
         }
     }
 }
@@ -1401,6 +1409,11 @@ pub enum ThunkImpl {
         implementation_function: TokenStream,
         cc_function_type: TokenStream,
     },
+    /// A function that implements `delete ptr`.
+    OperatorDelete {
+        thunk_ident: Ident,
+        type_cc_name: TokenStream,
+    },
 }
 
 impl ToTokens for ThunkImpl {
@@ -1465,6 +1478,14 @@ impl ToTokens for ThunkImpl {
                 }
                 .to_tokens(tokens);
             }
+            ThunkImpl::OperatorDelete { thunk_ident, type_cc_name } => {
+                quote! {
+                    extern "C" void #thunk_ident(#type_cc_name* ptr) {
+                        delete ptr;
+                    }
+                }
+                .to_tokens(tokens);
+            }
         }
     }
 }

rs_bindings_from_cc/generate_bindings/database/rs_snippet.rs

@@ -288,6 +288,11 @@ pub enum UniformReprTemplateType {
     StdUniquePtr {
         element_type: RsTypeKind,
     },
+    /// std::unique_ptr<T, std::default_delete<T>> where T has a virtual
+    /// destructor or overloads operator delete.
+    StdUniquePtrDyn {
+        element_type: RsTypeKind,
+    },
     AbslSpan {
         is_const: bool,
         include_lifetime: bool,
@@ -326,7 +331,9 @@ impl UniformReprTemplateType {
                 ensure!(element_type.is_complete(), "Rust std::unique_ptr<T> cannot be used with incomplete types, and `{}` is incomplete", element_type.display(db));
                 ensure!(element_type.is_destructible(), "Rust std::unique_ptr<T> requires that `T` be destructible, but the destructor of `{}` is non-public or deleted", element_type.display(db));
                 if element_type.overloads_operator_delete() {
-                    return Ok(None);
+                    return Ok(Some(Rc::new(UniformReprTemplateType::StdUniquePtrDyn {
+                        element_type,
+                    })));
                 }
                 Ok(Some(Rc::new(UniformReprTemplateType::StdUniquePtr { element_type })))
             }
@@ -377,6 +384,10 @@ impl UniformReprTemplateType {
                 let element_type_tokens = element_type.to_token_stream(db);
                 quote! { ::cc_std::std::unique_ptr::<#element_type_tokens> }
             }
+            Self::StdUniquePtrDyn { element_type } => {
+                let element_type_tokens = element_type.to_token_stream(db);
+                quote! { ::cc_std::std::unique_ptr_dyn::<#element_type_tokens> }
+            }
             Self::AbslSpan { is_const, include_lifetime, element_type } => {
                 let element_type_tokens = element_type.to_token_stream(db);
 

rs_bindings_from_cc/generate_bindings/generate_struct_and_union.rs

@@ -608,6 +608,14 @@ pub fn generate_record(db: &dyn BindingsGenerator, record: Rc<Record>) -> Result
         } else {
             vec![]
         };
+
+    if record.overloads_operator_delete {
+        let (thunk, thunk_impl, trait_impl) = cc_struct_operator_delete_impl(db, &record)?;
+        api_snippets.thunks.push(thunk);
+        api_snippets.cc_details.push(thunk_impl);
+        indirect_functions.push(trait_impl);
+    }
+
     let incomplete_definition = if crubit_features.contains(crubit_feature::CrubitFeature::Wrapper)
     {
         Some(quote! {
@@ -930,3 +938,42 @@ fn cc_struct_upcast_impl(
 
     Ok((upcast_impls, thunks, thunk_impls))
 }
+
+fn cc_struct_operator_delete_impl(
+    db: &dyn BindingsGenerator,
+    record: &Rc<Record>,
+) -> Result<(Thunk, ThunkImpl, TokenStream)> {
+    let thunk_ident = make_rs_ident(&format!(
+        "__crubit_delete__{}_{}",
+        record.mangled_cc_name,
+        record.owning_target.convert_to_cc_identifier(),
+    ));
+    let type_name = db.rs_type_kind(record.as_ref().into())?.to_token_stream(db);
+    let type_cc_name = cpp_type_name_for_record(record.as_ref(), db.ir())?;
+
+    let thunk =
+        Thunk::OperatorDelete { thunk_ident: thunk_ident.clone(), type_name: type_name.clone() };
+    let thunk_impl = ThunkImpl::OperatorDelete {
+        thunk_ident: thunk_ident.clone(),
+        type_cc_name: type_cc_name.to_token_stream(),
+    };
+
+    let crate_root_path = db.ir().crate_root_path_tokens();
+    let in_cc_std = db.ir().is_current_target(&record.owning_target)
+        && record.owning_target.target_name_escaped() == "cc_std";
+    let trait_path = if in_cc_std {
+        quote! { crate::std::OperatorDelete }
+    } else {
+        quote! { ::cc_std::std::OperatorDelete }
+    };
+
+    let trait_impl = quote! {
+        unsafe impl #trait_path for #type_name {
+            unsafe fn delete(ptr: *mut Self) {
+                #crate_root_path::detail::#thunk_ident(ptr as *mut #type_name);
+            }
+        }
+    };
+
+    Ok((thunk, thunk_impl, trait_impl))
+}

rs_bindings_from_cc/test/golden/inheritance_rs_api.rs

@@ -366,6 +366,12 @@ unsafe impl ::cxx::ExternType for MyAbstractClass {
 // //rs_bindings_from_cc/test/golden:inheritance_cc needs [//features:experimental] for MyAbstractClass::operator= (return type: references are not supported)
 // //rs_bindings_from_cc/test/golden:inheritance_cc needs [//features:experimental] for MyAbstractClass::operator= (the type of __param_0 (parameter #1): references are not supported)
 
+unsafe impl ::cc_std::std::OperatorDelete for crate::MyAbstractClass {
+    unsafe fn delete(ptr: *mut Self) {
+        crate::detail::__crubit_delete__15MyAbstractClass___2f_2fthird_5fparty_2fcrubit_2frs_5fbindings_5ffrom_5fcc_2ftest_2fgolden_3ainheritance_5fcc(ptr as*mut crate::MyAbstractClass);
+    }
+}
+
 /// Method inheritance
 #[derive(Clone, Copy, ::ctor::MoveAndAssignViaCopy)]
 #[repr(C)]
@@ -544,6 +550,9 @@ mod detail {
         pub(crate) unsafe fn __rust_thunk___ZN14VirtualDerivedC1Ev(
             __this: *mut ::core::ffi::c_void,
         );
+        pub(crate) unsafe fn __crubit_delete__15MyAbstractClass___2f_2fthird_5fparty_2fcrubit_2frs_5fbindings_5ffrom_5fcc_2ftest_2fgolden_3ainheritance_5fcc(
+            ptr: *mut crate::MyAbstractClass,
+        );
         pub(crate) unsafe fn __rust_thunk___ZN11MethodBase1C1Ev(__this: *mut ::core::ffi::c_void);
         #[link_name = "_ZN11MethodBase16PublicEv"]
         pub(crate) unsafe fn __rust_thunk___ZN11MethodBase16PublicEv<'a>(

rs_bindings_from_cc/test/golden/inheritance_rs_api_impl.cc

@@ -74,6 +74,12 @@ extern "C" void __rust_thunk___ZN14VirtualDerivedC1Ev(
 static_assert(CRUBIT_SIZEOF(class MyAbstractClass) == 8);
 static_assert(alignof(class MyAbstractClass) == 8);
 
+extern "C" void
+__crubit_delete__15MyAbstractClass___2f_2fthird_5fparty_2fcrubit_2frs_5fbindings_5ffrom_5fcc_2ftest_2fgolden_3ainheritance_5fcc(
+    class MyAbstractClass* ptr) {
+  delete ptr;
+}
+
 static_assert(sizeof(class MethodBase1) == 1);
 static_assert(alignof(class MethodBase1) == 1);
 

rs_bindings_from_cc/test/golden/polymorphic_rs_api.rs

@@ -65,6 +65,12 @@ impl ::ctor::PinnedDrop for PolymorphicBase {
     }
 }
 
+unsafe impl ::cc_std::std::OperatorDelete for crate::PolymorphicBase {
+    unsafe fn delete(ptr: *mut Self) {
+        crate::detail::__crubit_delete__15PolymorphicBase___2f_2fthird_5fparty_2fcrubit_2frs_5fbindings_5ffrom_5fcc_2ftest_2fgolden_3apolymorphic_5fcc(ptr as*mut crate::PolymorphicBase);
+    }
+}
+
 #[::ctor::recursively_pinned(PinnedDrop)]
 #[repr(C, align(8))]
 ///CRUBIT_ANNOTATE: cpp_type=PolymorphicBase2
@@ -116,6 +122,12 @@ impl ::ctor::PinnedDrop for PolymorphicBase2 {
     }
 }
 
+unsafe impl ::cc_std::std::OperatorDelete for crate::PolymorphicBase2 {
+    unsafe fn delete(ptr: *mut Self) {
+        crate::detail::__crubit_delete__16PolymorphicBase2___2f_2fthird_5fparty_2fcrubit_2frs_5fbindings_5ffrom_5fcc_2ftest_2fgolden_3apolymorphic_5fcc(ptr as*mut crate::PolymorphicBase2);
+    }
+}
+
 #[::ctor::recursively_pinned(PinnedDrop)]
 #[repr(C, align(8))]
 ///CRUBIT_ANNOTATE: cpp_type=PolymorphicDerived
@@ -170,6 +182,12 @@ impl ::ctor::PinnedDrop for PolymorphicDerived {
 // //rs_bindings_from_cc/test/golden:polymorphic_cc needs [//features:experimental] for PolymorphicDerived::operator= (return type: references are not supported)
 // //rs_bindings_from_cc/test/golden:polymorphic_cc needs [//features:experimental] for PolymorphicDerived::operator= (the type of __param_0 (parameter #1): references are not supported)
 
+unsafe impl ::cc_std::std::OperatorDelete for crate::PolymorphicDerived {
+    unsafe fn delete(ptr: *mut Self) {
+        crate::detail::__crubit_delete__18PolymorphicDerived___2f_2fthird_5fparty_2fcrubit_2frs_5fbindings_5ffrom_5fcc_2ftest_2fgolden_3apolymorphic_5fcc(ptr as*mut crate::PolymorphicDerived);
+    }
+}
+
 mod detail {
     #[allow(unused_imports)]
     use super::*;
@@ -180,6 +198,9 @@ mod detail {
         pub(crate) unsafe fn __rust_thunk___ZN15PolymorphicBaseD1Ev<'a>(
             __this: ::core::pin::Pin<&'a mut crate::PolymorphicBase>,
         );
+        pub(crate) unsafe fn __crubit_delete__15PolymorphicBase___2f_2fthird_5fparty_2fcrubit_2frs_5fbindings_5ffrom_5fcc_2ftest_2fgolden_3apolymorphic_5fcc(
+            ptr: *mut crate::PolymorphicBase,
+        );
         pub(crate) unsafe fn __rust_thunk___ZN16PolymorphicBase2C1Ev(
             __this: *mut ::core::ffi::c_void,
         );
@@ -189,12 +210,18 @@ mod detail {
         pub(crate) unsafe fn __rust_thunk___ZN16PolymorphicBase2D1Ev<'a>(
             __this: ::core::pin::Pin<&'a mut crate::PolymorphicBase2>,
         );
+        pub(crate) unsafe fn __crubit_delete__16PolymorphicBase2___2f_2fthird_5fparty_2fcrubit_2frs_5fbindings_5ffrom_5fcc_2ftest_2fgolden_3apolymorphic_5fcc(
+            ptr: *mut crate::PolymorphicBase2,
+        );
         pub(crate) unsafe fn __rust_thunk___ZN18PolymorphicDerivedC1Ev(
             __this: *mut ::core::ffi::c_void,
         );
         pub(crate) unsafe fn __rust_thunk___ZN18PolymorphicDerivedD1Ev<'a>(
             __this: ::core::pin::Pin<&'a mut crate::PolymorphicDerived>,
         );
+        pub(crate) unsafe fn __crubit_delete__18PolymorphicDerived___2f_2fthird_5fparty_2fcrubit_2frs_5fbindings_5ffrom_5fcc_2ftest_2fgolden_3apolymorphic_5fcc(
+            ptr: *mut crate::PolymorphicDerived,
+        );
     }
 }
 

rs_bindings_from_cc/test/golden/polymorphic_rs_api_impl.cc

@@ -31,6 +31,12 @@ extern "C" void __rust_thunk___ZN15PolymorphicBaseD1Ev(
   std::destroy_at(__this);
 }
 
+extern "C" void
+__crubit_delete__15PolymorphicBase___2f_2fthird_5fparty_2fcrubit_2frs_5fbindings_5ffrom_5fcc_2ftest_2fgolden_3apolymorphic_5fcc(
+    class PolymorphicBase* ptr) {
+  delete ptr;
+}
+
 static_assert(CRUBIT_SIZEOF(class PolymorphicBase2) == 8);
 static_assert(alignof(class PolymorphicBase2) == 8);
 
@@ -51,6 +57,12 @@ extern "C" void __rust_thunk___ZN16PolymorphicBase2D1Ev(
   std::destroy_at(__this);
 }
 
+extern "C" void
+__crubit_delete__16PolymorphicBase2___2f_2fthird_5fparty_2fcrubit_2frs_5fbindings_5ffrom_5fcc_2ftest_2fgolden_3apolymorphic_5fcc(
+    class PolymorphicBase2* ptr) {
+  delete ptr;
+}
+
 static_assert(CRUBIT_SIZEOF(class PolymorphicDerived) == 16);
 static_assert(alignof(class PolymorphicDerived) == 8);
 
@@ -64,4 +76,10 @@ extern "C" void __rust_thunk___ZN18PolymorphicDerivedD1Ev(
   std::destroy_at(__this);
 }
 
+extern "C" void
+__crubit_delete__18PolymorphicDerived___2f_2fthird_5fparty_2fcrubit_2frs_5fbindings_5ffrom_5fcc_2ftest_2fgolden_3apolymorphic_5fcc(
+    class PolymorphicDerived* ptr) {
+  delete ptr;
+}
+
 #pragma clang diagnostic pop

rs_bindings_from_cc/test/manual_bridge_vocabulary_types/unique_ptr_test.rs

@@ -37,17 +37,17 @@ fn test_unique_ptr_string() {
 
 #[gtest]
 fn test_unique_ptr_overloaded_delete() {
-    assert!(!item_exists::value_exists!(unique_ptr_lib::MakeUniquePtrOverloadedDelete))
+    assert!(item_exists::value_exists!(unique_ptr_lib::MakeUniquePtrOverloadedDelete))
 }
 
 #[gtest]
 fn test_unique_ptr_overloaded_destroying_delete() {
-    assert!(!item_exists::value_exists!(unique_ptr_lib::MakeUniquePtrOverloadedDestroyingDelete))
+    assert!(item_exists::value_exists!(unique_ptr_lib::MakeUniquePtrOverloadedDestroyingDelete))
 }
 
 #[gtest]
 fn test_unique_ptr_polymorphic_type() {
-    assert!(!item_exists::value_exists!(unique_ptr_lib::MakeUniquePtrPolymorphicType))
+    assert!(item_exists::value_exists!(unique_ptr_lib::MakeUniquePtrPolymorphicType))
 }
 
 #[gtest]

support/cc_std_impl/test/unique_ptr/BUILD

@@ -26,3 +26,24 @@ crubit_rust_test(
         "@crate_index//:googletest",
     ],
 )
+
+crubit_test_cc_library(
+    name = "unique_ptr_dyn_test_helpers",
+    testonly = 1,
+    srcs = ["unique_ptr_dyn_test_helpers.cc"],
+    hdrs = ["unique_ptr_dyn_test_helpers.h"],
+    aspect_hints = ["//features:experimental"],
+    deps = ["//support:annotations"],
+)
+
+crubit_rust_test(
+    name = "unique_ptr_dyn_test",
+    srcs = ["unique_ptr_dyn_test.rs"],
+    cc_deps = [
+        ":unique_ptr_dyn_test_helpers",
+        "//support/public:cc_std",
+    ],
+    deps = [
+        "@crate_index//:googletest",
+    ],
+)

support/cc_std_impl/test/unique_ptr/unique_ptr_dyn_test.rs

@@ -0,0 +1,29 @@
+// 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 googletest::prelude::*;
+use unique_ptr_dyn_test_helpers::unique_ptr_dyn_test::*;
+
+#[gtest]
+fn test_virtual_destructor() {
+    unsafe {
+        BaseWithVirtualDestructor::set_instances(1); // One for the upcoming allocation
+        {
+            let _up = create_base();
+            assert_eq!(BaseWithVirtualDestructor::instances(), 1);
+        }
+        assert_eq!(BaseWithVirtualDestructor::instances(), 0);
+    }
+}
+
+#[gtest]
+fn test_overloaded_delete() {
+    unsafe {
+        WithOverloadedDelete::set_delete_called(false);
+        {
+            let _up = create_with_overloaded_delete();
+        }
+        assert!(WithOverloadedDelete::delete_called());
+    }
+}

support/cc_std_impl/test/unique_ptr/unique_ptr_dyn_test_helpers.cc

@@ -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
+
+#include "support/cc_std_impl/test/unique_ptr/unique_ptr_dyn_test_helpers.h"
+
+namespace unique_ptr_dyn_test {
+
+static int instances_count = 0;
+
+int BaseWithVirtualDestructor::instances() { return instances_count; }
+
+void BaseWithVirtualDestructor::set_instances(int n) { instances_count = n; }
+
+BaseWithVirtualDestructor::~BaseWithVirtualDestructor() { --instances_count; }
+
+Derived::~Derived() {}
+
+static bool was_delete_called = false;
+
+bool WithOverloadedDelete::delete_called() { return was_delete_called; }
+
+void WithOverloadedDelete::set_delete_called(bool b) { was_delete_called = b; }
+
+void WithOverloadedDelete::operator delete(void* p) {
+  was_delete_called = true;
+  ::operator delete(p);
+}
+
+}  // namespace unique_ptr_dyn_test