Skip to content

Commit

Permalink
Populate and use ObjFn->arity correctly (wren-lang#731)
Browse files Browse the repository at this point in the history
`ObjFn->arity` was only populated for functions. Fix that with
wren call handles, and regular methods, so the stack can be properly
adjusted when invoking `wrenCallFunction`.
  • Loading branch information
mhermier committed Nov 29, 2022
1 parent c2a75f1 commit 9967840
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 3 deletions.
4 changes: 4 additions & 0 deletions src/vm/wren_compiler.c
Original file line number Diff line number Diff line change
Expand Up @@ -3282,6 +3282,8 @@ static void createConstructor(Compiler* compiler, Signature* signature,
Compiler methodCompiler;
initCompiler(&methodCompiler, compiler->parser, compiler, true);

methodCompiler.fn->arity = signature->arity;

// Allocate the instance.
emitOp(&methodCompiler, compiler->enclosingClass->isForeign
? CODE_FOREIGN_CONSTRUCT : CODE_CONSTRUCT);
Expand Down Expand Up @@ -3462,6 +3464,8 @@ static bool method(Compiler* compiler, Variable classVariable)
error(compiler, "A constructor cannot be static.");
}

methodCompiler.fn->arity = signature.arity;

// Include the full signature in debug messages in stack traces.
char fullSignature[MAX_METHOD_SIGNATURE];
int length;
Expand Down
6 changes: 4 additions & 2 deletions src/vm/wren_vm.c
Original file line number Diff line number Diff line change
Expand Up @@ -1422,6 +1422,7 @@ WrenHandle* wrenMakeCallHandle(WrenVM* vm, const char* signature)
// Create a little stub function that assumes the arguments are on the stack
// and calls the method.
ObjFn* fn = wrenNewFunction(vm, NULL, numParams + 1);
fn->arity = numParams;

// Wrap the function in a closure and then in a handle. Do this here so it
// doesn't get collected as we fill it in.
Expand Down Expand Up @@ -1460,9 +1461,10 @@ WrenInterpretResult wrenCall(WrenVM* vm, WrenHandle* method)

// Discard any extra temporary slots. We take for granted that the stub
// function has exactly one slot for each argument.
vm->fiber->stackTop = &vm->fiber->stack[closure->fn->maxSlots];
const int closureNumArgs = closure->fn->arity + 1;
vm->fiber->stackTop = &vm->fiber->stack[closureNumArgs];

wrenCallFunction(vm, vm->fiber, closure, 0);
wrenCallFunction(vm, vm->fiber, closure, closureNumArgs);
WrenInterpretResult result = runInterpreter(vm, vm->fiber);

// If the call didn't abort, then set up the API stack to point to the
Expand Down
8 changes: 7 additions & 1 deletion src/vm/wren_vm.h
Original file line number Diff line number Diff line change
Expand Up @@ -187,12 +187,18 @@ static inline void wrenCallFunction(WrenVM* vm, ObjFiber* fiber,
fiber->frameCapacity = max;
}

// Functions allows to be called with more arguments than required. So the
// the [fiber] [stackTop] has to be realined to match the [closure] [arity].
const int closureNumArgs = closure->fn->arity + 1;
ASSERT(closureNumArgs <= numArgs, "Expect more arguments");
fiber->stackTop -= numArgs - closureNumArgs;

// Grow the stack if needed.
int stackSize = (int)(fiber->stackTop - fiber->stack);
int needed = stackSize + closure->fn->maxSlots;
wrenEnsureStack(vm, fiber, needed);

wrenAppendCallFrame(vm, fiber, closure, fiber->stackTop - numArgs);
wrenAppendCallFrame(vm, fiber, closure, fiber->stackTop - closureNumArgs);
}

// Marks [obj] as a GC root so that it doesn't get collected.
Expand Down
5 changes: 5 additions & 0 deletions test/regression/731.wren
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@

Fn.new {
var foo
System.print(foo) // expect: null
}.call("Bug")

0 comments on commit 9967840

Please sign in to comment.