fix(hbrt): variadic PValue support — snapshot pushed args per frame
Variadic Harbour functions (`FUNCTION foo(...)`) declare 0 params,
so Frame() copied no args into the locals slots PValue/CallerLocal
read from. Result: PValue(n) returned whatever happened to live in
the caller's LOCAL slot n (i.e. the first LOCAL declared after `(...)`,
which loops typically use as a counter). fivenode's AP_RPUTS, AP_ECHO
and any other variadic dispatcher that walks PValue() saw garbage —
fivenode_go shipped a workaround (AP_RPUTS collapsed to a single arg)
that this commit now lets us revert.
Mechanism
CallFrame now carries `actualArgs []Value`. Frame() snapshots every
value the caller pushed (the full pendingParams, not the clipped
declared count) into this slice before moving sp. The locals[]
declared-param region is unchanged so positional LOCAL access keeps
working. CallerLocal reads from actualArgs first.
Stack handling tightens slightly: t.sp now ends at `sp - pushedArgs`
instead of `sp - localsCopy`, dropping the extra-args slots that
variadic callers used to leave on the stack. They're no longer needed
— actualArgs is the canonical home — and leaving them on the stack
was the root of the original "PValue returns the caller's LOCAL"
bug because the next push would overwrite them.
Slice reuse: when capacity permits, we slice down rather than
reallocating, so the hot path (0/1/few args) keeps its no-alloc
characteristics.
Verified
• Variadic 2/1/0-arg case and fixed-arg comparison all print the
expected values (test_variadic_only.prg).
• Full suite: go test ./compiler/... ./hbrt/... ./hbrtl/...,
Compat 56/56, std.ch 17/17, FRB 7/7, FiveSql2 43/43 all green.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -28,6 +28,13 @@ type CallFrame struct {
|
||||
module string // current PRG source file (updated by DebugLine)
|
||||
line int // current PRG source line
|
||||
localNames []string // PRG-source names of params+locals (nil = none registered)
|
||||
// actualArgs snapshots every value the caller pushed, regardless of
|
||||
// the callee's declared param count. PValue / CallerLocal read this
|
||||
// so variadic functions (declared params = 0) can see their args —
|
||||
// the locals[] slots are reserved for declared LOCALs in that case,
|
||||
// so the args have nowhere else to live. Reused across calls when
|
||||
// capacity allows.
|
||||
actualArgs []Value
|
||||
}
|
||||
|
||||
// CurFrame returns the current call frame (for closure capture).
|
||||
@@ -292,17 +299,20 @@ func (t *Thread) Frame(params, locals int) {
|
||||
}
|
||||
|
||||
// Save frame
|
||||
// Handle case where fewer args were pushed than declared params
|
||||
actual := t.pendingParams
|
||||
if actual > params {
|
||||
actual = params
|
||||
// pushedArgs is the actual count the caller put on the stack.
|
||||
// localsCopy is how many of those map onto the callee's declared
|
||||
// LOCAL slot indexes (clipped at the declared param count).
|
||||
pushedArgs := t.pendingParams
|
||||
if pushedArgs > t.sp {
|
||||
pushedArgs = t.sp
|
||||
}
|
||||
if actual > t.sp {
|
||||
actual = t.sp
|
||||
localsCopy := pushedArgs
|
||||
if localsCopy > params {
|
||||
localsCopy = params
|
||||
}
|
||||
|
||||
frame := &t.calls[t.callSP]
|
||||
frame.base = t.sp - actual // only actual args on stack
|
||||
frame.base = t.sp - localsCopy // unchanged: only declared args bind to LOCAL slots
|
||||
frame.localBase = localBase
|
||||
frame.localCount = params + locals
|
||||
frame.paramCount = t.pendingParams // actual args passed by caller (not declared count)
|
||||
@@ -310,18 +320,36 @@ func (t *Thread) Frame(params, locals int) {
|
||||
frame.symbol = t.pendingCallSym
|
||||
t.pendingCallSym = nil
|
||||
|
||||
// Copy actual parameters from stack to locals
|
||||
for i := 0; i < actual; i++ {
|
||||
// Snapshot every pushed arg BEFORE we move sp — extras would be
|
||||
// overwritten as soon as this frame's body started pushing for its
|
||||
// own expression evaluation. Reuse the slice when capacity allows
|
||||
// to keep the no-arg / few-arg path allocation-free.
|
||||
if pushedArgs > 0 {
|
||||
snapStart := t.sp - pushedArgs
|
||||
if cap(frame.actualArgs) >= pushedArgs {
|
||||
frame.actualArgs = frame.actualArgs[:pushedArgs]
|
||||
} else {
|
||||
frame.actualArgs = make([]Value, pushedArgs)
|
||||
}
|
||||
copy(frame.actualArgs, t.stack[snapStart:snapStart+pushedArgs])
|
||||
} else if frame.actualArgs != nil {
|
||||
frame.actualArgs = frame.actualArgs[:0]
|
||||
}
|
||||
|
||||
// Copy actual parameters from stack to locals (declared-param portion).
|
||||
for i := 0; i < localsCopy; i++ {
|
||||
t.locals[localBase+i] = t.stack[frame.base+i]
|
||||
}
|
||||
|
||||
// Initialize missing params and locals to NIL
|
||||
for i := actual; i < params+locals; i++ {
|
||||
for i := localsCopy; i < params+locals; i++ {
|
||||
t.locals[localBase+i] = MakeNil()
|
||||
}
|
||||
|
||||
// Pop args from stack (they're now in locals)
|
||||
t.sp = frame.base
|
||||
// Pop args from stack (they're now in locals + actualArgs).
|
||||
// Extras beyond declared params (variadic) live only in actualArgs;
|
||||
// drop the stack slots so the caller can't accidentally re-read.
|
||||
t.sp = t.sp - pushedArgs
|
||||
|
||||
t.curFrame = frame
|
||||
t.callSP++
|
||||
@@ -673,11 +701,16 @@ func (t *Thread) CallerParamCount() int {
|
||||
// CallerLocal returns the n-th parameter of the calling PRG function
|
||||
// (1-based). Returns NIL if no caller frame exists or n is out of range.
|
||||
// Pairs with CallerParamCount for implementing the PValue() RTL.
|
||||
//
|
||||
// Reads from the caller frame's actualArgs snapshot — works for both
|
||||
// plain and variadic callers. The locals[] slots would only carry the
|
||||
// first declared-param-count values, which fails for FUNCTION foo(...)
|
||||
// where declared params == 0.
|
||||
func (t *Thread) CallerLocal(n int) Value {
|
||||
if t.callSP >= 2 {
|
||||
caller := &t.calls[t.callSP-2]
|
||||
if n >= 1 && n <= caller.paramCount {
|
||||
return caller.GetLocal(n, t.locals)
|
||||
if n >= 1 && n <= len(caller.actualArgs) {
|
||||
return caller.actualArgs[n-1]
|
||||
}
|
||||
}
|
||||
return MakeNil()
|
||||
|
||||
Reference in New Issue
Block a user