From 4b3c818ec5e2a7231d5aefc802768f7ede93bd2e Mon Sep 17 00:00:00 2001 From: Bob Nystrom Date: Wed, 16 Dec 2015 10:22:42 -0800 Subject: [PATCH 01/26] Start moving embedding API towards "slot" register-API. - wrenGetArgumentCount() -> wrenGetSlotCount() - wrenGetArgument___() -> wrenGetSlot___() Also, the get functions assert that the value is the right type instead of checking at runtime. This puts the onus on the caller to be safe, but maximizes performance. --- src/include/wren.h | 43 ++++++++++++--------- src/module/io.c | 70 +++++++++++++++++----------------- src/module/timer.c | 4 +- src/optional/wren_opt_meta.c | 2 +- src/optional/wren_opt_random.c | 14 +++---- src/vm/wren_value.h | 1 + src/vm/wren_vm.c | 70 +++++++++++++++++----------------- test/api/benchmark.c | 13 ++++--- test/api/foreign_class.c | 26 ++++++------- test/api/value.c | 2 +- 10 files changed, 125 insertions(+), 120 deletions(-) diff --git a/src/include/wren.h b/src/include/wren.h index 29645f83..53cdc1a9 100644 --- a/src/include/wren.h +++ b/src/include/wren.h @@ -120,7 +120,7 @@ typedef struct // // If this is `NULL`, Wren discards any printed text. WrenWriteFn writeFn; - + // The number of bytes Wren will allocate before triggering the first garbage // collection. // @@ -237,8 +237,8 @@ void wrenReleaseValue(WrenVM* vm, WrenValue* value); // the foreign object's data. void* wrenAllocateForeign(WrenVM* vm, size_t size); -// Returns the number of arguments available to the current foreign method. -int wrenGetArgumentCount(WrenVM* vm); +// Returns the number of slots available to the current foreign method. +int wrenGetSlotCount(WrenVM* vm); // The following functions read one of the arguments passed to a foreign call. // They may only be called while within a function provided to @@ -260,32 +260,37 @@ int wrenGetArgumentCount(WrenVM* vm); // // It is an error to pass an invalid argument index. -// Reads a boolean argument for a foreign call. Returns false if the argument -// is not a boolean. -bool wrenGetArgumentBool(WrenVM* vm, int index); +// Reads a boolean value from [slot]. +// +// It is an error to call this if the slot does not contain a boolean value. +bool wrenGetSlotBool(WrenVM* vm, int slot); -// Reads a numeric argument for a foreign call. Returns 0 if the argument is not -// a number. -double wrenGetArgumentDouble(WrenVM* vm, int index); +// Reads a number from [slot]. +// +// It is an error to call this if the slot does not contain a number. +double wrenGetSlotDouble(WrenVM* vm, int slot); -// Reads a foreign object argument for a foreign call and returns a pointer to -// the foreign data stored with it. Returns NULL if the argument is not a -// foreign object. -void* wrenGetArgumentForeign(WrenVM* vm, int index); +// Reads a foreign object from [slot] and returns a pointer to the foreign data +// stored with it. +// +// It is an error to call this if the slot does not contain an instance of a +// foreign class. +void* wrenGetSlotForeign(WrenVM* vm, int slot); -// Reads an string argument for a foreign call. Returns NULL if the argument is -// not a string. +// Reads a string from [slot]. // // The memory for the returned string is owned by Wren. You can inspect it -// while in your foreign function, but cannot keep a pointer to it after the +// while in your foreign method, but cannot keep a pointer to it after the // function returns, since the garbage collector may reclaim it. -const char* wrenGetArgumentString(WrenVM* vm, int index); +// +// It is an error to call this if the slot does not contain a string. +const char* wrenGetSlotString(WrenVM* vm, int slot); -// Creates a handle for the value passed as an argument to a foreign call. +// Creates a handle for the value stored in [slot]. // // This will prevent the object that is referred to from being garbage collected // until the handle is released by calling [wrenReleaseValue()]. -WrenValue* wrenGetArgumentValue(WrenVM* vm, int index); +WrenValue* wrenGetSlotValue(WrenVM* vm, int slot); // The following functions provide the return value for a foreign method back // to Wren. Like above, they may only be called during a foreign call invoked diff --git a/src/module/io.c b/src/module/io.c index 9f3a836b..bd03bd52 100644 --- a/src/module/io.c +++ b/src/module/io.c @@ -33,7 +33,7 @@ static void shutdownStdin() free(stdinStream); stdinStream = NULL; } - + if (stdinOnData != NULL) { wrenReleaseValue(getVM(), stdinOnData); @@ -51,16 +51,16 @@ void fileAllocate(WrenVM* vm) // Store the file descriptor in the foreign data, so that we can get to it // in the finalizer. int* fd = (int*)wrenAllocateForeign(vm, sizeof(int)); - *fd = (int)wrenGetArgumentDouble(vm, 1); + *fd = (int)wrenGetSlotDouble(vm, 1); } void fileFinalize(WrenVM* vm) { - int fd = *(int*)wrenGetArgumentForeign(vm, 0); - + int fd = *(int*)wrenGetSlotForeign(vm, 0); + // Already closed. if (fd == -1) return; - + uv_fs_t request; uv_fs_close(getLoop(), &request, fd, NULL); uv_fs_req_cleanup(&request); @@ -73,7 +73,7 @@ void fileFinalize(WrenVM* vm) static bool handleRequestError(uv_fs_t* request) { if (request->result >= 0) return false; - + FileRequestData* data = (FileRequestData*)request->data; WrenValue* fiber = (WrenValue*)data->fiber; @@ -115,18 +115,18 @@ WrenValue* freeRequest(uv_fs_t* request) static void openCallback(uv_fs_t* request) { if (handleRequestError(request)) return; - + double fd = (double)request->result; WrenValue* fiber = freeRequest(request); - + schedulerResumeDouble(fiber, fd); } void fileOpen(WrenVM* vm) { - const char* path = wrenGetArgumentString(vm, 1); - uv_fs_t* request = createRequest(wrenGetArgumentValue(vm, 2)); - + const char* path = wrenGetSlotString(vm, 1); + uv_fs_t* request = createRequest(wrenGetSlotValue(vm, 2)); + // TODO: Allow controlling flags and modes. uv_fs_open(getLoop(), request, path, O_RDONLY, 0, openCallback); } @@ -135,51 +135,51 @@ void fileOpen(WrenVM* vm) static void sizeCallback(uv_fs_t* request) { if (handleRequestError(request)) return; - + double size = (double)request->statbuf.st_size; WrenValue* fiber = freeRequest(request); - + schedulerResumeDouble(fiber, size); } void fileSizePath(WrenVM* vm) { - const char* path = wrenGetArgumentString(vm, 1); - uv_fs_t* request = createRequest(wrenGetArgumentValue(vm, 2)); + const char* path = wrenGetSlotString(vm, 1); + uv_fs_t* request = createRequest(wrenGetSlotValue(vm, 2)); uv_fs_stat(getLoop(), request, path, sizeCallback); } static void closeCallback(uv_fs_t* request) { if (handleRequestError(request)) return; - + WrenValue* fiber = freeRequest(request); schedulerResume(fiber); } void fileClose(WrenVM* vm) { - int* foreign = (int*)wrenGetArgumentForeign(vm, 0); + int* foreign = (int*)wrenGetSlotForeign(vm, 0); int fd = *foreign; - + // If it's already closed, we're done. if (fd == -1) { wrenReturnBool(vm, true); return; } - + // Mark it closed immediately. *foreign = -1; - - uv_fs_t* request = createRequest(wrenGetArgumentValue(vm, 1)); + + uv_fs_t* request = createRequest(wrenGetSlotValue(vm, 1)); uv_fs_close(getLoop(), request, fd, closeCallback); wrenReturnBool(vm, false); } void fileDescriptor(WrenVM* vm) { - int* foreign = (int*)wrenGetArgumentForeign(vm, 0); + int* foreign = (int*)wrenGetSlotForeign(vm, 0); int fd = *foreign; wrenReturnDouble(vm, fd); } @@ -197,35 +197,35 @@ static void fileReadBytesCallback(uv_fs_t* request) // embedding API supported a way to *give* it bytes that were previously // allocated using Wren's own allocator. schedulerResumeBytes(fiber, buffer.base, buffer.len); - + // TODO: Likewise, freeing this after we resume is lame. free(buffer.base); } void fileReadBytes(WrenVM* vm) { - uv_fs_t* request = createRequest(wrenGetArgumentValue(vm, 2)); - - int fd = *(int*)wrenGetArgumentForeign(vm, 0); + uv_fs_t* request = createRequest(wrenGetSlotValue(vm, 2)); + + int fd = *(int*)wrenGetSlotForeign(vm, 0); // TODO: Assert fd != -1. FileRequestData* data = (FileRequestData*)request->data; - size_t length = (size_t)wrenGetArgumentDouble(vm, 1); + size_t length = (size_t)wrenGetSlotDouble(vm, 1); data->buffer.len = length; data->buffer.base = (char*)malloc(length); - + // TODO: Allow passing in offset. uv_fs_read(getLoop(), request, fd, &data->buffer, 1, 0, fileReadBytesCallback); } void fileSize(WrenVM* vm) { - uv_fs_t* request = createRequest(wrenGetArgumentValue(vm, 1)); + uv_fs_t* request = createRequest(wrenGetSlotValue(vm, 1)); - int fd = *(int*)wrenGetArgumentForeign(vm, 0); + int fd = *(int*)wrenGetSlotForeign(vm, 0); // TODO: Assert fd != -1. - + uv_fs_fstat(getLoop(), request, fd, sizeCallback); } @@ -247,9 +247,9 @@ static void stdinReadCallback(uv_stream_t* stream, ssize_t numRead, shutdownStdin(); return; } - + // TODO: Handle other errors. - + if (stdinOnData == NULL) { stdinOnData = wrenGetMethod(getVM(), "io", "Stdin", "onData_(_)"); @@ -259,7 +259,7 @@ static void stdinReadCallback(uv_stream_t* stream, ssize_t numRead, // embedding API supported a way to *give* it bytes that were previously // allocated using Wren's own allocator. wrenCall(getVM(), stdinOnData, NULL, "a", buffer->base, numRead); - + // TODO: Likewise, freeing this after we resume is lame. free(buffer->base); } @@ -284,7 +284,7 @@ void stdinReadStart(WrenVM* vm) stdinStream = (uv_stream_t*)handle; } } - + uv_read_start(stdinStream, allocCallback, stdinReadCallback); // TODO: Check return. } diff --git a/src/module/timer.c b/src/module/timer.c index 43b8d662..a9d98eca 100644 --- a/src/module/timer.c +++ b/src/module/timer.c @@ -27,8 +27,8 @@ static void timerCallback(uv_timer_t* handle) void timerStartTimer(WrenVM* vm) { - int milliseconds = (int)wrenGetArgumentDouble(vm, 1); - WrenValue* fiber = wrenGetArgumentValue(vm, 2); + int milliseconds = (int)wrenGetSlotDouble(vm, 1); + WrenValue* fiber = wrenGetSlotValue(vm, 2); // Store the fiber to resume when the timer completes. uv_timer_t* handle = (uv_timer_t*)malloc(sizeof(uv_timer_t)); diff --git a/src/optional/wren_opt_meta.c b/src/optional/wren_opt_meta.c index 435ab678..5428cc23 100644 --- a/src/optional/wren_opt_meta.c +++ b/src/optional/wren_opt_meta.c @@ -18,7 +18,7 @@ void metaCompile(WrenVM* vm) : AS_CLOSURE(callingFn)->fn->module; // Compile it. - ObjFn* fn = wrenCompile(vm, module, wrenGetArgumentString(vm, 1), false); + ObjFn* fn = wrenCompile(vm, module, wrenGetSlotString(vm, 1), false); if (fn == NULL) return; // Return the result. We can't use the public API for this since we have a diff --git a/src/optional/wren_opt_random.c b/src/optional/wren_opt_random.c index b02c7ca7..3735a4db 100644 --- a/src/optional/wren_opt_random.c +++ b/src/optional/wren_opt_random.c @@ -45,7 +45,7 @@ static void randomAllocate(WrenVM* vm) static void randomSeed0(WrenVM* vm) { - Well512* well = (Well512*)wrenGetArgumentForeign(vm, 0); + Well512* well = (Well512*)wrenGetSlotForeign(vm, 0); srand((uint32_t)time(NULL)); for (int i = 0; i < 16; i++) @@ -56,9 +56,9 @@ static void randomSeed0(WrenVM* vm) static void randomSeed1(WrenVM* vm) { - Well512* well = (Well512*)wrenGetArgumentForeign(vm, 0); + Well512* well = (Well512*)wrenGetSlotForeign(vm, 0); - srand((uint32_t)wrenGetArgumentDouble(vm, 1)); + srand((uint32_t)wrenGetSlotDouble(vm, 1)); for (int i = 0; i < 16; i++) { well->state[i] = rand(); @@ -67,17 +67,17 @@ static void randomSeed1(WrenVM* vm) static void randomSeed16(WrenVM* vm) { - Well512* well = (Well512*)wrenGetArgumentForeign(vm, 0); + Well512* well = (Well512*)wrenGetSlotForeign(vm, 0); for (int i = 0; i < 16; i++) { - well->state[i] = (uint32_t)wrenGetArgumentDouble(vm, i + 1); + well->state[i] = (uint32_t)wrenGetSlotDouble(vm, i + 1); } } static void randomFloat(WrenVM* vm) { - Well512* well = (Well512*)wrenGetArgumentForeign(vm, 0); + Well512* well = (Well512*)wrenGetSlotForeign(vm, 0); // A double has 53 bits of precision in its mantissa, and we'd like to take // full advantage of that, so we need 53 bits of random source data. @@ -97,7 +97,7 @@ static void randomFloat(WrenVM* vm) static void randomInt0(WrenVM* vm) { - Well512* well = (Well512*)wrenGetArgumentForeign(vm, 0); + Well512* well = (Well512*)wrenGetSlotForeign(vm, 0); wrenReturnDouble(vm, (double)advanceState(well)); } diff --git a/src/vm/wren_value.h b/src/vm/wren_value.h index 702538ff..21e871fd 100644 --- a/src/vm/wren_value.h +++ b/src/vm/wren_value.h @@ -74,6 +74,7 @@ #define IS_FN(value) (wrenIsObjType(value, OBJ_FN)) // ObjFn #define IS_FOREIGN(value) (wrenIsObjType(value, OBJ_FOREIGN)) // ObjForeign #define IS_INSTANCE(value) (wrenIsObjType(value, OBJ_INSTANCE)) // ObjInstance +#define IS_LIST(value) (wrenIsObjType(value, OBJ_LIST)) // ObjList #define IS_RANGE(value) (wrenIsObjType(value, OBJ_RANGE)) // ObjRange #define IS_STRING(value) (wrenIsObjType(value, OBJ_STRING)) // ObjString diff --git a/src/vm/wren_vm.c b/src/vm/wren_vm.c index e6157160..4d44d49f 100644 --- a/src/vm/wren_vm.c +++ b/src/vm/wren_vm.c @@ -439,21 +439,21 @@ static inline void callFunction( sizeof(CallFrame) * max); fiber->frameCapacity = max; } - + // Grow the stack if needed. int stackSize = (int)(fiber->stackTop - fiber->stack); int needed = stackSize + wrenUpwrapClosure(function)->maxSlots; - + if (fiber->stackCapacity < needed) { int capacity = wrenPowerOf2Ceil(needed); - + Value* oldStack = fiber->stack; fiber->stack = (Value*)wrenReallocate(vm, fiber->stack, sizeof(Value) * fiber->stackCapacity, sizeof(Value) * capacity); fiber->stackCapacity = capacity; - + // If the reallocation moves the stack, then we need to shift every pointer // into the stack to point to its new location. if (fiber->stack != oldStack) @@ -461,13 +461,13 @@ static inline void callFunction( // Top of the stack. long offset = fiber->stack - oldStack; fiber->stackTop += offset; - + // Stack pointer for each call frame. for (int i = 0; i < fiber->numFrames; i++) { fiber->frames[i].stackStart += offset; } - + // Open upvalues. for (ObjUpvalue* upvalue = fiber->openUpvalues; upvalue != NULL; @@ -1148,10 +1148,10 @@ static WrenInterpretResult runInterpreter(WrenVM* vm, register ObjFiber* fiber) // See if there's another fiber to return to. ObjFiber* callingFiber = fiber->caller; fiber->caller = NULL; - + fiber = callingFiber; vm->fiber = fiber; - + // If not, we're done. if (fiber == NULL) return WREN_RESULT_SUCCESS; @@ -1624,10 +1624,10 @@ void wrenPopRoot(WrenVM* vm) vm->numTempRoots--; } -int wrenGetArgumentCount(WrenVM* vm) +int wrenGetSlotCount(WrenVM* vm) { ASSERT(vm->foreignStackStart != NULL, "Must be in foreign call."); - + // If no fiber is executing, we must be in a finalizer, in which case the // "stack" just has one object, the object being finalized. if (vm->fiber == NULL) return 1; @@ -1635,54 +1635,52 @@ int wrenGetArgumentCount(WrenVM* vm) return (int)(vm->fiber->stackTop - vm->foreignStackStart); } -static void validateForeignArgument(WrenVM* vm, int index) +// Ensures that [slot] is a valid index into a foreign method's stack of slots. +static void validateForeignSlot(WrenVM* vm, int slot) { ASSERT(vm->foreignStackStart != NULL, "Must be in foreign call."); - ASSERT(index >= 0, "index cannot be negative."); - ASSERT(index < wrenGetArgumentCount(vm), "Not that many arguments."); + ASSERT(slot >= 0, "Slot cannot be negative."); + ASSERT(slot < wrenGetSlotCount(vm), "Not that many slots."); } -bool wrenGetArgumentBool(WrenVM* vm, int index) +bool wrenGetSlotBool(WrenVM* vm, int slot) { - validateForeignArgument(vm, index); + validateForeignSlot(vm, slot); + ASSERT(IS_BOOL(vm->foreignStackStart[slot]), "Slot must hold a bool."); - if (!IS_BOOL(vm->foreignStackStart[index])) return false; - - return AS_BOOL(vm->foreignStackStart[index]); + return AS_BOOL(vm->foreignStackStart[slot]); } -double wrenGetArgumentDouble(WrenVM* vm, int index) +double wrenGetSlotDouble(WrenVM* vm, int slot) { - validateForeignArgument(vm, index); + validateForeignSlot(vm, slot); + ASSERT(IS_NUM(vm->foreignStackStart[slot]), "Slot must hold a number."); - if (!IS_NUM(vm->foreignStackStart[index])) return 0.0; - - return AS_NUM(vm->foreignStackStart[index]); + return AS_NUM(vm->foreignStackStart[slot]); } -void* wrenGetArgumentForeign(WrenVM* vm, int index) +void* wrenGetSlotForeign(WrenVM* vm, int slot) { - validateForeignArgument(vm, index); + validateForeignSlot(vm, slot); + ASSERT(IS_FOREIGN(vm->foreignStackStart[slot]), + "Slot must hold a foreign instance."); - if (!IS_FOREIGN(vm->foreignStackStart[index])) return NULL; - - return AS_FOREIGN(vm->foreignStackStart[index])->data; + return AS_FOREIGN(vm->foreignStackStart[slot])->data; } -const char* wrenGetArgumentString(WrenVM* vm, int index) +const char* wrenGetSlotString(WrenVM* vm, int slot) { - validateForeignArgument(vm, index); + validateForeignSlot(vm, slot); + ASSERT(IS_STRING(vm->foreignStackStart[slot]), "Slot must hold a string."); - if (!IS_STRING(vm->foreignStackStart[index])) return NULL; - - return AS_CSTRING(vm->foreignStackStart[index]); + return AS_CSTRING(vm->foreignStackStart[slot]); } -WrenValue* wrenGetArgumentValue(WrenVM* vm, int index) +WrenValue* wrenGetSlotValue(WrenVM* vm, int slot) { - validateForeignArgument(vm, index); + validateForeignSlot(vm, slot); - return wrenCaptureValue(vm, vm->foreignStackStart[index]); + return wrenCaptureValue(vm, vm->foreignStackStart[slot]); } void wrenReturnBool(WrenVM* vm, bool value) diff --git a/test/api/benchmark.c b/test/api/benchmark.c index 5c7f4ccb..d5c84a8f 100644 --- a/test/api/benchmark.c +++ b/test/api/benchmark.c @@ -5,17 +5,18 @@ static void arguments(WrenVM* vm) { double result = 0; - result += wrenGetArgumentDouble(vm, 1); - result += wrenGetArgumentDouble(vm, 2); - result += wrenGetArgumentDouble(vm, 3); - result += wrenGetArgumentDouble(vm, 4); - + + result += wrenGetSlotDouble(vm, 1); + result += wrenGetSlotDouble(vm, 2); + result += wrenGetSlotDouble(vm, 3); + result += wrenGetSlotDouble(vm, 4); + wrenReturnDouble(vm, result); } WrenForeignMethodFn benchmarkBindMethod(const char* signature) { if (strcmp(signature, "static Benchmark.arguments(_,_,_,_)") == 0) return arguments; - + return NULL; } diff --git a/test/api/foreign_class.c b/test/api/foreign_class.c index cf9fbfeb..cd544def 100644 --- a/test/api/foreign_class.c +++ b/test/api/foreign_class.c @@ -18,15 +18,15 @@ static void counterAllocate(WrenVM* vm) static void counterIncrement(WrenVM* vm) { - double* value = (double*)wrenGetArgumentForeign(vm, 0); - double increment = wrenGetArgumentDouble(vm, 1); + double* value = (double*)wrenGetSlotForeign(vm, 0); + double increment = wrenGetSlotDouble(vm, 1); *value += increment; } static void counterValue(WrenVM* vm) { - double value = *(double*)wrenGetArgumentForeign(vm, 0); + double value = *(double*)wrenGetSlotForeign(vm, 0); wrenReturnDouble(vm, value); } @@ -34,9 +34,9 @@ static void pointAllocate(WrenVM* vm) { double* coordinates = (double*)wrenAllocateForeign(vm, sizeof(double[3])); - // This gets called by both constructors, so sniff the argument count to see + // This gets called by both constructors, so sniff the slot count to see // which one was invoked. - if (wrenGetArgumentCount(vm) == 1) + if (wrenGetSlotCount(vm) == 1) { coordinates[0] = 0.0; coordinates[1] = 0.0; @@ -44,23 +44,23 @@ static void pointAllocate(WrenVM* vm) } else { - coordinates[0] = wrenGetArgumentDouble(vm, 1); - coordinates[1] = wrenGetArgumentDouble(vm, 2); - coordinates[2] = wrenGetArgumentDouble(vm, 3); + coordinates[0] = wrenGetSlotDouble(vm, 1); + coordinates[1] = wrenGetSlotDouble(vm, 2); + coordinates[2] = wrenGetSlotDouble(vm, 3); } } static void pointTranslate(WrenVM* vm) { - double* coordinates = (double*)wrenGetArgumentForeign(vm, 0); - coordinates[0] += wrenGetArgumentDouble(vm, 1); - coordinates[1] += wrenGetArgumentDouble(vm, 2); - coordinates[2] += wrenGetArgumentDouble(vm, 3); + double* coordinates = (double*)wrenGetSlotForeign(vm, 0); + coordinates[0] += wrenGetSlotDouble(vm, 1); + coordinates[1] += wrenGetSlotDouble(vm, 2); + coordinates[2] += wrenGetSlotDouble(vm, 3); } static void pointToString(WrenVM* vm) { - double* coordinates = (double*)wrenGetArgumentForeign(vm, 0); + double* coordinates = (double*)wrenGetSlotForeign(vm, 0); char result[100]; sprintf(result, "(%g, %g, %g)", coordinates[0], coordinates[1], coordinates[2]); diff --git a/test/api/value.c b/test/api/value.c index 5b7b6225..e26d4978 100644 --- a/test/api/value.c +++ b/test/api/value.c @@ -6,7 +6,7 @@ static WrenValue* value; static void setValue(WrenVM* vm) { - value = wrenGetArgumentValue(vm, 1); + value = wrenGetSlotValue(vm, 1); } static void getValue(WrenVM* vm) From 7fcdcf2f1a5a14b5b20cf033b6efdcfb3095a5e4 Mon Sep 17 00:00:00 2001 From: Bob Nystrom Date: Wed, 16 Dec 2015 13:00:13 -0800 Subject: [PATCH 02/26] wrenReturn___() -> wrenSlotSet___(). This turns those functions into general-purpose functions for writing raw C values into slots on the foreign call stack. Writing a return just means writing a value to slot 0. --- src/include/wren.h | 55 +++++++++----- src/module/io.c | 6 +- src/optional/wren_opt_meta.c | 4 +- src/optional/wren_opt_random.c | 4 +- src/vm/wren_vm.c | 93 ++++++++++++++--------- test/api/benchmark.c | 2 +- test/api/foreign_class.c | 6 +- test/api/main.c | 4 +- test/api/returns.c | 51 ------------- test/api/returns.h | 3 - test/api/returns.wren | 23 ------ test/api/slots.c | 83 ++++++++++++++++++++ test/api/slots.h | 3 + test/api/slots.wren | 16 ++++ test/api/value.c | 2 +- util/xcode/wren.xcodeproj/project.pbxproj | 12 +-- 16 files changed, 215 insertions(+), 152 deletions(-) delete mode 100644 test/api/returns.c delete mode 100644 test/api/returns.h delete mode 100644 test/api/returns.wren create mode 100644 test/api/slots.c create mode 100644 test/api/slots.h create mode 100644 test/api/slots.wren diff --git a/src/include/wren.h b/src/include/wren.h index 53cdc1a9..f107f6e3 100644 --- a/src/include/wren.h +++ b/src/include/wren.h @@ -240,6 +240,8 @@ void* wrenAllocateForeign(WrenVM* vm, size_t size); // Returns the number of slots available to the current foreign method. int wrenGetSlotCount(WrenVM* vm); +// TODO: Update docs. + // The following functions read one of the arguments passed to a foreign call. // They may only be called while within a function provided to // [wrenDefineMethod] or [wrenDefineStaticMethod] that Wren has invoked. @@ -265,6 +267,18 @@ int wrenGetSlotCount(WrenVM* vm); // It is an error to call this if the slot does not contain a boolean value. bool wrenGetSlotBool(WrenVM* vm, int slot); +// Reads a byte array from [slot]. +// +// The memory for the returned string is owned by Wren. You can inspect it +// while in your foreign method, but cannot keep a pointer to it after the +// function returns, since the garbage collector may reclaim it. +// +// Returns a pointer to the first byte of the array and fill [length] with the +// number of bytes in the array. +// +// It is an error to call this if the slot does not contain a string. +const char* wrenGetSlotBytes(WrenVM* vm, int slot, int* length); + // Reads a number from [slot]. // // It is an error to call this if the slot does not contain a number. @@ -301,27 +315,32 @@ WrenValue* wrenGetSlotValue(WrenVM* vm, int slot); // call one of these once. It is an error to access any of the foreign calls // arguments after one of these has been called. -// Provides a boolean return value for a foreign call. -void wrenReturnBool(WrenVM* vm, bool value); +// Stores the boolean [value] in [slot]. +void wrenSetSlotBool(WrenVM* vm, int slot, bool value); -// Provides a numeric return value for a foreign call. -void wrenReturnDouble(WrenVM* vm, double value); +// Stores the array [length] of [bytes] in [slot]. +// +// The bytes are copied to a new string within Wren's heap, so you can free +// memory used by them after this is called. +void wrenSetSlotBytes(WrenVM* vm, int slot, const char* bytes, int length); -// Provides a string return value for a foreign call. -// -// The [text] will be copied to a new string within Wren's heap, so you can -// free memory used by it after this is called. -// -// If [length] is non-zero, Wren copies that many bytes from [text], including -// any null bytes. If it is -1, then the length of [text] is calculated using -// `strlen()`. If the string may contain any null bytes in the middle, then you -// must pass an explicit length. -void wrenReturnString(WrenVM* vm, const char* text, int length); +// Stores the numeric [value] in [slot]. +void wrenSetSlotDouble(WrenVM* vm, int slot, double value); -// Provides the return value for a foreign call. +// Stores null in [slot]. +void wrenSetSlotNull(WrenVM* vm, int slot); + +// Stores the string [text] in [slot]. // -// This uses the value referred to by the handle as the return value, but it -// does not release the handle. -void wrenReturnValue(WrenVM* vm, WrenValue* value); +// The [text] is copied to a new string within Wren's heap, so you can free +// memory used by it after this is called. The length is calculated using +// [strlen()]. If the string may contain any null bytes in the middle, then you +// should use [wrenSetSlotBytes()] instead. +void wrenSetSlotString(WrenVM* vm, int slot, const char* text); + +// Stores the value captured in [value] in [slot]. +// +// This does not release the handle for the value. +void wrenSetSlotValue(WrenVM* vm, int slot, WrenValue* value); #endif diff --git a/src/module/io.c b/src/module/io.c index bd03bd52..d118e6e1 100644 --- a/src/module/io.c +++ b/src/module/io.c @@ -165,7 +165,7 @@ void fileClose(WrenVM* vm) // If it's already closed, we're done. if (fd == -1) { - wrenReturnBool(vm, true); + wrenSetSlotBool(vm, 0, true); return; } @@ -174,14 +174,14 @@ void fileClose(WrenVM* vm) uv_fs_t* request = createRequest(wrenGetSlotValue(vm, 1)); uv_fs_close(getLoop(), request, fd, closeCallback); - wrenReturnBool(vm, false); + wrenSetSlotBool(vm, 0, false); } void fileDescriptor(WrenVM* vm) { int* foreign = (int*)wrenGetSlotForeign(vm, 0); int fd = *foreign; - wrenReturnDouble(vm, fd); + wrenSetSlotDouble(vm, 0, fd); } static void fileReadBytesCallback(uv_fs_t* request) diff --git a/src/optional/wren_opt_meta.c b/src/optional/wren_opt_meta.c index 5428cc23..6bebcd3e 100644 --- a/src/optional/wren_opt_meta.c +++ b/src/optional/wren_opt_meta.c @@ -19,12 +19,10 @@ void metaCompile(WrenVM* vm) // Compile it. ObjFn* fn = wrenCompile(vm, module, wrenGetSlotString(vm, 1), false); - if (fn == NULL) return; // Return the result. We can't use the public API for this since we have a // bare ObjFn. - *vm->foreignStackStart = OBJ_VAL(fn); - vm->foreignStackStart = NULL; + vm->foreignStackStart[0] = fn != NULL ? OBJ_VAL(fn) : NULL_VAL; } static WrenForeignMethodFn bindMetaForeignMethods(WrenVM* vm, diff --git a/src/optional/wren_opt_random.c b/src/optional/wren_opt_random.c index 3735a4db..120f5850 100644 --- a/src/optional/wren_opt_random.c +++ b/src/optional/wren_opt_random.c @@ -92,14 +92,14 @@ static void randomFloat(WrenVM* vm) // from 0 to 1.0 (half-inclusive). result /= 9007199254740992.0; - wrenReturnDouble(vm, result); + wrenSetSlotDouble(vm, 0, result); } static void randomInt0(WrenVM* vm) { Well512* well = (Well512*)wrenGetSlotForeign(vm, 0); - wrenReturnDouble(vm, (double)advanceState(well)); + wrenSetSlotDouble(vm, 0, (double)advanceState(well)); } // TODO: The way these are wired up is pretty verbose and tedious. Also, the diff --git a/src/vm/wren_vm.c b/src/vm/wren_vm.c index 4d44d49f..3ce874ff 100644 --- a/src/vm/wren_vm.c +++ b/src/vm/wren_vm.c @@ -349,18 +349,14 @@ static void callForeign(WrenVM* vm, ObjFiber* fiber, WrenForeignMethodFn foreign, int numArgs) { vm->foreignStackStart = fiber->stackTop - numArgs; + foreign(vm); - // Discard the stack slots for the arguments (but leave one for - // the result). - fiber->stackTop -= numArgs - 1; - - // If nothing was returned, implicitly return null. - if (vm->foreignStackStart != NULL) - { - *vm->foreignStackStart = NULL_VAL; - vm->foreignStackStart = NULL; - } + // Discard the stack slots for the arguments and temporaries but leave one + // for the result. + fiber->stackTop = vm->foreignStackStart + 1; + + vm->foreignStackStart = NULL; } // Handles the current fiber having aborted because of an error. Switches to @@ -475,6 +471,11 @@ static inline void callFunction( { upvalue->value += offset; } + + if (vm->foreignStackStart != NULL) + { + vm->foreignStackStart += offset; + } } } @@ -734,7 +735,7 @@ void wrenFinalizeForeign(WrenVM* vm, ObjForeign* foreign) ASSERT(method->type == METHOD_FOREIGN, "Finalizer should be foreign."); - // Pass the constructor arguments to the allocator as well. + // Pass the foreign object to the finalizer. Value slot = OBJ_VAL(foreign); vm->foreignStackStart = &slot; @@ -1628,9 +1629,15 @@ int wrenGetSlotCount(WrenVM* vm) { ASSERT(vm->foreignStackStart != NULL, "Must be in foreign call."); - // If no fiber is executing, we must be in a finalizer, in which case the - // "stack" just has one object, the object being finalized. - if (vm->fiber == NULL) return 1; + // If no fiber is executing or the foreign stack is not in it, we must be in + // a finalizer, in which case the "stack" just has one object, the object + // being finalized. + if (vm->fiber == NULL || + vm->foreignStackStart < vm->fiber->stack || + vm->foreignStackStart > vm->fiber->stackTop) + { + return 1; + } return (int)(vm->fiber->stackTop - vm->foreignStackStart); } @@ -1651,6 +1658,16 @@ bool wrenGetSlotBool(WrenVM* vm, int slot) return AS_BOOL(vm->foreignStackStart[slot]); } +const char* wrenGetSlotBytes(WrenVM* vm, int slot, int* length) +{ + validateForeignSlot(vm, slot); + ASSERT(IS_STRING(vm->foreignStackStart[slot]), "Slot must hold a string."); + + ObjString* string = AS_STRING(vm->foreignStackStart[slot]); + *length = string->length; + return string->value; +} + double wrenGetSlotDouble(WrenVM* vm, int slot) { validateForeignSlot(vm, slot); @@ -1683,39 +1700,43 @@ WrenValue* wrenGetSlotValue(WrenVM* vm, int slot) return wrenCaptureValue(vm, vm->foreignStackStart[slot]); } -void wrenReturnBool(WrenVM* vm, bool value) +// Stores [value] in [slot] in the foreign call stack. +static void setSlot(WrenVM* vm, int slot, Value value) { - ASSERT(vm->foreignStackStart != NULL, "Must be in foreign call."); - - *vm->foreignStackStart = BOOL_VAL(value); - vm->foreignStackStart = NULL; + validateForeignSlot(vm, slot); + vm->foreignStackStart[slot] = value; } -void wrenReturnDouble(WrenVM* vm, double value) +void wrenSetSlotBool(WrenVM* vm, int slot, bool value) { - ASSERT(vm->foreignStackStart != NULL, "Must be in foreign call."); - - *vm->foreignStackStart = NUM_VAL(value); - vm->foreignStackStart = NULL; + setSlot(vm, slot, BOOL_VAL(value)); } -void wrenReturnString(WrenVM* vm, const char* text, int length) +void wrenSetSlotBytes(WrenVM* vm, int slot, const char* bytes, int length) +{ + ASSERT(bytes != NULL, "Byte arraybytes cannot be NULL."); + setSlot(vm, slot, wrenNewString(vm, bytes, (size_t)length)); +} + +void wrenSetSlotDouble(WrenVM* vm, int slot, double value) +{ + setSlot(vm, slot, NUM_VAL(value)); +} + +void wrenSetSlotNull(WrenVM* vm, int slot) +{ + setSlot(vm, slot, NULL_VAL); +} + +void wrenSetSlotString(WrenVM* vm, int slot, const char* text) { - ASSERT(vm->foreignStackStart != NULL, "Must be in foreign call."); ASSERT(text != NULL, "String cannot be NULL."); - - size_t size = length; - if (length == -1) size = strlen(text); - - *vm->foreignStackStart = wrenNewString(vm, text, size); - vm->foreignStackStart = NULL; + setSlot(vm, slot, wrenNewString(vm, text, strlen(text))); } -void wrenReturnValue(WrenVM* vm, WrenValue* value) +void wrenSetSlotValue(WrenVM* vm, int slot, WrenValue* value) { - ASSERT(vm->foreignStackStart != NULL, "Must be in foreign call."); ASSERT(value != NULL, "Value cannot be NULL."); - *vm->foreignStackStart = value->value; - vm->foreignStackStart = NULL; + setSlot(vm, slot, value->value); } diff --git a/test/api/benchmark.c b/test/api/benchmark.c index d5c84a8f..afc1a9af 100644 --- a/test/api/benchmark.c +++ b/test/api/benchmark.c @@ -11,7 +11,7 @@ static void arguments(WrenVM* vm) result += wrenGetSlotDouble(vm, 3); result += wrenGetSlotDouble(vm, 4); - wrenReturnDouble(vm, result); + wrenSetSlotDouble(vm, 0, result); } WrenForeignMethodFn benchmarkBindMethod(const char* signature) diff --git a/test/api/foreign_class.c b/test/api/foreign_class.c index cd544def..119a5879 100644 --- a/test/api/foreign_class.c +++ b/test/api/foreign_class.c @@ -7,7 +7,7 @@ static int finalized = 0; static void apiFinalized(WrenVM* vm) { - wrenReturnDouble(vm, finalized); + wrenSetSlotDouble(vm, 0, finalized); } static void counterAllocate(WrenVM* vm) @@ -27,7 +27,7 @@ static void counterIncrement(WrenVM* vm) static void counterValue(WrenVM* vm) { double value = *(double*)wrenGetSlotForeign(vm, 0); - wrenReturnDouble(vm, value); + wrenSetSlotDouble(vm, 0, value); } static void pointAllocate(WrenVM* vm) @@ -64,7 +64,7 @@ static void pointToString(WrenVM* vm) char result[100]; sprintf(result, "(%g, %g, %g)", coordinates[0], coordinates[1], coordinates[2]); - wrenReturnString(vm, result, (int)strlen(result)); + wrenSetSlotString(vm, 0, result); } static void resourceAllocate(WrenVM* vm) diff --git a/test/api/main.c b/test/api/main.c index 7674db05..dbff716a 100644 --- a/test/api/main.c +++ b/test/api/main.c @@ -7,7 +7,7 @@ #include "benchmark.h" #include "call.h" #include "foreign_class.h" -#include "returns.h" +#include "slots.h" #include "value.h" // The name of the currently executing API test. @@ -36,7 +36,7 @@ static WrenForeignMethodFn bindForeignMethod( method = foreignClassBindMethod(fullName); if (method != NULL) return method; - method = returnsBindMethod(fullName); + method = slotsBindMethod(fullName); if (method != NULL) return method; method = valueBindMethod(fullName); diff --git a/test/api/returns.c b/test/api/returns.c deleted file mode 100644 index f622ded9..00000000 --- a/test/api/returns.c +++ /dev/null @@ -1,51 +0,0 @@ -#include - -#include "returns.h" - -static void implicitNull(WrenVM* vm) -{ - // Do nothing. -} - -static void returnInt(WrenVM* vm) -{ - wrenReturnDouble(vm, 123456); -} - -static void returnFloat(WrenVM* vm) -{ - wrenReturnDouble(vm, 123.456); -} - -static void returnTrue(WrenVM* vm) -{ - wrenReturnBool(vm, true); -} - -static void returnFalse(WrenVM* vm) -{ - wrenReturnBool(vm, false); -} - -static void returnString(WrenVM* vm) -{ - wrenReturnString(vm, "a string", -1); -} - -static void returnBytes(WrenVM* vm) -{ - wrenReturnString(vm, "a\0b\0c", 5); -} - -WrenForeignMethodFn returnsBindMethod(const char* signature) -{ - if (strcmp(signature, "static Returns.implicitNull") == 0) return implicitNull; - if (strcmp(signature, "static Returns.returnInt") == 0) return returnInt; - if (strcmp(signature, "static Returns.returnFloat") == 0) return returnFloat; - if (strcmp(signature, "static Returns.returnTrue") == 0) return returnTrue; - if (strcmp(signature, "static Returns.returnFalse") == 0) return returnFalse; - if (strcmp(signature, "static Returns.returnString") == 0) return returnString; - if (strcmp(signature, "static Returns.returnBytes") == 0) return returnBytes; - - return NULL; -} diff --git a/test/api/returns.h b/test/api/returns.h deleted file mode 100644 index 21fa57cd..00000000 --- a/test/api/returns.h +++ /dev/null @@ -1,3 +0,0 @@ -#include "wren.h" - -WrenForeignMethodFn returnsBindMethod(const char* signature); diff --git a/test/api/returns.wren b/test/api/returns.wren deleted file mode 100644 index 16a07e5e..00000000 --- a/test/api/returns.wren +++ /dev/null @@ -1,23 +0,0 @@ -class Returns { - foreign static implicitNull - - foreign static returnInt - foreign static returnFloat - - foreign static returnTrue - foreign static returnFalse - - foreign static returnString - foreign static returnBytes -} - -System.print(Returns.implicitNull == null) // expect: true - -System.print(Returns.returnInt) // expect: 123456 -System.print(Returns.returnFloat) // expect: 123.456 - -System.print(Returns.returnTrue) // expect: true -System.print(Returns.returnFalse) // expect: false - -System.print(Returns.returnString) // expect: a string -System.print(Returns.returnBytes.bytes.toList) // expect: [97, 0, 98, 0, 99] diff --git a/test/api/slots.c b/test/api/slots.c new file mode 100644 index 00000000..8a2c7308 --- /dev/null +++ b/test/api/slots.c @@ -0,0 +1,83 @@ +#include + +#include "slots.h" + +static void noSet(WrenVM* vm) +{ + // Do nothing. +} + +static void getSlots(WrenVM* vm) +{ + bool result = true; + if (wrenGetSlotBool(vm, 1) != true) result = false; + // TODO: Test wrenGetSlotForeign(). + + int length; + const char* bytes = wrenGetSlotBytes(vm, 2, &length); + if (length != 5) result = false; + if (memcmp(bytes, "by\0te", length) != 0) result = false; + + if (wrenGetSlotDouble(vm, 3) != 12.34) result = false; + if (strcmp(wrenGetSlotString(vm, 4), "str") != 0) result = false; + + WrenValue* value = wrenGetSlotValue(vm, 5); + + if (result) + { + // Otherwise, return the value so we can tell if we captured it correctly. + wrenSetSlotValue(vm, 0, value); + wrenReleaseValue(vm, value); + } + else + { + // If anything failed, return false. + wrenSetSlotBool(vm, 0, false); + } +} + +static void setSlots(WrenVM* vm) +{ + WrenValue* value = wrenGetSlotValue(vm, 1); + + wrenSetSlotBool(vm, 1, true); + wrenSetSlotBytes(vm, 2, "by\0te", 5); + wrenSetSlotDouble(vm, 3, 12.34); + wrenSetSlotString(vm, 4, "str"); + + // TODO: wrenSetSlotNull(). + + // Read the slots back to make sure they were set correctly. + + bool result = true; + if (wrenGetSlotBool(vm, 1) != true) result = false; + + int length; + const char* bytes = wrenGetSlotBytes(vm, 2, &length); + if (length != 5) result = false; + if (memcmp(bytes, "by\0te", length) != 0) result = false; + + if (wrenGetSlotDouble(vm, 3) != 12.34) result = false; + if (strcmp(wrenGetSlotString(vm, 4), "str") != 0) result = false; + + if (result) + { + // Move the value into the return position. + wrenSetSlotValue(vm, 0, value); + wrenReleaseValue(vm, value); + } + else + { + // If anything failed, return false. + wrenSetSlotBool(vm, 0, false); + } +} + +WrenForeignMethodFn slotsBindMethod(const char* signature) +{ + if (strcmp(signature, "static Slots.noSet") == 0) return noSet; + if (strcmp(signature, "static Slots.getSlots(_,_,_,_,_)") == 0) return getSlots; + if (strcmp(signature, "static Slots.setSlots(_,_,_,_)") == 0) return setSlots; + + return NULL; +} diff --git a/test/api/slots.h b/test/api/slots.h new file mode 100644 index 00000000..cf4392fa --- /dev/null +++ b/test/api/slots.h @@ -0,0 +1,3 @@ +#include "wren.h" + +WrenForeignMethodFn slotsBindMethod(const char* signature); diff --git a/test/api/slots.wren b/test/api/slots.wren new file mode 100644 index 00000000..119251d5 --- /dev/null +++ b/test/api/slots.wren @@ -0,0 +1,16 @@ +class Slots { + foreign static noSet + + foreign static getSlots(bool, num, string, bytes, value) + + foreign static setSlots(a, b, c, d) +} + +// If nothing is set in the return slot, it retains its previous value, the +// receiver. +System.print(Slots.noSet == Slots) // expect: true + +var value = ["value"] +System.print(Slots.getSlots(true, "by\0te", 12.34, "str", value) == value) // expect: true + +System.print(Slots.setSlots(value, 0, 0, 0) == value) // expect: true diff --git a/test/api/value.c b/test/api/value.c index e26d4978..837e8ce2 100644 --- a/test/api/value.c +++ b/test/api/value.c @@ -11,7 +11,7 @@ static void setValue(WrenVM* vm) static void getValue(WrenVM* vm) { - wrenReturnValue(vm, value); + wrenSetSlotValue(vm, 0, value); wrenReleaseValue(vm, value); } diff --git a/util/xcode/wren.xcodeproj/project.pbxproj b/util/xcode/wren.xcodeproj/project.pbxproj index fe7b67a4..a1fb096b 100644 --- a/util/xcode/wren.xcodeproj/project.pbxproj +++ b/util/xcode/wren.xcodeproj/project.pbxproj @@ -47,7 +47,7 @@ 29DC14A91BBA302F008A8274 /* wren_vm.c in Sources */ = {isa = PBXBuildFile; fileRef = 29205C981AB4E6430073018D /* wren_vm.c */; }; 29DC14AA1BBA3032008A8274 /* main.c in Sources */ = {isa = PBXBuildFile; fileRef = 29D009A61B7E3993000CE58C /* main.c */; }; 29DC14AB1BBA3038008A8274 /* foreign_class.c in Sources */ = {isa = PBXBuildFile; fileRef = 29D009A81B7E39A8000CE58C /* foreign_class.c */; }; - 29DC14AC1BBA303D008A8274 /* returns.c in Sources */ = {isa = PBXBuildFile; fileRef = 29D009AA1B7E39A8000CE58C /* returns.c */; }; + 29DC14AC1BBA303D008A8274 /* slots.c in Sources */ = {isa = PBXBuildFile; fileRef = 29D009AA1B7E39A8000CE58C /* slots.c */; }; 29DC14AD1BBA3040008A8274 /* value.c in Sources */ = {isa = PBXBuildFile; fileRef = 29D009AC1B7E39A8000CE58C /* value.c */; }; /* End PBXBuildFile section */ @@ -118,8 +118,8 @@ 29D009A61B7E3993000CE58C /* main.c */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.c; name = main.c; path = ../../test/api/main.c; sourceTree = ""; }; 29D009A81B7E39A8000CE58C /* foreign_class.c */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.c; name = foreign_class.c; path = ../../test/api/foreign_class.c; sourceTree = ""; }; 29D009A91B7E39A8000CE58C /* foreign_class.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = foreign_class.h; path = ../../test/api/foreign_class.h; sourceTree = ""; }; - 29D009AA1B7E39A8000CE58C /* returns.c */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.c; name = returns.c; path = ../../test/api/returns.c; sourceTree = ""; }; - 29D009AB1B7E39A8000CE58C /* returns.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = returns.h; path = ../../test/api/returns.h; sourceTree = ""; }; + 29D009AA1B7E39A8000CE58C /* slots.c */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.c; name = slots.c; path = ../../test/api/slots.c; sourceTree = ""; }; + 29D009AB1B7E39A8000CE58C /* slots.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = slots.h; path = ../../test/api/slots.h; sourceTree = ""; }; 29D009AC1B7E39A8000CE58C /* value.c */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.c; name = value.c; path = ../../test/api/value.c; sourceTree = ""; }; 29D009AD1B7E39A8000CE58C /* value.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = value.h; path = ../../test/api/value.h; sourceTree = ""; }; 29F384111BD19706002F84E0 /* io.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = io.h; path = ../../src/module/io.h; sourceTree = ""; }; @@ -249,8 +249,8 @@ 293D46951BB43F9900200083 /* call.h */, 29D009A81B7E39A8000CE58C /* foreign_class.c */, 29D009A91B7E39A8000CE58C /* foreign_class.h */, - 29D009AA1B7E39A8000CE58C /* returns.c */, - 29D009AB1B7E39A8000CE58C /* returns.h */, + 29D009AA1B7E39A8000CE58C /* slots.c */, + 29D009AB1B7E39A8000CE58C /* slots.h */, 29D009AC1B7E39A8000CE58C /* value.c */, 29D009AD1B7E39A8000CE58C /* value.h */, ); @@ -376,7 +376,7 @@ 293D46961BB43F9900200083 /* call.c in Sources */, 29A427351BDBE435001E6E22 /* wren_opt_meta.c in Sources */, 29DC14AB1BBA3038008A8274 /* foreign_class.c in Sources */, - 29DC14AC1BBA303D008A8274 /* returns.c in Sources */, + 29DC14AC1BBA303D008A8274 /* slots.c in Sources */, 29DC14AD1BBA3040008A8274 /* value.c in Sources */, ); runOnlyForDeploymentPostprocessing = 0; From 6f37d379f4fcfb315824e60bc569f0611c94bd2d Mon Sep 17 00:00:00 2001 From: Bob Nystrom Date: Wed, 16 Dec 2015 16:28:26 -0800 Subject: [PATCH 03/26] Add C API functions for working with lists: - wrenEnsureSlots() Lets you go the foreign slot stack to make room for a temporary work area. - wrenSetSlotNewList() Creates a new empty list and stores it in a slot. - wrenInsertInList() Takes a value from one slot and inserts it into the list in another. Still need more functions like getting elements from a list, removing, etc. but this at least lets you create, populate, and return lists from foreign methods. --- src/include/wren.h | 18 +++ src/vm/wren_vm.c | 143 ++++++++++++++-------- test/api/lists.c | 46 +++++++ test/api/lists.h | 3 + test/api/lists.wren | 10 ++ test/api/main.c | 4 + test/api/slots.c | 28 +++++ test/api/slots.wren | 5 +- util/xcode/wren.xcodeproj/project.pbxproj | 6 + 9 files changed, 210 insertions(+), 53 deletions(-) create mode 100644 test/api/lists.c create mode 100644 test/api/lists.h create mode 100644 test/api/lists.wren diff --git a/src/include/wren.h b/src/include/wren.h index f107f6e3..55aed491 100644 --- a/src/include/wren.h +++ b/src/include/wren.h @@ -240,6 +240,14 @@ void* wrenAllocateForeign(WrenVM* vm, size_t size); // Returns the number of slots available to the current foreign method. int wrenGetSlotCount(WrenVM* vm); +// Ensures that the foreign method stack has at least [numSlots] available for +// use, growing the stack if needed. +// +// Does not shrink the stack if it has more than enough slots. +// +// It is an error to call this from a finalizer. +void wrenEnsureSlots(WrenVM* vm, int numSlots); + // TODO: Update docs. // The following functions read one of the arguments passed to a foreign call. @@ -327,6 +335,9 @@ void wrenSetSlotBytes(WrenVM* vm, int slot, const char* bytes, int length); // Stores the numeric [value] in [slot]. void wrenSetSlotDouble(WrenVM* vm, int slot, double value); +// Stores a new empty list in [slot]. +void wrenSetSlotNewList(WrenVM* vm, int slot); + // Stores null in [slot]. void wrenSetSlotNull(WrenVM* vm, int slot); @@ -343,4 +354,11 @@ void wrenSetSlotString(WrenVM* vm, int slot, const char* text); // This does not release the handle for the value. void wrenSetSlotValue(WrenVM* vm, int slot, WrenValue* value); +// Takes the value stored at [elementSlot] and inserts it into the list stored +// at [listSlot] at [index]. +// +// As in Wren, negative indexes can be used to insert from the end. To append +// an element, use `-1` for the index. +void wrenInsertInList(WrenVM* vm, int listSlot, int index, int elementSlot); + #endif diff --git a/src/vm/wren_vm.c b/src/vm/wren_vm.c index 3ce874ff..e8439b63 100644 --- a/src/vm/wren_vm.c +++ b/src/vm/wren_vm.c @@ -420,6 +420,48 @@ static bool checkArity(WrenVM* vm, Value value, int numArgs) return false; } +// Ensures [fiber]'s stack has at least [needed] slots. +static void ensureStack(WrenVM* vm, ObjFiber* fiber, int needed) +{ + if (fiber->stackCapacity >= needed) return; + + int capacity = wrenPowerOf2Ceil(needed); + + Value* oldStack = fiber->stack; + fiber->stack = (Value*)wrenReallocate(vm, fiber->stack, + sizeof(Value) * fiber->stackCapacity, + sizeof(Value) * capacity); + fiber->stackCapacity = capacity; + + // If the reallocation moves the stack, then we need to shift every pointer + // into the stack to point to its new location. + if (fiber->stack != oldStack) + { + // Top of the stack. + long offset = fiber->stack - oldStack; + fiber->stackTop += offset; + + // Stack pointer for each call frame. + for (int i = 0; i < fiber->numFrames; i++) + { + fiber->frames[i].stackStart += offset; + } + + // Open upvalues. + for (ObjUpvalue* upvalue = fiber->openUpvalues; + upvalue != NULL; + upvalue = upvalue->next) + { + upvalue->value += offset; + } + + if (vm->foreignStackStart != NULL) + { + vm->foreignStackStart += offset; + } + } +} + // Pushes [function] onto [fiber]'s callstack and invokes it. Expects [numArgs] // arguments (including the receiver) to be on the top of the stack already. // [function] can be an `ObjFn` or `ObjClosure`. @@ -435,50 +477,12 @@ static inline void callFunction( sizeof(CallFrame) * max); fiber->frameCapacity = max; } - + // Grow the stack if needed. int stackSize = (int)(fiber->stackTop - fiber->stack); int needed = stackSize + wrenUpwrapClosure(function)->maxSlots; - - if (fiber->stackCapacity < needed) - { - int capacity = wrenPowerOf2Ceil(needed); - - Value* oldStack = fiber->stack; - fiber->stack = (Value*)wrenReallocate(vm, fiber->stack, - sizeof(Value) * fiber->stackCapacity, - sizeof(Value) * capacity); - fiber->stackCapacity = capacity; - - // If the reallocation moves the stack, then we need to shift every pointer - // into the stack to point to its new location. - if (fiber->stack != oldStack) - { - // Top of the stack. - long offset = fiber->stack - oldStack; - fiber->stackTop += offset; - - // Stack pointer for each call frame. - for (int i = 0; i < fiber->numFrames; i++) - { - fiber->frames[i].stackStart += offset; - } - - // Open upvalues. - for (ObjUpvalue* upvalue = fiber->openUpvalues; - upvalue != NULL; - upvalue = upvalue->next) - { - upvalue->value += offset; - } - - if (vm->foreignStackStart != NULL) - { - vm->foreignStackStart += offset; - } - } - } - + ensureStack(vm, fiber, needed); + wrenAppendCallFrame(vm, fiber, function, fiber->stackTop - numArgs); } @@ -1625,23 +1629,39 @@ void wrenPopRoot(WrenVM* vm) vm->numTempRoots--; } +// Returns true if the VM is in a foreign method that's a finalizer. +// +// Finalizers don't run in the context of a fiber and have a single magic stack +// slot, so need to be handled a little specially. +static bool isInFinalizer(WrenVM* vm) +{ + return vm->fiber == NULL || + vm->foreignStackStart < vm->fiber->stack || + vm->foreignStackStart > vm->fiber->stackTop; +} + int wrenGetSlotCount(WrenVM* vm) { ASSERT(vm->foreignStackStart != NULL, "Must be in foreign call."); - // If no fiber is executing or the foreign stack is not in it, we must be in - // a finalizer, in which case the "stack" just has one object, the object - // being finalized. - if (vm->fiber == NULL || - vm->foreignStackStart < vm->fiber->stack || - vm->foreignStackStart > vm->fiber->stackTop) - { - return 1; - } - + if (isInFinalizer(vm)) return 1; return (int)(vm->fiber->stackTop - vm->foreignStackStart); } +void wrenEnsureSlots(WrenVM* vm, int numSlots) +{ + ASSERT(!isInFinalizer(vm), "Cannot grow the stack in a finalizer."); + + int currentSize = (int)(vm->fiber->stackTop - vm->foreignStackStart); + if (currentSize >= numSlots) return; + + // Grow the stack if needed. + int needed = (int)(vm->foreignStackStart - vm->fiber->stack) + numSlots; + ensureStack(vm, vm->fiber, needed); + + vm->fiber->stackTop = vm->foreignStackStart + numSlots; +} + // Ensures that [slot] is a valid index into a foreign method's stack of slots. static void validateForeignSlot(WrenVM* vm, int slot) { @@ -1723,6 +1743,11 @@ void wrenSetSlotDouble(WrenVM* vm, int slot, double value) setSlot(vm, slot, NUM_VAL(value)); } +void wrenSetSlotNewList(WrenVM* vm, int slot) +{ + setSlot(vm, slot, OBJ_VAL(wrenNewList(vm, 0))); +} + void wrenSetSlotNull(WrenVM* vm, int slot) { setSlot(vm, slot, NULL_VAL); @@ -1740,3 +1765,19 @@ void wrenSetSlotValue(WrenVM* vm, int slot, WrenValue* value) setSlot(vm, slot, value->value); } + +void wrenInsertInList(WrenVM* vm, int listSlot, int index, int elementSlot) +{ + validateForeignSlot(vm, listSlot); + validateForeignSlot(vm, elementSlot); + ASSERT(IS_LIST(vm->foreignStackStart[listSlot]), "Must insert into a list."); + + ObjList* list = AS_LIST(vm->foreignStackStart[listSlot]); + + // Negative indices count from the end. + if (index < 0) index = list->elements.count + 1 + index; + + ASSERT(index <= list->elements.count, "Index out of bounds."); + + wrenListInsert(vm, list, vm->foreignStackStart[elementSlot], index); +} diff --git a/test/api/lists.c b/test/api/lists.c new file mode 100644 index 00000000..8a2f7a48 --- /dev/null +++ b/test/api/lists.c @@ -0,0 +1,46 @@ +#include + +#include "lists.h" + +static void newList(WrenVM* vm) +{ + wrenSetSlotNewList(vm, 0); +} + +// Helper function to store a double in a slot then insert it into the list at +// slot zero. +static void insertNumber(WrenVM* vm, int index, double value) +{ + wrenSetSlotDouble(vm, 1, value); + wrenInsertInList(vm, 0, index, 1); +} + +static void insert(WrenVM* vm) +{ + wrenSetSlotNewList(vm, 0); + + wrenEnsureSlots(vm, 2); + + // Appending. + insertNumber(vm, 0, 1.0); + insertNumber(vm, 1, 2.0); + insertNumber(vm, 2, 3.0); + + // Inserting. + insertNumber(vm, 0, 4.0); + insertNumber(vm, 1, 5.0); + insertNumber(vm, 2, 6.0); + + // Negative indexes. + insertNumber(vm, -1, 7.0); + insertNumber(vm, -2, 8.0); + insertNumber(vm, -3, 9.0); +} + +WrenForeignMethodFn listsBindMethod(const char* signature) +{ + if (strcmp(signature, "static Lists.newList()") == 0) return newList; + if (strcmp(signature, "static Lists.insert()") == 0) return insert; + + return NULL; +} diff --git a/test/api/lists.h b/test/api/lists.h new file mode 100644 index 00000000..9c74380b --- /dev/null +++ b/test/api/lists.h @@ -0,0 +1,3 @@ +#include "wren.h" + +WrenForeignMethodFn listsBindMethod(const char* signature); diff --git a/test/api/lists.wren b/test/api/lists.wren new file mode 100644 index 00000000..64135a5b --- /dev/null +++ b/test/api/lists.wren @@ -0,0 +1,10 @@ +class Lists { + foreign static newList() + foreign static insert() +} + +var list = Lists.newList() +System.print(list is List) // expect: true +System.print(list.count) // expect: 0 + +System.print(Lists.insert()) // expect: [4, 5, 6, 1, 2, 3, 9, 8, 7] diff --git a/test/api/main.c b/test/api/main.c index dbff716a..e9e67768 100644 --- a/test/api/main.c +++ b/test/api/main.c @@ -7,6 +7,7 @@ #include "benchmark.h" #include "call.h" #include "foreign_class.h" +#include "lists.h" #include "slots.h" #include "value.h" @@ -36,6 +37,9 @@ static WrenForeignMethodFn bindForeignMethod( method = foreignClassBindMethod(fullName); if (method != NULL) return method; + method = listsBindMethod(fullName); + if (method != NULL) return method; + method = slotsBindMethod(fullName); if (method != NULL) return method; diff --git a/test/api/slots.c b/test/api/slots.c index 8a2c7308..347b9d75 100644 --- a/test/api/slots.c +++ b/test/api/slots.c @@ -1,3 +1,4 @@ +#include #include #include "slots.h" @@ -73,11 +74,38 @@ static void setSlots(WrenVM* vm) } } +static void ensure(WrenVM* vm) +{ + int before = wrenGetSlotCount(vm); + + wrenEnsureSlots(vm, 20); + + int after = wrenGetSlotCount(vm); + + // Use the slots to make sure they're available. + for (int i = 0; i < 20; i++) + { + wrenSetSlotDouble(vm, i, i); + } + + int sum = 0; + + for (int i = 0; i < 20; i++) + { + sum += (int)wrenGetSlotDouble(vm, i); + } + + char result[100]; + sprintf(result, "%d -> %d (%d)", before, after, sum); + wrenSetSlotString(vm, 0, result); +} + WrenForeignMethodFn slotsBindMethod(const char* signature) { if (strcmp(signature, "static Slots.noSet") == 0) return noSet; if (strcmp(signature, "static Slots.getSlots(_,_,_,_,_)") == 0) return getSlots; if (strcmp(signature, "static Slots.setSlots(_,_,_,_)") == 0) return setSlots; + if (strcmp(signature, "static Slots.ensure()") == 0) return ensure; return NULL; } diff --git a/test/api/slots.wren b/test/api/slots.wren index 119251d5..70d403e8 100644 --- a/test/api/slots.wren +++ b/test/api/slots.wren @@ -1,9 +1,8 @@ class Slots { foreign static noSet - foreign static getSlots(bool, num, string, bytes, value) - foreign static setSlots(a, b, c, d) + foreign static ensure() } // If nothing is set in the return slot, it retains its previous value, the @@ -14,3 +13,5 @@ var value = ["value"] System.print(Slots.getSlots(true, "by\0te", 12.34, "str", value) == value) // expect: true System.print(Slots.setSlots(value, 0, 0, 0) == value) // expect: true + +System.print(Slots.ensure()) // expect: 1 -> 20 (190) diff --git a/util/xcode/wren.xcodeproj/project.pbxproj b/util/xcode/wren.xcodeproj/project.pbxproj index a1fb096b..f5cc72d5 100644 --- a/util/xcode/wren.xcodeproj/project.pbxproj +++ b/util/xcode/wren.xcodeproj/project.pbxproj @@ -26,6 +26,7 @@ 29729F331BA70A620099CA20 /* io.wren.inc in Sources */ = {isa = PBXBuildFile; fileRef = 29729F301BA70A620099CA20 /* io.wren.inc */; }; 2986F6D71ACF93BA00BCE26C /* wren_primitive.c in Sources */ = {isa = PBXBuildFile; fileRef = 2986F6D51ACF93BA00BCE26C /* wren_primitive.c */; }; 29932D511C20D8C900099DEE /* benchmark.c in Sources */ = {isa = PBXBuildFile; fileRef = 29932D4F1C20D8C900099DEE /* benchmark.c */; }; + 29932D541C210F8D00099DEE /* lists.c in Sources */ = {isa = PBXBuildFile; fileRef = 29932D521C210F8D00099DEE /* lists.c */; }; 29A427341BDBE435001E6E22 /* wren_opt_meta.c in Sources */ = {isa = PBXBuildFile; fileRef = 29A4272E1BDBE435001E6E22 /* wren_opt_meta.c */; }; 29A427351BDBE435001E6E22 /* wren_opt_meta.c in Sources */ = {isa = PBXBuildFile; fileRef = 29A4272E1BDBE435001E6E22 /* wren_opt_meta.c */; }; 29A427361BDBE435001E6E22 /* wren_opt_meta.wren.inc in Sources */ = {isa = PBXBuildFile; fileRef = 29A427301BDBE435001E6E22 /* wren_opt_meta.wren.inc */; }; @@ -106,6 +107,8 @@ 2986F6D61ACF93BA00BCE26C /* wren_primitive.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = wren_primitive.h; path = ../../src/vm/wren_primitive.h; sourceTree = ""; }; 29932D4F1C20D8C900099DEE /* benchmark.c */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.c; name = benchmark.c; path = ../../test/api/benchmark.c; sourceTree = ""; }; 29932D501C20D8C900099DEE /* benchmark.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = benchmark.h; path = ../../test/api/benchmark.h; sourceTree = ""; }; + 29932D521C210F8D00099DEE /* lists.c */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.c; name = lists.c; path = ../../test/api/lists.c; sourceTree = ""; }; + 29932D531C210F8D00099DEE /* lists.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = lists.h; path = ../../test/api/lists.h; sourceTree = ""; }; 29A4272E1BDBE435001E6E22 /* wren_opt_meta.c */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.c; name = wren_opt_meta.c; path = ../../src/optional/wren_opt_meta.c; sourceTree = ""; }; 29A4272F1BDBE435001E6E22 /* wren_opt_meta.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = wren_opt_meta.h; path = ../../src/optional/wren_opt_meta.h; sourceTree = ""; }; 29A427301BDBE435001E6E22 /* wren_opt_meta.wren.inc */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.pascal; name = wren_opt_meta.wren.inc; path = ../../src/optional/wren_opt_meta.wren.inc; sourceTree = ""; }; @@ -249,6 +252,8 @@ 293D46951BB43F9900200083 /* call.h */, 29D009A81B7E39A8000CE58C /* foreign_class.c */, 29D009A91B7E39A8000CE58C /* foreign_class.h */, + 29932D521C210F8D00099DEE /* lists.c */, + 29932D531C210F8D00099DEE /* lists.h */, 29D009AA1B7E39A8000CE58C /* slots.c */, 29D009AB1B7E39A8000CE58C /* slots.h */, 29D009AC1B7E39A8000CE58C /* value.c */, @@ -358,6 +363,7 @@ files = ( 29A427371BDBE435001E6E22 /* wren_opt_meta.wren.inc in Sources */, 29729F321BA70A620099CA20 /* io.c in Sources */, + 29932D541C210F8D00099DEE /* lists.c in Sources */, 291647C81BA5EC5E006142EE /* modules.c in Sources */, 29DC14A11BBA2FEC008A8274 /* scheduler.c in Sources */, 29A427391BDBE435001E6E22 /* wren_opt_random.c in Sources */, From 910049cd191678707f59376111a7284ca99541fe Mon Sep 17 00:00:00 2001 From: Bob Nystrom Date: Wed, 16 Dec 2015 16:39:27 -0800 Subject: [PATCH 04/26] Update the docs around slots. --- src/include/wren.h | 61 +++++++++++++++++++++++++++++----------------- 1 file changed, 39 insertions(+), 22 deletions(-) diff --git a/src/include/wren.h b/src/include/wren.h index 55aed491..dca07058 100644 --- a/src/include/wren.h +++ b/src/include/wren.h @@ -230,6 +230,45 @@ WrenInterpretResult wrenCallVarArgs(WrenVM* vm, WrenValue* method, // longer be used. void wrenReleaseValue(WrenVM* vm, WrenValue* value); +// The following functions are intended to be called from foreign methods or +// finalizers. The interface Wren provides to a foreign method is like a +// register machine: you are given a numbered array of slots that values can be +// read from and written to. Values always live in a slot (unless explicitly +// captured using wrenGetSlotValue(), which ensures the garbage collector can +// find them. +// +// When your foreign function is called, you are given one slot for the receiver +// and each argument to the method. The receiver is in slot 0 and the arguments +// are in increasingly numbered slots after that. You are free to read and +// write to those slots as you want. If you want more slots to use as scratch +// space, you can call wrenEnsureSlots() to add more. +// +// When your function returns, every slot except slot zero is discarded and the +// value in slot zero is used as the return value of the method. If you don't +// store a return value in that slot yourself, it will retain its previous +// value, the receiver. +// +// While Wren is dynamically typed, C is not. This means the C interface has to +// support the various types of primitive values a Wren variable can hold: bool, +// double, string, etc. If we supported this for every operation in the C API, +// there would be a combinatorial explosion of functions, like "get a +// double-valued element from a list", "insert a string key and double value +// into a map", etc. +// +// To avoid that, the only way to convert to and from a raw C value is by going +// into and out of a slot. All other functions work with values already in a +// slot. So, to add an element to a list, you put the list in one slot, and the +// element in another. Then there is a single API function wrenInsertInList() +// that takes the element out of that slot and puts it into the list. +// +// The goal of this API is to be easy to use while not compromising performance. +// The latter means it does not do type or bounds checking at runtime except +// using assertions which are generally removed from release builds. C is an +// unsafe language, so it's up to you to be careful to use it correctly. In +// return, you get a very fast FFI. + +// TODO: Generalize this to look up a foreign class in any slot and place the +// object in a desired slot. // This must be called once inside a foreign class's allocator function. // // It tells Wren how many bytes of raw data need to be stored in the foreign @@ -248,28 +287,6 @@ int wrenGetSlotCount(WrenVM* vm); // It is an error to call this from a finalizer. void wrenEnsureSlots(WrenVM* vm, int numSlots); -// TODO: Update docs. - -// The following functions read one of the arguments passed to a foreign call. -// They may only be called while within a function provided to -// [wrenDefineMethod] or [wrenDefineStaticMethod] that Wren has invoked. -// -// They retreive the argument at a given index which ranges from 0 to the number -// of parameters the method expects. The zeroth parameter is used for the -// receiver of the method. For example, given a foreign method "foo" on String -// invoked like: -// -// "receiver".foo("one", "two", "three") -// -// The foreign function will be able to access the arguments like so: -// -// 0: "receiver" -// 1: "one" -// 2: "two" -// 3: "three" -// -// It is an error to pass an invalid argument index. - // Reads a boolean value from [slot]. // // It is an error to call this if the slot does not contain a boolean value. From 6f3077d58269ff12313528c4e87330d44925ed32 Mon Sep 17 00:00:00 2001 From: Bob Nystrom Date: Wed, 16 Dec 2015 20:55:18 -0800 Subject: [PATCH 05/26] Fix off by one error in initial stack capacity. --- src/vm/wren_value.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/vm/wren_value.c b/src/vm/wren_value.c index c0970762..6d1bd4c3 100644 --- a/src/vm/wren_value.c +++ b/src/vm/wren_value.c @@ -150,7 +150,9 @@ ObjFiber* wrenNewFiber(WrenVM* vm, Obj* fn) // Allocate the arrays before the fiber in case it triggers a GC. CallFrame* frames = ALLOCATE_ARRAY(vm, CallFrame, INITIAL_CALL_FRAMES); - int stackCapacity = wrenPowerOf2Ceil(wrenUpwrapClosure(fn)->maxSlots); + // Add one slot for the unused implicit receiver slot that the compiler + // assumes all functions have. + int stackCapacity = wrenPowerOf2Ceil(wrenUpwrapClosure(fn)->maxSlots + 1); Value* stack = ALLOCATE_ARRAY(vm, Value, stackCapacity); ObjFiber* fiber = ALLOCATE(vm, ObjFiber); @@ -382,6 +384,8 @@ static uint32_t hashValue(Value value) case VAL_OBJ: return hashObject(AS_OBJ(value)); default: UNREACHABLE(); } + + return 0; #endif } From 15043b897fde1e0f42a0af106381a0339465465e Mon Sep 17 00:00:00 2001 From: Bob Nystrom Date: Wed, 23 Dec 2015 17:29:53 -0800 Subject: [PATCH 06/26] Add a benchmark for wrenCall(). --- src/include/wren.h | 6 +++++ src/vm/wren_vm.c | 12 +++++++-- test/api/benchmark.c | 48 ++++++++++++++++++++++++++++++++++++ test/api/main.c | 2 +- test/benchmark/api_call.wren | 9 +++++++ util/benchmark.py | 2 ++ 6 files changed, 76 insertions(+), 3 deletions(-) create mode 100644 test/benchmark/api_call.wren diff --git a/src/include/wren.h b/src/include/wren.h index dca07058..3b9465d3 100644 --- a/src/include/wren.h +++ b/src/include/wren.h @@ -226,6 +226,12 @@ WrenInterpretResult wrenCallVarArgs(WrenVM* vm, WrenValue* method, WrenValue** returnValue, const char* argTypes, va_list args); +// Gets the numeric value of [value]. +// +// It is an error to call this if the value is not a number. +double wrenGetValueDouble(WrenVM* vm, WrenValue* value); +// TODO: Functions for other types. + // Releases the reference stored in [value]. After calling this, [value] can no // longer be used. void wrenReleaseValue(WrenVM* vm, WrenValue* value); diff --git a/src/vm/wren_vm.c b/src/vm/wren_vm.c index e8439b63..1d3bd1f3 100644 --- a/src/vm/wren_vm.c +++ b/src/vm/wren_vm.c @@ -1474,9 +1474,17 @@ WrenValue* wrenCaptureValue(WrenVM* vm, Value value) return wrappedValue; } +double wrenGetValueDouble(WrenVM* vm, WrenValue* value) +{ + ASSERT(value != NULL, "Value cannot be NULL."); + ASSERT(IS_NUM(value->value), "Value must be a number."); + + return AS_NUM(value->value); +} + void wrenReleaseValue(WrenVM* vm, WrenValue* value) { - ASSERT(value != NULL, "NULL value."); + ASSERT(value != NULL, "Value cannot be NULL."); // Update the VM's head pointer if we're releasing the first handle. if (vm->valueHandles == value) vm->valueHandles = value->next; @@ -1734,7 +1742,7 @@ void wrenSetSlotBool(WrenVM* vm, int slot, bool value) void wrenSetSlotBytes(WrenVM* vm, int slot, const char* bytes, int length) { - ASSERT(bytes != NULL, "Byte arraybytes cannot be NULL."); + ASSERT(bytes != NULL, "Byte array cannot be NULL."); setSlot(vm, slot, wrenNewString(vm, bytes, (size_t)length)); } diff --git a/test/api/benchmark.c b/test/api/benchmark.c index afc1a9af..26d8daad 100644 --- a/test/api/benchmark.c +++ b/test/api/benchmark.c @@ -1,4 +1,5 @@ #include +#include #include "benchmark.h" @@ -14,9 +15,56 @@ static void arguments(WrenVM* vm) wrenSetSlotDouble(vm, 0, result); } +const char* testScript = +"class Test {\n" +" static method(a, b, c, d) { a + b + c + d }\n" +"}\n"; + +static void call(WrenVM* vm) +{ + int iterations = (int)wrenGetSlotDouble(vm, 1); + + // Since the VM is not re-entrant, we can't call from within this foreign + // method. Instead, make a new VM to run the call test in. + WrenConfiguration config; + wrenInitConfiguration(&config); + WrenVM* otherVM = wrenNewVM(&config); + + wrenInterpret(otherVM, testScript); + + WrenValue* method = wrenGetMethod(otherVM, "main", "Test", "method(_,_,_,_)"); + + double startTime = (double)clock() / CLOCKS_PER_SEC; + + double result = 0; + for (int i = 0; i < iterations; i++) + { + WrenValue* resultValue; + wrenCall(otherVM, method, &resultValue, "dddd", 1.0, 2.0, 3.0, 4.0); + result += wrenGetValueDouble(otherVM, resultValue); + wrenReleaseValue(otherVM, resultValue); + } + + double elapsed = (double)clock() / CLOCKS_PER_SEC - startTime; + + wrenReleaseValue(otherVM, method); + wrenFreeVM(otherVM); + + if (result == (1.0 + 2.0 + 3.0 + 4.0) * iterations) + { + wrenSetSlotDouble(vm, 0, elapsed); + } + else + { + // Got the wrong result. + wrenSetSlotBool(vm, 0, false); + } +} + WrenForeignMethodFn benchmarkBindMethod(const char* signature) { if (strcmp(signature, "static Benchmark.arguments(_,_,_,_)") == 0) return arguments; + if (strcmp(signature, "static Benchmark.call(_)") == 0) return call; return NULL; } diff --git a/test/api/main.c b/test/api/main.c index e9e67768..f4f57e74 100644 --- a/test/api/main.c +++ b/test/api/main.c @@ -64,7 +64,7 @@ static WrenForeignClassMethods bindForeignClass( } static void afterLoad(WrenVM* vm) { - if (strstr(testName, "call.wren") != NULL) callRunTests(vm); + if (strstr(testName, "/call.wren") != NULL) callRunTests(vm); } int main(int argc, const char* argv[]) diff --git a/test/benchmark/api_call.wren b/test/benchmark/api_call.wren new file mode 100644 index 00000000..568f2549 --- /dev/null +++ b/test/benchmark/api_call.wren @@ -0,0 +1,9 @@ +class Benchmark { + foreign static call(iterations) +} + +var result = Benchmark.call(1000000) +// Returns false if it didn't calculate the right value. Otherwise returns the +// elapsed time. +System.print(result is Num) +System.print("elapsed: %(result)") diff --git a/util/benchmark.py b/util/benchmark.py index 36e7952d..cb00a649 100755 --- a/util/benchmark.py +++ b/util/benchmark.py @@ -51,6 +51,8 @@ def BENCHMARK(name, pattern): regex = re.compile(pattern + "\n" + r"elapsed: (\d+\.\d+)", re.MULTILINE) BENCHMARKS.append([name, regex, None]) +BENCHMARK("api_call", "true") + BENCHMARK("api_foreign_method", "100000000") BENCHMARK("binary_trees", """stretch tree of depth 13 check: -1 From 0ac793d4f88daa62a8433dff7fbf95e34016334b Mon Sep 17 00:00:00 2001 From: Bob Nystrom Date: Thu, 24 Dec 2015 10:12:12 -0800 Subject: [PATCH 07/26] Add Directory.list() to io. --- src/cli/modules.c | 6 +- src/module/io.c | 152 +++++++++++++-------- src/module/io.wren | 33 +++++ src/module/io.wren.inc | 33 +++++ test/io/directory/dir/a.txt | 1 + test/io/directory/dir/b.txt | 1 + test/io/directory/dir/c.txt | 1 + test/io/directory/list.wren | 8 ++ test/io/directory/list_file.wren | 3 + test/io/directory/list_nonexistent.wren | 3 + test/io/directory/list_wrong_arg_type.wren | 3 + 11 files changed, 188 insertions(+), 56 deletions(-) create mode 100644 test/io/directory/dir/a.txt create mode 100644 test/io/directory/dir/b.txt create mode 100644 test/io/directory/dir/c.txt create mode 100644 test/io/directory/list.wren create mode 100644 test/io/directory/list_file.wren create mode 100644 test/io/directory/list_nonexistent.wren create mode 100644 test/io/directory/list_wrong_arg_type.wren diff --git a/src/cli/modules.c b/src/cli/modules.c index 093844e6..27111e4c 100644 --- a/src/cli/modules.c +++ b/src/cli/modules.c @@ -7,6 +7,7 @@ #include "scheduler.wren.inc" #include "timer.wren.inc" +extern void directoryList(WrenVM* vm); extern void fileAllocate(WrenVM* vm); extern void fileFinalize(WrenVM* vm); extern void fileOpen(WrenVM* vm); @@ -35,7 +36,7 @@ extern void timerStartTimer(WrenVM* vm); // If you add a new class to the largest module below, make sure to bump this. // Note that it also includes an extra slot for the sentinel value indicating // the end of the list. -#define MAX_CLASSES_PER_MODULE 3 +#define MAX_CLASSES_PER_MODULE 4 // Describes one foreign method in a class. typedef struct @@ -87,6 +88,9 @@ typedef struct static ModuleRegistry modules[] = { MODULE(io) + CLASS(Directory) + STATIC_METHOD("list_(_,_)", directoryList) + END_CLASS CLASS(File) STATIC_METHOD("", fileAllocate) STATIC_METHOD("", fileFinalize) diff --git a/src/module/io.c b/src/module/io.c index 9f3a836b..ac1cc789 100644 --- a/src/module/io.c +++ b/src/module/io.c @@ -46,6 +46,96 @@ void ioShutdown() shutdownStdin(); } +// If [request] failed with an error, sends the runtime error to the VM and +// frees the request. +// +// Returns true if an error was reported. +static bool handleRequestError(uv_fs_t* request) +{ + if (request->result >= 0) return false; + + FileRequestData* data = (FileRequestData*)request->data; + WrenValue* fiber = (WrenValue*)data->fiber; + + schedulerResumeError(fiber, uv_strerror((int)request->result)); + + free(data); + uv_fs_req_cleanup(request); + free(request); + return true; +} + +// Allocates a new request that resumes [fiber] when it completes. +uv_fs_t* createRequest(WrenValue* fiber) +{ + uv_fs_t* request = (uv_fs_t*)malloc(sizeof(uv_fs_t)); + + FileRequestData* data = (FileRequestData*)malloc(sizeof(FileRequestData)); + data->fiber = fiber; + + request->data = data; + return request; +} + +// Releases resources used by [request]. +// +// Returns the fiber that should be resumed after [request] completes. +WrenValue* freeRequest(uv_fs_t* request) +{ + FileRequestData* data = (FileRequestData*)request->data; + WrenValue* fiber = data->fiber; + + free(data); + uv_fs_req_cleanup(request); + free(request); + + return fiber; +} + +static void directoryListCallback(uv_fs_t* request) +{ + if (handleRequestError(request)) return; + + uv_dirent_t entry; + + // TODO: Right now, there's way to create a list using the C API so create a + // buffer containing all of the result paths terminated by '\0'. We'll split + // that back into a list in Wren. + size_t resultLength = 0; + size_t bufferSize = 1024; + char* result = (char*)malloc(bufferSize); + + while (uv_fs_scandir_next(request, &entry) != UV_EOF) + { + size_t length = strlen(entry.name); + + // Grow the buffer if needed. + while (resultLength + length + 1 > bufferSize) + { + bufferSize *= 2; + result = realloc(result, bufferSize); + } + + // Copy the path, including the null terminator. + memcpy(result + resultLength, entry.name, length + 1); + resultLength += length + 1; + } + + WrenValue* fiber = freeRequest(request); + schedulerResumeBytes(fiber, result, resultLength); + free(result); +} + +void directoryList(WrenVM* vm) +{ + const char* path = wrenGetArgumentString(vm, 1); + + uv_fs_t* request = createRequest(wrenGetArgumentValue(vm, 2)); + + // TODO: Check return. + uv_fs_scandir(getLoop(), request, path, 0, directoryListCallback); +} + void fileAllocate(WrenVM* vm) { // Store the file descriptor in the foreign data, so that we can get to it @@ -66,59 +156,12 @@ void fileFinalize(WrenVM* vm) uv_fs_req_cleanup(&request); } -// If [request] failed with an error, sends the runtime error to the VM and -// frees the request. -// -// Returns true if an error was reported. -static bool handleRequestError(uv_fs_t* request) -{ - if (request->result >= 0) return false; - - FileRequestData* data = (FileRequestData*)request->data; - WrenValue* fiber = (WrenValue*)data->fiber; - - schedulerResumeError(fiber, uv_strerror((int)request->result)); - - free(data); - uv_fs_req_cleanup(request); - free(request); - return true; -} - -// Allocates a new request that resumes [fiber] when it completes. -uv_fs_t* createRequest(WrenValue* fiber) -{ - uv_fs_t* request = (uv_fs_t*)malloc(sizeof(uv_fs_t)); - - FileRequestData* data = (FileRequestData*)malloc(sizeof(FileRequestData)); - data->fiber = fiber; - - request->data = data; - return request; -} - -// Releases resources used by [request]. -// -// Returns the fiber that should be resumed after [request] completes. -WrenValue* freeRequest(uv_fs_t* request) -{ - FileRequestData* data = (FileRequestData*)request->data; - WrenValue* fiber = data->fiber; - - free(data); - uv_fs_req_cleanup(request); - free(request); - - return fiber; -} - -static void openCallback(uv_fs_t* request) +static void fileOpenCallback(uv_fs_t* request) { if (handleRequestError(request)) return; double fd = (double)request->result; WrenValue* fiber = freeRequest(request); - schedulerResumeDouble(fiber, fd); } @@ -128,17 +171,16 @@ void fileOpen(WrenVM* vm) uv_fs_t* request = createRequest(wrenGetArgumentValue(vm, 2)); // TODO: Allow controlling flags and modes. - uv_fs_open(getLoop(), request, path, O_RDONLY, 0, openCallback); + uv_fs_open(getLoop(), request, path, O_RDONLY, 0, fileOpenCallback); } // Called by libuv when the stat call for size completes. -static void sizeCallback(uv_fs_t* request) +static void fileSizeCallback(uv_fs_t* request) { if (handleRequestError(request)) return; double size = (double)request->statbuf.st_size; WrenValue* fiber = freeRequest(request); - schedulerResumeDouble(fiber, size); } @@ -146,10 +188,10 @@ void fileSizePath(WrenVM* vm) { const char* path = wrenGetArgumentString(vm, 1); uv_fs_t* request = createRequest(wrenGetArgumentValue(vm, 2)); - uv_fs_stat(getLoop(), request, path, sizeCallback); + uv_fs_stat(getLoop(), request, path, fileSizeCallback); } -static void closeCallback(uv_fs_t* request) +static void fileCloseCallback(uv_fs_t* request) { if (handleRequestError(request)) return; @@ -173,7 +215,7 @@ void fileClose(WrenVM* vm) *foreign = -1; uv_fs_t* request = createRequest(wrenGetArgumentValue(vm, 1)); - uv_fs_close(getLoop(), request, fd, closeCallback); + uv_fs_close(getLoop(), request, fd, fileCloseCallback); wrenReturnBool(vm, false); } @@ -226,7 +268,7 @@ void fileSize(WrenVM* vm) int fd = *(int*)wrenGetArgumentForeign(vm, 0); // TODO: Assert fd != -1. - uv_fs_fstat(getLoop(), request, fd, sizeCallback); + uv_fs_fstat(getLoop(), request, fd, fileSizeCallback); } static void allocCallback(uv_handle_t* handle, size_t suggestedSize, diff --git a/src/module/io.wren b/src/module/io.wren index 03ca9ce0..d641743a 100644 --- a/src/module/io.wren +++ b/src/module/io.wren @@ -1,5 +1,37 @@ import "scheduler" for Scheduler +class Directory { + static list(path) { + if (!(path is String)) Fiber.abort("Path must be a string.") + + list_(path, Fiber.current) + + // We get back a byte array containing all of the paths, each terminated by + // a zero byte. + var entryBuffer = Scheduler.runNextScheduled_() + + // TODO: Add split() to String. + // Split it into an array of strings. + var entries = [] + var start = 0 + var i = 0 + var bytes = entryBuffer.bytes + while (i < bytes.count) { + if (bytes[i] == 0) { + var entry = entryBuffer[start...i] + start = i + 1 + entries.add(entry) + } + + i = i + 1 + } + + return entries + } + + foreign static list_(path, fiber) +} + foreign class File { static open(path) { if (!(path is String)) Fiber.abort("Path must be a string.") @@ -90,6 +122,7 @@ class Stdin { readStop_() if (__line != null) { + // Emit the last line. var line = __line __line = null if (__waitingFiber != null) __waitingFiber.transfer(line) diff --git a/src/module/io.wren.inc b/src/module/io.wren.inc index b9b5272f..32b62086 100644 --- a/src/module/io.wren.inc +++ b/src/module/io.wren.inc @@ -2,6 +2,38 @@ static const char* ioModuleSource = "import \"scheduler\" for Scheduler\n" "\n" +"class Directory {\n" +" static list(path) {\n" +" if (!(path is String)) Fiber.abort(\"Path must be a string.\")\n" +"\n" +" list_(path, Fiber.current)\n" +"\n" +" // We get back a byte array containing all of the paths, each terminated by\n" +" // a zero byte.\n" +" var entryBuffer = Scheduler.runNextScheduled_()\n" +"\n" +" // TODO: Add split() to String.\n" +" // Split it into an array of strings.\n" +" var entries = []\n" +" var start = 0\n" +" var i = 0\n" +" var bytes = entryBuffer.bytes\n" +" while (i < bytes.count) {\n" +" if (bytes[i] == 0) {\n" +" var entry = entryBuffer[start...i]\n" +" start = i + 1\n" +" entries.add(entry)\n" +" }\n" +"\n" +" i = i + 1\n" +" }\n" +"\n" +" return entries\n" +" }\n" +"\n" +" foreign static list_(path, fiber)\n" +"}\n" +"\n" "foreign class File {\n" " static open(path) {\n" " if (!(path is String)) Fiber.abort(\"Path must be a string.\")\n" @@ -92,6 +124,7 @@ static const char* ioModuleSource = " readStop_()\n" "\n" " if (__line != null) {\n" +" // Emit the last line.\n" " var line = __line\n" " __line = null\n" " if (__waitingFiber != null) __waitingFiber.transfer(line)\n" diff --git a/test/io/directory/dir/a.txt b/test/io/directory/dir/a.txt new file mode 100644 index 00000000..e068544d --- /dev/null +++ b/test/io/directory/dir/a.txt @@ -0,0 +1 @@ +this is a text file \ No newline at end of file diff --git a/test/io/directory/dir/b.txt b/test/io/directory/dir/b.txt new file mode 100644 index 00000000..e068544d --- /dev/null +++ b/test/io/directory/dir/b.txt @@ -0,0 +1 @@ +this is a text file \ No newline at end of file diff --git a/test/io/directory/dir/c.txt b/test/io/directory/dir/c.txt new file mode 100644 index 00000000..e068544d --- /dev/null +++ b/test/io/directory/dir/c.txt @@ -0,0 +1 @@ +this is a text file \ No newline at end of file diff --git a/test/io/directory/list.wren b/test/io/directory/list.wren new file mode 100644 index 00000000..bf61d759 --- /dev/null +++ b/test/io/directory/list.wren @@ -0,0 +1,8 @@ +import "io" for Directory + +var entries = Directory.list("test/io/directory/dir") + +// Ignore OS-specific dot files like ".DS_Store". +entries = entries.where {|entry| !entry.startsWith(".") }.toList + +System.print(entries) // expect: [a.txt, b.txt, c.txt] diff --git a/test/io/directory/list_file.wren b/test/io/directory/list_file.wren new file mode 100644 index 00000000..a411eeb0 --- /dev/null +++ b/test/io/directory/list_file.wren @@ -0,0 +1,3 @@ +import "io" for Directory + +var entries = Directory.list("test/io/directory/dir/a.txt") // expect runtime error: not a directory diff --git a/test/io/directory/list_nonexistent.wren b/test/io/directory/list_nonexistent.wren new file mode 100644 index 00000000..024e7de3 --- /dev/null +++ b/test/io/directory/list_nonexistent.wren @@ -0,0 +1,3 @@ +import "io" for Directory + +Directory.list("nonexistent") // expect runtime error: no such file or directory diff --git a/test/io/directory/list_wrong_arg_type.wren b/test/io/directory/list_wrong_arg_type.wren new file mode 100644 index 00000000..03bedcd5 --- /dev/null +++ b/test/io/directory/list_wrong_arg_type.wren @@ -0,0 +1,3 @@ +import "io" for Directory + +Directory.list(123) // expect runtime error: Path must be a string. From 1d16e85a85b15068a4a8c6004f433531d973cce7 Mon Sep 17 00:00:00 2001 From: Bob Nystrom Date: Sat, 26 Dec 2015 10:53:14 -0800 Subject: [PATCH 08/26] Add an API to load a top-level variable into a slot. --- src/include/wren.h | 5 +++ src/vm/wren_vm.c | 22 ++++++++++++ test/api/get_variable.c | 44 +++++++++++++++++++++++ test/api/get_variable.h | 3 ++ test/api/get_variable.wren | 24 +++++++++++++ test/api/get_variable_module.wren | 3 ++ test/api/main.c | 6 +++- util/xcode/wren.xcodeproj/project.pbxproj | 6 ++++ 8 files changed, 112 insertions(+), 1 deletion(-) create mode 100644 test/api/get_variable.c create mode 100644 test/api/get_variable.h create mode 100644 test/api/get_variable.wren create mode 100644 test/api/get_variable_module.wren diff --git a/src/include/wren.h b/src/include/wren.h index 3b9465d3..e55487b5 100644 --- a/src/include/wren.h +++ b/src/include/wren.h @@ -384,4 +384,9 @@ void wrenSetSlotValue(WrenVM* vm, int slot, WrenValue* value); // an element, use `-1` for the index. void wrenInsertInList(WrenVM* vm, int listSlot, int index, int elementSlot); +// Looks up the top level variable with [name] in [module] and stores it in +// [slot]. +void wrenGetVariable(WrenVM* vm, const char* module, const char* name, + int slot); + #endif diff --git a/src/vm/wren_vm.c b/src/vm/wren_vm.c index 1d3bd1f3..e517fb65 100644 --- a/src/vm/wren_vm.c +++ b/src/vm/wren_vm.c @@ -1789,3 +1789,25 @@ void wrenInsertInList(WrenVM* vm, int listSlot, int index, int elementSlot) wrenListInsert(vm, list, vm->foreignStackStart[elementSlot], index); } + +void wrenGetVariable(WrenVM* vm, const char* module, const char* name, + int slot) +{ + ASSERT(module != NULL, "Module cannot be NULL."); + ASSERT(module != NULL, "Variable name cannot be NULL."); + validateForeignSlot(vm, slot); + + Value moduleName = wrenStringFormat(vm, "$", module); + wrenPushRoot(vm, AS_OBJ(moduleName)); + + ObjModule* moduleObj = getModule(vm, moduleName); + ASSERT(moduleObj != NULL, "Could not find module."); + + wrenPopRoot(vm); // moduleName. + + int variableSlot = wrenSymbolTableFind(&moduleObj->variableNames, + name, strlen(name)); + ASSERT(variableSlot != -1, "Could not find variable."); + + setSlot(vm, slot, moduleObj->variables.data[variableSlot]); +} diff --git a/test/api/get_variable.c b/test/api/get_variable.c new file mode 100644 index 00000000..c5237214 --- /dev/null +++ b/test/api/get_variable.c @@ -0,0 +1,44 @@ +#include + +#include "get_variable.h" + +static void beforeDefined(WrenVM* vm) +{ + wrenGetVariable(vm, "main", "A", 0); +} + +static void afterDefined(WrenVM* vm) +{ + wrenGetVariable(vm, "main", "A", 0); +} + +static void afterAssigned(WrenVM* vm) +{ + wrenGetVariable(vm, "main", "A", 0); +} + +static void otherSlot(WrenVM* vm) +{ + wrenEnsureSlots(vm, 3); + wrenGetVariable(vm, "main", "B", 2); + + // Move it into return position. + const char* string = wrenGetSlotString(vm, 2); + wrenSetSlotString(vm, 0, string); +} + +static void otherModule(WrenVM* vm) +{ + wrenGetVariable(vm, "get_variable_module", "Variable", 0); +} + +WrenForeignMethodFn getVariableBindMethod(const char* signature) +{ + if (strcmp(signature, "static GetVariable.beforeDefined()") == 0) return beforeDefined; + if (strcmp(signature, "static GetVariable.afterDefined()") == 0) return afterDefined; + if (strcmp(signature, "static GetVariable.afterAssigned()") == 0) return afterAssigned; + if (strcmp(signature, "static GetVariable.otherSlot()") == 0) return otherSlot; + if (strcmp(signature, "static GetVariable.otherModule()") == 0) return otherModule; + + return NULL; +} diff --git a/test/api/get_variable.h b/test/api/get_variable.h new file mode 100644 index 00000000..8150ee23 --- /dev/null +++ b/test/api/get_variable.h @@ -0,0 +1,3 @@ +#include "wren.h" + +WrenForeignMethodFn getVariableBindMethod(const char* signature); diff --git a/test/api/get_variable.wren b/test/api/get_variable.wren new file mode 100644 index 00000000..89b92e4b --- /dev/null +++ b/test/api/get_variable.wren @@ -0,0 +1,24 @@ +import "get_variable_module" + +class GetVariable { + foreign static beforeDefined() + foreign static afterDefined() + foreign static afterAssigned() + foreign static otherSlot() + foreign static otherModule() +} + +System.print(GetVariable.beforeDefined()) // expect: null + +var A = "a" + +System.print(GetVariable.afterDefined()) // expect: a + +A = "changed" + +System.print(GetVariable.afterAssigned()) // expect: changed + +var B = "b" +System.print(GetVariable.otherSlot()) // expect: b + +System.print(GetVariable.otherModule()) // expect: value diff --git a/test/api/get_variable_module.wren b/test/api/get_variable_module.wren new file mode 100644 index 00000000..17c96844 --- /dev/null +++ b/test/api/get_variable_module.wren @@ -0,0 +1,3 @@ +// nontest + +var Variable = "value" diff --git a/test/api/main.c b/test/api/main.c index f4f57e74..7ec7f797 100644 --- a/test/api/main.c +++ b/test/api/main.c @@ -6,6 +6,7 @@ #include "benchmark.h" #include "call.h" +#include "get_variable.h" #include "foreign_class.h" #include "lists.h" #include "slots.h" @@ -33,7 +34,10 @@ static WrenForeignMethodFn bindForeignMethod( method = benchmarkBindMethod(fullName); if (method != NULL) return method; - + + method = getVariableBindMethod(fullName); + if (method != NULL) return method; + method = foreignClassBindMethod(fullName); if (method != NULL) return method; diff --git a/util/xcode/wren.xcodeproj/project.pbxproj b/util/xcode/wren.xcodeproj/project.pbxproj index f5cc72d5..c250b022 100644 --- a/util/xcode/wren.xcodeproj/project.pbxproj +++ b/util/xcode/wren.xcodeproj/project.pbxproj @@ -19,6 +19,7 @@ 29205C9E1AB4E6430073018D /* wren_value.c in Sources */ = {isa = PBXBuildFile; fileRef = 29205C971AB4E6430073018D /* wren_value.c */; }; 29205C9F1AB4E6430073018D /* wren_vm.c in Sources */ = {isa = PBXBuildFile; fileRef = 29205C981AB4E6430073018D /* wren_vm.c */; }; 293D46961BB43F9900200083 /* call.c in Sources */ = {isa = PBXBuildFile; fileRef = 293D46941BB43F9900200083 /* call.c */; }; + 2949AA8D1C2F14F000B106BA /* get_variable.c in Sources */ = {isa = PBXBuildFile; fileRef = 2949AA8B1C2F14F000B106BA /* get_variable.c */; }; 29512C811B91F8EB008C10E6 /* libuv.a in Frameworks */ = {isa = PBXBuildFile; fileRef = 29512C801B91F8EB008C10E6 /* libuv.a */; }; 29512C821B91F901008C10E6 /* libuv.a in Frameworks */ = {isa = PBXBuildFile; fileRef = 29512C801B91F8EB008C10E6 /* libuv.a */; }; 29729F311BA70A620099CA20 /* io.c in Sources */ = {isa = PBXBuildFile; fileRef = 29729F2E1BA70A620099CA20 /* io.c */; }; @@ -98,6 +99,8 @@ 29205CA81AB4E65E0073018D /* wren_vm.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = wren_vm.h; path = ../../src/vm/wren_vm.h; sourceTree = ""; }; 293D46941BB43F9900200083 /* call.c */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.c; name = call.c; path = ../../test/api/call.c; sourceTree = ""; }; 293D46951BB43F9900200083 /* call.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = call.h; path = ../../test/api/call.h; sourceTree = ""; }; + 2949AA8B1C2F14F000B106BA /* get_variable.c */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.c; name = get_variable.c; path = ../../test/api/get_variable.c; sourceTree = ""; }; + 2949AA8C1C2F14F000B106BA /* get_variable.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = get_variable.h; path = ../../test/api/get_variable.h; sourceTree = ""; }; 29512C7F1B91F86E008C10E6 /* api_test */ = {isa = PBXFileReference; explicitFileType = "compiled.mach-o.executable"; includeInIndex = 0; path = api_test; sourceTree = BUILT_PRODUCTS_DIR; }; 29512C801B91F8EB008C10E6 /* libuv.a */ = {isa = PBXFileReference; lastKnownFileType = archive.ar; name = libuv.a; path = ../../build/libuv.a; sourceTree = ""; }; 296371B31AC713D000079FDA /* wren_opcodes.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = wren_opcodes.h; path = ../../src/vm/wren_opcodes.h; sourceTree = ""; }; @@ -252,6 +255,8 @@ 293D46951BB43F9900200083 /* call.h */, 29D009A81B7E39A8000CE58C /* foreign_class.c */, 29D009A91B7E39A8000CE58C /* foreign_class.h */, + 2949AA8B1C2F14F000B106BA /* get_variable.c */, + 2949AA8C1C2F14F000B106BA /* get_variable.h */, 29932D521C210F8D00099DEE /* lists.c */, 29932D531C210F8D00099DEE /* lists.h */, 29D009AA1B7E39A8000CE58C /* slots.c */, @@ -365,6 +370,7 @@ 29729F321BA70A620099CA20 /* io.c in Sources */, 29932D541C210F8D00099DEE /* lists.c in Sources */, 291647C81BA5EC5E006142EE /* modules.c in Sources */, + 2949AA8D1C2F14F000B106BA /* get_variable.c in Sources */, 29DC14A11BBA2FEC008A8274 /* scheduler.c in Sources */, 29A427391BDBE435001E6E22 /* wren_opt_random.c in Sources */, 29932D511C20D8C900099DEE /* benchmark.c in Sources */, From 8203baf7bd2baf62c86a398fd0ecaf9adf1c89ef Mon Sep 17 00:00:00 2001 From: Bob Nystrom Date: Sat, 26 Dec 2015 20:08:20 -0800 Subject: [PATCH 09/26] Initialize match in the test script. Thanks, Michel! --- util/test.py | 1 + 1 file changed, 1 insertion(+) diff --git a/util/test.py b/util/test.py index 8e5bc0fb..1f9454b5 100755 --- a/util/test.py +++ b/util/test.py @@ -182,6 +182,7 @@ class Test: # Make sure the stack trace has the right line. Skip over any lines that # come from builtin libraries. + match = False stack_lines = error_lines[line + 1:] for stack_line in stack_lines: match = STACK_TRACE_PATTERN.search(stack_line) From 8ab329ff91553f49cbdb3161529efbd3e7627c5e Mon Sep 17 00:00:00 2001 From: Bob Nystrom Date: Sat, 26 Dec 2015 20:11:48 -0800 Subject: [PATCH 10/26] Make wren.mk log wren_to_c_string. Thanks, Michel! --- util/wren.mk | 3 +++ util/wren_to_c_string.py | 2 -- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/util/wren.mk b/util/wren.mk index b48f5ca1..88e8b457 100644 --- a/util/wren.mk +++ b/util/wren.mk @@ -206,12 +206,15 @@ $(LIBUV): $(LIBUV_DIR)/build/gyp/gyp util/libuv.py # Wren modules that get compiled into the binary as C strings. src/optional/wren_opt_%.wren.inc: src/optional/wren_opt_%.wren util/wren_to_c_string.py + @ printf "%10s %-30s %s\n" str $< @ ./util/wren_to_c_string.py $@ $< src/vm/wren_%.wren.inc: src/vm/wren_%.wren util/wren_to_c_string.py + @ printf "%10s %-30s %s\n" str $< @ ./util/wren_to_c_string.py $@ $< src/module/%.wren.inc: src/module/%.wren util/wren_to_c_string.py + @ printf "%10s %-30s %s\n" str $< @ ./util/wren_to_c_string.py $@ $< .PHONY: all cli test vm diff --git a/util/wren_to_c_string.py b/util/wren_to_c_string.py index 6715fb40..8129882f 100755 --- a/util/wren_to_c_string.py +++ b/util/wren_to_c_string.py @@ -50,7 +50,5 @@ def main(): with open(args.output, "w") as f: f.write(c_source) - print(" str " + args.input) - main() From 4ad4953196122e1412d4dff55032df853d388829 Mon Sep 17 00:00:00 2001 From: Bob Nystrom Date: Sat, 26 Dec 2015 20:12:35 -0800 Subject: [PATCH 11/26] Fix typo. Thanks, Michel! --- src/vm/wren_compiler.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vm/wren_compiler.c b/src/vm/wren_compiler.c index 14ff8781..80af5451 100644 --- a/src/vm/wren_compiler.c +++ b/src/vm/wren_compiler.c @@ -1321,7 +1321,7 @@ static int resolveLocal(Compiler* compiler, const char* name, int length) // Adds an upvalue to [compiler]'s function with the given properties. Does not // add one if an upvalue for that variable is already in the list. Returns the -// index of the uvpalue. +// index of the upvalue. static int addUpvalue(Compiler* compiler, bool isLocal, int index) { // Look for an existing one. From a5dc5e83903f1e4e1337213d9e3db57b661b7b6b Mon Sep 17 00:00:00 2001 From: Bob Nystrom Date: Sat, 26 Dec 2015 20:13:39 -0800 Subject: [PATCH 12/26] Sort cases alphabetically. Thanks, Michel! --- src/vm/wren_value.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vm/wren_value.c b/src/vm/wren_value.c index c0970762..c40cb423 100644 --- a/src/vm/wren_value.c +++ b/src/vm/wren_value.c @@ -1156,10 +1156,10 @@ void wrenFreeObj(WrenVM* vm, Obj* obj) wrenValueBufferClear(vm, &((ObjModule*)obj)->variables); break; - case OBJ_STRING: case OBJ_CLOSURE: case OBJ_INSTANCE: case OBJ_RANGE: + case OBJ_STRING: case OBJ_UPVALUE: break; } From 5b90896fa847a5e8ab79502d4a1d383ed0c5d58f Mon Sep 17 00:00:00 2001 From: Bob Nystrom Date: Sat, 26 Dec 2015 20:15:00 -0800 Subject: [PATCH 13/26] Print object type for unknown object error. Thanks, Michel! --- src/vm/wren_debug.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vm/wren_debug.c b/src/vm/wren_debug.c index c44522ad..572c826d 100644 --- a/src/vm/wren_debug.c +++ b/src/vm/wren_debug.c @@ -50,7 +50,7 @@ static void dumpObject(Obj* obj) case OBJ_RANGE: printf("[range %p]", obj); break; case OBJ_STRING: printf("%s", ((ObjString*)obj)->value); break; case OBJ_UPVALUE: printf("[upvalue %p]", obj); break; - default: printf("[unknown object]"); break; + default: printf("[unknown object %d]", obj->type); break; } } From fbc76cdc058da54fb1ec9750f8c1b0484f607a29 Mon Sep 17 00:00:00 2001 From: Bob Nystrom Date: Sat, 26 Dec 2015 20:26:05 -0800 Subject: [PATCH 14/26] Fix C++ compile error in Directory.list(). --- src/module/io.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/module/io.c b/src/module/io.c index ac1cc789..580bf876 100644 --- a/src/module/io.c +++ b/src/module/io.c @@ -98,32 +98,32 @@ static void directoryListCallback(uv_fs_t* request) uv_dirent_t entry; - // TODO: Right now, there's way to create a list using the C API so create a - // buffer containing all of the result paths terminated by '\0'. We'll split - // that back into a list in Wren. - size_t resultLength = 0; - size_t bufferSize = 1024; - char* result = (char*)malloc(bufferSize); + // TODO: Right now, there's no way to create a list using the C API, so + // create a buffer containing all of the result paths terminated by '\0'. + // We'll split that back into a list in Wren. + size_t bufferLength = 0; + size_t bufferCapacity = 1024; + char* buffer = (char*)malloc(bufferCapacity); while (uv_fs_scandir_next(request, &entry) != UV_EOF) { size_t length = strlen(entry.name); // Grow the buffer if needed. - while (resultLength + length + 1 > bufferSize) + while (bufferLength + length + 1 > bufferCapacity) { - bufferSize *= 2; - result = realloc(result, bufferSize); + bufferCapacity *= 2; + buffer = (char*)realloc(buffer, bufferCapacity); } // Copy the path, including the null terminator. - memcpy(result + resultLength, entry.name, length + 1); - resultLength += length + 1; + memcpy(buffer + bufferLength, entry.name, length + 1); + bufferLength += length + 1; } WrenValue* fiber = freeRequest(request); - schedulerResumeBytes(fiber, result, resultLength); - free(result); + schedulerResumeBytes(fiber, buffer, bufferLength); + free(buffer); } void directoryList(WrenVM* vm) From a09892de326ffeca993e82fad2ebc7a27c6c45e7 Mon Sep 17 00:00:00 2001 From: Bob Nystrom Date: Sun, 27 Dec 2015 21:43:08 -0800 Subject: [PATCH 15/26] Use a fiber for a finalizer's foreign stack. --- Makefile | 2 +- src/vm/wren_value.c | 8 +++++--- src/vm/wren_vm.c | 30 ++++++++++++++---------------- src/vm/wren_vm.h | 6 ++++++ 4 files changed, 26 insertions(+), 20 deletions(-) diff --git a/Makefile b/Makefile index c402486a..493b5868 100644 --- a/Makefile +++ b/Makefile @@ -48,7 +48,7 @@ test: debug benchmark: release @ $(MAKE) -f util/wren.mk test - @ ./util/benchmark.py $(suite) + @ ./util/benchmark.py -l wren $(suite) # Generate the Wren site. docs: diff --git a/src/vm/wren_value.c b/src/vm/wren_value.c index ea576b9a..ab3c8f1f 100644 --- a/src/vm/wren_value.c +++ b/src/vm/wren_value.c @@ -152,7 +152,9 @@ ObjFiber* wrenNewFiber(WrenVM* vm, Obj* fn) // Add one slot for the unused implicit receiver slot that the compiler // assumes all functions have. - int stackCapacity = wrenPowerOf2Ceil(wrenUpwrapClosure(fn)->maxSlots + 1); + int stackCapacity = fn == NULL + ? 1 + : wrenPowerOf2Ceil(wrenUpwrapClosure(fn)->maxSlots + 1); Value* stack = ALLOCATE_ARRAY(vm, Value, stackCapacity); ObjFiber* fiber = ALLOCATE(vm, ObjFiber); @@ -174,10 +176,10 @@ void wrenResetFiber(WrenVM* vm, ObjFiber* fiber, Obj* fn) fiber->caller = NULL; fiber->error = NULL_VAL; fiber->callerIsTrying = false; + fiber->numFrames = 0; // Initialize the first call frame. - fiber->numFrames = 0; - wrenAppendCallFrame(vm, fiber, fn, fiber->stack); + if (fn != NULL) wrenAppendCallFrame(vm, fiber, fn, fiber->stack); } ObjForeign* wrenNewForeign(WrenVM* vm, ObjClass* classObj, size_t size) diff --git a/src/vm/wren_vm.c b/src/vm/wren_vm.c index e517fb65..83ee85e5 100644 --- a/src/vm/wren_vm.c +++ b/src/vm/wren_vm.c @@ -65,6 +65,8 @@ WrenVM* wrenNewVM(WrenConfiguration* config) vm->modules = wrenNewMap(vm); wrenInitializeCore(vm); + + vm->finalizerFiber = wrenNewFiber(vm, NULL); // TODO: Lazy load these. #if WREN_OPT_META @@ -137,8 +139,9 @@ void wrenCollectGarbage(WrenVM* vm) wrenGrayObj(vm, vm->tempRoots[i]); } - // The current fiber. + // The rooted fibers. wrenGrayObj(vm, (Obj*)vm->fiber); + wrenGrayObj(vm, (Obj*)vm->finalizerFiber); // The value handles. for (WrenValue* value = vm->valueHandles; @@ -740,10 +743,16 @@ void wrenFinalizeForeign(WrenVM* vm, ObjForeign* foreign) ASSERT(method->type == METHOD_FOREIGN, "Finalizer should be foreign."); // Pass the foreign object to the finalizer. - Value slot = OBJ_VAL(foreign); - vm->foreignStackStart = &slot; + *vm->finalizerFiber->stackTop++ = OBJ_VAL(foreign); + ObjFiber* previousFiber = vm->fiber; + vm->fiber = vm->finalizerFiber; + vm->foreignStackStart = vm->finalizerFiber->stack; method->fn.foreign(vm); + + vm->fiber = previousFiber; + vm->foreignStackStart = NULL; + wrenResetFiber(vm, vm->finalizerFiber, NULL); } // The main bytecode interpreter loop. This is where the magic happens. It is @@ -1637,28 +1646,17 @@ void wrenPopRoot(WrenVM* vm) vm->numTempRoots--; } -// Returns true if the VM is in a foreign method that's a finalizer. -// -// Finalizers don't run in the context of a fiber and have a single magic stack -// slot, so need to be handled a little specially. -static bool isInFinalizer(WrenVM* vm) -{ - return vm->fiber == NULL || - vm->foreignStackStart < vm->fiber->stack || - vm->foreignStackStart > vm->fiber->stackTop; -} - int wrenGetSlotCount(WrenVM* vm) { ASSERT(vm->foreignStackStart != NULL, "Must be in foreign call."); - if (isInFinalizer(vm)) return 1; return (int)(vm->fiber->stackTop - vm->foreignStackStart); } void wrenEnsureSlots(WrenVM* vm, int numSlots) { - ASSERT(!isInFinalizer(vm), "Cannot grow the stack in a finalizer."); + ASSERT(vm->fiber != vm->finalizerFiber, + "Cannot grow the stack in a finalizer."); int currentSize = (int)(vm->fiber->stackTop - vm->foreignStackStart); if (currentSize >= numSlots) return; diff --git a/src/vm/wren_vm.h b/src/vm/wren_vm.h index 1d0b0776..5508e7d3 100644 --- a/src/vm/wren_vm.h +++ b/src/vm/wren_vm.h @@ -102,6 +102,12 @@ struct WrenVM // There is a single global symbol table for all method names on all classes. // Method calls are dispatched directly by index in this table. SymbolTable methodNames; + + // A special fiber whose stack is used for communicating with foreign + // finalizer methods. Finalizers run during GC, so we don't want to do any + // allocation in order to execute one, so we set up this fiber ahead of time + // to ensure it's available. + ObjFiber* finalizerFiber; }; // A generic allocation function that handles all explicit memory management. From a447b663806116e17c2758cbd600d7c2d01217ea Mon Sep 17 00:00:00 2001 From: Bob Nystrom Date: Mon, 28 Dec 2015 07:43:17 -0800 Subject: [PATCH 16/26] Make slots available to the API outside of foreign methods. At any point in time, the user can call wrenEnsureSlots() which will implicitly create a fiber and point to its stack if needed. --- src/optional/wren_opt_meta.c | 2 +- src/vm/wren_value.c | 42 ++++++++++ src/vm/wren_value.h | 3 + src/vm/wren_vm.c | 146 ++++++++++++++--------------------- src/vm/wren_vm.h | 13 ++-- test/api/slots.c | 37 ++++++++- test/api/slots.wren | 13 +++- 7 files changed, 157 insertions(+), 99 deletions(-) diff --git a/src/optional/wren_opt_meta.c b/src/optional/wren_opt_meta.c index 6bebcd3e..a4f922df 100644 --- a/src/optional/wren_opt_meta.c +++ b/src/optional/wren_opt_meta.c @@ -22,7 +22,7 @@ void metaCompile(WrenVM* vm) // Return the result. We can't use the public API for this since we have a // bare ObjFn. - vm->foreignStackStart[0] = fn != NULL ? OBJ_VAL(fn) : NULL_VAL; + vm->apiStack[0] = fn != NULL ? OBJ_VAL(fn) : NULL_VAL; } static WrenForeignMethodFn bindMetaForeignMethods(WrenVM* vm, diff --git a/src/vm/wren_value.c b/src/vm/wren_value.c index ab3c8f1f..22cfb995 100644 --- a/src/vm/wren_value.c +++ b/src/vm/wren_value.c @@ -182,6 +182,48 @@ void wrenResetFiber(WrenVM* vm, ObjFiber* fiber, Obj* fn) if (fn != NULL) wrenAppendCallFrame(vm, fiber, fn, fiber->stack); } +void wrenEnsureStack(WrenVM* vm, ObjFiber* fiber, int needed) +{ + if (fiber->stackCapacity >= needed) return; + + int capacity = wrenPowerOf2Ceil(needed); + + Value* oldStack = fiber->stack; + fiber->stack = (Value*)wrenReallocate(vm, fiber->stack, + sizeof(Value) * fiber->stackCapacity, + sizeof(Value) * capacity); + fiber->stackCapacity = capacity; + + // If the reallocation moves the stack, then we need to shift every pointer + // into the stack to point to its new location. + if (fiber->stack != oldStack) + { + // Top of the stack. + long offset = fiber->stack - oldStack; + + if (vm->apiStack >= oldStack && vm->apiStack <= fiber->stackTop) + { + vm->apiStack += offset; + } + + // Stack pointer for each call frame. + for (int i = 0; i < fiber->numFrames; i++) + { + fiber->frames[i].stackStart += offset; + } + + // Open upvalues. + for (ObjUpvalue* upvalue = fiber->openUpvalues; + upvalue != NULL; + upvalue = upvalue->next) + { + upvalue->value += offset; + } + + fiber->stackTop += offset; + } +} + ObjForeign* wrenNewForeign(WrenVM* vm, ObjClass* classObj, size_t size) { ObjForeign* object = ALLOCATE_FLEX(vm, ObjForeign, uint8_t, size); diff --git a/src/vm/wren_value.h b/src/vm/wren_value.h index 21e871fd..b34a8801 100644 --- a/src/vm/wren_value.h +++ b/src/vm/wren_value.h @@ -660,6 +660,9 @@ static inline void wrenAppendCallFrame(WrenVM* vm, ObjFiber* fiber, frame->ip = wrenUpwrapClosure(frame->fn)->bytecode; } +// Ensures [fiber]'s stack has at least [needed] slots. +void wrenEnsureStack(WrenVM* vm, ObjFiber* fiber, int needed); + ObjForeign* wrenNewForeign(WrenVM* vm, ObjClass* classObj, size_t size); // TODO: The argument list here is getting a bit gratuitous. diff --git a/src/vm/wren_vm.c b/src/vm/wren_vm.c index 83ee85e5..add81798 100644 --- a/src/vm/wren_vm.c +++ b/src/vm/wren_vm.c @@ -351,15 +351,14 @@ static void bindMethod(WrenVM* vm, int methodType, int symbol, static void callForeign(WrenVM* vm, ObjFiber* fiber, WrenForeignMethodFn foreign, int numArgs) { - vm->foreignStackStart = fiber->stackTop - numArgs; + vm->apiStack = fiber->stackTop - numArgs; foreign(vm); // Discard the stack slots for the arguments and temporaries but leave one // for the result. - fiber->stackTop = vm->foreignStackStart + 1; - - vm->foreignStackStart = NULL; + fiber->stackTop = vm->apiStack + 1; + vm->apiStack = NULL; } // Handles the current fiber having aborted because of an error. Switches to @@ -423,48 +422,6 @@ static bool checkArity(WrenVM* vm, Value value, int numArgs) return false; } -// Ensures [fiber]'s stack has at least [needed] slots. -static void ensureStack(WrenVM* vm, ObjFiber* fiber, int needed) -{ - if (fiber->stackCapacity >= needed) return; - - int capacity = wrenPowerOf2Ceil(needed); - - Value* oldStack = fiber->stack; - fiber->stack = (Value*)wrenReallocate(vm, fiber->stack, - sizeof(Value) * fiber->stackCapacity, - sizeof(Value) * capacity); - fiber->stackCapacity = capacity; - - // If the reallocation moves the stack, then we need to shift every pointer - // into the stack to point to its new location. - if (fiber->stack != oldStack) - { - // Top of the stack. - long offset = fiber->stack - oldStack; - fiber->stackTop += offset; - - // Stack pointer for each call frame. - for (int i = 0; i < fiber->numFrames; i++) - { - fiber->frames[i].stackStart += offset; - } - - // Open upvalues. - for (ObjUpvalue* upvalue = fiber->openUpvalues; - upvalue != NULL; - upvalue = upvalue->next) - { - upvalue->value += offset; - } - - if (vm->foreignStackStart != NULL) - { - vm->foreignStackStart += offset; - } - } -} - // Pushes [function] onto [fiber]'s callstack and invokes it. Expects [numArgs] // arguments (including the receiver) to be on the top of the stack already. // [function] can be an `ObjFn` or `ObjClosure`. @@ -484,7 +441,7 @@ static inline void callFunction( // Grow the stack if needed. int stackSize = (int)(fiber->stackTop - fiber->stack); int needed = stackSize + wrenUpwrapClosure(function)->maxSlots; - ensureStack(vm, fiber, needed); + wrenEnsureStack(vm, fiber, needed); wrenAppendCallFrame(vm, fiber, function, fiber->stackTop - numArgs); } @@ -717,7 +674,7 @@ static void createForeign(WrenVM* vm, ObjFiber* fiber, Value* stack) ASSERT(method->type == METHOD_FOREIGN, "Allocator should be foreign."); // Pass the constructor arguments to the allocator as well. - vm->foreignStackStart = stack; + vm->apiStack = stack; method->fn.foreign(vm); @@ -746,12 +703,18 @@ void wrenFinalizeForeign(WrenVM* vm, ObjForeign* foreign) *vm->finalizerFiber->stackTop++ = OBJ_VAL(foreign); ObjFiber* previousFiber = vm->fiber; vm->fiber = vm->finalizerFiber; - vm->foreignStackStart = vm->finalizerFiber->stack; + + // TODO: What if apiStack is not currently NULL? This can happen if a foreign + // method causes an allocation which triggers a GC. The simplest solution + // might be to make finalizers have a different function type which only + // passes in the foreign bytes and not the VM. Then we wouldn't need a fiber + // for the finalizer. + vm->apiStack = vm->finalizerFiber->stack; method->fn.foreign(vm); vm->fiber = previousFiber; - vm->foreignStackStart = NULL; + vm->apiStack = NULL; wrenResetFiber(vm, vm->finalizerFiber, NULL); } @@ -1512,14 +1475,14 @@ void wrenReleaseValue(WrenVM* vm, WrenValue* value) void* wrenAllocateForeign(WrenVM* vm, size_t size) { - ASSERT(vm->foreignStackStart != NULL, "Must be in foreign call."); + ASSERT(vm->apiStack != NULL, "Must be in foreign call."); // TODO: Validate this. It can fail if the user calls this inside another // foreign method, or calls one of the return functions. - ObjClass* classObj = AS_CLASS(vm->foreignStackStart[0]); + ObjClass* classObj = AS_CLASS(vm->apiStack[0]); ObjForeign* foreign = wrenNewForeign(vm, classObj, size); - vm->foreignStackStart[0] = OBJ_VAL(foreign); + vm->apiStack[0] = OBJ_VAL(foreign); return (void*)foreign->data; } @@ -1648,9 +1611,9 @@ void wrenPopRoot(WrenVM* vm) int wrenGetSlotCount(WrenVM* vm) { - ASSERT(vm->foreignStackStart != NULL, "Must be in foreign call."); - - return (int)(vm->fiber->stackTop - vm->foreignStackStart); + if (vm->apiStack == NULL) return 0; + + return (int)(vm->fiber->stackTop - vm->apiStack); } void wrenEnsureSlots(WrenVM* vm, int numSlots) @@ -1658,79 +1621,84 @@ void wrenEnsureSlots(WrenVM* vm, int numSlots) ASSERT(vm->fiber != vm->finalizerFiber, "Cannot grow the stack in a finalizer."); - int currentSize = (int)(vm->fiber->stackTop - vm->foreignStackStart); + // If we don't have a fiber accessible, create one for the API to use. + if (vm->fiber == NULL && vm->apiStack == NULL) + { + vm->fiber = wrenNewFiber(vm, NULL); + vm->apiStack = vm->fiber->stack; + } + + int currentSize = (int)(vm->fiber->stackTop - vm->apiStack); if (currentSize >= numSlots) return; // Grow the stack if needed. - int needed = (int)(vm->foreignStackStart - vm->fiber->stack) + numSlots; - ensureStack(vm, vm->fiber, needed); + int needed = (int)(vm->apiStack - vm->fiber->stack) + numSlots; + wrenEnsureStack(vm, vm->fiber, needed); - vm->fiber->stackTop = vm->foreignStackStart + numSlots; + vm->fiber->stackTop = vm->apiStack + numSlots; } -// Ensures that [slot] is a valid index into a foreign method's stack of slots. -static void validateForeignSlot(WrenVM* vm, int slot) +// Ensures that [slot] is a valid index into the API's stack of slots. +static void validateApiSlot(WrenVM* vm, int slot) { - ASSERT(vm->foreignStackStart != NULL, "Must be in foreign call."); ASSERT(slot >= 0, "Slot cannot be negative."); ASSERT(slot < wrenGetSlotCount(vm), "Not that many slots."); } bool wrenGetSlotBool(WrenVM* vm, int slot) { - validateForeignSlot(vm, slot); - ASSERT(IS_BOOL(vm->foreignStackStart[slot]), "Slot must hold a bool."); + validateApiSlot(vm, slot); + ASSERT(IS_BOOL(vm->apiStack[slot]), "Slot must hold a bool."); - return AS_BOOL(vm->foreignStackStart[slot]); + return AS_BOOL(vm->apiStack[slot]); } const char* wrenGetSlotBytes(WrenVM* vm, int slot, int* length) { - validateForeignSlot(vm, slot); - ASSERT(IS_STRING(vm->foreignStackStart[slot]), "Slot must hold a string."); + validateApiSlot(vm, slot); + ASSERT(IS_STRING(vm->apiStack[slot]), "Slot must hold a string."); - ObjString* string = AS_STRING(vm->foreignStackStart[slot]); + ObjString* string = AS_STRING(vm->apiStack[slot]); *length = string->length; return string->value; } double wrenGetSlotDouble(WrenVM* vm, int slot) { - validateForeignSlot(vm, slot); - ASSERT(IS_NUM(vm->foreignStackStart[slot]), "Slot must hold a number."); + validateApiSlot(vm, slot); + ASSERT(IS_NUM(vm->apiStack[slot]), "Slot must hold a number."); - return AS_NUM(vm->foreignStackStart[slot]); + return AS_NUM(vm->apiStack[slot]); } void* wrenGetSlotForeign(WrenVM* vm, int slot) { - validateForeignSlot(vm, slot); - ASSERT(IS_FOREIGN(vm->foreignStackStart[slot]), + validateApiSlot(vm, slot); + ASSERT(IS_FOREIGN(vm->apiStack[slot]), "Slot must hold a foreign instance."); - return AS_FOREIGN(vm->foreignStackStart[slot])->data; + return AS_FOREIGN(vm->apiStack[slot])->data; } const char* wrenGetSlotString(WrenVM* vm, int slot) { - validateForeignSlot(vm, slot); - ASSERT(IS_STRING(vm->foreignStackStart[slot]), "Slot must hold a string."); + validateApiSlot(vm, slot); + ASSERT(IS_STRING(vm->apiStack[slot]), "Slot must hold a string."); - return AS_CSTRING(vm->foreignStackStart[slot]); + return AS_CSTRING(vm->apiStack[slot]); } WrenValue* wrenGetSlotValue(WrenVM* vm, int slot) { - validateForeignSlot(vm, slot); - - return wrenCaptureValue(vm, vm->foreignStackStart[slot]); + validateApiSlot(vm, slot); + return wrenCaptureValue(vm, vm->apiStack[slot]); } // Stores [value] in [slot] in the foreign call stack. static void setSlot(WrenVM* vm, int slot, Value value) { - validateForeignSlot(vm, slot); - vm->foreignStackStart[slot] = value; + validateApiSlot(vm, slot); + vm->apiStack[slot] = value; } void wrenSetSlotBool(WrenVM* vm, int slot, bool value) @@ -1774,18 +1742,18 @@ void wrenSetSlotValue(WrenVM* vm, int slot, WrenValue* value) void wrenInsertInList(WrenVM* vm, int listSlot, int index, int elementSlot) { - validateForeignSlot(vm, listSlot); - validateForeignSlot(vm, elementSlot); - ASSERT(IS_LIST(vm->foreignStackStart[listSlot]), "Must insert into a list."); + validateApiSlot(vm, listSlot); + validateApiSlot(vm, elementSlot); + ASSERT(IS_LIST(vm->apiStack[listSlot]), "Must insert into a list."); - ObjList* list = AS_LIST(vm->foreignStackStart[listSlot]); + ObjList* list = AS_LIST(vm->apiStack[listSlot]); // Negative indices count from the end. if (index < 0) index = list->elements.count + 1 + index; ASSERT(index <= list->elements.count, "Index out of bounds."); - wrenListInsert(vm, list, vm->foreignStackStart[elementSlot], index); + wrenListInsert(vm, list, vm->apiStack[elementSlot], index); } void wrenGetVariable(WrenVM* vm, const char* module, const char* name, @@ -1793,7 +1761,7 @@ void wrenGetVariable(WrenVM* vm, const char* module, const char* name, { ASSERT(module != NULL, "Module cannot be NULL."); ASSERT(module != NULL, "Variable name cannot be NULL."); - validateForeignSlot(vm, slot); + validateApiSlot(vm, slot); Value moduleName = wrenStringFormat(vm, "$", module); wrenPushRoot(vm, AS_OBJ(moduleName)); diff --git a/src/vm/wren_vm.h b/src/vm/wren_vm.h index 5508e7d3..402f4233 100644 --- a/src/vm/wren_vm.h +++ b/src/vm/wren_vm.h @@ -84,11 +84,14 @@ struct WrenVM // NULL if there are no handles. WrenValue* valueHandles; - // Foreign function data: - - // During a foreign function call, this will point to the first argument (the - // receiver) of the call on the fiber's stack. - Value* foreignStackStart; + // Pointer to the bottom of the range of stack slots available for use from + // the C API. During a foreign method, this will be in the stack of the fiber + // that is executing a method. + // + // If not in a foreign method, this is initially NULL. If the user requests + // slots by calling wrenEnsureSlots(), a stack is created and this is + // initialized. + Value* apiStack; WrenConfiguration config; diff --git a/test/api/slots.c b/test/api/slots.c index 347b9d75..9e995b9b 100644 --- a/test/api/slots.c +++ b/test/api/slots.c @@ -94,7 +94,41 @@ static void ensure(WrenVM* vm) { sum += (int)wrenGetSlotDouble(vm, i); } - + + char result[100]; + sprintf(result, "%d -> %d (%d)", before, after, sum); + wrenSetSlotString(vm, 0, result); +} + +static void ensureOutsideForeign(WrenVM* vm) +{ + // To test the behavior outside of a foreign method (which we're currently + // in), create a new separate VM. + WrenConfiguration config; + wrenInitConfiguration(&config); + WrenVM* otherVM = wrenNewVM(&config); + + int before = wrenGetSlotCount(otherVM); + + wrenEnsureSlots(otherVM, 20); + + int after = wrenGetSlotCount(otherVM); + + // Use the slots to make sure they're available. + for (int i = 0; i < 20; i++) + { + wrenSetSlotDouble(otherVM, i, i); + } + + int sum = 0; + + for (int i = 0; i < 20; i++) + { + sum += (int)wrenGetSlotDouble(otherVM, i); + } + + wrenFreeVM(otherVM); + char result[100]; sprintf(result, "%d -> %d (%d)", before, after, sum); wrenSetSlotString(vm, 0, result); @@ -106,6 +140,7 @@ WrenForeignMethodFn slotsBindMethod(const char* signature) if (strcmp(signature, "static Slots.getSlots(_,_,_,_,_)") == 0) return getSlots; if (strcmp(signature, "static Slots.setSlots(_,_,_,_)") == 0) return setSlots; if (strcmp(signature, "static Slots.ensure()") == 0) return ensure; + if (strcmp(signature, "static Slots.ensureOutsideForeign()") == 0) return ensureOutsideForeign; return NULL; } diff --git a/test/api/slots.wren b/test/api/slots.wren index 70d403e8..cf63988f 100644 --- a/test/api/slots.wren +++ b/test/api/slots.wren @@ -3,6 +3,7 @@ class Slots { foreign static getSlots(bool, num, string, bytes, value) foreign static setSlots(a, b, c, d) foreign static ensure() + foreign static ensureOutsideForeign() } // If nothing is set in the return slot, it retains its previous value, the @@ -10,8 +11,14 @@ class Slots { System.print(Slots.noSet == Slots) // expect: true var value = ["value"] -System.print(Slots.getSlots(true, "by\0te", 12.34, "str", value) == value) // expect: true +System.print(Slots.getSlots(true, "by\0te", 12.34, "str", value) == value) +// expect: true -System.print(Slots.setSlots(value, 0, 0, 0) == value) // expect: true +System.print(Slots.setSlots(value, 0, 0, 0) == value) +// expect: true -System.print(Slots.ensure()) // expect: 1 -> 20 (190) +System.print(Slots.ensure()) +// expect: 1 -> 20 (190) + +System.print(Slots.ensureOutsideForeign()) +// expect: 0 -> 20 (190) From ed6fad6153d8686d5e23837f345563792dc07716 Mon Sep 17 00:00:00 2001 From: Bob Nystrom Date: Mon, 28 Dec 2015 08:06:29 -0800 Subject: [PATCH 17/26] Get rid of fiber for finalizers. Instead, finalizers just get access to the foreign object's raw bytes. This is deliberately limiting, since it discourages the user from interacting with the VM in the middle of a GC. --- src/cli/modules.c | 2 +- src/include/wren.h | 8 +++++++- src/module/io.c | 4 ++-- src/vm/wren_vm.c | 29 ++++------------------------- src/vm/wren_vm.h | 6 ------ test/api/foreign_class.c | 9 +++++++-- test/io/file/finalize.wren | 10 ++++++++++ 7 files changed, 31 insertions(+), 37 deletions(-) create mode 100644 test/io/file/finalize.wren diff --git a/src/cli/modules.c b/src/cli/modules.c index 27111e4c..581f5f2d 100644 --- a/src/cli/modules.c +++ b/src/cli/modules.c @@ -208,7 +208,7 @@ WrenForeignClassMethods bindBuiltInForeignClass( if (clas == NULL) return methods; methods.allocate = findMethod(clas, true, ""); - methods.finalize = findMethod(clas, true, ""); + methods.finalize = (WrenFinalizerFn)findMethod(clas, true, ""); return methods; } diff --git a/src/include/wren.h b/src/include/wren.h index e55487b5..a73a69b8 100644 --- a/src/include/wren.h +++ b/src/include/wren.h @@ -38,6 +38,12 @@ typedef void* (*WrenReallocateFn)(void* memory, size_t newSize); // A function callable from Wren code, but implemented in C. typedef void (*WrenForeignMethodFn)(WrenVM* vm); +// A finalizer function for freeing resources owned by an instance of a foreign +// class. Unlike most foreign methods, finalizers do not have access to the VM +// and should not interact with it since it's in the middle of a garbage +// collection. +typedef void (*WrenFinalizerFn)(void* data); + // Loads and returns the source code for the module [name]. typedef char* (*WrenLoadModuleFn)(WrenVM* vm, const char* name); @@ -64,7 +70,7 @@ typedef struct // foreign object's memory. // // This may be `NULL` if the foreign class does not need to finalize. - WrenForeignMethodFn finalize; + WrenFinalizerFn finalize; } WrenForeignClassMethods; // Returns a pair of pointers to the foreign methods used to allocate and diff --git a/src/module/io.c b/src/module/io.c index 284f5a20..5ec141ab 100644 --- a/src/module/io.c +++ b/src/module/io.c @@ -144,9 +144,9 @@ void fileAllocate(WrenVM* vm) *fd = (int)wrenGetSlotDouble(vm, 1); } -void fileFinalize(WrenVM* vm) +void fileFinalize(void* data) { - int fd = *(int*)wrenGetSlotForeign(vm, 0); + int fd = *(int*)data; // Already closed. if (fd == -1) return; diff --git a/src/vm/wren_vm.c b/src/vm/wren_vm.c index add81798..eca8eca6 100644 --- a/src/vm/wren_vm.c +++ b/src/vm/wren_vm.c @@ -66,8 +66,6 @@ WrenVM* wrenNewVM(WrenConfiguration* config) wrenInitializeCore(vm); - vm->finalizerFiber = wrenNewFiber(vm, NULL); - // TODO: Lazy load these. #if WREN_OPT_META wrenLoadMetaModule(vm); @@ -139,9 +137,8 @@ void wrenCollectGarbage(WrenVM* vm) wrenGrayObj(vm, vm->tempRoots[i]); } - // The rooted fibers. + // The rooted fiber. wrenGrayObj(vm, (Obj*)vm->fiber); - wrenGrayObj(vm, (Obj*)vm->finalizerFiber); // The value handles. for (WrenValue* value = vm->valueHandles; @@ -628,7 +625,7 @@ static void bindForeignClass(WrenVM* vm, ObjClass* classObj, ObjModule* module) if (methods.finalize != NULL) { - method.fn.foreign = methods.finalize; + method.fn.foreign = (WrenForeignMethodFn)methods.finalize; wrenBindMethod(vm, classObj, symbol, method); } } @@ -699,23 +696,8 @@ void wrenFinalizeForeign(WrenVM* vm, ObjForeign* foreign) ASSERT(method->type == METHOD_FOREIGN, "Finalizer should be foreign."); - // Pass the foreign object to the finalizer. - *vm->finalizerFiber->stackTop++ = OBJ_VAL(foreign); - ObjFiber* previousFiber = vm->fiber; - vm->fiber = vm->finalizerFiber; - - // TODO: What if apiStack is not currently NULL? This can happen if a foreign - // method causes an allocation which triggers a GC. The simplest solution - // might be to make finalizers have a different function type which only - // passes in the foreign bytes and not the VM. Then we wouldn't need a fiber - // for the finalizer. - vm->apiStack = vm->finalizerFiber->stack; - - method->fn.foreign(vm); - - vm->fiber = previousFiber; - vm->apiStack = NULL; - wrenResetFiber(vm, vm->finalizerFiber, NULL); + WrenFinalizerFn finalizer = (WrenFinalizerFn)method->fn.foreign; + finalizer(foreign->data); } // The main bytecode interpreter loop. This is where the magic happens. It is @@ -1618,9 +1600,6 @@ int wrenGetSlotCount(WrenVM* vm) void wrenEnsureSlots(WrenVM* vm, int numSlots) { - ASSERT(vm->fiber != vm->finalizerFiber, - "Cannot grow the stack in a finalizer."); - // If we don't have a fiber accessible, create one for the API to use. if (vm->fiber == NULL && vm->apiStack == NULL) { diff --git a/src/vm/wren_vm.h b/src/vm/wren_vm.h index 402f4233..2b9d8d01 100644 --- a/src/vm/wren_vm.h +++ b/src/vm/wren_vm.h @@ -105,12 +105,6 @@ struct WrenVM // There is a single global symbol table for all method names on all classes. // Method calls are dispatched directly by index in this table. SymbolTable methodNames; - - // A special fiber whose stack is used for communicating with foreign - // finalizer methods. Finalizers run during GC, so we don't want to do any - // allocation in order to execute one, so we set up this fiber ahead of time - // to ensure it's available. - ObjFiber* finalizerFiber; }; // A generic allocation function that handles all explicit memory management. diff --git a/test/api/foreign_class.c b/test/api/foreign_class.c index 119a5879..f77bf17d 100644 --- a/test/api/foreign_class.c +++ b/test/api/foreign_class.c @@ -69,11 +69,16 @@ static void pointToString(WrenVM* vm) static void resourceAllocate(WrenVM* vm) { - wrenAllocateForeign(vm, 0); + int* value = (int*)wrenAllocateForeign(vm, sizeof(int)); + *value = 123; } -static void resourceFinalize(WrenVM* vm) +static void resourceFinalize(void* data) { + // Make sure we get the right data back. + int* value = (int*)data; + if (*value != 123) exit(1); + finalized++; } diff --git a/test/io/file/finalize.wren b/test/io/file/finalize.wren new file mode 100644 index 00000000..5502fbab --- /dev/null +++ b/test/io/file/finalize.wren @@ -0,0 +1,10 @@ +import "io" for File + +// Don't store in a variable. +File.open("test/io/file/finalize.wren") + +System.gc() +// We can't really test what the finalizer *does* from Wren, since the object +// is unreachable, but this at least ensures it doesn't crash. + +System.print("ok") // expect: ok \ No newline at end of file From e0ac88c22abcf2ce53e7e7630f365eabb8ef4d01 Mon Sep 17 00:00:00 2001 From: Bob Nystrom Date: Tue, 29 Dec 2015 07:58:47 -0800 Subject: [PATCH 18/26] Revamp wrenCall to work with slots. Now, you call wrenEnsureSlots() and then wrenSetSlot___() to set up the receiver and arguments before the call. Then wrenCall() is passed a handle to the stub function that makes the call. After that, you can get the result using wrenGetSlot___(). This is a little more verbose to use, but it's more flexible, simpler, and much faster in the VM. The call benchmark is 185% of the previous speed. --- src/include/wren.h | 57 +++--------- src/module/io.c | 67 ++++++++++---- src/module/scheduler.c | 71 ++++++++------- src/module/scheduler.h | 14 ++- src/module/timer.c | 2 +- src/vm/wren_core.c | 2 + src/vm/wren_vm.c | 201 ++++++++++------------------------------- test/api/benchmark.c | 21 ++++- test/api/call.c | 78 ++++++++++++---- test/api/call.wren | 9 +- 10 files changed, 239 insertions(+), 283 deletions(-) diff --git a/src/include/wren.h b/src/include/wren.h index a73a69b8..de2b7874 100644 --- a/src/include/wren.h +++ b/src/include/wren.h @@ -189,54 +189,27 @@ void wrenCollectGarbage(WrenVM* vm); // Runs [source], a string of Wren source code in a new fiber in [vm]. WrenInterpretResult wrenInterpret(WrenVM* vm, const char* source); -// Creates a handle that can be used to invoke a method with [signature] on the -// object in [module] currently stored in top-level [variable]. +// Creates a handle that can be used to invoke a method with [signature] on +// using a receiver and arguments that are set up on the stack. // // 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 using [wrenReleaseValue]. -WrenValue* wrenGetMethod(WrenVM* vm, const char* module, const char* variable, - const char* signature); +// When you are done with this handle, it must be released using +// [wrenReleaseValue]. +WrenValue* wrenMakeCallHandle(WrenVM* vm, 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 -// type of a single argument, in order. The allowed types are: +// Calls [method], using the receiver and arguments previously set up on the +// stack. // -// - "b" - A C `int` converted to a Wren Bool. -// - "d" - A C `double` converted to a Wren Num. -// - "i" - A C `int` converted to a Wren Num. -// - "s" - A C null-terminated `const char*` converted to a Wren String. Wren -// will allocate its own string and copy the characters from this, so -// you don't have to worry about the lifetime of the string you pass to -// Wren. -// - "a" - An array of bytes converted to a Wren String. This requires two -// consecutive arguments in the argument list: `const char*` pointing -// to the array of bytes, followed by an `int` defining the length of -// the array. This is used when the passed string may contain null -// bytes, or just to avoid the implicit `strlen()` call of "s" if you -// happen to already know the length. -// - "v" - A previously acquired WrenValue*. Passing this in does not implicitly -// release the value. If the passed argument is NULL, this becomes a -// Wren NULL. +// [method] must have been created by a call to [wrenMakeCallHandle]. The +// arguments to the method must be already on the stack. The receiver should be +// in slot 0 with the remaining arguments following it, in order. It is an +// error if the number of arguments provided does not match the method's +// signature. // -// [method] must have been created by a call to [wrenGetMethod]. If -// [returnValue] is not `NULL`, the return value of the method will be stored -// in a new [WrenValue] that [returnValue] will point to. Don't forget to -// release it, when done with it. -WrenInterpretResult wrenCall(WrenVM* vm, WrenValue* method, - WrenValue** returnValue, - const char* argTypes, ...); - -WrenInterpretResult wrenCallVarArgs(WrenVM* vm, WrenValue* method, - WrenValue** returnValue, - const char* argTypes, va_list args); - -// Gets the numeric value of [value]. -// -// It is an error to call this if the value is not a number. -double wrenGetValueDouble(WrenVM* vm, WrenValue* value); -// TODO: Functions for other types. +// After this returns, you can access the return value from slot 0 on the stack. +WrenInterpretResult wrenCall(WrenVM* vm, WrenValue* method); // Releases the reference stored in [value]. After calling this, [value] can no // longer be used. @@ -359,7 +332,7 @@ void wrenSetSlotBool(WrenVM* vm, int slot, bool value); // // The bytes are copied to a new string within Wren's heap, so you can free // memory used by them after this is called. -void wrenSetSlotBytes(WrenVM* vm, int slot, const char* bytes, int length); +void wrenSetSlotBytes(WrenVM* vm, int slot, const char* bytes, size_t length); // Stores the numeric [value] in [slot]. void wrenSetSlotDouble(WrenVM* vm, int slot, double value); diff --git a/src/module/io.c b/src/module/io.c index 5ec141ab..3300ec40 100644 --- a/src/module/io.c +++ b/src/module/io.c @@ -18,7 +18,10 @@ typedef struct sFileRequestData static const int stdinDescriptor = 0; -// Handle to Stdin.onData_(). Called when libuv provides data on stdin. +// Handle to the Stdin class object. +static WrenValue* stdinClass = NULL; + +// Handle to an onData_() method call. Called when libuv provides data on stdin. static WrenValue* stdinOnData = NULL; // The stream used to read from stdin. Initialized on the first read. @@ -33,7 +36,13 @@ static void shutdownStdin() free(stdinStream); stdinStream = NULL; } - + + if (stdinClass != NULL) + { + wrenReleaseValue(getVM(), stdinClass); + stdinClass = NULL; + } + if (stdinOnData != NULL) { wrenReleaseValue(getVM(), stdinOnData); @@ -121,8 +130,9 @@ static void directoryListCallback(uv_fs_t* request) bufferLength += length + 1; } - WrenValue* fiber = freeRequest(request); - schedulerResumeBytes(fiber, buffer, bufferLength); + schedulerResume(freeRequest(request), true); + wrenSetSlotBytes(getVM(), 2, buffer, bufferLength); + schedulerFinishResume(); free(buffer); } @@ -161,8 +171,9 @@ static void fileOpenCallback(uv_fs_t* request) if (handleRequestError(request)) return; double fd = (double)request->result; - WrenValue* fiber = freeRequest(request); - schedulerResumeDouble(fiber, fd); + schedulerResume(freeRequest(request), true); + wrenSetSlotDouble(getVM(), 2, fd); + schedulerFinishResume(); } void fileOpen(WrenVM* vm) @@ -180,8 +191,9 @@ static void fileSizeCallback(uv_fs_t* request) if (handleRequestError(request)) return; double size = (double)request->statbuf.st_size; - WrenValue* fiber = freeRequest(request); - schedulerResumeDouble(fiber, size); + schedulerResume(freeRequest(request), true); + wrenSetSlotDouble(getVM(), 2, size); + schedulerFinishResume(); } void fileSizePath(WrenVM* vm) @@ -195,8 +207,7 @@ static void fileCloseCallback(uv_fs_t* request) { if (handleRequestError(request)) return; - WrenValue* fiber = freeRequest(request); - schedulerResume(fiber); + schedulerResume(freeRequest(request), false); } void fileClose(WrenVM* vm) @@ -233,12 +244,12 @@ static void fileReadBytesCallback(uv_fs_t* request) FileRequestData* data = (FileRequestData*)request->data; uv_buf_t buffer = data->buffer; - WrenValue* fiber = freeRequest(request); - // TODO: Having to copy the bytes here is a drag. It would be good if Wren's // embedding API supported a way to *give* it bytes that were previously // allocated using Wren's own allocator. - schedulerResumeBytes(fiber, buffer.base, buffer.len); + schedulerResume(freeRequest(request), true); + wrenSetSlotBytes(getVM(), 2, buffer.base, buffer.len); + schedulerFinishResume(); // TODO: Likewise, freeing this after we resume is lame. free(buffer.base); @@ -282,25 +293,41 @@ static void allocCallback(uv_handle_t* handle, size_t suggestedSize, static void stdinReadCallback(uv_stream_t* stream, ssize_t numRead, const uv_buf_t* buffer) { + WrenVM* vm = getVM(); + + if (stdinClass == NULL) + { + wrenEnsureSlots(vm, 1); + wrenGetVariable(vm, "io", "Stdin", 0); + stdinClass = wrenGetSlotValue(vm, 0); + } + + if (stdinOnData == NULL) + { + stdinOnData = wrenMakeCallHandle(vm, "onData_(_)"); + } + // If stdin was closed, send null to let io.wren know. if (numRead == UV_EOF) { - wrenCall(getVM(), stdinOnData, NULL, "v", NULL); + wrenEnsureSlots(vm, 2); + wrenSetSlotValue(vm, 0, stdinClass); + wrenSetSlotNull(vm, 1); + wrenCall(vm, stdinOnData); + shutdownStdin(); return; } // TODO: Handle other errors. - if (stdinOnData == NULL) - { - stdinOnData = wrenGetMethod(getVM(), "io", "Stdin", "onData_(_)"); - } - // TODO: Having to copy the bytes here is a drag. It would be good if Wren's // embedding API supported a way to *give* it bytes that were previously // allocated using Wren's own allocator. - wrenCall(getVM(), stdinOnData, NULL, "a", buffer->base, numRead); + wrenEnsureSlots(vm, 2); + wrenSetSlotValue(vm, 0, stdinClass); + wrenSetSlotBytes(vm, 1, buffer->base, numRead); + wrenCall(vm, stdinOnData); // TODO: Likewise, freeing this after we resume is lame. free(buffer->base); diff --git a/src/module/scheduler.c b/src/module/scheduler.c index 20347eb8..28f4989f 100644 --- a/src/module/scheduler.c +++ b/src/module/scheduler.c @@ -7,30 +7,19 @@ #include "wren.h" #include "vm.h" +// A handle to the "Scheduler" class object. Used to call static methods on it. +static WrenValue* schedulerClass; + // This method resumes a fiber that is suspended waiting on an asynchronous // operation. The first resumes it with zero arguments, and the second passes // one. -static WrenValue* resume; -static WrenValue* resumeWithArg; +static WrenValue* resume1; +static WrenValue* resume2; static WrenValue* resumeError; -void schedulerCaptureMethods(WrenVM* vm) +static void resume(WrenValue* method) { - resume = wrenGetMethod(vm, "scheduler", "Scheduler", "resume_(_)"); - resumeWithArg = wrenGetMethod(vm, "scheduler", "Scheduler", "resume_(_,_)"); - resumeError = wrenGetMethod(vm, "scheduler", "Scheduler", "resumeError_(_,_)"); -} - -static void callResume(WrenValue* resumeMethod, WrenValue* fiber, - const char* argTypes, ...) -{ - va_list args; - va_start(args, argTypes); - WrenInterpretResult result = wrenCallVarArgs(getVM(), resumeMethod, NULL, - argTypes, args); - va_end(args); - - wrenReleaseValue(getVM(), fiber); + WrenInterpretResult result = wrenCall(getVM(), method); // If a runtime error occurs in response to an async operation and nothing // catches the error in the fiber, then exit the CLI. @@ -41,34 +30,50 @@ static void callResume(WrenValue* resumeMethod, WrenValue* fiber, } } -void schedulerResume(WrenValue* fiber) +void schedulerCaptureMethods(WrenVM* vm) { - callResume(resume, fiber, "v", fiber); + wrenEnsureSlots(vm, 1); + wrenGetVariable(vm, "scheduler", "Scheduler", 0); + schedulerClass = wrenGetSlotValue(vm, 0); + + resume1 = wrenMakeCallHandle(vm, "resume_(_)"); + resume2 = wrenMakeCallHandle(vm, "resume_(_,_)"); + resumeError = wrenMakeCallHandle(vm, "resumeError_(_,_)"); } -void schedulerResumeBytes(WrenValue* fiber, const char* bytes, size_t length) +void schedulerResume(WrenValue* fiber, bool hasArgument) { - callResume(resumeWithArg, fiber, "va", fiber, bytes, length); + WrenVM* vm = getVM(); + wrenEnsureSlots(vm, 2 + (hasArgument ? 1 : 0)); + wrenSetSlotValue(vm, 0, schedulerClass); + wrenSetSlotValue(vm, 1, fiber); + wrenReleaseValue(vm, fiber); + + // If we don't need to wait for an argument to be stored on the stack, resume + // it now. + if (!hasArgument) resume(resume1); } -void schedulerResumeDouble(WrenValue* fiber, double value) +void schedulerFinishResume() { - callResume(resumeWithArg, fiber, "vd", fiber, value); -} - -void schedulerResumeString(WrenValue* fiber, const char* text) -{ - callResume(resumeWithArg, fiber, "vs", fiber, text); + resume(resume2); } void schedulerResumeError(WrenValue* fiber, const char* error) { - callResume(resumeError, fiber, "vs", fiber, error); + schedulerResume(fiber, true); + wrenSetSlotString(getVM(), 2, error); + resume(resumeError); } void schedulerShutdown() { - if (resume != NULL) wrenReleaseValue(getVM(), resume); - if (resumeWithArg != NULL) wrenReleaseValue(getVM(), resumeWithArg); - if (resumeError != NULL) wrenReleaseValue(getVM(), resumeError); + // If the module was never loaded, we don't have anything to release. + if (schedulerClass == NULL) return; + + WrenVM* vm = getVM(); + wrenReleaseValue(vm, schedulerClass); + wrenReleaseValue(vm, resume1); + wrenReleaseValue(vm, resume2); + wrenReleaseValue(vm, resumeError); } diff --git a/src/module/scheduler.h b/src/module/scheduler.h index e90ce1e4..943d5780 100644 --- a/src/module/scheduler.h +++ b/src/module/scheduler.h @@ -3,10 +3,16 @@ #include "wren.h" -void schedulerResume(WrenValue* fiber); -void schedulerResumeBytes(WrenValue* fiber, const char* bytes, size_t length); -void schedulerResumeDouble(WrenValue* fiber, double value); -void schedulerResumeString(WrenValue* fiber, const char* text); +// Sets up the API stack to call one of the resume methods on Scheduler. +// +// If [hasArgument] is false, this just sets up the stack to have another +// argument stored in slot 2 and returns. The module must store the argument +// on the stack and then call [schedulerFinishResume] to complete the call. +// +// Otherwise, the call resumes immediately. Releases [fiber] when called. +void schedulerResume(WrenValue* fiber, bool hasArgument); + +void schedulerFinishResume(); void schedulerResumeError(WrenValue* fiber, const char* error); void schedulerShutdown(); diff --git a/src/module/timer.c b/src/module/timer.c index a9d98eca..406635f0 100644 --- a/src/module/timer.c +++ b/src/module/timer.c @@ -22,7 +22,7 @@ static void timerCallback(uv_timer_t* handle) uv_close((uv_handle_t*)handle, timerCloseCallback); // Run the fiber that was sleeping. - schedulerResume(fiber); + schedulerResume(fiber, false); } void timerStartTimer(WrenVM* vm) diff --git a/src/vm/wren_core.c b/src/vm/wren_core.c index 79ef463a..a5b38dd3 100644 --- a/src/vm/wren_core.c +++ b/src/vm/wren_core.c @@ -150,6 +150,8 @@ DEF_PRIMITIVE(fiber_suspend) { // Switching to a null fiber tells the interpreter to stop and exit. vm->fiber = NULL; + vm->apiStack = NULL; + return false; } diff --git a/src/vm/wren_vm.c b/src/vm/wren_vm.c index eca8eca6..f281a62f 100644 --- a/src/vm/wren_vm.c +++ b/src/vm/wren_vm.c @@ -1104,18 +1104,23 @@ static WrenInterpretResult runInterpreter(WrenVM* vm, register ObjFiber* fiber) // If the fiber is complete, end it. if (fiber->numFrames == 0) { - // See if there's another fiber to return to. - ObjFiber* callingFiber = fiber->caller; + // See if there's another fiber to return to. If not, we're done. + if (fiber->caller == NULL) + { + // Store the final result value at the beginning of the stack so the + // C API can get it. + fiber->stack[0] = result; + fiber->stackTop = fiber->stack + 1; + return WREN_RESULT_SUCCESS; + } + + ObjFiber* resumingFiber = fiber->caller; fiber->caller = NULL; - - fiber = callingFiber; - vm->fiber = fiber; - - // If not, we're done. - if (fiber == NULL) return WREN_RESULT_SUCCESS; - + fiber = resumingFiber; + vm->fiber = resumingFiber; + // Store the result in the resuming fiber. - *(fiber->stackTop - 1) = result; + fiber->stackTop[-1] = result; } else { @@ -1127,7 +1132,7 @@ static WrenInterpretResult runInterpreter(WrenVM* vm, register ObjFiber* fiber) // result). fiber->stackTop = frame->stackStart + 1; } - + LOAD_FRAME(); DISPATCH(); } @@ -1255,11 +1260,10 @@ static WrenInterpretResult runInterpreter(WrenVM* vm, register ObjFiber* fiber) #undef READ_SHORT } -// Creates an [ObjFn] that invokes a method with [signature] when called. -static ObjFn* makeCallStub(WrenVM* vm, ObjModule* module, const char* signature) +WrenValue* wrenMakeCallHandle(WrenVM* vm, const char* signature) { int signatureLength = (int)strlen(signature); - + // Count the number parameters the method expects. int numParams = 0; if (signature[signatureLength - 1] == ')') @@ -1270,147 +1274,46 @@ static ObjFn* makeCallStub(WrenVM* vm, ObjModule* module, const char* signature) if (*s == '_') numParams++; } } - + + // Add the signatue to the method table. int method = wrenSymbolTableEnsure(vm, &vm->methodNames, signature, signatureLength); - + + // Create a little stub function that assumes the arguments are on the stack + // and calls the method. uint8_t* bytecode = ALLOCATE_ARRAY(vm, uint8_t, 5); bytecode[0] = (uint8_t)(CODE_CALL_0 + numParams); bytecode[1] = (method >> 8) & 0xff; bytecode[2] = method & 0xff; bytecode[3] = CODE_RETURN; bytecode[4] = CODE_END; - + int* debugLines = ALLOCATE_ARRAY(vm, int, 5); memset(debugLines, 1, 5); + + ObjFn* fn = wrenNewFunction(vm, NULL, NULL, 0, 0, numParams + 1, 0, bytecode, + 5, signature, signatureLength, debugLines); - return wrenNewFunction(vm, module, NULL, 0, 0, numParams + 1, 0, bytecode, 5, - signature, signatureLength, debugLines); + // Wrap the function in a handle. + return wrenCaptureValue(vm, OBJ_VAL(fn)); } -WrenValue* wrenGetMethod(WrenVM* vm, const char* module, const char* variable, - const char* signature) +WrenInterpretResult wrenCall(WrenVM* vm, WrenValue* method) { - Value moduleName = wrenStringFormat(vm, "$", module); - wrenPushRoot(vm, AS_OBJ(moduleName)); - - ObjModule* moduleObj = getModule(vm, moduleName); - // TODO: Handle module not being found. - - int variableSlot = wrenSymbolTableFind(&moduleObj->variableNames, - variable, strlen(variable)); - // TODO: Handle the variable not being found. - - ObjFn* fn = makeCallStub(vm, moduleObj, signature); - wrenPushRoot(vm, (Obj*)fn); - - // Create a single fiber that we can reuse each time the method is invoked. - ObjFiber* fiber = wrenNewFiber(vm, (Obj*)fn); - wrenPushRoot(vm, (Obj*)fiber); - - // Create a handle that keeps track of the function that calls the method. - 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]; - - wrenPopRoot(vm); // fiber. - wrenPopRoot(vm); // fn. - wrenPopRoot(vm); // moduleName. - - return method; -} - -WrenInterpretResult wrenCall(WrenVM* vm, WrenValue* method, - WrenValue** returnValue, - const char* argTypes, ...) -{ - va_list args; - va_start(args, argTypes); - WrenInterpretResult result = wrenCallVarArgs(vm, method, returnValue, - argTypes, args); - va_end(args); - - return result; -} - -WrenInterpretResult wrenCallVarArgs(WrenVM* vm, WrenValue* method, - WrenValue** returnValue, - const char* argTypes, va_list args) -{ - // 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. - for (const char* argType = argTypes; *argType != '\0'; argType++) - { - Value value = NULL_VAL; - switch (*argType) - { - case 'a': - { - const char* bytes = va_arg(args, const char*); - int length = va_arg(args, int); - value = wrenNewString(vm, bytes, (size_t)length); - break; - } - - case 'b': value = BOOL_VAL(va_arg(args, int)); break; - case 'd': value = NUM_VAL(va_arg(args, double)); break; - case 'i': value = NUM_VAL((double)va_arg(args, int)); break; - case 'n': value = NULL_VAL; va_arg(args, void*); break; - case 's': - value = wrenStringFormat(vm, "$", va_arg(args, const char*)); - break; - - case 'v': - { - // Allow a NULL value pointer for Wren null. - WrenValue* wrenValue = va_arg(args, WrenValue*); - if (wrenValue != NULL) value = wrenValue->value; - break; - } - - default: - ASSERT(false, "Unknown argument type."); - break; - } - - *fiber->stackTop++ = value; - } - - Value receiver = fiber->stack[0]; - Obj* fn = fiber->frames[0].fn; - wrenPushRoot(vm, (Obj*)fn); - - WrenInterpretResult result = runInterpreter(vm, fiber); - - if (result == WREN_RESULT_SUCCESS) - { - if (returnValue != NULL) - { - // Make sure the return value doesn't get collected while capturing it. - fiber->stackTop++; - *returnValue = wrenCaptureValue(vm, fiber->stack[0]); - } - - // Reset the fiber to get ready for the next call. - wrenResetFiber(vm, fiber, fn); - - // Push the receiver back on the stack. - *fiber->stackTop++ = receiver; - } - else - { - if (returnValue != NULL) *returnValue = NULL; - } - - wrenPopRoot(vm); - - return result; + ASSERT(method != NULL, "Method cannot be NULL."); + ASSERT(IS_FN(method->value), "Method must be a method handle."); + ASSERT(vm->fiber != NULL, "Must set up arguments for call first."); + ASSERT(vm->apiStack != NULL, "Must set up arguments for call first."); + ASSERT(vm->fiber->numFrames == 0, "Can not call from a foreign method."); + + ObjFn* fn = AS_FN(method->value); + + ASSERT(vm->fiber->stackTop - vm->fiber->stack >= fn->arity, + "Stack must have enough arguments for method."); + + callFunction(vm, vm->fiber, (Obj*)fn, fn->arity); + + return runInterpreter(vm, vm->fiber); } WrenValue* wrenCaptureValue(WrenVM* vm, Value value) @@ -1428,14 +1331,6 @@ WrenValue* wrenCaptureValue(WrenVM* vm, Value value) return wrappedValue; } -double wrenGetValueDouble(WrenVM* vm, WrenValue* value) -{ - ASSERT(value != NULL, "Value cannot be NULL."); - ASSERT(IS_NUM(value->value), "Value must be a number."); - - return AS_NUM(value->value); -} - void wrenReleaseValue(WrenVM* vm, WrenValue* value) { ASSERT(value != NULL, "Value cannot be NULL."); @@ -1601,7 +1496,7 @@ int wrenGetSlotCount(WrenVM* vm) void wrenEnsureSlots(WrenVM* vm, int numSlots) { // If we don't have a fiber accessible, create one for the API to use. - if (vm->fiber == NULL && vm->apiStack == NULL) + if (vm->apiStack == NULL) { vm->fiber = wrenNewFiber(vm, NULL); vm->apiStack = vm->fiber->stack; @@ -1685,10 +1580,10 @@ void wrenSetSlotBool(WrenVM* vm, int slot, bool value) setSlot(vm, slot, BOOL_VAL(value)); } -void wrenSetSlotBytes(WrenVM* vm, int slot, const char* bytes, int length) +void wrenSetSlotBytes(WrenVM* vm, int slot, const char* bytes, size_t length) { ASSERT(bytes != NULL, "Byte array cannot be NULL."); - setSlot(vm, slot, wrenNewString(vm, bytes, (size_t)length)); + setSlot(vm, slot, wrenNewString(vm, bytes, length)); } void wrenSetSlotDouble(WrenVM* vm, int slot, double value) @@ -1735,6 +1630,8 @@ void wrenInsertInList(WrenVM* vm, int listSlot, int index, int elementSlot) wrenListInsert(vm, list, vm->apiStack[elementSlot], index); } +// TODO: Maybe just have this always return a WrenValue* instead of having to +// deal with slots? void wrenGetVariable(WrenVM* vm, const char* module, const char* name, int slot) { diff --git a/test/api/benchmark.c b/test/api/benchmark.c index 26d8daad..672e97ba 100644 --- a/test/api/benchmark.c +++ b/test/api/benchmark.c @@ -32,21 +32,32 @@ static void call(WrenVM* vm) wrenInterpret(otherVM, testScript); - WrenValue* method = wrenGetMethod(otherVM, "main", "Test", "method(_,_,_,_)"); + WrenValue* method = wrenMakeCallHandle(otherVM, "method(_,_,_,_)"); + + wrenEnsureSlots(otherVM, 1); + wrenGetVariable(otherVM, "main", "Test", 0); + WrenValue* testClass = wrenGetSlotValue(otherVM, 0); double startTime = (double)clock() / CLOCKS_PER_SEC; double result = 0; for (int i = 0; i < iterations; i++) { - WrenValue* resultValue; - wrenCall(otherVM, method, &resultValue, "dddd", 1.0, 2.0, 3.0, 4.0); - result += wrenGetValueDouble(otherVM, resultValue); - wrenReleaseValue(otherVM, resultValue); + wrenEnsureSlots(otherVM, 5); + wrenSetSlotValue(otherVM, 0, testClass); + wrenSetSlotDouble(otherVM, 1, 1.0); + wrenSetSlotDouble(otherVM, 2, 2.0); + wrenSetSlotDouble(otherVM, 3, 3.0); + wrenSetSlotDouble(otherVM, 4, 4.0); + + wrenCall(otherVM, method); + + result += wrenGetSlotDouble(otherVM, 0); } double elapsed = (double)clock() / CLOCKS_PER_SEC - startTime; + wrenReleaseValue(otherVM, testClass); wrenReleaseValue(otherVM, method); wrenFreeVM(otherVM); diff --git a/test/api/call.c b/test/api/call.c index 5207a456..f7d8af8a 100644 --- a/test/api/call.c +++ b/test/api/call.c @@ -5,33 +5,75 @@ void callRunTests(WrenVM* vm) { - WrenValue* noParams = wrenGetMethod(vm, "main", "Call", "noParams"); - WrenValue* zero = wrenGetMethod(vm, "main", "Call", "zero()"); - WrenValue* one = wrenGetMethod(vm, "main", "Call", "one(_)"); - WrenValue* two = wrenGetMethod(vm, "main", "Call", "two(_,_)"); + wrenEnsureSlots(vm, 1); + wrenGetVariable(vm, "main", "Call", 0); + WrenValue* callClass = wrenGetSlotValue(vm, 0); + + WrenValue* noParams = wrenMakeCallHandle(vm, "noParams"); + WrenValue* zero = wrenMakeCallHandle(vm, "zero()"); + WrenValue* one = wrenMakeCallHandle(vm, "one(_)"); + WrenValue* two = wrenMakeCallHandle(vm, "two(_,_)"); // Different arity. - wrenCall(vm, noParams, NULL, ""); - wrenCall(vm, zero, NULL, ""); - wrenCall(vm, one, NULL, "i", 1); - wrenCall(vm, two, NULL, "ii", 1, 2); - - WrenValue* getValue = wrenGetMethod(vm, "main", "Call", "getValue(_)"); + wrenEnsureSlots(vm, 1); + wrenSetSlotValue(vm, 0, callClass); + wrenCall(vm, noParams); + + wrenEnsureSlots(vm, 1); + wrenSetSlotValue(vm, 0, callClass); + wrenCall(vm, zero); + + wrenEnsureSlots(vm, 2); + wrenSetSlotValue(vm, 0, callClass); + wrenSetSlotDouble(vm, 1, 1.0); + wrenCall(vm, one); + + wrenEnsureSlots(vm, 3); + wrenSetSlotValue(vm, 0, callClass); + wrenSetSlotDouble(vm, 1, 1.0); + wrenSetSlotDouble(vm, 2, 2.0); + wrenCall(vm, two); // Returning a value. - WrenValue* value = NULL; - wrenCall(vm, getValue, &value, "v", NULL); + WrenValue* getValue = wrenMakeCallHandle(vm, "getValue()"); + wrenEnsureSlots(vm, 1); + wrenSetSlotValue(vm, 0, callClass); + wrenCall(vm, getValue); + WrenValue* value = wrenGetSlotValue(vm, 0); // Different argument types. - wrenCall(vm, two, NULL, "bb", true, false); - wrenCall(vm, two, NULL, "dd", 1.2, 3.4); - wrenCall(vm, two, NULL, "ii", 3, 4); - wrenCall(vm, two, NULL, "ss", "string", "another"); - wrenCall(vm, two, NULL, "vv", NULL, value); + wrenEnsureSlots(vm, 3); + wrenSetSlotValue(vm, 0, callClass); + wrenSetSlotBool(vm, 1, true); + wrenSetSlotBool(vm, 2, false); + wrenCall(vm, two); + + wrenEnsureSlots(vm, 3); + wrenSetSlotValue(vm, 0, callClass); + wrenSetSlotDouble(vm, 1, 1.2); + wrenSetSlotDouble(vm, 2, 3.4); + wrenCall(vm, two); + + wrenEnsureSlots(vm, 3); + wrenSetSlotValue(vm, 0, callClass); + wrenSetSlotString(vm, 1, "string"); + wrenSetSlotString(vm, 2, "another"); + wrenCall(vm, two); + + wrenEnsureSlots(vm, 3); + wrenSetSlotValue(vm, 0, callClass); + wrenSetSlotNull(vm, 1); + wrenSetSlotValue(vm, 2, value); + wrenCall(vm, two); // Truncate a string, or allow null bytes. - wrenCall(vm, two, NULL, "aa", "string", 3, "b\0y\0t\0e", 7); + wrenEnsureSlots(vm, 3); + wrenSetSlotValue(vm, 0, callClass); + wrenSetSlotBytes(vm, 1, "string", 3); + wrenSetSlotBytes(vm, 2, "b\0y\0t\0e", 7); + wrenCall(vm, two); + wrenReleaseValue(vm, callClass); wrenReleaseValue(vm, noParams); wrenReleaseValue(vm, zero); wrenReleaseValue(vm, one); diff --git a/test/api/call.wren b/test/api/call.wren index 35feaa5e..889a7936 100644 --- a/test/api/call.wren +++ b/test/api/call.wren @@ -20,13 +20,7 @@ class Call { System.print("two %(one) %(two)") } - static getValue(value) { - // Return a new value if we aren't given one. - if (value == null) return ["a", "b"] - - // Otherwise print it. - System.print(value) - } + static getValue() { ["a", "b"] } } // expect: noParams @@ -36,7 +30,6 @@ class Call { // expect: two true false // expect: two 1.2 3.4 -// expect: two 3 4 // expect: two string another // expect: two null [a, b] // expect: two str [98, 0, 121, 0, 116, 0, 101] From 2c31985e541721c3924a5552ccd79e7e04f8128b Mon Sep 17 00:00:00 2001 From: Bob Nystrom Date: Tue, 29 Dec 2015 08:37:47 -0800 Subject: [PATCH 19/26] Make Directory.list() create and return a list from C. --- src/module/io.c | 25 +++++-------------------- src/module/io.wren | 23 +---------------------- src/module/io.wren.inc | 23 +---------------------- src/vm/wren_debug.c | 3 +++ src/vm/wren_vm.c | 11 +++++++++-- test/api/call.c | 9 +++++++++ test/api/call.wren | 1 + 7 files changed, 29 insertions(+), 66 deletions(-) diff --git a/src/module/io.c b/src/module/io.c index 3300ec40..2355d14c 100644 --- a/src/module/io.c +++ b/src/module/io.c @@ -107,33 +107,18 @@ static void directoryListCallback(uv_fs_t* request) uv_dirent_t entry; - // TODO: Right now, there's no way to create a list using the C API, so - // create a buffer containing all of the result paths terminated by '\0'. - // We'll split that back into a list in Wren. - size_t bufferLength = 0; - size_t bufferCapacity = 1024; - char* buffer = (char*)malloc(bufferCapacity); + WrenVM* vm = getVM(); + wrenEnsureSlots(vm, 3); + wrenSetSlotNewList(vm, 2); while (uv_fs_scandir_next(request, &entry) != UV_EOF) { - size_t length = strlen(entry.name); - - // Grow the buffer if needed. - while (bufferLength + length + 1 > bufferCapacity) - { - bufferCapacity *= 2; - buffer = (char*)realloc(buffer, bufferCapacity); - } - - // Copy the path, including the null terminator. - memcpy(buffer + bufferLength, entry.name, length + 1); - bufferLength += length + 1; + wrenSetSlotString(vm, 1, entry.name); + wrenInsertInList(vm, 2, -1, 1); } schedulerResume(freeRequest(request), true); - wrenSetSlotBytes(getVM(), 2, buffer, bufferLength); schedulerFinishResume(); - free(buffer); } void directoryList(WrenVM* vm) diff --git a/src/module/io.wren b/src/module/io.wren index d641743a..9694d2f9 100644 --- a/src/module/io.wren +++ b/src/module/io.wren @@ -5,28 +5,7 @@ class Directory { if (!(path is String)) Fiber.abort("Path must be a string.") list_(path, Fiber.current) - - // We get back a byte array containing all of the paths, each terminated by - // a zero byte. - var entryBuffer = Scheduler.runNextScheduled_() - - // TODO: Add split() to String. - // Split it into an array of strings. - var entries = [] - var start = 0 - var i = 0 - var bytes = entryBuffer.bytes - while (i < bytes.count) { - if (bytes[i] == 0) { - var entry = entryBuffer[start...i] - start = i + 1 - entries.add(entry) - } - - i = i + 1 - } - - return entries + return Scheduler.runNextScheduled_() } foreign static list_(path, fiber) diff --git a/src/module/io.wren.inc b/src/module/io.wren.inc index 32b62086..770657c9 100644 --- a/src/module/io.wren.inc +++ b/src/module/io.wren.inc @@ -7,28 +7,7 @@ static const char* ioModuleSource = " if (!(path is String)) Fiber.abort(\"Path must be a string.\")\n" "\n" " list_(path, Fiber.current)\n" -"\n" -" // We get back a byte array containing all of the paths, each terminated by\n" -" // a zero byte.\n" -" var entryBuffer = Scheduler.runNextScheduled_()\n" -"\n" -" // TODO: Add split() to String.\n" -" // Split it into an array of strings.\n" -" var entries = []\n" -" var start = 0\n" -" var i = 0\n" -" var bytes = entryBuffer.bytes\n" -" while (i < bytes.count) {\n" -" if (bytes[i] == 0) {\n" -" var entry = entryBuffer[start...i]\n" -" start = i + 1\n" -" entries.add(entry)\n" -" }\n" -"\n" -" i = i + 1\n" -" }\n" -"\n" -" return entries\n" +" return Scheduler.runNextScheduled_()\n" " }\n" "\n" " foreign static list_(path, fiber)\n" diff --git a/src/vm/wren_debug.c b/src/vm/wren_debug.c index 572c826d..2d5d7063 100644 --- a/src/vm/wren_debug.c +++ b/src/vm/wren_debug.c @@ -20,6 +20,9 @@ void wrenDebugPrintStackTrace(ObjFiber* fiber) CallFrame* frame = &fiber->frames[i]; ObjFn* fn = wrenUpwrapClosure(frame->fn); + // Skip over stub functions for calling methods from the C API. + if (fn->module == NULL) continue; + // The built-in core module has no name. We explicitly omit it from stack // traces since we don't want to highlight to a user the implementation // detail of what part of the core module is written in C and what is Wren. diff --git a/src/vm/wren_vm.c b/src/vm/wren_vm.c index f281a62f..2d6f0029 100644 --- a/src/vm/wren_vm.c +++ b/src/vm/wren_vm.c @@ -1311,23 +1311,30 @@ WrenInterpretResult wrenCall(WrenVM* vm, WrenValue* method) ASSERT(vm->fiber->stackTop - vm->fiber->stack >= fn->arity, "Stack must have enough arguments for method."); - callFunction(vm, vm->fiber, (Obj*)fn, fn->arity); + // Discard any extra temporary slots. We take for granted that the stub + // function has exactly one slot for each arguments. + vm->fiber->stackTop = &vm->fiber->stack[fn->maxSlots]; + callFunction(vm, vm->fiber, (Obj*)fn, 0); return runInterpreter(vm, vm->fiber); } WrenValue* wrenCaptureValue(WrenVM* vm, Value value) { + if (IS_OBJ(value)) wrenPushRoot(vm, AS_OBJ(value)); + // Make a handle for it. WrenValue* wrappedValue = ALLOCATE(vm, WrenValue); wrappedValue->value = value; + if (IS_OBJ(value)) wrenPopRoot(vm); + // 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; - + return wrappedValue; } diff --git a/test/api/call.c b/test/api/call.c index f7d8af8a..d119dde9 100644 --- a/test/api/call.c +++ b/test/api/call.c @@ -73,6 +73,15 @@ void callRunTests(WrenVM* vm) wrenSetSlotBytes(vm, 2, "b\0y\0t\0e", 7); wrenCall(vm, two); + // Call ignores with extra temporary slots on stack. + wrenEnsureSlots(vm, 10); + wrenSetSlotValue(vm, 0, callClass); + for (int i = 1; i < 10; i++) + { + wrenSetSlotDouble(vm, i, i * 0.1); + } + wrenCall(vm, one); + wrenReleaseValue(vm, callClass); wrenReleaseValue(vm, noParams); wrenReleaseValue(vm, zero); diff --git a/test/api/call.wren b/test/api/call.wren index 889a7936..afe8e9e3 100644 --- a/test/api/call.wren +++ b/test/api/call.wren @@ -33,3 +33,4 @@ class Call { // expect: two string another // expect: two null [a, b] // expect: two str [98, 0, 121, 0, 116, 0, 101] +// expect: one 0.1 From 678251a00c674acb471bee13d4fae5e0b4e57616 Mon Sep 17 00:00:00 2001 From: Bob Nystrom Date: Tue, 29 Dec 2015 21:07:26 -0800 Subject: [PATCH 20/26] Fix type for fileFinalizer(). --- src/cli/modules.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/cli/modules.c b/src/cli/modules.c index 581f5f2d..4e6e8e3f 100644 --- a/src/cli/modules.c +++ b/src/cli/modules.c @@ -9,7 +9,7 @@ extern void directoryList(WrenVM* vm); extern void fileAllocate(WrenVM* vm); -extern void fileFinalize(WrenVM* vm); +extern void fileFinalize(void* data); extern void fileOpen(WrenVM* vm); extern void fileSizePath(WrenVM* vm); extern void fileClose(WrenVM* vm); @@ -83,6 +83,7 @@ typedef struct #define METHOD(signature, fn) { false, signature, fn }, #define STATIC_METHOD(signature, fn) { true, signature, fn }, +#define FINALIZER(fn) { true, "", (WrenForeignMethodFn)fn }, // The array of built-in modules. static ModuleRegistry modules[] = @@ -93,7 +94,7 @@ static ModuleRegistry modules[] = END_CLASS CLASS(File) STATIC_METHOD("", fileAllocate) - STATIC_METHOD("", fileFinalize) + FINALIZER(fileFinalize) STATIC_METHOD("open_(_,_)", fileOpen) STATIC_METHOD("sizePath_(_,_)", fileSizePath) METHOD("close_(_)", fileClose) From 8854dfb07e2b42a86ec442de0736fb06421eeb4f Mon Sep 17 00:00:00 2001 From: Bob Nystrom Date: Tue, 29 Dec 2015 21:56:44 -0800 Subject: [PATCH 21/26] Docs for the io module! --- doc/site/modules/io/directory.markdown | 10 ++++ doc/site/modules/io/file.markdown | 63 +++++++++++++++++++++++++- doc/site/modules/io/index.markdown | 3 +- doc/site/modules/io/stdin.markdown | 11 +++-- doc/site/modules/io/template.html | 1 + src/module/io.wren | 3 +- 6 files changed, 84 insertions(+), 7 deletions(-) create mode 100644 doc/site/modules/io/directory.markdown diff --git a/doc/site/modules/io/directory.markdown b/doc/site/modules/io/directory.markdown new file mode 100644 index 00000000..0e41a33b --- /dev/null +++ b/doc/site/modules/io/directory.markdown @@ -0,0 +1,10 @@ +^title Directory Class + +A directory on the file system. + +## Static Methods + +### Directory.**list**(path) + +Lists the contents of the directory at `path`. Returns a sorted list of path +strings for all of the contents of the directory. diff --git a/doc/site/modules/io/file.markdown b/doc/site/modules/io/file.markdown index b9b690cd..60a23f63 100644 --- a/doc/site/modules/io/file.markdown +++ b/doc/site/modules/io/file.markdown @@ -1,7 +1,66 @@ ^title File Class -**TODO** +Lets you work with files on the file system. An instance of this class +represents an open file with a file descriptor. + +When you are done with a file object, it's a good idea to explicitly close it. +If you don't, the GC will close it when the file is no longer used and gets +finalized, but that may take a while. In the meantime, leaving it open wastes +a file descriptor. + +## Static Methods + +### File.**open**(path, fn) + +Opens the file at [path] and passes it to [fn]. After the function returns, the +file is automatically closed. + + :::wren + File.open("words.txt") {|file| + file.readBytes(5) + } + +### File.**read**(path) + +Reads the entire contents of the file at [path] and returns it as a string. + + :::wren + File.read("words.txt") + +The encoding or decoding is done. If the file is UTF-8, then the resulting +string will be a UTF-8 string. Otherwise, it will be a string of bytes in +whatever encoding the file uses. + +### File.**size**(path) + +Returns the size in bytes of the contents of the file at [path]. + +## Constructors + +### File.**open**(path) + +Opens the file at [path] for reading. ## Methods -**TODO** +### **descriptor** + +The numeric file descriptor used to access the file. + +### **isOpen** + +Whether the file is still open or has been closed. + +### **size** + +The size of the contents of the file in bytes. + +### **close**() + +Closes the file. After calling this, you can read or write from it. + +### **readBytes**(count) + +Reads up to [count] bytes starting from the beginning of the file. + +(Allowing an offset to read elsewhere from the file isn't implemented yet.) diff --git a/doc/site/modules/io/index.markdown b/doc/site/modules/io/index.markdown index ba41efbd..1dc81f49 100644 --- a/doc/site/modules/io/index.markdown +++ b/doc/site/modules/io/index.markdown @@ -1,6 +1,7 @@ ^title Module "io" -**TODO** +Provides access to operating system streams and the file system. +* [Directory](directory.html) * [File](file.html) * [Stdin](stdin.html) diff --git a/doc/site/modules/io/stdin.markdown b/doc/site/modules/io/stdin.markdown index 399a9af5..66377f85 100644 --- a/doc/site/modules/io/stdin.markdown +++ b/doc/site/modules/io/stdin.markdown @@ -1,7 +1,12 @@ ^title Stdin Class -**TODO** +The standard input stream. -## Methods +## Static Methods -**TODO** +### **readLine**() + +Reads one line of input from stdin. Blocks the current fiber until a full line +of input has been received. + +Returns the string of input or `null` if stdin is closed. diff --git a/doc/site/modules/io/template.html b/doc/site/modules/io/template.html index ab36a796..ebce39a1 100644 --- a/doc/site/modules/io/template.html +++ b/doc/site/modules/io/template.html @@ -27,6 +27,7 @@

