diff --git a/Source/LuaBridge/detail/CFunctions.h b/Source/LuaBridge/detail/CFunctions.h index 7a1352f1..a4a2eb9c 100644 --- a/Source/LuaBridge/detail/CFunctions.h +++ b/Source/LuaBridge/detail/CFunctions.h @@ -793,6 +793,26 @@ int invoke_proxy_functor(lua_State* L) return function::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 +int invoke_proxy_constructor(lua_State* L) +{ + using FnTraits = function_traits; + + LUABRIDGE_ASSERT(isfulluserdata(L, lua_upvalueindex(1))); + + auto& func = *align(lua_touserdata(L, lua_upvalueindex(1))); + + function::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 -struct constructor; - -template -struct constructor -{ - using empty = std::tuple<>; - - static T* call(const empty&) - { - return new T; - } - - static T* call(void* ptr, const empty&) - { - return new (ptr) T; - } -}; - template struct constructor { @@ -1105,12 +1106,6 @@ struct placement_constructor return std::apply(alloc, args); } - - template - static T* construct(void* ptr, const F& func) - { - return func(ptr); - } }; //================================================================================================= @@ -1127,12 +1122,6 @@ struct external_constructor return std::apply(alloc, args); } - - template - static T* construct(const F& func) - { - return func(); - } }; //================================================================================================= diff --git a/Source/LuaBridge/detail/Namespace.h b/Source/LuaBridge/detail/Namespace.h index 82e4b41e..81f80d24 100644 --- a/Source/LuaBridge/detail/Namespace.h +++ b/Source/LuaBridge/detail/Namespace.h @@ -1148,7 +1148,10 @@ class Namespace : public detail::Registrar { ([&] { - detail::push_function(L, detail::constructor_forwarder(std::move(functions))); // Stack: co, cl, st, function + using F = detail::constructor_forwarder; + + lua_newuserdata_aligned(L, F(std::move(functions))); // Stack: co, cl, st, upvalue + lua_pushcclosure_x(L, &detail::invoke_proxy_constructor, 1); // Stack: co, cl, st, function } (), ...); } @@ -1161,6 +1164,8 @@ class Namespace : public detail::Registrar ([&] { + using F = detail::constructor_forwarder; + lua_createtable(L, 2, 0); // reserve space for: function, arity lua_pushinteger(L, 1); if constexpr (detail::is_any_cfunction_pointer_v) @@ -1169,7 +1174,8 @@ class Namespace : public detail::Registrar lua_pushinteger(L, static_cast(detail::function_arity_excluding_v) - 1); // 1: for void* ptr lua_settable(L, -3); lua_pushinteger(L, 2); - detail::push_function(L, detail::constructor_forwarder(std::move(functions))); + lua_newuserdata_aligned(L, F(std::move(functions))); + lua_pushcclosure_x(L, &detail::invoke_proxy_constructor, 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(std::move(allocator), std::move(deallocator))); + using F = detail::factory_forwarder; + + lua_newuserdata_aligned(L, F(std::move(allocator), std::move(deallocator))); // Stack: co, cl, st, upvalue + lua_pushcclosure_x(L, &detail::invoke_proxy_constructor, 1); // Stack: co, cl, st, function rawsetfield(L, -2, "__call"); // Stack: co, cl, st return *this; diff --git a/Tests/Source/ClassTests.cpp b/Tests/Source/ClassTests.cpp index 0d434787..42cf36af 100644 --- a/Tests/Source/ClassTests.cpp +++ b/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") + .addFunction("val", &Node::val) + .endClass() + .beginClass("foo") + .addConstructor([](void* p, int size) { return new(p) foo(size); }) + .addFunction("first", &foo::first) + .addFunction("next", &foo::next) + .endClass() + .beginClass("bar") + .addConstructor() + .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") + .addFunction("val", &Node::val) + .endClass() + .beginClass("foo") + .addFactory( + +[]() -> foo* { return new foo(5000); }, + +[](foo* x) { delete x; }) + .addFunction("first", &foo::first) + .addFunction("next", &foo::next) + .endClass() + .beginClass("bar") + .addConstructor() + .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(); +}