From 6b85ed29365e9bcec40cf5d72664dd740c2e7f35 Mon Sep 17 00:00:00 2001 From: ThePhD Date: Mon, 11 Jul 2016 12:41:17 -0400 Subject: [PATCH] At least I have users who can help me catch my dumbness. One day, I'll find someone who wants to work on something and isn't somehow insufferable. --- sol/function_types.hpp | 23 ++++++++++++++--- sol/raii.hpp | 2 +- sol/simple_usertype_metatable.hpp | 27 +++++++++++++++++--- sol/usertype_metatable.hpp | 15 +++++------ test_simple_usertypes.cpp | 41 ++++++++++++++++++++++++++++++- test_usertypes.cpp | 40 ++++++++++++++++++++++++++++++ 6 files changed, 133 insertions(+), 15 deletions(-) diff --git a/sol/function_types.hpp b/sol/function_types.hpp index 59a9c468..c1606185 100644 --- a/sol/function_types.hpp +++ b/sol/function_types.hpp @@ -286,15 +286,15 @@ namespace sol { }; template - struct pusher>> { - static int push(lua_State* L, detail::constructors_for>) { + struct pusher>> { + static int push(lua_State* L, detail::tagged>) { lua_CFunction cf = call_detail::construct; return stack::push(L, cf); } }; template - struct pusher>> { + struct pusher>> { static int push(lua_State* L, constructor_wrapper c) { lua_CFunction cf = call_detail::call_user>; int closures = stack::push>(L, std::move(c)); @@ -302,6 +302,23 @@ namespace sol { } }; + template + struct pusher>> { + static int push(lua_State* L, detail::tagged>) { + lua_CFunction cf = call_detail::destruct; + return stack::push(L, cf); + } + }; + + template + struct pusher>> { + static int push(lua_State* L, destructor_wrapper c) { + lua_CFunction cf = call_detail::call_user>; + int closures = stack::push>(L, std::move(c)); + return stack::push(L, c_closure(cf, closures)); + } + }; + } // stack } // sol diff --git a/sol/raii.hpp b/sol/raii.hpp index c3effa74..a185bdab 100644 --- a/sol/raii.hpp +++ b/sol/raii.hpp @@ -66,7 +66,7 @@ namespace sol { } template - struct constructors_for { + struct tagged { List l; }; } // detail diff --git a/sol/simple_usertype_metatable.hpp b/sol/simple_usertype_metatable.hpp index 0d10e7c8..a163f7a4 100644 --- a/sol/simple_usertype_metatable.hpp +++ b/sol/simple_usertype_metatable.hpp @@ -36,19 +36,24 @@ namespace sol { std::vector> registrations; object callconstructfunc; - template + template >> = meta::enabler> + void add(lua_State* L, N&& n, F&& f) { + registrations.emplace_back(make_object(L, std::forward(n)), make_object(L, function_args(std::forward(f)))); + } + + template >> = meta::enabler> void add(lua_State* L, N&& n, F&& f) { registrations.emplace_back(make_object(L, std::forward(n)), make_object(L, std::forward(f))); } template void add(lua_State* L, N&& n, constructor_wrapper c) { - registrations.emplace_back(make_object(L, std::forward(n)), make_object(L, detail::constructors_for>{std::move(c)})); + registrations.emplace_back(make_object(L, std::forward(n)), make_object(L, detail::tagged>{std::move(c)})); } template void add(lua_State* L, N&& n, constructor_list c) { - registrations.emplace_back(make_object(L, std::forward(n)), make_object(L, detail::constructors_for>{std::move(c)})); + registrations.emplace_back(make_object(L, std::forward(n)), make_object(L, detail::tagged>{std::move(c)})); } template @@ -123,6 +128,22 @@ namespace sol { luaL_newmetatable(L, metakey); stack_reference t(L, -1); for (auto& kvp : umx.registrations) { + switch (i) { + case 0: + if (kvp.first.template is() && kvp.first.template as() == "__gc") { + continue; + } + break; + case 1: + if (kvp.first.template is() && kvp.first.template as() == "__gc") { + stack::set_field(L, kvp.first, detail::unique_destruct, t.stack_index()); + continue; + } + break; + case 2: + default: + break; + } stack::set_field(L, kvp.first, kvp.second, t.stack_index()); } diff --git a/sol/usertype_metatable.hpp b/sol/usertype_metatable.hpp index c58c3012..b6aebef7 100644 --- a/sol/usertype_metatable.hpp +++ b/sol/usertype_metatable.hpp @@ -349,10 +349,12 @@ namespace sol { (void)detail::swallow{ 0, (um.template make_regs<(I * 2)>(value_table, lastreg, std::get<(I * 2)>(um.functions), std::get<(I * 2 + 1)>(um.functions)), 0)... }; um.finish_regs(value_table, lastreg); value_table[lastreg] = { nullptr, nullptr }; + bool hasdestructor = !value_table.empty() && name_of(meta_function::garbage_collect) == value_table[lastreg - 1].name; regs_t ref_table = value_table; - bool hasdestructor = lastreg > 0 && name_of(meta_function::garbage_collect) == ref_table[lastreg - 1].name; + regs_t unique_table = value_table; if (hasdestructor) { ref_table[lastreg - 1] = { nullptr, nullptr }; + unique_table[lastreg - 1] = { value_table[lastreg - 1].name, detail::unique_destruct }; } // Now use um @@ -360,27 +362,26 @@ namespace sol { for (std::size_t i = 0; i < 3; ++i) { // Pointer types, AKA "references" from C++ const char* metakey = nullptr; + luaL_Reg* metaregs = nullptr; switch (i) { case 0: metakey = &usertype_traits::metatable[0]; + metaregs = ref_table.data(); break; case 1: metakey = &usertype_traits>::metatable[0]; + metaregs = unique_table.data(); break; case 2: default: metakey = &usertype_traits::metatable[0]; + metaregs = value_table.data(); break; } luaL_newmetatable(L, metakey); stack_reference t(L, -1); stack::push(L, make_light(um)); - if (i < 2) { - luaL_setfuncs(L, ref_table.data(), 1); - } - else { - luaL_setfuncs(L, value_table.data(), 1); - } + luaL_setfuncs(L, metaregs, 1); if (um.baseclasscheck != nullptr) { stack::set_field(L, detail::base_class_check_key(), um.baseclasscheck, t.stack_index()); diff --git a/test_simple_usertypes.cpp b/test_simple_usertypes.cpp index 5bf62ed8..3a04ee61 100644 --- a/test_simple_usertypes.cpp +++ b/test_simple_usertypes.cpp @@ -4,9 +4,11 @@ #include #include +#include +#include #include -TEST_CASE("usertypes/simple_usertypes", "Ensure that simple usertypes properly work here") { +TEST_CASE("usertypes/simple-usertypes", "Ensure that simple usertypes properly work here") { struct marker { bool value = false; }; @@ -170,3 +172,40 @@ TEST_CASE("usertypes/simple-usertypes-constructors", "Ensure that calls with spe REQUIRE(z == 29); } +TEST_CASE("usertype/simple-shared-ptr-regression", "simple usertype metatables should not screw over unique usertype metatables") { + static int created = 0; + static int destroyed = 0; + struct test { + test() { + ++created; + } + + ~test() { + ++destroyed; + } + }; + { + std::list> tests; + sol::state lua; + lua.open_libraries(); + + lua.new_simple_usertype("test", + "create", [&]() -> std::shared_ptr { + tests.push_back(std::make_shared()); + return tests.back(); + } + ); + REQUIRE(created == 0); + REQUIRE(destroyed == 0); + lua.script("x = test.create()"); + REQUIRE(created == 1); + REQUIRE(destroyed == 0); + REQUIRE_FALSE(tests.empty()); + std::shared_ptr& x = lua["x"]; + std::size_t xuse = x.use_count(); + std::size_t tuse = tests.back().use_count(); + REQUIRE(xuse == tuse); + } + REQUIRE(created == 1); + REQUIRE(destroyed == 1); +} diff --git a/test_usertypes.cpp b/test_usertypes.cpp index 12b30d52..6e9ce29b 100644 --- a/test_usertypes.cpp +++ b/test_usertypes.cpp @@ -4,6 +4,8 @@ #include #include +#include +#include #include struct vars { @@ -1085,3 +1087,41 @@ value = pcall(pm.gen,pm) bool value = lua["value"]; REQUIRE_FALSE(value); } + +TEST_CASE("usertype/shared-ptr-regression", "usertype metatables should not screw over unique usertype metatables") { + static int created = 0; + static int destroyed = 0; + struct test { + test() { + ++created; + } + + ~test() { + ++destroyed; + } + }; + { + std::list> tests; + sol::state lua; + lua.open_libraries(); + + lua.new_usertype("test", + "create", [&]() -> std::shared_ptr { + tests.push_back(std::make_shared()); + return tests.back(); + } + ); + REQUIRE(created == 0); + REQUIRE(destroyed == 0); + lua.script("x = test.create()"); + REQUIRE(created == 1); + REQUIRE(destroyed == 0); + REQUIRE_FALSE(tests.empty()); + std::shared_ptr& x = lua["x"]; + std::size_t xuse = x.use_count(); + std::size_t tuse = tests.back().use_count(); + REQUIRE(xuse == tuse); + } + REQUIRE(created == 1); + REQUIRE(destroyed == 1); +}