diff --git a/.gitignore b/.gitignore index 59b4e3e2..948f96c2 100644 --- a/.gitignore +++ b/.gitignore @@ -14,3 +14,4 @@ build/ .DS_store .idea/ cmake-build-*/ +tmpunwind.o diff --git a/Source/LuaBridge/detail/CFunctions.h b/Source/LuaBridge/detail/CFunctions.h index dde44344..56b86fc1 100644 --- a/Source/LuaBridge/detail/CFunctions.h +++ b/Source/LuaBridge/detail/CFunctions.h @@ -429,55 +429,78 @@ inline std::optional try_call_newindex_extensible(lua_State* L, const char* LUABRIDGE_ASSERT(key != nullptr); LUABRIDGE_ASSERT(lua_istable(L, -1)); // Stack: mt - lua_pushvalue(L, -1); // Stack: mt, mt + lua_pushvalue(L, -1); // Stack: mt, mt_iter (initially a copy of mt) for (;;) { - push_class_or_const_table(L, -1); // Stack: mt, mt, class table (ct) | nil - if (! lua_istable(L, -1)) // Stack: mt, mt, nil + push_class_or_const_table(L, -1); // Stack: mt, mt_iter, class table (ct) | nil + if (! lua_istable(L, -1)) // Stack: mt, mt_iter, nil { lua_pop(L, 2); // Stack: mt return std::nullopt; } - lua_pushvalue(L, 2); // Stack: mt, mt, ct | co, field name - lua_rawget(L, -2); // Stack: mt, mt, ct | co, field | nil + lua_pushvalue(L, 2); // Stack: mt, mt_iter, ct | co, field name + lua_rawget(L, -2); // Stack: mt, mt_iter, ct | co, field | nil - if (! lua_isnil(L, -1)) // Stack: mt, mt, ct | co, field + if (! lua_isnil(L, -1)) // Stack: mt, mt_iter, ct | co, field { if (! lua_iscfunction(L, -1)) { - lua_pop(L, 1); + lua_pop(L, 1); // Stack: mt, mt_iter, ct | co break; } // Obtain class options - const Options options = get_class_options(L, -2); // Stack: mt, mt, ct | co, field + const Options options = get_class_options(L, -2); // Stack: mt, mt_iter, ct | co, field if (! options.test(allowOverridingMethods)) luaL_error(L, "immutable member '%s'", key); - rawsetfield(L, -2, make_super_method_name(key).c_str()); // Stack: mt, mt, ct | co + // Copy the original cfunction as "super_" into the ORIGINAL (target/derived) + // class table, not into the parent class table where it was found. This ensures + // that base-class instances continue to find and call the original cfunction via + // their own class table, while derived-class instances gain a "super_" alias. + const int cfunction_idx = lua_gettop(L); // absolute stack index of the cfunction to copy + lua_getmetatable(L, 1); // Stack: mt, mt_iter, ct | co, field, orig_mt + push_class_or_const_table(L, -1); // Stack: mt, mt_iter, ct | co, field, orig_mt, orig_ct + lua_remove(L, -2); // Stack: mt, mt_iter, ct | co, field, orig_ct + lua_getmetatable(L, -1); // Stack: mt, mt_iter, ct | co, field, orig_ct, orig_ct_meta + lua_pushvalue(L, cfunction_idx); // push copy of the cfunction + rawsetfield(L, -2, make_super_method_name(key).c_str()); // Stack: mt, mt_iter, ct | co, field, orig_ct, orig_ct_meta + lua_pop(L, 3); // pop orig_ct_meta, orig_ct, field -> Stack: mt, mt_iter, ct | co break; } - lua_pop(L, 1); // Stack: mt, mt, ct | co + lua_pop(L, 1); // Stack: mt, mt_iter, ct | co - lua_rawgetp_x(L, -2, getParentKey()); // Stack: mt, mt, ct | co, parent mt (pmt) | nil - if (lua_isnil(L, -1)) // Stack: mt, mt, ct | co, nil + lua_rawgetp_x(L, -2, getParentKey()); // Stack: mt, mt_iter, ct | co, parent mt (pmt) | nil + if (lua_isnil(L, -1)) // Stack: mt, mt_iter, ct | co, nil { - lua_pop(L, 1); // Stack: mt, mt, ct | co + lua_pop(L, 1); // Stack: mt, mt_iter, ct | co break; } - LUABRIDGE_ASSERT(lua_istable(L, -1)); // Stack: mt, mt, ct | co, pmt - lua_remove(L, -2); // Stack: mt, mt, pmt + LUABRIDGE_ASSERT(lua_istable(L, -1)); // Stack: mt, mt_iter, ct | co, pmt + lua_remove(L, -2); // Stack: mt, mt_iter, pmt lua_remove(L, -2); // Stack: mt, pmt } - lua_remove(L, -2); // Stack: mt, ct | co - lua_getmetatable(L, -1); // Stack: mt, ct | co, mt2 - lua_pushvalue(L, 3); // Stack: mt, ct | co, mt2, arg3 - rawsetfield(L, -2, key); // Stack: mt, ct | co, mt2 + // Stack: mt, mt_iter, ct | co at every break point. + // Discard the search context and store the new value in the ORIGINAL (target/derived) class + // table obtained from arg1's metatable. This fixes the inheritance bug where a method + // defined on a derived class was written into the base class, making it visible to + // base-class instances as well. + lua_pop(L, 2); // Stack: mt + + lua_getmetatable(L, 1); // Stack: mt, orig_mt (metatable of the original static table, arg1) + push_class_or_const_table(L, -1); // Stack: mt, orig_mt, orig_ct + lua_remove(L, -2); // Stack: mt, orig_ct + // In LuaBridge, each class/const table is set as its own metatable (see createConstTable), + // so getmetatable(orig_ct) == orig_ct. rawsetfield on the metatable therefore writes + // directly into orig_ct, matching the storage convention used throughout this file. + lua_getmetatable(L, -1); // Stack: mt, orig_ct, orig_ct_meta (== orig_ct; class tables are own-metatables) + lua_pushvalue(L, 3); // Stack: mt, orig_ct, orig_ct_meta, arg3 + rawsetfield(L, -2, key); // Stack: mt, orig_ct, orig_ct_meta lua_pop(L, 2); // Stack: mt diff --git a/Tests/Source/ClassExtensibleTests.cpp b/Tests/Source/ClassExtensibleTests.cpp index 2ccdc276..70f921a2 100644 --- a/Tests/Source/ClassExtensibleTests.cpp +++ b/Tests/Source/ClassExtensibleTests.cpp @@ -396,6 +396,89 @@ TEST_F(ClassExtensibleTests, ExtensibleDerivedClassAndBaseCascading) EXPECT_EQ(1, result()); } +TEST_F(ClassExtensibleTests, ExtensibleDerivedDoesNotPollutBaseMethod) +{ + // Regression test: defining a Lua method on a derived class must NOT overwrite the + // same-named method on the base class (the originally reported inheritance bug). + luabridge::getGlobalNamespace(L) + .beginClass("ExtensibleBase", luabridge::extensibleClass) + .addConstructor() + .endClass() + .deriveClass("ExtensibleDerived", luabridge::extensibleClass) + .addConstructor() + .endClass() + ; + + runLua(R"( + function ExtensibleBase:init() return 100 end + function ExtensibleDerived:init() return 200 end + + local base = ExtensibleBase() + local derived = ExtensibleDerived() + result = base:init() + )"); + + // base:init() must return 100 (the Base version), not 200 (the Derived version) + EXPECT_EQ(100, result()); +} + +TEST_F(ClassExtensibleTests, ExtensibleDerivedMethodIsolatedFromBase) +{ + // Verify both sides independently after same-name Lua methods are defined on each class. + luabridge::getGlobalNamespace(L) + .beginClass("ExtensibleBase", luabridge::extensibleClass) + .addConstructor() + .endClass() + .deriveClass("ExtensibleDerived", luabridge::extensibleClass) + .addConstructor() + .endClass() + ; + + runLua(R"( + function ExtensibleBase:getValue() return 10 end + function ExtensibleDerived:getValue() return 20 end + + local base = ExtensibleBase() + local derived = ExtensibleDerived() + result = derived:getValue() * 100 + base:getValue() + )"); + + // derived:getValue() == 20, base:getValue() == 10 => 20*100 + 10 == 2010 + EXPECT_EQ(2010, result()); +} + +TEST_F(ClassExtensibleTests, ExtensibleDerivedOverridePreservesCppBaseMethod) +{ + // When allowOverridingMethods is set and a derived class overrides a C++ method, + // the base class should still call the original C++ implementation. + constexpr auto options = luabridge::extensibleClass | luabridge::allowOverridingMethods; + + luabridge::getGlobalNamespace(L) + .beginClass("ExtensibleBase", options) + .addConstructor() + .addFunction("baseClass", &ExtensibleBase::baseClass) + .endClass() + .deriveClass("ExtensibleDerived", options) + .addConstructor() + .endClass() + ; + + runLua(R"( + -- Override baseClass only on ExtensibleDerived; base must remain unaffected + function ExtensibleDerived:baseClass() + return 100 + self:super_baseClass() + end + + local base = ExtensibleBase() + local derived = ExtensibleDerived() + result = derived:baseClass() * 1000 + base:baseClass() + )"); + + // derived:baseClass() == 100 + 1 == 101, base:baseClass() == 1 (original C++) + // => 101 * 1000 + 1 == 101001 + EXPECT_EQ(101001, result()); +} + TEST_F(ClassExtensibleTests, ExtensibleDerivedClassAndBaseSameMethod) { luabridge::getGlobalNamespace(L)