Fix issue with placement and factory constructors

Source/LuaBridge/detail/CFunctions.h

@@ -793,6 +793,26 @@ int invoke_proxy_functor(lua_State* L)
     return function<typename FnTraits::result_type, typename FnTraits::argument_types, 1>::call(L, func);
 }
 
+//=================================================================================================
+/**
+ * @brief lua_CFunction to call on a object constructor via functor (lambda wrapped in a std::function).
+ *
+ * The proxy std::function (lightuserdata) is in the first upvalue. The class userdata object will be pushed at the top of the Lua stack.
+ */
+template <class F>
+int invoke_proxy_constructor(lua_State* L)
+{
+    using FnTraits = function_traits<F>;
+
+    LUABRIDGE_ASSERT(isfulluserdata(L, lua_upvalueindex(1)));
+
+    auto& func = *align<F>(lua_touserdata(L, lua_upvalueindex(1)));
+
+    function<void, typename FnTraits::argument_types, 1>::call(L, func);
+
+    return 1;
+}
+
 //=================================================================================================
 /**
  * @brief lua_CFunction to resolve an invocation between several overloads.
@@ -1054,25 +1074,6 @@ void push_member_function(lua_State* L, int (U::*mfp)(lua_State*) const)
  * These templates call operator new with the contents of a type/value list passed to the constructor. Two versions of call() are provided.
  * One performs a regular new, the other performs a placement new.
  */
-template <class T, class Args>
-struct constructor;
-
-template <class T>
-struct constructor<T, void>
-{
-    using empty = std::tuple<>;
-
-    static T* call(const empty&)
-    {
-        return new T;
-    }
-
-    static T* call(void* ptr, const empty&)
-    {
-        return new (ptr) T;
-    }
-};
-
 template <class T, class Args>
 struct constructor
 {
@@ -1105,12 +1106,6 @@ struct placement_constructor
 
         return std::apply(alloc, args);
     }
-
-    template <class F>
-    static T* construct(void* ptr, const F& func)
-    {
-        return func(ptr);
-    }
 };
 
 //=================================================================================================
@@ -1127,12 +1122,6 @@ struct external_constructor
 
         return std::apply(alloc, args);
     }
-
-    template <class F>
-    static T* construct(const F& func)
-    {
-        return func();
-    }
 };
 
 //=================================================================================================

Source/LuaBridge/detail/Namespace.h

@@ -1148,7 +1148,10 @@ class Namespace : public detail::Registrar
             {
                 ([&]
                 {
-                    detail::push_function(L, detail::constructor_forwarder<T, Functions>(std::move(functions))); // Stack: co, cl, st, function
+                    using F = detail::constructor_forwarder<T, Functions>;
+
+                    lua_newuserdata_aligned<F>(L, F(std::move(functions))); // Stack: co, cl, st, upvalue
+                    lua_pushcclosure_x(L, &detail::invoke_proxy_constructor<F>, 1); // Stack: co, cl, st, function
 
                 } (), ...);
             }
