Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,4 @@ build/
.DS_store
.idea/
cmake-build-*/
tmpunwind.o
61 changes: 42 additions & 19 deletions Source/LuaBridge/detail/CFunctions.h
Original file line number Diff line number Diff line change
Expand Up @@ -429,55 +429,78 @@ inline std::optional<int> 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_<name>" 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_<name>" 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

Expand Down
83 changes: 83 additions & 0 deletions Tests/Source/ClassExtensibleTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,89 @@ TEST_F(ClassExtensibleTests, ExtensibleDerivedClassAndBaseCascading)
EXPECT_EQ(1, result<int>());
}

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>("ExtensibleBase", luabridge::extensibleClass)
.addConstructor<void(*)()>()
.endClass()
.deriveClass<ExtensibleDerived, ExtensibleBase>("ExtensibleDerived", luabridge::extensibleClass)
.addConstructor<void(*)()>()
.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<int>());
}

TEST_F(ClassExtensibleTests, ExtensibleDerivedMethodIsolatedFromBase)
{
// Verify both sides independently after same-name Lua methods are defined on each class.
luabridge::getGlobalNamespace(L)
.beginClass<ExtensibleBase>("ExtensibleBase", luabridge::extensibleClass)
.addConstructor<void(*)()>()
.endClass()
.deriveClass<ExtensibleDerived, ExtensibleBase>("ExtensibleDerived", luabridge::extensibleClass)
.addConstructor<void(*)()>()
.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<int>());
}

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>("ExtensibleBase", options)
.addConstructor<void(*)()>()
.addFunction("baseClass", &ExtensibleBase::baseClass)
.endClass()
.deriveClass<ExtensibleDerived, ExtensibleBase>("ExtensibleDerived", options)
.addConstructor<void(*)()>()
.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<int>());
}

TEST_F(ClassExtensibleTests, ExtensibleDerivedClassAndBaseSameMethod)
{
luabridge::getGlobalNamespace(L)
Expand Down