Improve Cpp::Destruct to indicate success/failure, adapt C-API and tests

Author: aaronj0

include/CppInterOp/CppInterOp.h

@@ -865,7 +865,8 @@ CPPINTEROP_API TCppObject_t Construct(TCppScope_t scope, void* arena = nullptr,
 /// destructor
 /// \param[in] count indicate the number of objects to destruct, if \c This
 /// points to an array of objects
-CPPINTEROP_API void Destruct(TCppObject_t This, TCppScope_t type,
+/// \returns true if wrapper generation and invocation succeeded.
+CPPINTEROP_API bool Destruct(TCppObject_t This, TCppScope_t type,
                              bool withFree = true, TCppIndex_t count = 0UL);
 
 /// @name Stream Redirection

include/clang-c/CXCppInterOp.h

@@ -361,8 +361,10 @@ CINDEX_LINKAGE void clang_invoke(CXScope func, void* result, void** args,
  * \param type The type of the object.
  *
  * \param withFree Whether to call operator delete/free or not.
+ *
+ * \returns true if wrapper generation and invocation succeeded.
  */
-CINDEX_LINKAGE void clang_destruct(CXObject This, CXScope S,
+CINDEX_LINKAGE bool clang_destruct(CXObject This, CXScope S,
                                    bool withFree = true, size_t nary = 0UL);
 
 /**

lib/CppInterOp/CXCppInterOp.cpp

@@ -613,10 +613,10 @@ void clang_invoke(CXScope func, void* result, void** args, size_t n,
 }
 
 namespace Cpp {
-void Destruct(compat::Interpreter& interp, TCppObject_t This,
+bool Destruct(compat::Interpreter& interp, TCppObject_t This,
               clang::Decl* Class, bool withFree, size_t nary);
 } // namespace Cpp
 
-void clang_destruct(CXObject This, CXScope S, bool withFree, size_t nary) {
-  Cpp::Destruct(*getInterpreter(S), This, getDecl(S), withFree, nary);
+bool clang_destruct(CXObject This, CXScope S, bool withFree, size_t nary) {
+  return Cpp::Destruct(*getInterpreter(S), This, getDecl(S), withFree, nary);
 }

lib/CppInterOp/CppInterOp.cpp

@@ -3812,19 +3812,20 @@ TCppObject_t Construct(TCppScope_t scope, void* arena /*=nullptr*/,
   return Construct(getInterp(), scope, arena, count);
 }
 
-void Destruct(compat::Interpreter& interp, TCppObject_t This, Decl* Class,
+bool Destruct(compat::Interpreter& interp, TCppObject_t This, Decl* Class,
               bool withFree, TCppIndex_t nary) {
   if (auto wrapper = make_dtor_wrapper(interp, Class)) {
     (*wrapper)(This, nary, withFree);
-    return;
+    return true;
   }
-  // FIXME: Diagnose.
+  return false;
+  // FIXME: Enable stronger diagnostics
 }
 
-void Destruct(TCppObject_t This, TCppScope_t scope, bool withFree /*=true*/,
+bool Destruct(TCppObject_t This, TCppScope_t scope, bool withFree /*=true*/,
               TCppIndex_t count /*=0UL*/) {
   auto* Class = static_cast<Decl*>(scope);
-  Destruct(getInterp(), This, Class, withFree, count);
+  return Destruct(getInterp(), This, Class, withFree, count);
 }
 
 class StreamCaptureInfo {

unittests/CppInterOp/FunctionReflectionTest.cpp

@@ -1477,7 +1477,7 @@ TEST(FunctionReflectionTest, JitCallAdvanced) {
   Ctor.Invoke(&object);
   EXPECT_TRUE(object) << "Failed to call the ctor.";
   // Building a wrapper with a typedef decl must be possible.
-  Cpp::Destruct(object, Decls[1]);
+  EXPECT_TRUE(Cpp::Destruct(object, Decls[1]));
 
   // C API
   auto* I = clang_createInterpreterFromRawPtr(Cpp::GetInterpreter());
@@ -2342,7 +2342,7 @@ TEST(FunctionReflectionTest, ConstructArray) {
   obj = reinterpret_cast<int*>(reinterpret_cast<char*>(where) +
                                (Cpp::SizeOf(scope) * 4));
   EXPECT_TRUE(*obj == 42);
-  Cpp::Destruct(where, scope, /*withFree=*/false, 5);
+  EXPECT_TRUE(Cpp::Destruct(where, scope, /*withFree=*/false, 5));
   Cpp::Deallocate(scope, where, 5);
   output = testing::internal::GetCapturedStdout();
   EXPECT_EQ(output,
@@ -2379,7 +2379,7 @@ TEST(FunctionReflectionTest, Destruct) {
   testing::internal::CaptureStdout();
   Cpp::TCppScope_t scope = Cpp::GetNamed("C");
   Cpp::TCppObject_t object = Cpp::Construct(scope);
-  Cpp::Destruct(object, scope);
+  EXPECT_TRUE(Cpp::Destruct(object, scope));
   std::string output = testing::internal::GetCapturedStdout();
 
   EXPECT_EQ(output, "Destructor Executed");
@@ -2389,7 +2389,7 @@ TEST(FunctionReflectionTest, Destruct) {
   object = Cpp::Construct(scope);
   // Make sure we do not call delete by adding an explicit Deallocate. If we
   // called delete the Deallocate will cause a double deletion error.
-  Cpp::Destruct(object, scope, /*withFree=*/false);
+  EXPECT_TRUE(Cpp::Destruct(object, scope, /*withFree=*/false));
   Cpp::Deallocate(scope, object);
   output = testing::internal::GetCapturedStdout();
   EXPECT_EQ(output, "Destructor Executed");
@@ -2454,7 +2454,7 @@ TEST(FunctionReflectionTest, DestructArray) {
 
   testing::internal::CaptureStdout();
   // destruct 3 out of 5 objects
-  Cpp::Destruct(where, scope, false, 3);
+  EXPECT_TRUE(Cpp::Destruct(where, scope, false, 3));
   output = testing::internal::GetCapturedStdout();
 
   EXPECT_EQ(
@@ -2466,7 +2466,7 @@ TEST(FunctionReflectionTest, DestructArray) {
   // destruct the rest
   auto *new_head = reinterpret_cast<void*>(reinterpret_cast<char*>(where) +
                                           (Cpp::SizeOf(scope) * 3));
-  Cpp::Destruct(new_head, scope, false, 2);
+  EXPECT_TRUE(Cpp::Destruct(new_head, scope, false, 2));
 
   output = testing::internal::GetCapturedStdout();
   EXPECT_EQ(output, "\nDestructor Executed\n\nDestructor Executed\n");
@@ -2481,7 +2481,7 @@ TEST(FunctionReflectionTest, DestructArray) {
   testing::internal::CaptureStdout();
   // FIXME : This should work with the array of objects as well
   // Cpp::Destruct(where, scope, true, 5);
-  Cpp::Destruct(where, scope, true);
+  EXPECT_TRUE(Cpp::Destruct(where, scope, true));
   output = testing::internal::GetCapturedStdout();
   EXPECT_EQ(output, "\nDestructor Executed\n");
   output.clear();