@@ -1161,6 +1164,8 @@ class Namespace : public detail::Registrar
 
                 ([&]
                 {
+                    using F = detail::constructor_forwarder<T, Functions>;
+
                     lua_createtable(L, 2, 0); // reserve space for: function, arity
                     lua_pushinteger(L, 1);
                     if constexpr (detail::is_any_cfunction_pointer_v<Functions>)
@@ -1169,7 +1174,8 @@ class Namespace : public detail::Registrar
                         lua_pushinteger(L, static_cast<int>(detail::function_arity_excluding_v<Functions, lua_State*>) - 1); // 1: for void* ptr
                     lua_settable(L, -3);
                     lua_pushinteger(L, 2);
-                    detail::push_function(L, detail::constructor_forwarder<T, Functions>(std::move(functions)));
+                    lua_newuserdata_aligned<F>(L, F(std::move(functions)));
+                    lua_pushcclosure_x(L, &detail::invoke_proxy_constructor<F>, 1);
                     lua_settable(L, -3);
                     lua_rawseti(L, -2, idx);
                     ++idx;
@@ -1198,7 +1204,10 @@ class Namespace : public detail::Registrar
         {
             assertStackState(); // Stack: const table (co), class table (cl), static table (st)
 
-            detail::push_function(L, detail::factory_forwarder<T, Allocator, Deallocator>(std::move(allocator), std::move(deallocator)));
+            using F = detail::factory_forwarder<T, Allocator, Deallocator>;
+
+            lua_newuserdata_aligned<F>(L, F(std::move(allocator), std::move(deallocator))); // Stack: co, cl, st, upvalue
+            lua_pushcclosure_x(L, &detail::invoke_proxy_constructor<F>, 1); // Stack: co, cl, st, function
             rawsetfield(L, -2, "__call"); // Stack: co, cl, st
 
             return *this;

Tests/Source/ClassTests.cpp

@@ -2885,3 +2885,162 @@ TEST_F(ClassTests, WrongThrowBadArgObjectDescription)
 
 #endif
 }
+
+namespace {
+struct Node
+{
+    int data = 0;
+    Node* next = nullptr;
+    Node* prev = nullptr;
+
+    explicit Node(int value)
+        : data(value)
+    {
+    }
+
+    int val() const { return data; }
+    Node* next_node() const { return next; }
+};
+
+class DoublyLinkedList
+{
+public:
+    DoublyLinkedList(int numToAdd = 0)
+    {
+        for (int value = 1; value <= numToAdd; value++)
+            addBack(value);
+    }
+
+    ~DoublyLinkedList()
+    {
+        while (head)
+            removeBack();
+    }
+
+    void addBack(int value)
+    {
+        Node* newNode = new Node(value);
+        if (tail == nullptr)
+        {
+            head = newNode;
+            tail = newNode;
+        }
+        else
+        {
+            newNode->prev = tail;
+            tail->next = newNode;
+            tail = newNode;
+        }
+    }
+
+    void removeBack()
+    {
+        if (tail == nullptr)
+            return;
+
+        Node* temp = tail;
+        tail = tail->prev;
+
+        if (tail != nullptr)
+            tail->next = nullptr;
+        else
+            head = nullptr;
+
+        delete temp;
+    }
+
+    Node* first() const { return head; }
+
+private:
+    Node* head = nullptr;
+    Node* tail = nullptr;
+};
+
+class foo
+{
+public:
+    foo(int size = 5000) : list(size) {}
+
+    Node* first() const { return list.first(); }
+    Node* next(Node* curr) const { return curr->next_node(); }
+
+private:
+    DoublyLinkedList list;
+};
+
+class bar
+{
+public:
+   bar(foo* f) : foo_(f) {}
+
+   Node* first() const { return foo_->first(); }
+   Node* next(Node* curr) const { return foo_->next(curr); }
+
+private:
+   foo* foo_;
+};
+} // namespace
+
+TEST_F(ClassTests, BugWithPlacementConstructor)
+{
+    luabridge::getGlobalNamespace(L)
+        .beginNamespace("foobar")
+            .beginClass<Node>("Node")
+                .addFunction("val", &Node::val)
+            .endClass()
+            .beginClass<foo>("foo")
+                .addConstructor([](void* p, int size) { return new(p) foo(size); })
+                .addFunction("first", &foo::first)
+                .addFunction("next", &foo::next)
+            .endClass()
+            .beginClass<bar>("bar")
+                .addConstructor<void(*)(foo *)>()
+                .addFunction("first", &bar::first)
+                .addFunction("next", &bar::next)
+            .endClass()
+        .endNamespace();
+
+    runLua(R"(
+        local foo = foobar.foo(5000)
+        local bar = foobar.bar(foo)
+        local next = bar:first()
+        while next do
+            next = bar:next(next)
+        end
+    )");
+
+    SUCCEED();
+}
+
+TEST_F(ClassTests, BugWithFactoryConstructor)
+{
+    luabridge::getGlobalNamespace(L)
+        .beginNamespace("foobar")
+            .beginClass<Node>("Node")
+                .addFunction("val", &Node::val)
+            .endClass()
+            .beginClass<foo>("foo")
+                .addFactory(
+                    +[]() -> foo* { return new foo(5000); },
+                    +[](foo* x) { delete x; })
+                .addFunction("first", &foo::first)
+                .addFunction("next", &foo::next)
+            .endClass()
+            .beginClass<bar>("bar")
+                .addConstructor<void(*)(foo *)>()
+                .addFunction("first", &bar::first)
+                .addFunction("next", &bar::next)
+            .endClass()
+        .endNamespace();
+
+    runLua(R"(
+        local foo = foobar.foo()
+        local bar = foobar.bar(foo)
+        local next = bar:first()
+        while next do
+            next = bar:next(next)
+        end
+    )");
+
+    SUCCEED();
+}