Merge branch 'main' into out-of-process

Author: kr-2003

include/CppInterOp/CppInterOp.h

@@ -36,6 +36,7 @@
 namespace Cpp {
 using TCppIndex_t = size_t;
 using TCppScope_t = void*;
+using TCppConstScope_t = const void*;
 using TCppType_t = void*;
 using TCppFunction_t = void*;
 using TCppConstFunction_t = const void*;
@@ -839,14 +840,14 @@ enum : long int {
 CPPINTEROP_API std::vector<long int> GetDimensions(TCppType_t type);
 
 /// Allocates memory required by an object of a given class
-/// \c scope Given class for which to allocate memory for
-/// \c count is used to indicate the number of objects to allocate for.
+/// \param[in] scope Given class for which to allocate memory for
+/// \param[in] count is used to indicate the number of objects to allocate for.
 CPPINTEROP_API TCppObject_t Allocate(TCppScope_t scope,
                                      TCppIndex_t count = 1UL);
 
 /// Deallocates memory for a given class.
-/// \c scope Class to indicate size of memory to deallocate
-/// \c count is used to indicate the number of objects to dallocate for
+/// \param[in] scope Class to indicate size of memory to deallocate
+/// \param[in] count is used to indicate the number of objects to dallocate for
 CPPINTEROP_API void Deallocate(TCppScope_t scope, TCppObject_t address,
                                TCppIndex_t count = 1UL);
 
@@ -856,6 +857,8 @@ CPPINTEROP_API void Deallocate(TCppScope_t scope, TCppObject_t address,
 /// \param[in] arena If set, this API uses placement new to construct at this
 /// address.
 /// \param[in] is used to indicate the number of objects to construct.
+/// \returns a pointer to the constructed object, which is arena if placement
+/// new is used.
 CPPINTEROP_API TCppObject_t Construct(TCppScope_t scope, void* arena = nullptr,
                                       TCppIndex_t count = 1UL);
 
@@ -866,7 +869,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, TCppConstScope_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,
-              clang::Decl* Class, bool withFree, size_t nary);
+bool Destruct(compat::Interpreter& interp, TCppObject_t This,
+              const 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

@@ -3817,19 +3817,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, const 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*/,
-              TCppIndex_t count /*=0UL*/) {
-  auto* Class = static_cast<Decl*>(scope);
-  Destruct(getInterp(), This, Class, withFree, count);
+bool Destruct(TCppObject_t This, TCppConstScope_t scope,
+              bool withFree /*=true*/, TCppIndex_t count /*=0UL*/) {
+  const auto* Class = static_cast<const Decl*>(scope);
+  return Destruct(getInterp(), This, Class, withFree, count);
 }
 
 class StreamCaptureInfo {

unittests/CppInterOp/CMakeLists.txt

@@ -98,7 +98,7 @@ target_link_libraries(DynamicLibraryManagerTests
 )
 
 if(EMSCRIPTEN)
-  set(TEST_SHARED_LIBRARY_PATH "${CMAKE_CURRENT_BINARY_DIR}/TestSharedLib/unittests/bin/Release/")
+  set(TEST_SHARED_LIBRARY_PATH "${CMAKE_CURRENT_BINARY_DIR}/TestSharedLib/unittests/bin/$<CONFIG>/")
   string(REPLACE "@" "@@" ESCAPED_TEST_SHARED_LIBRARY_PATH "${TEST_SHARED_LIBRARY_PATH}")
   # Check explanation of Emscripten-specific link flags for CppInterOpTests above for DynamicLibraryManagerTests as well.
   target_link_options(DynamicLibraryManagerTests

unittests/CppInterOp/FunctionReflectionTest.cpp

@@ -1481,7 +1481,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());
@@ -2365,7 +2365,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,
@@ -2405,7 +2405,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");
@@ -2415,7 +2415,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");
@@ -2432,6 +2432,20 @@ TEST(FunctionReflectionTest, Destruct) {
   // Clean up resources
   clang_Interpreter_takeInterpreterAsPtr(I);
   clang_Interpreter_dispose(I);
+
+  // Failure test, this wrapper should not compile since we explicitly delete
+  // the destructor
+  Interp->declare(R"(
+  class D {
+    public:
+      D() {}
+      ~D() = delete;
+  };
+  )");
+
+  scope = Cpp::GetNamed("D");
+  object = Cpp::Construct(scope);
+  EXPECT_FALSE(Cpp::Destruct(object, scope));
 }
 
 TEST(FunctionReflectionTest, DestructArray) {
@@ -2483,7 +2497,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(
@@ -2495,7 +2509,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");
@@ -2510,7 +2524,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();