Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Populate and use ObjFn->arity correctly (#731) #32

Merged
merged 1 commit into from
Jun 1, 2024
Merged
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
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")