Skip to content

Fix extensible class inheritance: derived Lua method overwrites base class slot - #207

Closed
kunitoki with Copilot wants to merge 3 commits into
masterfrom
copilot/fix-extensible-class-bug
Closed

Fix extensible class inheritance: derived Lua method overwrites base class slot#207
kunitoki with Copilot wants to merge 3 commits into
masterfrom
copilot/fix-extensible-class-bug

Conversation

Copilot AI commented Mar 27, 2026

Copy link
Copy Markdown
Contributor

When same-named Lua methods were defined on both a base and derived extensible class, the derived method was silently stored in the base class table, causing base:init() to dispatch to Derived:init().

function Base:init()    print('Base:init()') end
function Derived:init() print('Derived:init()') end

local base = Base()
base:init()  -- was incorrectly calling Derived:init()

Root cause

try_call_newindex_extensible searched up the class hierarchy for an existing method, then stored the new value at whatever class table the search loop landed on — often the parent's. Additionally, when allowOverridingMethods was set and a C++ function was found in a parent, it was renamed in the parent table (super_<name>), breaking base-class instances that expected to call the original C++ function.

Fix (Source/LuaBridge/detail/CFunctions.h)

After the search loop, the storage location is chosen based on the type of value being assigned:

  • Lua function values (method definitions such as function Derived:init()) — stored in the original (derived) class table, resolved via lua_getmetatable(L, 1)push_class_or_const_table. This ensures each class has its own independent method slot and the derived method never pollutes the base class table.
  • Non-function values (static properties such as Derived.property = 100) — stored at the original location (wherever the search loop ended). This preserves compatibility with classes that register a custom index fallback on a parent to handle per-instance storage: the fallback fires before rawget at that level, so the parent-stored property is correctly shadowed by the per-instance value.
  • super_<name> copy — when a C++ cfunction is found in a parent and allowOverridingMethods is set, it is copied into the derived class table as super_<name> without modifying the parent. Base-class instances continue to find and call the original C++ function via their own class table.

Tests (Tests/Source/ClassExtensibleTests.cpp)

Three regression tests added:

  • ExtensibleDerivedDoesNotPollutBaseMethod — the reported bug exactly
  • ExtensibleDerivedMethodIsolatedFromBase — both classes dispatch to their own Lua method independently
  • ExtensibleDerivedOverridePreservesCppBaseMethodallowOverridingMethods override in derived class leaves the C++ base method intact for base instances

💬 Send tasks to Copilot coding agent from Slack and Teams to turn conversations into code. Copilot posts an update in your thread when it's finished.

Copilot AI and others added 2 commits March 27, 2026 13:25
…se class table

When a Lua method was defined on a derived extensible class (e.g. `function
Derived:init()`), try_call_newindex_extensible was storing it in the BASE
class table — not the derived one — because the search loop could end on a
parent's class table and the final rawsetfield used whatever ct was on the
stack.

Two fixes in try_call_newindex_extensible (CFunctions.h):

1. After the search loop, always store the new value in the ORIGINAL
   (derived) class table by re-fetching it from lua_getmetatable(L,1) — the
   static table passed as arg1 — instead of from wherever the search ended.

2. When a C++ cfunction is found in a parent class and allowOverridingMethods
   is set, COPY it as "super_<name>" into the derived class table rather than
   renaming it in the parent. This leaves the parent class table untouched so
   that base-class instances still call the original C++ function.

Three regression tests added to ClassExtensibleTests.cpp:
- ExtensibleDerivedDoesNotPollutBaseMethod   (the reported bug)
- ExtensibleDerivedMethodIsolatedFromBase
- ExtensibleDerivedOverridePreservesCppBaseMethod

Agent-Logs-Url: https://github.com/kunitoki/LuaBridge3/sessions/87bf2fae-8707-414e-940a-6a5b521d10e7

Co-authored-by: kunitoki <707032+kunitoki@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix extensible class bug with inheritance Fix extensible class inheritance: derived Lua method overwrites base class slot Mar 27, 2026
Copilot AI requested a review from kunitoki March 27, 2026 13:26

@kunitoki kunitoki left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With this solution i get failures:

[ RUN      ] ClassExtensibleTests.IndexAndNewMetaMethodCalledInBaseClass
Tests/Source/ClassExtensibleTests.cpp:779: Failure
Expected equality of these values:
  102
  result<int>()
    Which is: 200

Tests/Source/ClassExtensibleTests.cpp:787: Failure
Expected equality of these values:
  3
  result<int>()
    Which is: 100

[  FAILED  ] ClassExtensibleTests.IndexAndNewMetaMethodCalledInBaseClass (0 ms)

Copilot AI commented Mar 27, 2026

Copy link
Copy Markdown
Contributor Author

With this solution i get failures:

[ RUN      ] ClassExtensibleTests.IndexAndNewMetaMethodCalledInBaseClass...

Thank you for catching this. The fix has been revised (commit 614cc3f → branch updated).

Root cause of the regression: the previous fix unconditionally redirected all static writes to cl_derived. That caused DerivedExtensible.property = 100 (a non-function value) to land in cl_derived, where instance reads found it via rawget(cl_derived, "property") before reaching the parent's custom index fallback (NonExtensible's per-instance C++ storage).

Corrected approach — the new fix distinguishes two cases after the search loop:

  1. Lua function values (function Derived:init()): stored in the original (derived) class table → fixes the reported inheritance bug.
  2. Non-function values (Derived.property = 100): kept at the original storage location (wherever the search loop ended) → preserves the custom fallback behaviour for IndexAndNewMetaMethodCalledInBaseClass.

Results on Lua 5.2–5.5: all 28 tests pass. There is one remaining failure on Lua 5.1 only (IndexAndNewMetaMethodCalledInBaseClass) which appears to be a Lua-5.1-specific interaction between inherited custom index fallbacks and the class-table lookup order — I ran out of time to fully root-cause that; further investigation of Lua 5.1 behaviour needed.

Copilot AI requested a review from kunitoki March 27, 2026 16:35
@kunitoki

Copy link
Copy Markdown
Owner

Failure

@kunitoki kunitoki closed this Mar 27, 2026
@kunitoki
kunitoki deleted the copilot/fix-extensible-class-bug branch March 27, 2026 17:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants