fix call-fn stack leak and set-field index bug#8
Conversation
There was a problem hiding this comment.
Build & Tests
Build: pass. Compiles cleanly on both platforms.
Tests: pass. CI green on macOS and Ubuntu.
Findings
-
Stack leak fix (call-fn) is correct.
Lua.callwithnresults=1pushes exactly one value. The getter functions (get-int,get-float, etc.) peek without consuming. The old code read the result but never popped it, leaking one slot per call. The fix wraps both success and error paths inlet-doblocks that pop after reading. Placement is correct — theletbinding captures the value before the pop. -
Index adjustment fix (set-*-field, table-length) is correct. In Lua's C API, pushing a value shifts negative (relative) indices by -1 but leaves positive (absolute) indices unchanged. The old
(- index 1)was unconditional, breaking positive indices.(if (< index 0) (- index 1) index)is the right fix. Bothset-*-field(line 95) andtable-length(line 421) are fixed consistently. Index 0 is already invalid in Lua's API (1-based positive, -1-based negative), so no guard needed there. -
Example version bumps are correct. All four examples updated from
@0.1.0to@0.2.0, matching the latest tag and the existinggendocs.carpreference. -
Pre-existing stack leaks (not regressions, just noting):
get-string-global(line ~390): callsget-globalwhich pushes a value, reads it, but never pops — same class of bug as the call-fn fix.do-in/eval-fileerror paths: read the error message from -1 but don't pop it.
These could be follow-up work.
Verdict: merge
All three fixes are correct, well-scoped, and CI is green. The pre-existing leaks are worth a follow-up but don't block this PR.
Summary
(- index 1)adjusted the table index. this is correct for negative (relative) indices but wrong for positive (absolute) ones—positive indices don't shift when the stack grows. the same bug existed intable-length, fixed there too.@0.1.0to@0.2.0to match the README.Opened by the carpentry-org heartbeat agent (Claude). hellerve has not reviewed this yet.