Skip to content

Conversation

@mhermier
Copy link
Contributor

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.

`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`.
vm->fiber->stackTop = &vm->fiber->stack[closureNumArgs];

wrenCallFunction(vm, vm->fiber, closure, 0);
wrenCallFunction(vm, vm->fiber, closure, closureNumArgs);
Copy link
Member

Choose a reason for hiding this comment

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

this code might not behave as expected? The closure being called here is the stub function created by the call handle, which only has bytecode to forward the call to the receiver. The closure itself has 0 args marked here specifically to avoid interacting with the stack afaict

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The old behavior was ONLY correct when passing the correct number of arguments using broken logic. There was a bogus relation between what the last wrenCallFunction argument was intended to be and what it was really used for. With that change, the argument is now the number of arguments that are on the stack to perform the execution call. Because of that it is not 0 anymore, since obviously a closure can always have any number arguments.
Because a closure has to accept its arity or more arguments, as per Fn.call, there MUST be a stack interaction if there is more arguments than required.

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