From eecf8b8c8f2d0cb15dc1f54592a3d225e27cf2e3 Mon Sep 17 00:00:00 2001 From: kunitoki Date: Tue, 24 Nov 2020 00:11:51 +0100 Subject: [PATCH 01/10] Added support for passing around reference wrappers --- Source/LuaBridge/detail/CFunctions.h | 10 ++++---- Source/LuaBridge/detail/Stack.h | 35 ++++++++++++++++++++++++++++ Tests/Source/ClassTests.cpp | 24 +++++++++++++++++++ 3 files changed, 64 insertions(+), 5 deletions(-) diff --git a/Source/LuaBridge/detail/CFunctions.h b/Source/LuaBridge/detail/CFunctions.h index e720fc89..ceba1a6f 100644 --- a/Source/LuaBridge/detail/CFunctions.h +++ b/Source/LuaBridge/detail/CFunctions.h @@ -209,7 +209,7 @@ struct property_getter } }; -/* +#if 0 template struct property_getter, void> { @@ -225,7 +225,7 @@ struct property_getter, void> return 1; } }; -*/ +#endif /** * @brief lua_CFunction to get a class data member. @@ -295,7 +295,7 @@ struct property_setter } }; -/* +#if 0 template struct property_setter, void> { @@ -311,8 +311,8 @@ struct property_setter, void> return 0; } }; -*/ - +#endif + /** * @brief lua_CFunction to set a class data member. * diff --git a/Source/LuaBridge/detail/Stack.h b/Source/LuaBridge/detail/Stack.h index 1ff0fc65..71f4a1e6 100644 --- a/Source/LuaBridge/detail/Stack.h +++ b/Source/LuaBridge/detail/Stack.h @@ -567,6 +567,41 @@ struct Stack> } }; +//================================================================================================= +/** + * @brief Stack specialization for `std::reference_wrapper`. + */ +template +struct Stack> +{ + static void push(lua_State* L, const std::reference_wrapper& ref) + { + std::reference_wrapper* ptr = reinterpret_cast*>(lua_newuserdata_aligned>(L, ref.get())); + + lua_newtable(L); + lua_pushcfunction(L, &lua_deleteuserdata_aligned>); + rawsetfield(L, -2, "__gc"); + lua_setmetatable(L, -2); + + return ptr; + } + + static std::reference_wrapper get(lua_State* L, int index) + { + assert(lua_isuserdata(L, index)); + + std::reference_wrapper* ptr = reinterpret_cast*>(lua_touserdata(L, index)); + assert(ptr != nullptr); + + return *ptr; + } + + static bool isInstance(lua_State* L, int index) + { + return lua_type(L, index) == LUA_TUSERDATA; + } +}; + namespace detail { template diff --git a/Tests/Source/ClassTests.cpp b/Tests/Source/ClassTests.cpp index 52cdfa22..c169bfc5 100644 --- a/Tests/Source/ClassTests.cpp +++ b/Tests/Source/ClassTests.cpp @@ -1886,3 +1886,27 @@ TEST_F(ClassTests, NonVirtualMethodInBaseClassCannotBeExposed) lua_close(L); L = nullptr; } + +TEST_F(ClassTests, ReferenceWrapper) +{ + int x = 13; + std::reference_wrapper ref_wrap_x(x); + + luabridge::getGlobalNamespace(L) + .beginNamespace("test") + .addProperty("ref_wrap_x", &ref_wrap_x) + .addFunction("changeReference", [](std::reference_wrapper r) { r.get() = 100; }) + .endNamespace(); + + runLua(R"( + result = test.ref_wrap_x + test.changeReference(test.ref_wrap_x) + )"); + + EXPECT_TRUE(result().isUserdata()); + EXPECT_EQ(x, result().cast>().get()); + EXPECT_EQ(100, x); + + lua_close(L); + L = nullptr; +} From a6997e8cbec7f0ec6495a294410aec5e074e5b77 Mon Sep 17 00:00:00 2001 From: kunitoki Date: Tue, 24 Nov 2020 00:33:34 +0100 Subject: [PATCH 02/10] More testing --- Source/LuaBridge/detail/LuaRef.h | 2 +- Source/LuaBridge/detail/Namespace.h | 2 +- Tests/Source/ClassTests.cpp | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Source/LuaBridge/detail/LuaRef.h b/Source/LuaBridge/detail/LuaRef.h index 9ba9034a..a46a3063 100644 --- a/Source/LuaBridge/detail/LuaRef.h +++ b/Source/LuaBridge/detail/LuaRef.h @@ -1041,7 +1041,7 @@ struct Stack * @brief Stack specialization for `TableItem`. */ template <> -struct Stack +struct Stack { static void push(lua_State* L, const LuaRef::TableItem& v) { diff --git a/Source/LuaBridge/detail/Namespace.h b/Source/LuaBridge/detail/Namespace.h index c5e1524f..5fb3de7c 100644 --- a/Source/LuaBridge/detail/Namespace.h +++ b/Source/LuaBridge/detail/Namespace.h @@ -376,7 +376,7 @@ class Namespace : public detail::Registrar createStaticTable(name); // Stack: ns, co, cl, st ++m_stackSize; - lua_rawgetp( L, LUA_REGISTRYINDEX, staticKey); // Stack: ns, co, cl, st, parent st (pst) | nil + lua_rawgetp(L, LUA_REGISTRYINDEX, staticKey); // Stack: ns, co, cl, st, parent st (pst) | nil if (lua_isnil(L, -1)) // Stack: ns, co, cl, st, nil { ++m_stackSize; diff --git a/Tests/Source/ClassTests.cpp b/Tests/Source/ClassTests.cpp index c169bfc5..faeabb3f 100644 --- a/Tests/Source/ClassTests.cpp +++ b/Tests/Source/ClassTests.cpp @@ -1900,7 +1900,7 @@ TEST_F(ClassTests, ReferenceWrapper) runLua(R"( result = test.ref_wrap_x - test.changeReference(test.ref_wrap_x) + test.changeReference(result) )"); EXPECT_TRUE(result().isUserdata()); From 83dac3b39d3e83b8e40902058db476b3070c7f55 Mon Sep 17 00:00:00 2001 From: kunitoki Date: Tue, 24 Nov 2020 00:36:15 +0100 Subject: [PATCH 03/10] Fix compilation --- Source/LuaBridge/detail/Stack.h | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/Source/LuaBridge/detail/Stack.h b/Source/LuaBridge/detail/Stack.h index 71f4a1e6..045ebeca 100644 --- a/Source/LuaBridge/detail/Stack.h +++ b/Source/LuaBridge/detail/Stack.h @@ -576,14 +576,12 @@ struct Stack> { static void push(lua_State* L, const std::reference_wrapper& ref) { - std::reference_wrapper* ptr = reinterpret_cast*>(lua_newuserdata_aligned>(L, ref.get())); + lua_newuserdata_aligned>(L, ref.get()); lua_newtable(L); lua_pushcfunction(L, &lua_deleteuserdata_aligned>); rawsetfield(L, -2, "__gc"); lua_setmetatable(L, -2); - - return ptr; } static std::reference_wrapper get(lua_State* L, int index) From 40bea78acb0e9d5c084dfeb1fa7c7a9591f0abe4 Mon Sep 17 00:00:00 2001 From: kunitoki Date: Sun, 16 Oct 2022 00:15:36 +0200 Subject: [PATCH 04/10] More reference_wrapper handling --- Source/LuaBridge/detail/CFunctions.h | 54 ++++++++--------- Source/LuaBridge/detail/LuaHelpers.h | 15 ++++- Source/LuaBridge/detail/Stack.h | 40 ++++++++++--- Tests/Source/ClassTests.cpp | 86 ++++++++++++++++++++++++++-- third_party/gtest | 1 + 5 files changed, 155 insertions(+), 41 deletions(-) create mode 160000 third_party/gtest diff --git a/Source/LuaBridge/detail/CFunctions.h b/Source/LuaBridge/detail/CFunctions.h index 34382375..8c6b2bef 100644 --- a/Source/LuaBridge/detail/CFunctions.h +++ b/Source/LuaBridge/detail/CFunctions.h @@ -30,10 +30,21 @@ template auto unwrap_argument_or_error(lua_State* L, std::size_t index) { auto result = Stack::get(L, static_cast(index)); - if (! result) - luaL_error(L, "Error decoding argument #%d: %s", static_cast(index), result.message().c_str()); + if (result) + return std::move(*result); + + if constexpr (! std::is_lvalue_reference_v) + { + using U = std::reference_wrapper>; + + auto resultRef = Stack::get(L, static_cast(index)); + if (resultRef) + return (*resultRef).get(); + } + + luaL_error(L, "Error decoding argument #%d: %s", static_cast(index), result.message().c_str()); - return std::move(*result); + unreachable(); } template @@ -335,26 +346,6 @@ struct property_getter } }; -#if 0 -template -struct property_getter, void> -{ - static int call(lua_State* L) - { - assert(lua_islightuserdata(L, lua_upvalueindex(1))); - - std::reference_wrapper* ptr = static_cast*>(lua_touserdata(L, lua_upvalueindex(1))); - assert(ptr != nullptr); - - auto result = Stack::push(L, ptr->get()); - if (! result) - luaL_error(L, "%s", result.message().c_str()); - - return 1; - } -}; -#endif - /** * @brief lua_CFunction to get a class data member. * @@ -441,7 +432,6 @@ struct property_setter } }; -#if 0 template struct property_setter, void> { @@ -452,12 +442,22 @@ struct property_setter, void> std::reference_wrapper* ptr = static_cast*>(lua_touserdata(L, lua_upvalueindex(1))); assert(ptr != nullptr); - ptr->get() = Stack::get(L, 1); + auto result = Stack*>::get(L, 1); + if (result && *result) + { + *ptr = (*result).get(); + return 0; + } + else + { + auto result = Stack::get(L, 1); + if (result) + ptr->get() = std::move(*result); - return 0; + return 0; + } } }; -#endif /** * @brief lua_CFunction to set a class data member. diff --git a/Source/LuaBridge/detail/LuaHelpers.h b/Source/LuaBridge/detail/LuaHelpers.h index 6df5ef3a..86846ddc 100644 --- a/Source/LuaBridge/detail/LuaHelpers.h +++ b/Source/LuaBridge/detail/LuaHelpers.h @@ -427,7 +427,7 @@ template * @brief Deallocate lua userdata taking into account alignment. */ template -int lua_deleteuserdata_aligned(lua_State* L) +int lua_destroyuserdata_aligned(lua_State* L) { assert(isfulluserdata(L, 1)); @@ -437,6 +437,17 @@ int lua_deleteuserdata_aligned(lua_State* L) return 0; } +template +int lua_deleteuserdata_aligned(lua_State* L) +{ + assert(isfulluserdata(L, 1)); + + T** aligned = align(lua_touserdata(L, 1)); + delete *aligned; + + return 0; +} + /** * @brief Allocate lua userdata taking into account alignment. * @@ -455,7 +466,7 @@ void* lua_newuserdata_aligned(lua_State* L, Args&&... args) void* pointer = lua_newuserdata_x(L, maximum_space_needed_to_align()); lua_newtable(L); - lua_pushcfunction_x(L, &lua_deleteuserdata_aligned); + lua_pushcfunction_x(L, &lua_destroyuserdata_aligned); rawsetfield(L, -2, "__gc"); lua_setmetatable(L, -2); #endif diff --git a/Source/LuaBridge/detail/Stack.h b/Source/LuaBridge/detail/Stack.h index b15412c7..95676ae2 100644 --- a/Source/LuaBridge/detail/Stack.h +++ b/Source/LuaBridge/detail/Stack.h @@ -1341,30 +1341,54 @@ struct Stack template struct Stack> { - static void push(lua_State* L, const std::reference_wrapper& ref) + using storage_type = std::reference_wrapper*; + + static Result push(lua_State* L, const std::reference_wrapper& ref) { - lua_newuserdata_aligned>(L, ref.get()); + lua_newuserdata_aligned(L, new std::reference_wrapper(ref.get())); lua_newtable(L); - lua_pushcfunction(L, &lua_deleteuserdata_aligned>); + lua_pushcclosure_x(L, &get_reference_value, 0); + rawsetfield(L, -2, "__call"); + lua_pushcfunction(L, &lua_deleteuserdata_aligned); rawsetfield(L, -2, "__gc"); lua_setmetatable(L, -2); + + return {}; } - static std::reference_wrapper get(lua_State* L, int index) + static TypeResult> get(lua_State* L, int index) { - assert(lua_isuserdata(L, index)); + if (! lua_isuserdata(L, index)) + return makeErrorCode(ErrorCode::InvalidTypeCast); - std::reference_wrapper* ptr = reinterpret_cast*>(lua_touserdata(L, index)); - assert(ptr != nullptr); + storage_type* ptr = reinterpret_cast(lua_touserdata(L, index)); + if (ptr == nullptr || *ptr == nullptr) + return makeErrorCode(ErrorCode::InvalidTypeCast); - return *ptr; + return **ptr; } static bool isInstance(lua_State* L, int index) { return lua_type(L, index) == LUA_TUSERDATA; } + +private: + template + static int get_reference_value(lua_State* L) + { + assert(lua_isuserdata(L, -1)); + + std::reference_wrapper** ptr = static_cast**>(lua_touserdata(L, -1)); + assert(ptr != nullptr); + + auto result = Stack::push(L, (*ptr)->get()); + if (! result) + luaL_error(L, "%s", result.message().c_str()); + + return 1; + } }; namespace detail { diff --git a/Tests/Source/ClassTests.cpp b/Tests/Source/ClassTests.cpp index f7d5a641..753658d6 100644 --- a/Tests/Source/ClassTests.cpp +++ b/Tests/Source/ClassTests.cpp @@ -2393,7 +2393,7 @@ TEST_F(ClassTests, NewIndexFallbackMetaMethodFreeFunctor) ASSERT_EQ(246, result()); } -TEST_F(ClassTests, ReferenceWrapper) +TEST_F(ClassTests, ReferenceWrapperRead) { int x = 13; std::reference_wrapper ref_wrap_x(x); @@ -2410,9 +2410,87 @@ TEST_F(ClassTests, ReferenceWrapper) )"); EXPECT_TRUE(result().isUserdata()); - EXPECT_EQ(x, result().cast>().get()); + EXPECT_EQ(x, result().unsafe_cast>().get()); EXPECT_EQ(100, x); +} + +TEST_F(ClassTests, ReferenceWrapperWrite) +{ + int x = 13; + std::reference_wrapper ref_wrap_x(x); + + luabridge::getGlobalNamespace(L) + .beginNamespace("test") + .addProperty("ref_wrap_x", &ref_wrap_x) + .endNamespace(); + + runLua(R"( + test.ref_wrap_x = 100 + result = test.ref_wrap_x + )"); + + EXPECT_TRUE(result().isUserdata()); + EXPECT_EQ(x, result().unsafe_cast>().get()); + EXPECT_EQ(100, x); +} + +TEST_F(ClassTests, ReferenceWrapperRedirect) +{ + int x = 13; + int y = 100; + std::reference_wrapper ref_wrap_x(x); + std::reference_wrapper ref_wrap_y(y); + + luabridge::getGlobalNamespace(L) + .beginNamespace("test") + .addProperty("ref_wrap_x", &ref_wrap_x) + .addProperty("ref_wrap_y", &ref_wrap_y) + .endNamespace(); + + runLua(R"( + test.ref_wrap_x = test.ref_wrap_y + result = test.ref_wrap_x + )"); + + EXPECT_TRUE(result().isUserdata()); + EXPECT_EQ(y, result().unsafe_cast>().get()); +} + +TEST_F(ClassTests, ReferenceWrapperDecaysToType) +{ + int x = 13; + std::reference_wrapper ref_wrap_x(x); + + luabridge::getGlobalNamespace(L) + .beginNamespace("test") + .addProperty("ref_wrap_x", &ref_wrap_x) + .addFunction("takeReference", [](int r) { return r * 10; }) + .endNamespace(); + + runLua(R"( + result = test.takeReference(test.ref_wrap_x) + )"); + + EXPECT_EQ(130, result().unsafe_cast()); +} + +TEST_F(ClassTests, ReferenceWrapperDecaysToLuaType) +{ + int x = 13; + std::reference_wrapper ref_wrap_x(x); + + luabridge::getGlobalNamespace(L) + .beginNamespace("test") + .addProperty("ref_wrap_x", &ref_wrap_x) + .endNamespace(); + + runLua(R"( + function xyz(x) + return x() * 10 + end + + result = xyz(test.ref_wrap_x) + )"); - lua_close(L); - L = nullptr; + EXPECT_EQ(130, result().unsafe_cast()); } diff --git a/third_party/gtest b/third_party/gtest new file mode 160000 index 00000000..58f3f100 --- /dev/null +++ b/third_party/gtest @@ -0,0 +1 @@ +Subproject commit 58f3f1005cffce8a9d005d0361d3471cd9947501 From a7c268cc927d07c741d9e62961d0ce9047e55ac4 Mon Sep 17 00:00:00 2001 From: kunitoki Date: Sun, 16 Oct 2022 00:17:07 +0200 Subject: [PATCH 05/10] Remove old gtest reference --- third_party/gtest | 1 - 1 file changed, 1 deletion(-) delete mode 160000 third_party/gtest diff --git a/third_party/gtest b/third_party/gtest deleted file mode 160000 index 58f3f100..00000000 --- a/third_party/gtest +++ /dev/null @@ -1 +0,0 @@ -Subproject commit 58f3f1005cffce8a9d005d0361d3471cd9947501 From 6c4812b792a13fdcc00d95275a0dfd2822384e0c Mon Sep 17 00:00:00 2001 From: kunitoki Date: Sun, 16 Oct 2022 01:20:38 +0200 Subject: [PATCH 06/10] More tweaks --- Source/LuaBridge/detail/FuncTraits.h | 4 +- Source/LuaBridge/detail/LuaHelpers.h | 75 +++++++++++++++++++++++----- Source/LuaBridge/detail/Stack.h | 21 ++++---- Source/LuaBridge/detail/Userdata.h | 4 +- 4 files changed, 78 insertions(+), 26 deletions(-) diff --git a/Source/LuaBridge/detail/FuncTraits.h b/Source/LuaBridge/detail/FuncTraits.h index d661f209..e5871ea9 100644 --- a/Source/LuaBridge/detail/FuncTraits.h +++ b/Source/LuaBridge/detail/FuncTraits.h @@ -24,9 +24,9 @@ namespace detail { */ [[noreturn]] inline void unreachable() { -#if __GNUC__ // GCC, Clang, ICC +#if __GNUC__ __builtin_unreachable(); -#elif _MSC_VER // MSVC +#elif _MSC_VER __assume(false); #endif } diff --git a/Source/LuaBridge/detail/LuaHelpers.h b/Source/LuaBridge/detail/LuaHelpers.h index 86846ddc..53970c53 100644 --- a/Source/LuaBridge/detail/LuaHelpers.h +++ b/Source/LuaBridge/detail/LuaHelpers.h @@ -199,6 +199,25 @@ inline int lua_compare(lua_State* L, int idx1, int idx2, int op) } } +#if ! LUABRIDGE_ON_LUAJIT +inline void* luaL_testudata(lua_State* L, int ud, const char* tname) +{ + void* p = lua_touserdata(L, ud); + if (p == nullptr) + return nullptr; + + if (! lua_getmetatable(L, ud)) + return nullptr; + + luaL_getmetatable(L, tname); + if (! lua_rawequal(L, -1, -2)) + p = nullptr; + + lua_pop(L, 2); + return p; +} +#endif + inline int get_length(lua_State* L, int idx) { return static_cast(lua_objlen(L, idx)); @@ -427,7 +446,7 @@ template * @brief Deallocate lua userdata taking into account alignment. */ template -int lua_destroyuserdata_aligned(lua_State* L) +int lua_deleteuserdata_aligned(lua_State* L) { assert(isfulluserdata(L, 1)); @@ -437,17 +456,6 @@ int lua_destroyuserdata_aligned(lua_State* L) return 0; } -template -int lua_deleteuserdata_aligned(lua_State* L) -{ - assert(isfulluserdata(L, 1)); - - T** aligned = align(lua_touserdata(L, 1)); - delete *aligned; - - return 0; -} - /** * @brief Allocate lua userdata taking into account alignment. * @@ -466,7 +474,7 @@ void* lua_newuserdata_aligned(lua_State* L, Args&&... args) void* pointer = lua_newuserdata_x(L, maximum_space_needed_to_align()); lua_newtable(L); - lua_pushcfunction_x(L, &lua_destroyuserdata_aligned); + lua_pushcfunction_x(L, &lua_deleteuserdata_aligned); rawsetfield(L, -2, "__gc"); lua_setmetatable(L, -2); #endif @@ -478,6 +486,47 @@ void* lua_newuserdata_aligned(lua_State* L, Args&&... args) return pointer; } +/** + * @brief Deallocate lua userdata from pointer. + */ +template +int lua_deleteuserdata_pointer(lua_State* L) +{ + assert(isfulluserdata(L, 1)); + + T** aligned = align(lua_touserdata(L, 1)); + delete *aligned; + + return 0; +} + +/** + * @brief Allocate lua userdata from pointer. + */ +template +void* lua_newuserdata_pointer(lua_State* L, T* ptr) +{ +#if LUABRIDGE_ON_LUAU + void* pointer = lua_newuserdatadtor(L, maximum_space_needed_to_align(), [](void* x) + { + T** aligned = align(x); + delete *aligned; + }); +#else + void* pointer = lua_newuserdata_x(L, maximum_space_needed_to_align()); + + lua_newtable(L); + lua_pushcfunction_x(L, &lua_deleteuserdata_pointer); + rawsetfield(L, -2, "__gc"); + lua_setmetatable(L, -2); +#endif + + T** aligned = align(pointer); + *aligned = ptr; + + return pointer; +} + /** * @brief Safe error able to walk backwards for error reporting correctly. */ diff --git a/Source/LuaBridge/detail/Stack.h b/Source/LuaBridge/detail/Stack.h index 95676ae2..0443d343 100644 --- a/Source/LuaBridge/detail/Stack.h +++ b/Source/LuaBridge/detail/Stack.h @@ -1341,17 +1341,14 @@ struct Stack template struct Stack> { - using storage_type = std::reference_wrapper*; static Result push(lua_State* L, const std::reference_wrapper& ref) { - lua_newuserdata_aligned(L, new std::reference_wrapper(ref.get())); + lua_newuserdata_pointer(L, new std::reference_wrapper(ref.get())); - lua_newtable(L); + luaL_newmetatable(L, typeName()); lua_pushcclosure_x(L, &get_reference_value, 0); rawsetfield(L, -2, "__call"); - lua_pushcfunction(L, &lua_deleteuserdata_aligned); - rawsetfield(L, -2, "__gc"); lua_setmetatable(L, -2); return {}; @@ -1359,10 +1356,10 @@ struct Stack> static TypeResult> get(lua_State* L, int index) { - if (! lua_isuserdata(L, index)) + if (luaL_testudata(L, index, typeName()) == nullptr) return makeErrorCode(ErrorCode::InvalidTypeCast); - storage_type* ptr = reinterpret_cast(lua_touserdata(L, index)); + std::reference_wrapper** ptr = reinterpret_cast**>(lua_touserdata(L, index)); if (ptr == nullptr || *ptr == nullptr) return makeErrorCode(ErrorCode::InvalidTypeCast); @@ -1371,10 +1368,16 @@ struct Stack> static bool isInstance(lua_State* L, int index) { - return lua_type(L, index) == LUA_TUSERDATA; + return luaL_testudata(L, index, typeName()) != nullptr; } private: + static const char* typeName() + { + static const std::string s{ detail::typeName>() }; + return s.c_str(); + } + template static int get_reference_value(lua_State* L) { @@ -1392,7 +1395,6 @@ struct Stack> }; namespace detail { - template struct StackOpSelector { @@ -1440,7 +1442,6 @@ struct StackOpSelector static bool isInstance(lua_State* L, int index) { return Stack::isInstance(L, index); } }; - } // namespace detail template diff --git a/Source/LuaBridge/detail/Userdata.h b/Source/LuaBridge/detail/Userdata.h index 7d576294..5603993f 100644 --- a/Source/LuaBridge/detail/Userdata.h +++ b/Source/LuaBridge/detail/Userdata.h @@ -120,7 +120,7 @@ class Userdata lua_remove(L, -2); // Stack: rt, pot } - // no return + unreachable(); } static bool isInstance(lua_State* L, int index, const void* registryClassKey) @@ -159,6 +159,8 @@ class Userdata lua_remove(L, -2); // Stack: rt, pot } + + unreachable(); } static Userdata* throwBadArg(lua_State* L, int index) From e4df09a8c10fa3d0d000622d93a779b519a67b42 Mon Sep 17 00:00:00 2001 From: kunitoki Date: Sun, 16 Oct 2022 01:31:06 +0200 Subject: [PATCH 07/10] Improved code --- Source/LuaBridge/detail/Stack.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Source/LuaBridge/detail/Stack.h b/Source/LuaBridge/detail/Stack.h index 0443d343..36382e9a 100644 --- a/Source/LuaBridge/detail/Stack.h +++ b/Source/LuaBridge/detail/Stack.h @@ -1341,7 +1341,6 @@ struct Stack template struct Stack> { - static Result push(lua_State* L, const std::reference_wrapper& ref) { lua_newuserdata_pointer(L, new std::reference_wrapper(ref.get())); @@ -1356,14 +1355,15 @@ struct Stack> static TypeResult> get(lua_State* L, int index) { - if (luaL_testudata(L, index, typeName()) == nullptr) + auto ptr = luaL_testudata(L, index, typeName()); + if (ptr == nullptr) return makeErrorCode(ErrorCode::InvalidTypeCast); - std::reference_wrapper** ptr = reinterpret_cast**>(lua_touserdata(L, index)); - if (ptr == nullptr || *ptr == nullptr) + std::reference_wrapper** ref = reinterpret_cast**>(ptr); + if (ref == nullptr || *ref == nullptr) return makeErrorCode(ErrorCode::InvalidTypeCast); - return **ptr; + return **ref; } static bool isInstance(lua_State* L, int index) From 403bc2b58368ec854467e33f8e2f264aa07e6ff2 Mon Sep 17 00:00:00 2001 From: kunitoki Date: Sun, 16 Oct 2022 01:34:48 +0200 Subject: [PATCH 08/10] Cosmetics --- Source/LuaBridge/detail/Stack.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Source/LuaBridge/detail/Stack.h b/Source/LuaBridge/detail/Stack.h index 36382e9a..59b863ee 100644 --- a/Source/LuaBridge/detail/Stack.h +++ b/Source/LuaBridge/detail/Stack.h @@ -1341,9 +1341,9 @@ struct Stack template struct Stack> { - static Result push(lua_State* L, const std::reference_wrapper& ref) + static Result push(lua_State* L, const std::reference_wrapper& reference) { - lua_newuserdata_pointer(L, new std::reference_wrapper(ref.get())); + lua_newuserdata_pointer(L, new std::reference_wrapper(reference.get())); luaL_newmetatable(L, typeName()); lua_pushcclosure_x(L, &get_reference_value, 0); @@ -1359,11 +1359,11 @@ struct Stack> if (ptr == nullptr) return makeErrorCode(ErrorCode::InvalidTypeCast); - std::reference_wrapper** ref = reinterpret_cast**>(ptr); - if (ref == nullptr || *ref == nullptr) + auto reference = reinterpret_cast**>(ptr); + if (reference == nullptr || *reference == nullptr) return makeErrorCode(ErrorCode::InvalidTypeCast); - return **ref; + return **reference; } static bool isInstance(lua_State* L, int index) From 629afd521705017dda5ee328d6c5664dfa40a18d Mon Sep 17 00:00:00 2001 From: kunitoki Date: Sun, 16 Oct 2022 01:41:32 +0200 Subject: [PATCH 09/10] Reference wrapper failures --- Tests/Source/ClassTests.cpp | 33 ++++++++++++++++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-) diff --git a/Tests/Source/ClassTests.cpp b/Tests/Source/ClassTests.cpp index 753658d6..27059b8e 100644 --- a/Tests/Source/ClassTests.cpp +++ b/Tests/Source/ClassTests.cpp @@ -2474,7 +2474,38 @@ TEST_F(ClassTests, ReferenceWrapperDecaysToType) EXPECT_EQ(130, result().unsafe_cast()); } -TEST_F(ClassTests, ReferenceWrapperDecaysToLuaType) +TEST_F(ClassTests, ReferenceWrapperFailsOnInvalidType) +{ + int x = 13; + std::reference_wrapper ref_wrap_x(x); + + float y = 1.0f; + std::reference_wrapper ref_wrap_y(y); + + luabridge::getGlobalNamespace(L) + .beginNamespace("test") + .addProperty("ref_wrap_x", &ref_wrap_x) + .addProperty("ref_wrap_y", &ref_wrap_y) + .addFunction("takeReference1", [](float r) { return r * 10; }) + .addFunction("takeReference2", [](int r) { return r * 10; }) + .addFunction("takeReference3", [](std::reference_wrapper r) { return r.get() * 10; }) + .addFunction("takeReference4", [](std::reference_wrapper r) { return r.get() * 10; }) + .endNamespace(); + +#if LUABRIDGE_HAS_EXCEPTIONS + EXPECT_THROW(runLua("result = test.takeReference1(test.ref_wrap_x)"), std::exception); + EXPECT_THROW(runLua("result = test.takeReference2(test.ref_wrap_y)"), std::exception); + EXPECT_THROW(runLua("result = test.takeReference3(test.ref_wrap_x)"), std::exception); + EXPECT_THROW(runLua("result = test.takeReference4(test.ref_wrap_y)"), std::exception); +#else + EXPECT_FALSE(runLua("result = test.takeReference1(test.ref_wrap_x)")); + EXPECT_FALSE(runLua("result = test.takeReference2(test.ref_wrap_y)")); + EXPECT_FALSE(runLua("result = test.takeReference3(test.ref_wrap_x)")); + EXPECT_FALSE(runLua("result = test.takeReference4(test.ref_wrap_y)")); +#endif +} + +TEST_F(ClassTests, ReferenceWrapperAccessFromLua) { int x = 13; std::reference_wrapper ref_wrap_x(x); From dcf6be9bb8439103cb258c24544706ccdb01d152 Mon Sep 17 00:00:00 2001 From: kunitoki Date: Sun, 23 Apr 2023 21:32:36 +0200 Subject: [PATCH 10/10] More tweaks to reference wrapper --- Source/LuaBridge/detail/CFunctions.h | 1 + Source/LuaBridge/detail/Stack.h | 40 +++++++++++++++++++--------- Tests/Source/ClassTests.cpp | 2 +- 3 files changed, 29 insertions(+), 14 deletions(-) diff --git a/Source/LuaBridge/detail/CFunctions.h b/Source/LuaBridge/detail/CFunctions.h index 454bd11a..cf7cf626 100644 --- a/Source/LuaBridge/detail/CFunctions.h +++ b/Source/LuaBridge/detail/CFunctions.h @@ -37,6 +37,7 @@ auto unwrap_argument_or_error(lua_State* L, std::size_t index, std::size_t start if (result) return std::move(*result); + // TODO - this might be costly, how to deal with it ? if constexpr (! std::is_lvalue_reference_v) { using U = std::reference_wrapper>; diff --git a/Source/LuaBridge/detail/Stack.h b/Source/LuaBridge/detail/Stack.h index d9738c70..dfb1dee2 100644 --- a/Source/LuaBridge/detail/Stack.h +++ b/Source/LuaBridge/detail/Stack.h @@ -1352,10 +1352,11 @@ struct Stack> { static Result push(lua_State* L, const std::reference_wrapper& reference) { - lua_newuserdata_pointer(L, new std::reference_wrapper(reference.get())); + lua_newuserdata_aligned>(L, reference.get()); luaL_newmetatable(L, typeName()); - lua_pushcclosure_x(L, &get_reference_value, 0); + lua_pushvalue(L, -2); + lua_pushcclosure_x(L, &get_set_reference_value, 1); rawsetfield(L, -2, "__call"); lua_setmetatable(L, -2); @@ -1368,11 +1369,11 @@ struct Stack> if (ptr == nullptr) return makeErrorCode(ErrorCode::InvalidTypeCast); - auto reference = reinterpret_cast**>(ptr); - if (reference == nullptr || *reference == nullptr) + auto reference = reinterpret_cast*>(ptr); + if (reference == nullptr) return makeErrorCode(ErrorCode::InvalidTypeCast); - return **reference; + return *reference; } static bool isInstance(lua_State* L, int index) @@ -1388,18 +1389,31 @@ struct Stack> } template - static int get_reference_value(lua_State* L) + static int get_set_reference_value(lua_State* L) { - assert(lua_isuserdata(L, -1)); + LUABRIDGE_ASSERT(lua_isuserdata(L, lua_upvalueindex(1))); - std::reference_wrapper** ptr = static_cast**>(lua_touserdata(L, -1)); - assert(ptr != nullptr); + std::reference_wrapper* ptr = static_cast*>(lua_touserdata(L, lua_upvalueindex(1))); + LUABRIDGE_ASSERT(ptr != nullptr); - auto result = Stack::push(L, (*ptr)->get()); - if (! result) - luaL_error(L, "%s", result.message().c_str()); + if (lua_gettop(L) > 1) + { + auto result = Stack::get(L, 2); + if (! result) + luaL_error(L, "%s", result.message().c_str()); + + ptr->get() = *result; + + return 0; + } + else + { + auto result = Stack::push(L, ptr->get()); + if (! result) + luaL_error(L, "%s", result.message().c_str()); - return 1; + return 1; + } } }; diff --git a/Tests/Source/ClassTests.cpp b/Tests/Source/ClassTests.cpp index 44243e11..10e4b4ed 100644 --- a/Tests/Source/ClassTests.cpp +++ b/Tests/Source/ClassTests.cpp @@ -2818,7 +2818,7 @@ TEST_F(ClassTests, ReferenceWrapperWrite) .endNamespace(); runLua(R"( - test.ref_wrap_x = 100 + test.ref_wrap_x(100) result = test.ref_wrap_x )");