From 1e0a2e603630b300169b503defff2917cbab5ebb Mon Sep 17 00:00:00 2001 From: Bob Nystrom Date: Mon, 31 Aug 2015 07:57:48 -0700 Subject: [PATCH] Get rid of WrenMethod and just use WrenValue for both. Thanks, Michel! --- src/include/wren.h | 24 ++++++------- src/module/timer.c | 6 ++-- src/vm/wren_vm.c | 86 +++++++++++++++++----------------------------- src/vm/wren_vm.h | 25 +++----------- 4 files changed, 49 insertions(+), 92 deletions(-) diff --git a/src/include/wren.h b/src/include/wren.h index e13291fb..8eea11ab 100644 --- a/src/include/wren.h +++ b/src/include/wren.h @@ -10,11 +10,6 @@ // here. typedef struct WrenVM WrenVM; -// A handle to a method, bound to a receiver. -// -// This is used to call a Wren method on some object from C code. -typedef struct WrenMethod WrenMethod; - // A handle to a Wren object. // // This lets code outside of the VM hold a persistent reference to an object. @@ -191,10 +186,9 @@ WrenInterpretResult wrenInterpret(WrenVM* vm, const char* sourcePath, // This handle can be used repeatedly to directly invoke that method from C // code using [wrenCall]. // -// When done with this handle, it must be released by calling -// [wrenReleaseMethod]. -WrenMethod* wrenGetMethod(WrenVM* vm, const char* module, const char* variable, - const char* signature); +// When done with this handle, it must be released using [wrenReleaseValue]. +WrenValue* wrenGetMethod(WrenVM* vm, const char* module, const char* variable, + const char* signature); // Calls [method], passing in a series of arguments whose types must match the // specifed [argTypes]. This is a string where each character identifies the @@ -209,11 +203,13 @@ WrenMethod* wrenGetMethod(WrenVM* vm, const char* module, const char* variable, // Wren. // - "v" - A previously acquired WrenValue*. Passing this in does not implicitly // release the value. -void wrenCall(WrenVM* vm, WrenMethod* method, const char* argTypes, ...); - -// Releases memory associated with [method]. After calling this, [method] can -// no longer be used. -void wrenReleaseMethod(WrenVM* vm, WrenMethod* method); +// +// [method] must have been created by a call to [wrenGetMethod]. If [result] is +// not `NULL`, the return value of the method will be stored in a new +// [WrenValue] that [result] will point to. Don't forget to release it, when +// done with it. +WrenInterpretResult wrenCall(WrenVM* vm, WrenValue* method, WrenValue** result, + const char* argTypes, ...); // Releases the reference stored in [value]. After calling this, [value] can no // longer be used. diff --git a/src/module/timer.c b/src/module/timer.c index 466a269d..0862ac75 100644 --- a/src/module/timer.c +++ b/src/module/timer.c @@ -46,7 +46,7 @@ static const char* timerLibSource = "\n"; // The Wren method to call when a timer has completed. -static WrenMethod* resumeTimer; +static WrenValue* resumeTimer; // Called by libuv when the timer finished closing. static void timerCloseCallback(uv_handle_t* handle) @@ -63,7 +63,7 @@ static void timerCallback(uv_timer_t* handle) uv_close((uv_handle_t*)handle, timerCloseCallback); // Run the fiber that was sleeping. - wrenCall(getVM(), resumeTimer, "v", fiber); + wrenCall(getVM(), resumeTimer, NULL, "v", fiber); wrenReleaseValue(getVM(), fiber); } @@ -107,5 +107,5 @@ WrenForeignMethodFn timerBindForeign( void timerReleaseMethods() { - if (resumeTimer != NULL) wrenReleaseMethod(getVM(), resumeTimer); + if (resumeTimer != NULL) wrenReleaseValue(getVM(), resumeTimer); } diff --git a/src/vm/wren_vm.c b/src/vm/wren_vm.c index cb2177b4..8959d46f 100644 --- a/src/vm/wren_vm.c +++ b/src/vm/wren_vm.c @@ -113,7 +113,6 @@ void wrenFreeVM(WrenVM* vm) // Tell the user if they didn't free any handles. We don't want to just free // them here because the host app may still have pointers to them that they // may try to use. Better to tell them about the bug early. - ASSERT(vm->methodHandles == NULL, "All methods have not been released."); ASSERT(vm->valueHandles == NULL, "All values have not been released."); wrenSymbolTableClear(vm, &vm->methodNames); @@ -158,14 +157,6 @@ static void collectGarbage(WrenVM* vm) // The current fiber. wrenMarkObj(vm, (Obj*)vm->fiber); - // The method handles. - for (WrenMethod* handle = vm->methodHandles; - handle != NULL; - handle = handle->next) - { - wrenMarkObj(vm, (Obj*)handle->fiber); - } - // The value handles. for (WrenValue* value = vm->valueHandles; value != NULL; @@ -1296,8 +1287,8 @@ static ObjFn* makeCallStub(WrenVM* vm, ObjModule* module, const char* signature) signature, signatureLength, debugLines); } -WrenMethod* wrenGetMethod(WrenVM* vm, const char* module, const char* variable, - const char* signature) +WrenValue* wrenGetMethod(WrenVM* vm, const char* module, const char* variable, + const char* signature) { Value moduleName = wrenStringFormat(vm, "$", module); wrenPushRoot(vm, AS_OBJ(moduleName)); @@ -1317,18 +1308,11 @@ WrenMethod* wrenGetMethod(WrenVM* vm, const char* module, const char* variable, wrenPushRoot(vm, (Obj*)fiber); // Create a handle that keeps track of the function that calls the method. - WrenMethod* method = ALLOCATE(vm, WrenMethod); - method->fiber = fiber; + WrenValue* method = wrenCaptureValue(vm, OBJ_VAL(fiber)); // Store the receiver in the fiber's stack so we can use it later in the call. *fiber->stackTop++ = moduleObj->variables.data[variableSlot]; - // Add it to the front of the linked list of handles. - if (vm->methodHandles != NULL) vm->methodHandles->prev = method; - method->prev = NULL; - method->next = vm->methodHandles; - vm->methodHandles = method; - wrenPopRoot(vm); // fiber. wrenPopRoot(vm); // fn. wrenPopRoot(vm); // moduleName. @@ -1336,11 +1320,15 @@ WrenMethod* wrenGetMethod(WrenVM* vm, const char* module, const char* variable, return method; } -void wrenCall(WrenVM* vm, WrenMethod* method, const char* argTypes, ...) +WrenInterpretResult wrenCall(WrenVM* vm, WrenValue* method, WrenValue** result, + const char* argTypes, ...) { // TODO: Validate that the number of arguments matches what the method // expects. + ASSERT(IS_FIBER(method->value), "Value must come from wrenGetMethod()."); + ObjFiber* fiber = AS_FIBER(method->value); + // Push the arguments. va_list argList; va_start(argList, argTypes); @@ -1359,7 +1347,7 @@ void wrenCall(WrenVM* vm, WrenMethod* method, const char* argTypes, ...) value = wrenStringFormat(vm, "$", va_arg(argList, const char*)); break; } - + case 'v': value = va_arg(argList, WrenValue*)->value; break; default: @@ -1367,41 +1355,41 @@ void wrenCall(WrenVM* vm, WrenMethod* method, const char* argTypes, ...) break; } - *method->fiber->stackTop++ = value; + *fiber->stackTop++ = value; } va_end(argList); - Value receiver = method->fiber->stack[0]; - Obj* fn = method->fiber->frames[0].fn; - + Value receiver = fiber->stack[0]; + Obj* fn = fiber->frames[0].fn; + // TODO: How does this handle a runtime error occurring? - runInterpreter(vm, method->fiber); + runInterpreter(vm, fiber); + + if (result != NULL) *result = wrenCaptureValue(vm, fiber->stack[0]); // Reset the fiber to get ready for the next call. - wrenResetFiber(vm, method->fiber, fn); - + wrenResetFiber(vm, fiber, fn); + // Push the receiver back on the stack. - *method->fiber->stackTop++ = receiver; + *fiber->stackTop++ = receiver; + + return WREN_RESULT_SUCCESS; } -void wrenReleaseMethod(WrenVM* vm, WrenMethod* method) +WrenValue* wrenCaptureValue(WrenVM* vm, Value value) { - ASSERT(method != NULL, "NULL method."); + // Make a handle for it. + WrenValue* wrappedValue = ALLOCATE(vm, WrenValue); + wrappedValue->value = value; - // Update the VM's head pointer if we're releasing the first handle. - if (vm->methodHandles == method) vm->methodHandles = method->next; + // Add it to the front of the linked list of handles. + if (vm->valueHandles != NULL) vm->valueHandles->prev = wrappedValue; + wrappedValue->prev = NULL; + wrappedValue->next = vm->valueHandles; + vm->valueHandles = wrappedValue; - // Unlink it from the list. - if (method->prev != NULL) method->prev->next = method->next; - if (method->next != NULL) method->next->prev = method->prev; - - // Clear it out. This isn't strictly necessary since we're going to free it, - // but it makes for easier debugging. - method->prev = NULL; - method->next = NULL; - method->fiber = NULL; - DEALLOCATE(vm, method); + return wrappedValue; } void wrenReleaseValue(WrenVM* vm, WrenValue* value) @@ -1626,18 +1614,8 @@ const char* wrenGetArgumentString(WrenVM* vm, int index) WrenValue* wrenGetArgumentValue(WrenVM* vm, int index) { validateForeignArgument(vm, index); - - // Make a handle for it. - WrenValue* value = ALLOCATE(vm, WrenValue); - value->value = *(vm->foreignCallSlot + index); - // Add it to the front of the linked list of handles. - if (vm->valueHandles != NULL) vm->valueHandles->prev = value; - value->prev = NULL; - value->next = vm->valueHandles; - vm->valueHandles = value; - - return value; + return wrenCaptureValue(vm, *(vm->foreignCallSlot + index)); } void wrenReturnBool(WrenVM* vm, bool value) diff --git a/src/vm/wren_vm.h b/src/vm/wren_vm.h index 7e605537..24186636 100644 --- a/src/vm/wren_vm.h +++ b/src/vm/wren_vm.h @@ -17,29 +17,13 @@ typedef enum #undef OPCODE } Code; -// A handle to a method. -// -// It is a node in the doubly-linked list of currently allocate method handles. -// Each node has a reference to the fiber containing the method stub to call -// the method. -struct WrenMethod -{ - // The fiber that invokes the method. Its stack is pre-populated with the - // receiver for the method, and it contains a single callframe whose function - // is the bytecode stub to invoke the method. - ObjFiber* fiber; - - WrenMethod* prev; - WrenMethod* next; -}; - // A handle to a value, basically just a linked list of extra GC roots. // // Note that even non-heap-allocated values can be stored here. struct WrenValue { Value value; - + WrenValue* prev; WrenValue* next; }; @@ -112,10 +96,6 @@ struct WrenVM // receiver) of the call on the fiber's stack. Value* foreignCallSlot; - // Pointer to the first node in the linked list of active method handles or - // NULL if there are no handles. - WrenMethod* methodHandles; - // Pointer to the first node in the linked list of active value handles or // NULL if there are no handles. WrenValue* valueHandles; @@ -163,6 +143,9 @@ struct WrenVM // [oldSize] will be zero. It should return NULL. void* wrenReallocate(WrenVM* vm, void* memory, size_t oldSize, size_t newSize); +// Creates a new [WrenValue] for [value]. +WrenValue* wrenCaptureValue(WrenVM* vm, Value value); + // Looks up the core module in the module map. ObjModule* wrenGetCoreModule(WrenVM* vm);