io classes

diff --git a/src/module/io.wren b/src/module/io.wren index 9694d2f9..a4135a15 100644 --- a/src/module/io.wren +++ b/src/module/io.wren @@ -51,6 +51,8 @@ foreign class File { Scheduler.runNextScheduled_() } + foreign descriptor + isOpen { descriptor != -1 } size { @@ -74,7 +76,6 @@ foreign class File { foreign static sizePath_(path, fiber) foreign close_(fiber) - foreign descriptor foreign readBytes_(count, fiber) foreign size_(fiber) } From 452cc88082c4ba0711c754bb778efe479cd97a92 Mon Sep 17 00:00:00 2001 From: Bob Nystrom Date: Tue, 29 Dec 2015 22:29:13 -0800 Subject: [PATCH 22/26] Fix a few typos in the docs. --- doc/site/method-calls.markdown | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/doc/site/method-calls.markdown b/doc/site/method-calls.markdown index f13305b2..d9284b94 100644 --- a/doc/site/method-calls.markdown +++ b/doc/site/method-calls.markdown @@ -61,8 +61,8 @@ any "if I got two arguments do this..." logic. ## Getters -Some methods exist to expose some stored or computed property of an object. -These are *getters* and have no parentheses: +Some methods exist to expose a stored or computed property of an object. These +are *getters* and have no parentheses: :::wren "string".count //> 6 @@ -70,8 +70,9 @@ These are *getters* and have no parentheses: 1.23.sin //> 0.9424888019317 [1, 2, 3].isEmpty //> false -Sometimes you have a method doesn't need any parameters, but modifies the object -or has some other side effect. For those, it's better to use empty parentheses: +Sometimes you have a method that doesn't need any parameters, but modifies the +object or has some other side effect. For those, it's better to use empty +parentheses: :::wren list.clear() @@ -89,7 +90,7 @@ like a getter and a `()` method like a `()` method. These don't work: :::wren "string".count() - list.clear + [1, 2, 3].clear ## Setters @@ -104,7 +105,7 @@ language's perspective, the above line is just a call to the `height=(_)` method, passing in `74`. Since the `=(_)` is in the setter's signature, an object can have both a getter -and setter with the same name without any collision. This way, you can have +and setter with the same name without a collision. This way, you can have read/write properties. ## Operators From 6e2ec92e0d51cfa97326ace022c996c9282599c7 Mon Sep 17 00:00:00 2001 From: Bob Nystrom Date: Tue, 29 Dec 2015 22:35:14 -0800 Subject: [PATCH 23/26] Fix parameter format in docs. --- doc/site/modules/core/system.markdown | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/site/modules/core/system.markdown b/doc/site/modules/core/system.markdown index 1104c34d..ac091193 100644 --- a/doc/site/modules/core/system.markdown +++ b/doc/site/modules/core/system.markdown @@ -21,7 +21,7 @@ Prints a single newline to the console. ### System.**print**(object) -Prints [object] to the console followed by a newline. If not already a string, +Prints `object` to the console followed by a newline. If not already a string, the object is converted to a string by calling `toString` on it. :::wren @@ -29,7 +29,7 @@ the object is converted to a string by calling `toString` on it. ### System.**printAll**(sequence) -Iterates over [sequence] and prints each element, then prints a single newline +Iterates over `sequence` and prints each element, then prints a single newline at the end. Each element is converted to a string by calling `toString` on it. :::wren From b054526df818b84e8d6abcb337155295a84eaea0 Mon Sep 17 00:00:00 2001 From: Bob Nystrom Date: Wed, 30 Dec 2015 08:13:19 -0800 Subject: [PATCH 24/26] Add an optional offset to File.readBytes(). --- doc/site/modules/io/file.markdown | 27 ++++++++++++++----- src/cli/modules.c | 2 +- src/module/io.c | 10 ++++--- src/module/io.wren | 12 ++++++--- src/module/io.wren.inc | 15 ++++++++--- test/io/file/read_bytes.wren | 6 +++++ ...ve.wren => read_bytes_count_negative.wren} | 0 ...wren => read_bytes_count_not_integer.wren} | 0 test/io/file/read_bytes_from.wren | 20 ++++++++++++++ test/io/file/read_bytes_from_after_close.wren | 6 +++++ .../file/read_bytes_from_count_negative.wren | 4 +++ .../read_bytes_from_count_not_integer.wren | 4 +++ .../file/read_bytes_from_count_not_num.wren | 4 +++ .../file/read_bytes_from_offset_negative.wren | 4 +++ .../read_bytes_from_offset_not_integer.wren | 4 +++ .../file/read_bytes_from_offset_not_num.wren | 4 +++ 16 files changed, 104 insertions(+), 18 deletions(-) rename test/io/file/{read_bytes_negative.wren => read_bytes_count_negative.wren} (100%) rename test/io/file/{read_bytes_not_integer.wren => read_bytes_count_not_integer.wren} (100%) create mode 100644 test/io/file/read_bytes_from.wren create mode 100644 test/io/file/read_bytes_from_after_close.wren create mode 100644 test/io/file/read_bytes_from_count_negative.wren create mode 100644 test/io/file/read_bytes_from_count_not_integer.wren create mode 100644 test/io/file/read_bytes_from_count_not_num.wren create mode 100644 test/io/file/read_bytes_from_offset_negative.wren create mode 100644 test/io/file/read_bytes_from_offset_not_integer.wren create mode 100644 test/io/file/read_bytes_from_offset_not_num.wren diff --git a/doc/site/modules/io/file.markdown b/doc/site/modules/io/file.markdown index 60a23f63..9cd717fb 100644 --- a/doc/site/modules/io/file.markdown +++ b/doc/site/modules/io/file.markdown @@ -12,7 +12,7 @@ a file descriptor. ### File.**open**(path, fn) -Opens the file at [path] and passes it to [fn]. After the function returns, the +Opens the file at `path` and passes it to `fn`. After the function returns, the file is automatically closed. :::wren @@ -22,7 +22,7 @@ file is automatically closed. ### File.**read**(path) -Reads the entire contents of the file at [path] and returns it as a string. +Reads the entire contents of the file at `path` and returns it as a string. :::wren File.read("words.txt") @@ -33,13 +33,13 @@ whatever encoding the file uses. ### File.**size**(path) -Returns the size in bytes of the contents of the file at [path]. +Returns the size in bytes of the contents of the file at `path`. ## Constructors ### File.**open**(path) -Opens the file at [path] for reading. +Opens the file at `path` for reading. ## Methods @@ -61,6 +61,21 @@ Closes the file. After calling this, you can read or write from it. ### **readBytes**(count) -Reads up to [count] bytes starting from the beginning of the file. +Reads up to `count` bytes starting from the beginning of the file. -(Allowing an offset to read elsewhere from the file isn't implemented yet.) + :::wren + // Assume this file contains "I am a file!". + File.open("example.txt") {|file| + System.print(file.readBytes(6)) //> I am a + } + +### **readBytes**(count, offset) + +Reads up to `count` bytes starting at `offset` bytes from the beginning of +the file. + + :::wren + // Assume this file contains "I am a file!". + File.open("example.txt") {|file| + System.print(file.readBytes(6, 2)) //> am a f + } diff --git a/src/cli/modules.c b/src/cli/modules.c index 4e6e8e3f..cf4e8075 100644 --- a/src/cli/modules.c +++ b/src/cli/modules.c @@ -99,7 +99,7 @@ static ModuleRegistry modules[] = STATIC_METHOD("sizePath_(_,_)", fileSizePath) METHOD("close_(_)", fileClose) METHOD("descriptor", fileDescriptor) - METHOD("readBytes_(_,_)", fileReadBytes) + METHOD("readBytes_(_,_,_)", fileReadBytes) METHOD("size_(_)", fileSize) END_CLASS CLASS(Stdin) diff --git a/src/module/io.c b/src/module/io.c index 2355d14c..6b6c7c98 100644 --- a/src/module/io.c +++ b/src/module/io.c @@ -228,12 +228,13 @@ static void fileReadBytesCallback(uv_fs_t* request) FileRequestData* data = (FileRequestData*)request->data; uv_buf_t buffer = data->buffer; + size_t count = request->result; // TODO: Having to copy the bytes here is a drag. It would be good if Wren's // embedding API supported a way to *give* it bytes that were previously // allocated using Wren's own allocator. schedulerResume(freeRequest(request), true); - wrenSetSlotBytes(getVM(), 2, buffer.base, buffer.len); + wrenSetSlotBytes(getVM(), 2, buffer.base, count); schedulerFinishResume(); // TODO: Likewise, freeing this after we resume is lame. @@ -242,19 +243,20 @@ static void fileReadBytesCallback(uv_fs_t* request) void fileReadBytes(WrenVM* vm) { - uv_fs_t* request = createRequest(wrenGetSlotValue(vm, 2)); + uv_fs_t* request = createRequest(wrenGetSlotValue(vm, 3)); int fd = *(int*)wrenGetSlotForeign(vm, 0); // TODO: Assert fd != -1. FileRequestData* data = (FileRequestData*)request->data; size_t length = (size_t)wrenGetSlotDouble(vm, 1); + size_t offset = (size_t)wrenGetSlotDouble(vm, 2); data->buffer.len = length; data->buffer.base = (char*)malloc(length); - // TODO: Allow passing in offset. - uv_fs_read(getLoop(), request, fd, &data->buffer, 1, 0, fileReadBytesCallback); + uv_fs_read(getLoop(), request, fd, &data->buffer, 1, offset, + fileReadBytesCallback); } void fileSize(WrenVM* vm) diff --git a/src/module/io.wren b/src/module/io.wren index a4135a15..ea707624 100644 --- a/src/module/io.wren +++ b/src/module/io.wren @@ -62,13 +62,19 @@ foreign class File { return Scheduler.runNextScheduled_() } - readBytes(count) { + readBytes(count) { readBytes(count, 0) } + + readBytes(count, offset) { if (!isOpen) Fiber.abort("File is not open.") if (!(count is Num)) Fiber.abort("Count must be an integer.") if (!count.isInteger) Fiber.abort("Count must be an integer.") if (count < 0) Fiber.abort("Count cannot be negative.") - readBytes_(count, Fiber.current) + if (!(offset is Num)) Fiber.abort("Offset must be an integer.") + if (!offset.isInteger) Fiber.abort("Offset must be an integer.") + if (offset < 0) Fiber.abort("Offset cannot be negative.") + + readBytes_(count, offset, Fiber.current) return Scheduler.runNextScheduled_() } @@ -76,7 +82,7 @@ foreign class File { foreign static sizePath_(path, fiber) foreign close_(fiber) - foreign readBytes_(count, fiber) + foreign readBytes_(count, start, fiber) foreign size_(fiber) } diff --git a/src/module/io.wren.inc b/src/module/io.wren.inc index 770657c9..7ba842d5 100644 --- a/src/module/io.wren.inc +++ b/src/module/io.wren.inc @@ -53,6 +53,8 @@ static const char* ioModuleSource = " Scheduler.runNextScheduled_()\n" " }\n" "\n" +" foreign descriptor\n" +"\n" " isOpen { descriptor != -1 }\n" "\n" " size {\n" @@ -62,13 +64,19 @@ static const char* ioModuleSource = " return Scheduler.runNextScheduled_()\n" " }\n" "\n" -" readBytes(count) {\n" +" readBytes(count) { readBytes(count, 0) }\n" +"\n" +" readBytes(count, offset) {\n" " if (!isOpen) Fiber.abort(\"File is not open.\")\n" " if (!(count is Num)) Fiber.abort(\"Count must be an integer.\")\n" " if (!count.isInteger) Fiber.abort(\"Count must be an integer.\")\n" " if (count < 0) Fiber.abort(\"Count cannot be negative.\")\n" "\n" -" readBytes_(count, Fiber.current)\n" +" if (!(offset is Num)) Fiber.abort(\"Offset must be an integer.\")\n" +" if (!offset.isInteger) Fiber.abort(\"Offset must be an integer.\")\n" +" if (offset < 0) Fiber.abort(\"Offset cannot be negative.\")\n" +"\n" +" readBytes_(count, offset, Fiber.current)\n" " return Scheduler.runNextScheduled_()\n" " }\n" "\n" @@ -76,8 +84,7 @@ static const char* ioModuleSource = " foreign static sizePath_(path, fiber)\n" "\n" " foreign close_(fiber)\n" -" foreign descriptor\n" -" foreign readBytes_(count, fiber)\n" +" foreign readBytes_(count, start, fiber)\n" " foreign size_(fiber)\n" "}\n" "\n" diff --git a/test/io/file/read_bytes.wren b/test/io/file/read_bytes.wren index 44297cb4..a9d56a87 100644 --- a/test/io/file/read_bytes.wren +++ b/test/io/file/read_bytes.wren @@ -10,4 +10,10 @@ System.print(file.readBytes(7)) // expect: this is // Allows zero. System.print(file.readBytes(0).bytes.count) // expect: 0 +// A longer number reads the whole file. +System.print(file.readBytes(100)) // expect: this is a text file + +// Reading past the end truncates the buffer. +System.print(file.readBytes(100).bytes.count) // expect: 19 + file.close() diff --git a/test/io/file/read_bytes_negative.wren b/test/io/file/read_bytes_count_negative.wren similarity index 100% rename from test/io/file/read_bytes_negative.wren rename to test/io/file/read_bytes_count_negative.wren diff --git a/test/io/file/read_bytes_not_integer.wren b/test/io/file/read_bytes_count_not_integer.wren similarity index 100% rename from test/io/file/read_bytes_not_integer.wren rename to test/io/file/read_bytes_count_not_integer.wren diff --git a/test/io/file/read_bytes_from.wren b/test/io/file/read_bytes_from.wren new file mode 100644 index 00000000..a05cbc97 --- /dev/null +++ b/test/io/file/read_bytes_from.wren @@ -0,0 +1,20 @@ +import "io" for File + +var file = File.open("test/io/file/file.txt") + +// Zero starts at the beginning. +System.print(file.readBytes(3, 0)) // expect: thi + +// Starts at the offset. +System.print(file.readBytes(8, 3)) // expect: s is a t + +// Allows zero. +System.print(file.readBytes(0, 4).bytes.count) // expect: 0 + +// A longer number length reads until the end. +System.print(file.readBytes(100, 2)) // expect: is is a text file + +// An offset past the end returns an empty string. +System.print(file.readBytes(100, 30).bytes.count) // expect: 0 + +file.close() diff --git a/test/io/file/read_bytes_from_after_close.wren b/test/io/file/read_bytes_from_after_close.wren new file mode 100644 index 00000000..4cc8a753 --- /dev/null +++ b/test/io/file/read_bytes_from_after_close.wren @@ -0,0 +1,6 @@ +import "io" for File + +var file = File.open("test/io/file/file.txt") +file.close() + +file.readBytes(3, 0) // expect runtime error: File is not open. diff --git a/test/io/file/read_bytes_from_count_negative.wren b/test/io/file/read_bytes_from_count_negative.wren new file mode 100644 index 00000000..9d251303 --- /dev/null +++ b/test/io/file/read_bytes_from_count_negative.wren @@ -0,0 +1,4 @@ +import "io" for File + +var file = File.open("test/io/file/file.txt") +file.readBytes(-1, 0) // expect runtime error: Count cannot be negative. diff --git a/test/io/file/read_bytes_from_count_not_integer.wren b/test/io/file/read_bytes_from_count_not_integer.wren new file mode 100644 index 00000000..8a2ba576 --- /dev/null +++ b/test/io/file/read_bytes_from_count_not_integer.wren @@ -0,0 +1,4 @@ +import "io" for File + +var file = File.open("test/io/file/file.txt") +file.readBytes(1.2, 0) // expect runtime error: Count must be an integer. diff --git a/test/io/file/read_bytes_from_count_not_num.wren b/test/io/file/read_bytes_from_count_not_num.wren new file mode 100644 index 00000000..7783b522 --- /dev/null +++ b/test/io/file/read_bytes_from_count_not_num.wren @@ -0,0 +1,4 @@ +import "io" for File + +var file = File.open("test/io/file/file.txt") +file.readBytes("not num", 0) // expect runtime error: Count must be an integer. diff --git a/test/io/file/read_bytes_from_offset_negative.wren b/test/io/file/read_bytes_from_offset_negative.wren new file mode 100644 index 00000000..5a303082 --- /dev/null +++ b/test/io/file/read_bytes_from_offset_negative.wren @@ -0,0 +1,4 @@ +import "io" for File + +var file = File.open("test/io/file/file.txt") +file.readBytes(1, -1) // expect runtime error: Offset cannot be negative. diff --git a/test/io/file/read_bytes_from_offset_not_integer.wren b/test/io/file/read_bytes_from_offset_not_integer.wren new file mode 100644 index 00000000..dc573cea --- /dev/null +++ b/test/io/file/read_bytes_from_offset_not_integer.wren @@ -0,0 +1,4 @@ +import "io" for File + +var file = File.open("test/io/file/file.txt") +file.readBytes(1, 1.2) // expect runtime error: Offset must be an integer. diff --git a/test/io/file/read_bytes_from_offset_not_num.wren b/test/io/file/read_bytes_from_offset_not_num.wren new file mode 100644 index 00000000..528d0097 --- /dev/null +++ b/test/io/file/read_bytes_from_offset_not_num.wren @@ -0,0 +1,4 @@ +import "io" for File + +var file = File.open("test/io/file/file.txt") +file.readBytes(1, "not num") // expect runtime error: Offset must be an integer. From 5cd8a06fa0934c3fce7ba3393beeb970e5efa93d Mon Sep 17 00:00:00 2001 From: Bob Nystrom Date: Fri, 1 Jan 2016 09:58:44 -0800 Subject: [PATCH 25/26] File.stat(). --- doc/site/modules/io/index.markdown | 1 + doc/site/modules/io/stat.markdown | 50 +++++++++++++++++ src/cli/modules.c | 4 +- src/module/io.c | 56 +++++++++++++++++++ src/module/io.wren | 25 +++++++++ src/module/io.wren.inc | 25 +++++++++ test/io/file/size.wren | 13 +---- test/io/file/size_static.wren | 13 +++++ ...tent.wren => size_static_nonexistent.wren} | 0 test/io/file/stat_static.wren | 16 ++++++ test/io/file/stat_static_nonexistent.wren | 3 + 11 files changed, 195 insertions(+), 11 deletions(-) create mode 100644 doc/site/modules/io/stat.markdown create mode 100644 test/io/file/size_static.wren rename test/io/file/{size_nonexistent.wren => size_static_nonexistent.wren} (100%) create mode 100644 test/io/file/stat_static.wren create mode 100644 test/io/file/stat_static_nonexistent.wren diff --git a/doc/site/modules/io/index.markdown b/doc/site/modules/io/index.markdown index 1dc81f49..2b5a765c 100644 --- a/doc/site/modules/io/index.markdown +++ b/doc/site/modules/io/index.markdown @@ -4,4 +4,5 @@ Provides access to operating system streams and the file system. * [Directory](directory.html) * [File](file.html) +* [Stat](stat.html) * [Stdin](stdin.html) diff --git a/doc/site/modules/io/stat.markdown b/doc/site/modules/io/stat.markdown new file mode 100644 index 00000000..0b69b219 --- /dev/null +++ b/doc/site/modules/io/stat.markdown @@ -0,0 +1,50 @@ +^title Stat Class + +Contains the data returned by [File.stat()][stat]. + +[stat]: file.html#file.stat(path) + +## Methods + +### **device** + +The ID of the device containing the entry. + +### **inode** + +The [inode][] number of the entry. + +[inode]: https://en.wikipedia.org/wiki/Inode + +### **mode** + +A bit field describing the entry's type and protection flags. + +### **linkCount** + +The number of hard links to the entry. + +### **user** + +Numeric user ID of the file's owner. + +### **group** + +Numeric group ID of the file's owner. + +### **specialDevice** + +The device ID for the entry, if it's a special file. + +### **size** + +The size of the entry in bytes. + +### **blockSize** + +The preferred block size in bytes for interacting with the file. It may vary +from file to file. + +### **blockCount** + +The number of system blocks allocated on disk for the file. diff --git a/src/cli/modules.c b/src/cli/modules.c index cf4e8075..024156e3 100644 --- a/src/cli/modules.c +++ b/src/cli/modules.c @@ -12,6 +12,7 @@ extern void fileAllocate(WrenVM* vm); extern void fileFinalize(void* data); extern void fileOpen(WrenVM* vm); extern void fileSizePath(WrenVM* vm); +extern void fileStatPath(WrenVM* vm); extern void fileClose(WrenVM* vm); extern void fileDescriptor(WrenVM* vm); extern void fileReadBytes(WrenVM* vm); @@ -29,7 +30,7 @@ extern void timerStartTimer(WrenVM* vm); // If you add a new method to the longest class below, make sure to bump this. // Note that it also includes an extra slot for the sentinel value indicating // the end of the list. -#define MAX_METHODS_PER_CLASS 9 +#define MAX_METHODS_PER_CLASS 10 // The maximum number of foreign classes a single built-in module defines. // @@ -97,6 +98,7 @@ static ModuleRegistry modules[] = FINALIZER(fileFinalize) STATIC_METHOD("open_(_,_)", fileOpen) STATIC_METHOD("sizePath_(_,_)", fileSizePath) + STATIC_METHOD("statPath_(_,_)", fileStatPath) METHOD("close_(_)", fileClose) METHOD("descriptor", fileDescriptor) METHOD("readBytes_(_,_,_)", fileReadBytes) diff --git a/src/module/io.c b/src/module/io.c index 6b6c7c98..06371264 100644 --- a/src/module/io.c +++ b/src/module/io.c @@ -188,6 +188,62 @@ void fileSizePath(WrenVM* vm) uv_fs_stat(getLoop(), request, path, fileSizeCallback); } +// Called by libuv when the stat call completes. +static void fileStatPathCallback(uv_fs_t* request) +{ + if (handleRequestError(request)) return; + + WrenVM* vm = getVM(); + wrenEnsureSlots(vm, 4); + wrenSetSlotNewList(vm, 2); + + wrenSetSlotDouble(vm, 3, (double)request->statbuf.st_dev); + wrenInsertInList(vm, 2, -1, 3); + + wrenSetSlotDouble(vm, 3, (double)request->statbuf.st_ino); + wrenInsertInList(vm, 2, -1, 3); + + wrenSetSlotDouble(vm, 3, (double)request->statbuf.st_mode); + wrenInsertInList(vm, 2, -1, 3); + + wrenSetSlotDouble(vm, 3, (double)request->statbuf.st_nlink); + wrenInsertInList(vm, 2, -1, 3); + + wrenSetSlotDouble(vm, 3, (double)request->statbuf.st_uid); + wrenInsertInList(vm, 2, -1, 3); + + wrenSetSlotDouble(vm, 3, (double)request->statbuf.st_gid); + wrenInsertInList(vm, 2, -1, 3); + + wrenSetSlotDouble(vm, 3, (double)request->statbuf.st_rdev); + wrenInsertInList(vm, 2, -1, 3); + + wrenSetSlotDouble(vm, 3, (double)request->statbuf.st_size); + wrenInsertInList(vm, 2, -1, 3); + + wrenSetSlotDouble(vm, 3, (double)request->statbuf.st_blksize); + wrenInsertInList(vm, 2, -1, 3); + + wrenSetSlotDouble(vm, 3, (double)request->statbuf.st_blocks); + wrenInsertInList(vm, 2, -1, 3); + + // TODO: Include access, modification, and change times once we figure out + // how we want to represent it. + // time_t st_atime; /* time of last access */ + // time_t st_mtime; /* time of last modification */ + // time_t st_ctime; /* time of last status change */ + + schedulerResume(freeRequest(request), true); + schedulerFinishResume(); +} + +void fileStatPath(WrenVM* vm) +{ + const char* path = wrenGetSlotString(vm, 1); + uv_fs_t* request = createRequest(wrenGetSlotValue(vm, 2)); + uv_fs_stat(getLoop(), request, path, fileStatPathCallback); +} + static void fileCloseCallback(uv_fs_t* request) { if (handleRequestError(request)) return; diff --git a/src/module/io.wren b/src/module/io.wren index ea707624..e501c67a 100644 --- a/src/module/io.wren +++ b/src/module/io.wren @@ -44,6 +44,13 @@ foreign class File { return Scheduler.runNextScheduled_() } + static stat(path) { + if (!(path is String)) Fiber.abort("Path must be a string.") + + statPath_(path, Fiber.current) + return Stat.new_(Scheduler.runNextScheduled_()) + } + construct new_(fd) {} close() { @@ -80,12 +87,30 @@ foreign class File { foreign static open_(path, fiber) foreign static sizePath_(path, fiber) + foreign static statPath_(path, fiber) foreign close_(fiber) foreign readBytes_(count, start, fiber) foreign size_(fiber) } +class Stat { + construct new_(fields) { + _fields = fields + } + + device { _fields[0] } + inode { _fields[1] } + mode { _fields[2] } + linkCount { _fields[3] } + user { _fields[4] } + group { _fields[5] } + specialDevice { _fields[6] } + size { _fields[7] } + blockSize { _fields[8] } + blockCount { _fields[9] } +} + class Stdin { static readLine() { if (__isClosed == true) { diff --git a/src/module/io.wren.inc b/src/module/io.wren.inc index 7ba842d5..6f83dc7b 100644 --- a/src/module/io.wren.inc +++ b/src/module/io.wren.inc @@ -46,6 +46,13 @@ static const char* ioModuleSource = " return Scheduler.runNextScheduled_()\n" " }\n" "\n" +" static stat(path) {\n" +" if (!(path is String)) Fiber.abort(\"Path must be a string.\")\n" +"\n" +" statPath_(path, Fiber.current)\n" +" return Stat.new_(Scheduler.runNextScheduled_())\n" +" }\n" +"\n" " construct new_(fd) {}\n" "\n" " close() {\n" @@ -82,12 +89,30 @@ static const char* ioModuleSource = "\n" " foreign static open_(path, fiber)\n" " foreign static sizePath_(path, fiber)\n" +" foreign static statPath_(path, fiber)\n" "\n" " foreign close_(fiber)\n" " foreign readBytes_(count, start, fiber)\n" " foreign size_(fiber)\n" "}\n" "\n" +"class Stat {\n" +" construct new_(fields) {\n" +" _fields = fields\n" +" }\n" +"\n" +" device { _fields[0] }\n" +" inode { _fields[1] }\n" +" mode { _fields[2] }\n" +" linkCount { _fields[3] }\n" +" user { _fields[4] }\n" +" group { _fields[5] }\n" +" specialDevice { _fields[6] }\n" +" size { _fields[7] }\n" +" blockSize { _fields[8] }\n" +" blockCount { _fields[9] }\n" +"}\n" +"\n" "class Stdin {\n" " static readLine() {\n" " if (__isClosed == true) {\n" diff --git a/test/io/file/size.wren b/test/io/file/size.wren index 84e72eab..b1cc17fe 100644 --- a/test/io/file/size.wren +++ b/test/io/file/size.wren @@ -1,13 +1,6 @@ import "io" for File import "scheduler" for Scheduler -System.print(File.size("test/io/file/size.wren")) // expect: 270 - -// Runs asynchronously. -Scheduler.add { - System.print("async") -} - -System.print(File.size("test/io/file/size.wren")) -// expect: async -// expect: 270 +var file = File.open("test/io/file/file.txt") +System.print(file.size) // expect: 19 +file.close() diff --git a/test/io/file/size_static.wren b/test/io/file/size_static.wren new file mode 100644 index 00000000..c9b720f0 --- /dev/null +++ b/test/io/file/size_static.wren @@ -0,0 +1,13 @@ +import "io" for File +import "scheduler" for Scheduler + +System.print(File.size("test/io/file/file.txt")) // expect: 19 + +// Runs asynchronously. +Scheduler.add { + System.print("async") +} + +System.print(File.size("test/io/file/file.txt")) +// expect: async +// expect: 19 diff --git a/test/io/file/size_nonexistent.wren b/test/io/file/size_static_nonexistent.wren similarity index 100% rename from test/io/file/size_nonexistent.wren rename to test/io/file/size_static_nonexistent.wren diff --git a/test/io/file/stat_static.wren b/test/io/file/stat_static.wren new file mode 100644 index 00000000..0ca59d3f --- /dev/null +++ b/test/io/file/stat_static.wren @@ -0,0 +1,16 @@ +import "io" for File, Stat +import "scheduler" for Scheduler + +var stat = File.stat("test/io/file/file.txt") + +System.print(stat is Stat) // expect: true +System.print(stat.device is Num) // expect: true +System.print(stat.inode is Num) // expect: true +System.print(stat.mode is Num) // expect: true +System.print(stat.linkCount) // expect: 1 +System.print(stat.user is Num) // expect: true +System.print(stat.group is Num) // expect: true +System.print(stat.specialDevice) // expect: 0 +System.print(stat.size) // expect: 19 +System.print(stat.blockSize is Num) // expect: true +System.print(stat.blockCount is Num) // expect: true diff --git a/test/io/file/stat_static_nonexistent.wren b/test/io/file/stat_static_nonexistent.wren new file mode 100644 index 00000000..8bc17802 --- /dev/null +++ b/test/io/file/stat_static_nonexistent.wren @@ -0,0 +1,3 @@ +import "io" for File + +File.stat("nonexistent") // expect runtime error: no such file or directory From 94208647ab86d02d0f1e413818503324a70d7d5d Mon Sep 17 00:00:00 2001 From: Bob Nystrom Date: Fri, 1 Jan 2016 10:05:31 -0800 Subject: [PATCH 26/26] Test File.stat() on a directory. --- doc/site/modules/io/file.markdown | 7 +++++++ test/io/file/stat_static_directory.wren | 16 ++++++++++++++++ 2 files changed, 23 insertions(+) create mode 100644 test/io/file/stat_static_directory.wren diff --git a/doc/site/modules/io/file.markdown b/doc/site/modules/io/file.markdown index 9cd717fb..cb32454e 100644 --- a/doc/site/modules/io/file.markdown +++ b/doc/site/modules/io/file.markdown @@ -35,6 +35,13 @@ whatever encoding the file uses. Returns the size in bytes of the contents of the file at `path`. +### File.**stat**(path) + +"Stats" the file or directory at `path`. Returns a [Stat][] object describing +the low-level details of the file system entry. + +[stat]: stat.html + ## Constructors ### File.**open**(path) diff --git a/test/io/file/stat_static_directory.wren b/test/io/file/stat_static_directory.wren new file mode 100644 index 00000000..b5aa8ad3 --- /dev/null +++ b/test/io/file/stat_static_directory.wren @@ -0,0 +1,16 @@ +import "io" for File, Stat +import "scheduler" for Scheduler + +var stat = File.stat("test/io/directory/dir") + +System.print(stat is Stat) // expect: true +System.print(stat.device is Num) // expect: true +System.print(stat.inode is Num) // expect: true +System.print(stat.mode is Num) // expect: true +System.print(stat.linkCount) // expect: 5 +System.print(stat.user is Num) // expect: true +System.print(stat.group is Num) // expect: true +System.print(stat.specialDevice) // expect: 0 +System.print(stat.size) // expect: 170 +System.print(stat.blockSize is Num) // expect: true +System.print(stat.blockCount is Num) // expect: true