Improve Cpp::Construct pointer logic for memory arena flags

Also handle both ctors and class scopes

Author: aaronj0

include/CppInterOp/CppInterOp.h

@@ -224,6 +224,8 @@ class JitCall {
   ///\param[in] nary - Use array new if we have to construct an array of
   ///           objects (nary > 1).
   ///\param[in] args - a pointer to a argument list and argument size.
+  ///\param[in] is_arena - a pointer that indicates if placement new is to be
+  /// used
   // FIXME: Change the type of withFree from int to bool in the wrapper code.
   void InvokeConstructor(void* result, unsigned long nary = 1,
                          ArgList args = {}, void* is_arena = nullptr) const {
@@ -849,7 +851,7 @@ CPPINTEROP_API void Deallocate(TCppScope_t scope, TCppObject_t address,
 
 /// Creates one or more objects of class \c scope by calling its default
 /// constructor.
-/// \param[in] scope Class to construct
+/// \param[in] scope Class to construct, or handle to Constructor
 /// \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.

lib/CppInterOp/CppInterOp.cpp

@@ -3783,22 +3783,27 @@ void Deallocate(TCppScope_t scope, TCppObject_t address, TCppIndex_t count) {
 // FIXME: Add optional arguments to the operator new.
 TCppObject_t Construct(compat::Interpreter& interp, TCppScope_t scope,
                        void* arena /*=nullptr*/, TCppIndex_t count /*=1UL*/) {
-  auto* Class = (Decl*)scope;
-  // FIXME: Diagnose.
-  if (!HasDefaultConstructor(Class))
+
+  if (!Cpp::IsConstructor(scope) && !Cpp::IsClass(scope))
+    return nullptr;
+  if (Cpp::IsClass(scope) && !HasDefaultConstructor(scope))
     return nullptr;
 
-  auto* const Ctor = GetDefaultConstructor(interp, Class);
-  if (JitCall JC = MakeFunctionCallable(&interp, Ctor)) {
-    if (arena) {
-      JC.InvokeConstructor(&arena, count, {},
-                           (void*)~0); // Tell Invoke to use placement new.
-      return arena;
-    }
+  TCppFunction_t ctor = nullptr;
+  if (Cpp::IsClass(scope))
+    ctor = Cpp::GetDefaultConstructor(scope);
+  else // a ctor
+    ctor = scope;
 
-    void* obj = nullptr;
-    JC.InvokeConstructor(&obj, count, {}, nullptr);
-    return obj;
+  if (JitCall JC = MakeFunctionCallable(&interp, ctor)) {
+    if (JC.getKind() != Cpp::JitCall::kConstructorCall)
+      return nullptr;
+    // invoke the constructor (placement/heap) in one shot
+    // flag is non-null for placement new, null for normal new
+    void* flag = arena ? reinterpret_cast<void*>(1) : nullptr;
+    void* result = arena;
+    JC.InvokeConstructor(&result, count, {}, flag);
+    return result;
   }
   return nullptr;
 }

unittests/CppInterOp/FunctionReflectionTest.cpp

@@ -2107,20 +2107,24 @@ TEST(FunctionReflectionTest, Construct) {
   GTEST_SKIP() << "Disabled on Windows. Needs fixing.";
 #endif
   std::vector<const char*> interpreter_args = {"-include", "new"};
-  Cpp::CreateInterpreter(interpreter_args);
+  std::vector<Decl*> Decls, SubDecls;
 
-  Interp->declare(R"(
+  std::string code = R"(
     #include <new>
     extern "C" int printf(const char*,...);
     class C {
+    public:
       int x;
       C() {
         x = 12345;
         printf("Constructor Executed");
       }
     };
-    )");
+    void construct() { return; }
+    )";
 
+  GetAllTopLevelDecls(code, Decls, false, interpreter_args);
+  GetAllSubDecls(Decls[1], SubDecls);
   testing::internal::CaptureStdout();
   Cpp::TCppScope_t scope = Cpp::GetNamed("C");
   Cpp::TCppObject_t object = Cpp::Construct(scope);
@@ -2140,6 +2144,20 @@ TEST(FunctionReflectionTest, Construct) {
   EXPECT_EQ(output, "Constructor Executed");
   output.clear();
 
+  // Pass a constructor
+  testing::internal::CaptureStdout();
+  where = Cpp::Allocate(scope);
+  EXPECT_TRUE(where == Cpp::Construct(SubDecls[3], where));
+  EXPECT_TRUE(*(int*)where == 12345);
+  Cpp::Deallocate(scope, where);
+  output = testing::internal::GetCapturedStdout();
+  EXPECT_EQ(output, "Constructor Executed");
+  output.clear();
+
+  // Pass a non-class decl, this should fail
+  where = Cpp::Allocate(scope);
+  where = Cpp::Construct(Decls[2], where);
+  EXPECT_TRUE(where == nullptr);
   // C API
   testing::internal::CaptureStdout();
   auto* I = clang_createInterpreterFromRawPtr(Cpp::GetInterpreter());