diff --git a/src/vm/wren_compiler.c b/src/vm/wren_compiler.c index 66af1432..582548da 100644 --- a/src/vm/wren_compiler.c +++ b/src/vm/wren_compiler.c @@ -498,7 +498,7 @@ static int addConstant(Compiler* compiler, Value constant) // Initializes [compiler]. static void initCompiler(Compiler* compiler, Parser* parser, Compiler* parent, - bool isFunction) + bool isMethod) { compiler->parser = parser; compiler->parent = parent; @@ -512,41 +512,41 @@ static void initCompiler(Compiler* compiler, Parser* parser, Compiler* parent, parser->vm->compiler = compiler; + // Declare a local slot for either the closure or method receiver so that we + // don't try to reuse that slot for a user-defined local variable. For + // methods, we name it "this", so that we can resolve references to that like + // a normal variable. For functions, they have no explicit "this", so we use + // an empty name. That way references to "this" inside a function walks up + // the parent chain to find a method enclosing the function whose "this" we + // can close over. + compiler->numLocals = 1; + compiler->numSlots = compiler->numLocals; + + if (isMethod) + { + compiler->locals[0].name = "this"; + compiler->locals[0].length = 4; + } + else + { + compiler->locals[0].name = NULL; + compiler->locals[0].length = 0; + } + + compiler->locals[0].depth = -1; + compiler->locals[0].isUpvalue = false; + if (parent == NULL) { - compiler->numLocals = 0; - // Compiling top-level code, so the initial scope is module-level. compiler->scopeDepth = -1; } else { - // Declare a fake local variable for the receiver so that it's slot in the - // stack is taken. For methods, we call this "this", so that we can resolve - // references to that like a normal variable. For functions, they have no - // explicit "this". So we pick a bogus name. That way references to "this" - // inside a function will try to walk up the parent chain to find a method - // enclosing the function whose "this" we can close over. - compiler->numLocals = 1; - if (isFunction) - { - compiler->locals[0].name = NULL; - compiler->locals[0].length = 0; - } - else - { - compiler->locals[0].name = "this"; - compiler->locals[0].length = 4; - } - compiler->locals[0].depth = -1; - compiler->locals[0].isUpvalue = false; - - // The initial scope for function or method is a local scope. + // The initial scope for functions and methods is local scope. compiler->scopeDepth = 0; } - compiler->numSlots = compiler->numLocals; - compiler->fn = wrenNewFunction(parser->vm, parser->module, compiler->numLocals); } @@ -1867,7 +1867,7 @@ static void methodCall(Compiler* compiler, Code instruction, called.arity++; Compiler fnCompiler; - initCompiler(&fnCompiler, compiler->parser, compiler, true); + initCompiler(&fnCompiler, compiler->parser, compiler, false); // Make a dummy signature to track the arity. Signature fnSignature = { "", 0, SIG_METHOD, 0 }; @@ -3080,7 +3080,7 @@ static void createConstructor(Compiler* compiler, Signature* signature, int initializerSymbol) { Compiler methodCompiler; - initCompiler(&methodCompiler, compiler->parser, compiler, false); + initCompiler(&methodCompiler, compiler->parser, compiler, true); // Allocate the instance. emitOp(&methodCompiler, compiler->enclosingClass->isForeign @@ -3166,7 +3166,7 @@ static bool method(Compiler* compiler, Variable classVariable) compiler->enclosingClass->signature = &signature; Compiler methodCompiler; - initCompiler(&methodCompiler, compiler->parser, compiler, false); + initCompiler(&methodCompiler, compiler->parser, compiler, true); // Compile the method signature. signatureFn(&methodCompiler, &signature); @@ -3445,7 +3445,7 @@ ObjFn* wrenCompile(WrenVM* vm, ObjModule* module, const char* source, int numExistingVariables = module->variables.count; Compiler compiler; - initCompiler(&compiler, &parser, NULL, true); + initCompiler(&compiler, &parser, NULL, false); ignoreNewlines(&compiler); if (isExpression) diff --git a/src/vm/wren_core.c b/src/vm/wren_core.c index a827b057..f45823b3 100644 --- a/src/vm/wren_core.c +++ b/src/vm/wren_core.c @@ -59,15 +59,7 @@ DEF_PRIMITIVE(fiber_new) RETURN_ERROR("Function cannot take more than one parameter."); } - ObjFiber* newFiber = wrenNewFiber(vm, closure); - - // The compiler expects the first slot of a function to hold the receiver. - // Since a fiber's stack is invoked directly, it doesn't have one, so put it - // in here. - newFiber->stack[0] = NULL_VAL; - newFiber->stackTop++; - - RETURN_OBJ(newFiber); + RETURN_OBJ(wrenNewFiber(vm, closure)); } DEF_PRIMITIVE(fiber_abort) diff --git a/src/vm/wren_value.c b/src/vm/wren_value.c index e16f25f4..548a8008 100644 --- a/src/vm/wren_value.c +++ b/src/vm/wren_value.c @@ -159,27 +159,31 @@ ObjFiber* wrenNewFiber(WrenVM* vm, ObjClosure* closure) ObjFiber* fiber = ALLOCATE(vm, ObjFiber); initObj(vm, &fiber->obj, OBJ_FIBER, vm->fiberClass); + + fiber->stack = stack; + fiber->stackTop = fiber->stack; + fiber->stackCapacity = stackCapacity; + fiber->frames = frames; fiber->frameCapacity = INITIAL_CALL_FRAMES; - fiber->stack = stack; - fiber->stackCapacity = stackCapacity; - wrenResetFiber(vm, fiber, closure); + fiber->numFrames = 0; - return fiber; -} - -void wrenResetFiber(WrenVM* vm, ObjFiber* fiber, ObjClosure* closure) -{ - // Reset everything. - fiber->stackTop = fiber->stack; fiber->openUpvalues = NULL; fiber->caller = NULL; fiber->error = NULL_VAL; fiber->callerIsTrying = false; - fiber->numFrames = 0; + + if (closure != NULL) + { + // Initialize the first call frame. + wrenAppendCallFrame(vm, fiber, closure, fiber->stack); - // Initialize the first call frame. - if (closure != NULL) wrenAppendCallFrame(vm, fiber, closure, fiber->stack); + // The first slot always holds the closure. + fiber->stackTop[0] = OBJ_VAL(closure); + fiber->stackTop++; + } + + return fiber; } void wrenEnsureStack(WrenVM* vm, ObjFiber* fiber, int needed) diff --git a/src/vm/wren_value.h b/src/vm/wren_value.h index ffb79f08..d6d5ff43 100644 --- a/src/vm/wren_value.h +++ b/src/vm/wren_value.h @@ -628,10 +628,6 @@ ObjClosure* wrenNewClosure(WrenVM* vm, ObjFn* fn); // Creates a new fiber object that will invoke [closure]. ObjFiber* wrenNewFiber(WrenVM* vm, ObjClosure* closure); -// Resets [fiber] back to an initial state where it is ready to invoke -// [closure]. -void wrenResetFiber(WrenVM* vm, ObjFiber* fiber, ObjClosure* closure); - // Adds a new [CallFrame] to [fiber] invoking [closure] whose stack starts at // [stackStart]. static inline void wrenAppendCallFrame(WrenVM* vm, ObjFiber* fiber, diff --git a/src/vm/wren_vm.c b/src/vm/wren_vm.c index 33c18e45..bc7e9e7b 100644 --- a/src/vm/wren_vm.c +++ b/src/vm/wren_vm.c @@ -1249,17 +1249,18 @@ static WrenInterpretResult runInterpreter(WrenVM* vm, register ObjFiber* fiber) Value name = fn->constants.data[READ_SHORT()]; // Make a slot on the stack for the module's fiber to place the return - // value. It will be popped after this fiber is resumed. - PUSH(NULL_VAL); + // value. It will be popped after this fiber is resumed. Store the + // imported module's closure in the slot in case a GC happens when + // invoking the closure. + PUSH(wrenImportModule(vm, name)); - Value result = wrenImportModule(vm, name); if (!IS_NULL(fiber->error)) RUNTIME_ERROR(); // If we get a closure, call it to execute the module body. - if (IS_CLOSURE(result)) + if (IS_CLOSURE(PEEK())) { STORE_FRAME(); - ObjClosure* closure = AS_CLOSURE(result); + ObjClosure* closure = AS_CLOSURE(PEEK()); callFunction(vm, fiber, closure, 1); LOAD_FRAME(); } @@ -1417,7 +1418,7 @@ WrenInterpretResult wrenInterpretInModule(WrenVM* vm, const char* module, wrenPushRoot(vm, (Obj*)closure); ObjFiber* fiber = wrenNewFiber(vm, closure); wrenPopRoot(vm); // closure. - + return runInterpreter(vm, fiber); } diff --git a/test/language/variable/many_locals.wren b/test/language/variable/many_locals.wren index 97cc20c7..f2716d66 100644 --- a/test/language/variable/many_locals.wren +++ b/test/language/variable/many_locals.wren @@ -1,6 +1,6 @@ { - var a0 = "value" - var a1 = a0 + // Slot zero is always taken to hold the closure or receiver. + var a1 = "value" var a2 = a1 var a3 = a2 var a4 = a3 diff --git a/test/language/variable/many_nonsimultaneous_locals.wren b/test/language/variable/many_nonsimultaneous_locals.wren index 8320c1e9..fde7a127 100644 --- a/test/language/variable/many_nonsimultaneous_locals.wren +++ b/test/language/variable/many_nonsimultaneous_locals.wren @@ -1,10 +1,10 @@ -// Can have more than 256 local variables in a local scope, as long as they +// Can have more than 255 local variables in a local scope, as long as they // aren't all in scope at the same time. { { - var a0 = "value a" - var a1 = a0 + // Slot zero is always taken to hold the closure or receiver. + var a1 = "value a" var a2 = a1 var a3 = a2 var a4 = a3 @@ -263,8 +263,8 @@ } { - var b0 = "value b" - var b1 = b0 + // Slot zero is always taken to hold the closure or receiver. + var b1 = "value b" var b2 = b1 var b3 = b2 var b4 = b3 diff --git a/test/language/variable/too_many_locals.wren b/test/language/variable/too_many_locals.wren index 0c942ac0..3e6bf6d1 100644 --- a/test/language/variable/too_many_locals.wren +++ b/test/language/variable/too_many_locals.wren @@ -1,6 +1,6 @@ { - var a0 = "value" - var a1 = a0 + // Slot zero is always taken to hold the closure or receiver. + var a1 = "value" var a2 = a1 var a3 = a2 var a4 = a3 diff --git a/test/language/variable/too_many_locals_nested.wren b/test/language/variable/too_many_locals_nested.wren index 0c9736ac..9cb4801e 100644 --- a/test/language/variable/too_many_locals_nested.wren +++ b/test/language/variable/too_many_locals_nested.wren @@ -1,6 +1,6 @@ { - var a0 = "value" - var a1 = a0 + // Slot zero is always taken to hold the closure or receiver. + var a1 = "value" var a2 = a1 var a3 = a2 var a4 = a3