From b05a74da196429f87d5d40fcdb0bce4b2d6fe7aa Mon Sep 17 00:00:00 2001 From: Bob Nystrom Date: Tue, 29 Sep 2015 22:57:03 -0700 Subject: [PATCH] Revamp how runtime errors and fiber switching is handled. - Add Fiber.transferError(_). - Primitives place runtime errors directly in the fiber instead of on the stack. - Primitives that change fibers set it directly in the VM. - Allow a fiber's error to be any object (except null). --- doc/site/core/fiber.markdown | 4 + src/vm/wren_core.c | 178 ++++++++---------- src/vm/wren_debug.c | 11 +- src/vm/wren_meta.c | 6 +- src/vm/wren_primitive.c | 110 +++++------ src/vm/wren_primitive.h | 47 ++--- src/vm/wren_value.c | 4 +- src/vm/wren_value.h | 4 +- src/vm/wren_vm.c | 148 +++++++-------- test/core/fiber/abort_not_string.wren | 7 + test/core/fiber/abort_null.wren | 9 + test/core/fiber/abort_wrong_arg_type.wren | 1 - test/core/fiber/transfer_error.wren | 15 ++ .../core/fiber/transfer_error_not_string.wren | 10 + 14 files changed, 285 insertions(+), 269 deletions(-) create mode 100644 test/core/fiber/abort_not_string.wren create mode 100644 test/core/fiber/abort_null.wren delete mode 100644 test/core/fiber/abort_wrong_arg_type.wren create mode 100644 test/core/fiber/transfer_error.wren create mode 100644 test/core/fiber/transfer_error_not_string.wren diff --git a/doc/site/core/fiber.markdown b/doc/site/core/fiber.markdown index f0230e2b..6cb9de39 100644 --- a/doc/site/core/fiber.markdown +++ b/doc/site/core/fiber.markdown @@ -134,3 +134,7 @@ run. This returns `false` if the fiber is currently running or has yielded. ### **transfer**(value) **TODO** + +### **transferError**(error) + +**TODO** diff --git a/src/vm/wren_core.c b/src/vm/wren_core.c index f1a28f9e..691f568c 100644 --- a/src/vm/wren_core.c +++ b/src/vm/wren_core.c @@ -50,7 +50,7 @@ DEF_PRIMITIVE(class_toString) DEF_PRIMITIVE(fiber_new) { - if (!validateFn(vm, args, 1, "Argument")) return PRIM_ERROR; + if (!validateFn(vm, args[1], "Argument")) return PRIM_ERROR; ObjFiber* newFiber = wrenNewFiber(vm, AS_OBJ(args[1])); @@ -67,11 +67,10 @@ DEF_PRIMITIVE(fiber_new) DEF_PRIMITIVE(fiber_abort) { - if (!validateString(vm, args, 1, "Error message")) return PRIM_ERROR; - - // Move the error message to the return position. - args[0] = args[1]; - return PRIM_ERROR; + vm->fiber->error = args[1]; + + // If the error is explicitly null, it's not really an abort. + return IS_NULL(args[1]) ? PRIM_VALUE : PRIM_ERROR; } // Transfer execution to [fiber] coming from the current fiber whose stack has @@ -91,24 +90,17 @@ static PrimitiveResult runFiber(WrenVM* vm, ObjFiber* fiber, Value* args, // Remember who ran it. fiber->caller = vm->fiber; } - else - { - // Edge case: If we are transferring to ourself, immediately return the - // value. We can't treat this like an actual transfer since when we set the - // return below, it would overwrite the fiber being transferred to. - if (fiber == vm->fiber) RETURN_VAL(hasValue ? args[1] : NULL_VAL); - } if (fiber->numFrames == 0) { - args[0] = wrenStringFormat(vm, "Cannot $ a finished fiber.", + vm->fiber->error = wrenStringFormat(vm, "Cannot $ a finished fiber.", isCall ? "call" : "transfer to"); return PRIM_ERROR; } - if (fiber->error != NULL) + if (!IS_NULL(fiber->error)) { - args[0] = wrenStringFormat(vm, "Cannot $ an aborted fiber.", + vm->fiber->error = wrenStringFormat(vm, "Cannot $ an aborted fiber.", isCall ? "call" : "transfer to"); return PRIM_ERROR; } @@ -121,9 +113,11 @@ static PrimitiveResult runFiber(WrenVM* vm, ObjFiber* fiber, Value* args, // If the fiber was paused, make yield() or transfer() return the result. if (fiber->stackTop > fiber->stack) { - *(fiber->stackTop - 1) = hasValue ? args[1] : NULL_VAL; + fiber->stackTop[-1] = hasValue ? args[1] : NULL_VAL; } + vm->fiber = fiber; + return PRIM_RUN_FIBER; } @@ -144,21 +138,19 @@ DEF_PRIMITIVE(fiber_current) DEF_PRIMITIVE(fiber_error) { - ObjFiber* runFiber = AS_FIBER(args[0]); - if (runFiber->error == NULL) RETURN_NULL; - RETURN_OBJ(runFiber->error); + RETURN_VAL(AS_FIBER(args[0])->error); } DEF_PRIMITIVE(fiber_isDone) { ObjFiber* runFiber = AS_FIBER(args[0]); - RETURN_BOOL(runFiber->numFrames == 0 || runFiber->error != NULL); + RETURN_BOOL(runFiber->numFrames == 0 || !IS_NULL(runFiber->error)); } DEF_PRIMITIVE(fiber_suspend) { // Switching to a null fiber tells the interpreter to stop and exit. - args[0] = NULL_VAL; + vm->fiber = NULL; return PRIM_RUN_FIBER; } @@ -172,78 +164,71 @@ DEF_PRIMITIVE(fiber_transfer1) return runFiber(vm, AS_FIBER(args[0]), args, false, true); } +DEF_PRIMITIVE(fiber_transferError) +{ + PrimitiveResult result = runFiber(vm, AS_FIBER(args[0]), args, false, true); + if (result == PRIM_RUN_FIBER) vm->fiber->error = args[1]; + + return result; +} + DEF_PRIMITIVE(fiber_try) { - ObjFiber* runFiber = AS_FIBER(args[0]); - + ObjFiber* tried = AS_FIBER(args[0]); // TODO: Use runFiber(). - if (runFiber->numFrames == 0) RETURN_ERROR("Cannot try a finished fiber."); - if (runFiber->caller != NULL) RETURN_ERROR("Fiber has already been called."); + if (tried->numFrames == 0) RETURN_ERROR("Cannot try a finished fiber."); + if (tried->caller != NULL) RETURN_ERROR("Fiber has already been called."); + vm->fiber = tried; + // Remember who ran it. - runFiber->caller = fiber; - runFiber->callerIsTrying = true; + vm->fiber->caller = fiber; + vm->fiber->callerIsTrying = true; // If the fiber was yielded, make the yield call return null. - if (runFiber->stackTop > runFiber->stack) + if (vm->fiber->stackTop > vm->fiber->stack) { - *(runFiber->stackTop - 1) = NULL_VAL; + vm->fiber->stackTop[-1] = NULL_VAL; } - + return PRIM_RUN_FIBER; } DEF_PRIMITIVE(fiber_yield) { + vm->fiber = fiber->caller; + // Unhook this fiber from the one that called it. - ObjFiber* caller = fiber->caller; fiber->caller = NULL; fiber->callerIsTrying = false; - // If we don't have any other pending fibers, jump all the way out of the - // interpreter. - if (caller == NULL) - { - args[0] = NULL_VAL; - } - else + if (vm->fiber != NULL) { // Make the caller's run method return null. - *(caller->stackTop - 1) = NULL_VAL; - - // Return the fiber to resume. - args[0] = OBJ_VAL(caller); + vm->fiber->stackTop[-1] = NULL_VAL; } - + return PRIM_RUN_FIBER; } DEF_PRIMITIVE(fiber_yield1) { + vm->fiber = fiber->caller; + // Unhook this fiber from the one that called it. - ObjFiber* caller = fiber->caller; fiber->caller = NULL; fiber->callerIsTrying = false; - // If we don't have any other pending fibers, jump all the way out of the - // interpreter. - if (caller == NULL) - { - args[0] = NULL_VAL; - } - else + if (vm->fiber != NULL) { // Make the caller's run method return the argument passed to yield. - *(caller->stackTop - 1) = args[1]; + vm->fiber->stackTop[-1] = args[1]; - // When the yielding fiber resumes, we'll store the result of the yield call - // in its stack. Since Fiber.yield(value) has two arguments (the Fiber class - // and the value) and we only need one slot for the result, discard the other - // slot now. + // When the yielding fiber resumes, we'll store the result of the yield + // call in its stack. Since Fiber.yield(value) has two arguments (the Fiber + // class and the value) and we only need one slot for the result, discard + // the other slot now. fiber->stackTop--; - - // Return the fiber to resume. - args[0] = OBJ_VAL(caller); } return PRIM_RUN_FIBER; @@ -251,7 +236,7 @@ DEF_PRIMITIVE(fiber_yield1) DEF_PRIMITIVE(fn_new) { - if (!validateFn(vm, args, 1, "Argument")) return PRIM_ERROR; + if (!validateFn(vm, args[1], "Argument")) return PRIM_ERROR; // The block argument is already a function, so just return it. RETURN_VAL(args[1]); @@ -329,7 +314,7 @@ DEF_PRIMITIVE(list_insert) ObjList* list = AS_LIST(args[0]); // count + 1 here so you can "insert" at the very end. - uint32_t index = validateIndex(vm, args, list->elements.count + 1, 1, + uint32_t index = validateIndex(vm, args[1], list->elements.count + 1, "Index"); if (index == UINT32_MAX) return PRIM_ERROR; @@ -348,7 +333,7 @@ DEF_PRIMITIVE(list_iterate) RETURN_NUM(0); } - if (!validateInt(vm, args, 1, "Iterator")) return PRIM_ERROR; + if (!validateInt(vm, args[1], "Iterator")) return PRIM_ERROR; // Stop if we're out of bounds. double index = AS_NUM(args[1]); @@ -361,7 +346,7 @@ DEF_PRIMITIVE(list_iterate) DEF_PRIMITIVE(list_iteratorValue) { ObjList* list = AS_LIST(args[0]); - uint32_t index = validateIndex(vm, args, list->elements.count, 1, "Iterator"); + uint32_t index = validateIndex(vm, args[1], list->elements.count, "Iterator"); if (index == UINT32_MAX) return PRIM_ERROR; RETURN_VAL(list->elements.data[index]); @@ -370,7 +355,7 @@ DEF_PRIMITIVE(list_iteratorValue) DEF_PRIMITIVE(list_removeAt) { ObjList* list = AS_LIST(args[0]); - uint32_t index = validateIndex(vm, args, list->elements.count, 1, "Index"); + uint32_t index = validateIndex(vm, args[1], list->elements.count, "Index"); if (index == UINT32_MAX) return PRIM_ERROR; RETURN_VAL(wrenListRemoveAt(vm, list, index)); @@ -382,7 +367,7 @@ DEF_PRIMITIVE(list_subscript) if (IS_NUM(args[1])) { - uint32_t index = validateIndex(vm, args, list->elements.count, 1, + uint32_t index = validateIndex(vm, args[1], list->elements.count, "Subscript"); if (index == UINT32_MAX) return PRIM_ERROR; @@ -396,7 +381,7 @@ DEF_PRIMITIVE(list_subscript) int step; uint32_t count = list->elements.count; - uint32_t start = calculateRange(vm, args, AS_RANGE(args[1]), &count, &step); + uint32_t start = calculateRange(vm, AS_RANGE(args[1]), &count, &step); if (start == UINT32_MAX) return PRIM_ERROR; ObjList* result = wrenNewList(vm, count); @@ -411,7 +396,7 @@ DEF_PRIMITIVE(list_subscript) DEF_PRIMITIVE(list_subscriptSetter) { ObjList* list = AS_LIST(args[0]); - uint32_t index = validateIndex(vm, args, list->elements.count, 1, + uint32_t index = validateIndex(vm, args[1], list->elements.count, "Subscript"); if (index == UINT32_MAX) return PRIM_ERROR; @@ -426,7 +411,7 @@ DEF_PRIMITIVE(map_new) DEF_PRIMITIVE(map_subscript) { - if (!validateKey(vm, args, 1)) return PRIM_ERROR; + if (!validateKey(vm, args[1])) return PRIM_ERROR; ObjMap* map = AS_MAP(args[0]); Value value = wrenMapGet(map, args[1]); @@ -437,7 +422,7 @@ DEF_PRIMITIVE(map_subscript) DEF_PRIMITIVE(map_subscriptSetter) { - if (!validateKey(vm, args, 1)) return PRIM_ERROR; + if (!validateKey(vm, args[1])) return PRIM_ERROR; wrenMapSet(vm, AS_MAP(args[0]), args[1], args[2]); RETURN_VAL(args[2]); @@ -451,7 +436,7 @@ DEF_PRIMITIVE(map_clear) DEF_PRIMITIVE(map_containsKey) { - if (!validateKey(vm, args, 1)) return PRIM_ERROR; + if (!validateKey(vm, args[1])) return PRIM_ERROR; RETURN_BOOL(!IS_UNDEFINED(wrenMapGet(AS_MAP(args[0]), args[1]))); } @@ -473,7 +458,7 @@ DEF_PRIMITIVE(map_iterate) // Otherwise, start one past the last entry we stopped at. if (!IS_NULL(args[1])) { - if (!validateInt(vm, args, 1, "Iterator")) return PRIM_ERROR; + if (!validateInt(vm, args[1], "Iterator")) return PRIM_ERROR; if (AS_NUM(args[1]) < 0) RETURN_FALSE; index = (uint32_t)AS_NUM(args[1]); @@ -496,7 +481,7 @@ DEF_PRIMITIVE(map_iterate) DEF_PRIMITIVE(map_remove) { - if (!validateKey(vm, args, 1)) return PRIM_ERROR; + if (!validateKey(vm, args[1])) return PRIM_ERROR; RETURN_VAL(wrenMapRemoveKey(vm, AS_MAP(args[0]), args[1])); } @@ -504,7 +489,7 @@ DEF_PRIMITIVE(map_remove) DEF_PRIMITIVE(map_keyIteratorValue) { ObjMap* map = AS_MAP(args[0]); - uint32_t index = validateIndex(vm, args, map->capacity, 1, "Iterator"); + uint32_t index = validateIndex(vm, args[1], map->capacity, "Iterator"); if (index == UINT32_MAX) return PRIM_ERROR; MapEntry* entry = &map->entries[index]; @@ -519,7 +504,7 @@ DEF_PRIMITIVE(map_keyIteratorValue) DEF_PRIMITIVE(map_valueIteratorValue) { ObjMap* map = AS_MAP(args[0]); - uint32_t index = validateIndex(vm, args, map->capacity, 1, "Iterator"); + uint32_t index = validateIndex(vm, args[1], map->capacity, "Iterator"); if (index == UINT32_MAX) return PRIM_ERROR; MapEntry* entry = &map->entries[index]; @@ -543,7 +528,7 @@ DEF_PRIMITIVE(null_toString) DEF_PRIMITIVE(num_fromString) { - if (!validateString(vm, args, 1, "Argument")) return PRIM_ERROR; + if (!validateString(vm, args[1], "Argument")) return PRIM_ERROR; ObjString* string = AS_STRING(args[1]); @@ -559,7 +544,7 @@ DEF_PRIMITIVE(num_fromString) if (errno == ERANGE) { - args[0] = CONST_STRING(vm, "Number literal is too large."); + vm->fiber->error = CONST_STRING(vm, "Number literal is too large."); return PRIM_ERROR; } @@ -579,7 +564,7 @@ DEF_PRIMITIVE(num_pi) #define DEF_NUM_INFIX(name, op, type) \ DEF_PRIMITIVE(num_##name) \ { \ - if (!validateNum(vm, args, 1, "Right operand")) return PRIM_ERROR; \ + if (!validateNum(vm, args[1], "Right operand")) return PRIM_ERROR; \ RETURN_##type(AS_NUM(args[0]) op AS_NUM(args[1])); \ } @@ -596,7 +581,7 @@ DEF_NUM_INFIX(gte, >=, BOOL) #define DEF_NUM_BITWISE(name, op) \ DEF_PRIMITIVE(num_bitwise##name) \ { \ - if (!validateNum(vm, args, 1, "Right operand")) return PRIM_ERROR; \ + if (!validateNum(vm, args[1], "Right operand")) return PRIM_ERROR; \ uint32_t left = (uint32_t)AS_NUM(args[0]); \ uint32_t right = (uint32_t)AS_NUM(args[1]); \ RETURN_NUM(left op right); \ @@ -629,7 +614,7 @@ DEF_NUM_FN(tan, tan) DEF_PRIMITIVE(num_mod) { - if (!validateNum(vm, args, 1, "Right operand")) return PRIM_ERROR; + if (!validateNum(vm, args[1], "Right operand")) return PRIM_ERROR; RETURN_NUM(fmod(AS_NUM(args[0]), AS_NUM(args[1]))); } @@ -653,7 +638,7 @@ DEF_PRIMITIVE(num_bitwiseNot) DEF_PRIMITIVE(num_dotDot) { - if (!validateNum(vm, args, 1, "Right hand side of range")) return PRIM_ERROR; + if (!validateNum(vm, args[1], "Right hand side of range")) return PRIM_ERROR; double from = AS_NUM(args[0]); double to = AS_NUM(args[1]); @@ -662,7 +647,7 @@ DEF_PRIMITIVE(num_dotDot) DEF_PRIMITIVE(num_dotDotDot) { - if (!validateNum(vm, args, 1, "Right hand side of range")) return PRIM_ERROR; + if (!validateNum(vm, args[1], "Right hand side of range")) return PRIM_ERROR; double from = AS_NUM(args[0]); double to = AS_NUM(args[1]); @@ -805,7 +790,7 @@ DEF_PRIMITIVE(range_iterate) // Start the iteration. if (IS_NULL(args[1])) RETURN_NUM(range->from); - if (!validateNum(vm, args, 1, "Iterator")) return PRIM_ERROR; + if (!validateNum(vm, args[1], "Iterator")) return PRIM_ERROR; double iterator = AS_NUM(args[1]); @@ -852,7 +837,7 @@ DEF_PRIMITIVE(range_toString) DEF_PRIMITIVE(string_fromCodePoint) { - if (!validateInt(vm, args, 1, "Code point")) return PRIM_ERROR; + if (!validateInt(vm, args[1], "Code point")) return PRIM_ERROR; int codePoint = (int)AS_NUM(args[1]); if (codePoint < 0) @@ -871,7 +856,7 @@ DEF_PRIMITIVE(string_byteAt) { ObjString* string = AS_STRING(args[0]); - uint32_t index = validateIndex(vm, args, string->length, 1, "Index"); + uint32_t index = validateIndex(vm, args[1], string->length, "Index"); if (index == UINT32_MAX) return PRIM_ERROR; RETURN_NUM((uint8_t)string->value[index]); @@ -886,7 +871,7 @@ DEF_PRIMITIVE(string_codePointAt) { ObjString* string = AS_STRING(args[0]); - uint32_t index = validateIndex(vm, args, string->length, 1, "Index"); + uint32_t index = validateIndex(vm, args[1], string->length, "Index"); if (index == UINT32_MAX) return PRIM_ERROR; // If we are in the middle of a UTF-8 sequence, indicate that. @@ -900,7 +885,7 @@ DEF_PRIMITIVE(string_codePointAt) DEF_PRIMITIVE(string_contains) { - if (!validateString(vm, args, 1, "Argument")) return PRIM_ERROR; + if (!validateString(vm, args[1], "Argument")) return PRIM_ERROR; ObjString* string = AS_STRING(args[0]); ObjString* search = AS_STRING(args[1]); @@ -910,7 +895,7 @@ DEF_PRIMITIVE(string_contains) DEF_PRIMITIVE(string_endsWith) { - if (!validateString(vm, args, 1, "Argument")) return PRIM_ERROR; + if (!validateString(vm, args[1], "Argument")) return PRIM_ERROR; ObjString* string = AS_STRING(args[0]); ObjString* search = AS_STRING(args[1]); @@ -924,7 +909,7 @@ DEF_PRIMITIVE(string_endsWith) DEF_PRIMITIVE(string_indexOf) { - if (!validateString(vm, args, 1, "Argument")) return PRIM_ERROR; + if (!validateString(vm, args[1], "Argument")) return PRIM_ERROR; ObjString* string = AS_STRING(args[0]); ObjString* search = AS_STRING(args[1]); @@ -944,7 +929,7 @@ DEF_PRIMITIVE(string_iterate) RETURN_NUM(0); } - if (!validateInt(vm, args, 1, "Iterator")) return PRIM_ERROR; + if (!validateInt(vm, args[1], "Iterator")) return PRIM_ERROR; if (AS_NUM(args[1]) < 0) RETURN_FALSE; uint32_t index = (uint32_t)AS_NUM(args[1]); @@ -970,7 +955,7 @@ DEF_PRIMITIVE(string_iterateByte) RETURN_NUM(0); } - if (!validateInt(vm, args, 1, "Iterator")) return PRIM_ERROR; + if (!validateInt(vm, args[1], "Iterator")) return PRIM_ERROR; if (AS_NUM(args[1]) < 0) RETURN_FALSE; uint32_t index = (uint32_t)AS_NUM(args[1]); @@ -985,7 +970,7 @@ DEF_PRIMITIVE(string_iterateByte) DEF_PRIMITIVE(string_iteratorValue) { ObjString* string = AS_STRING(args[0]); - uint32_t index = validateIndex(vm, args, string->length, 1, "Iterator"); + uint32_t index = validateIndex(vm, args[1], string->length, "Iterator"); if (index == UINT32_MAX) return PRIM_ERROR; RETURN_VAL(wrenStringCodePointAt(vm, string, index)); @@ -993,7 +978,7 @@ DEF_PRIMITIVE(string_iteratorValue) DEF_PRIMITIVE(string_startsWith) { - if (!validateString(vm, args, 1, "Argument")) return PRIM_ERROR; + if (!validateString(vm, args[1], "Argument")) return PRIM_ERROR; ObjString* string = AS_STRING(args[0]); ObjString* search = AS_STRING(args[1]); @@ -1006,7 +991,7 @@ DEF_PRIMITIVE(string_startsWith) DEF_PRIMITIVE(string_plus) { - if (!validateString(vm, args, 1, "Right operand")) return PRIM_ERROR; + if (!validateString(vm, args[1], "Right operand")) return PRIM_ERROR; RETURN_VAL(wrenStringFormat(vm, "@@", args[0], args[1])); } @@ -1016,7 +1001,7 @@ DEF_PRIMITIVE(string_subscript) if (IS_NUM(args[1])) { - int index = validateIndex(vm, args, string->length, 1, "Subscript"); + int index = validateIndex(vm, args[1], string->length, "Subscript"); if (index == -1) return PRIM_ERROR; RETURN_VAL(wrenStringCodePointAt(vm, string, index)); @@ -1029,7 +1014,7 @@ DEF_PRIMITIVE(string_subscript) int step; uint32_t count = string->length; - int start = calculateRange(vm, args, AS_RANGE(args[1]), &count, &step); + int start = calculateRange(vm, AS_RANGE(args[1]), &count, &step); if (start == -1) return PRIM_ERROR; RETURN_VAL(wrenNewStringFromRange(vm, string, start, count, step)); @@ -1146,6 +1131,7 @@ void wrenInitializeCore(WrenVM* vm) PRIMITIVE(vm->fiberClass, "isDone", fiber_isDone); PRIMITIVE(vm->fiberClass, "transfer()", fiber_transfer); PRIMITIVE(vm->fiberClass, "transfer(_)", fiber_transfer1); + PRIMITIVE(vm->fiberClass, "transferError(_)", fiber_transferError); PRIMITIVE(vm->fiberClass, "try()", fiber_try); vm->fnClass = AS_CLASS(wrenFindVariable(vm, coreModule, "Fn")); diff --git a/src/vm/wren_debug.c b/src/vm/wren_debug.c index 902a2f2f..e34307b0 100644 --- a/src/vm/wren_debug.c +++ b/src/vm/wren_debug.c @@ -4,7 +4,16 @@ void wrenDebugPrintStackTrace(ObjFiber* fiber) { - fprintf(stderr, "%s\n", fiber->error->value); + if (IS_STRING(fiber->error)) + { + fprintf(stderr, "%s\n", AS_CSTRING(fiber->error)); + } + else + { + // TODO: Print something a little useful here. Maybe the name of the error's + // class? + fprintf(stderr, "[error object]\n"); + } for (int i = fiber->numFrames - 1; i >= 0; i--) { diff --git a/src/vm/wren_meta.c b/src/vm/wren_meta.c index 435eb22e..77f603cf 100644 --- a/src/vm/wren_meta.c +++ b/src/vm/wren_meta.c @@ -10,7 +10,7 @@ DEF_PRIMITIVE(meta_eval) { - if (!validateString(vm, args, 1, "Source code")) return PRIM_ERROR; + if (!validateString(vm, args[1], "Source code")) return PRIM_ERROR; // Eval the code in the module where the calling function was defined. Value callingFn = OBJ_VAL(fiber->frames[fiber->numFrames - 1].fn); @@ -33,8 +33,8 @@ DEF_PRIMITIVE(meta_eval) evalFiber->caller = fiber; // Switch to the fiber. - args[0] = OBJ_VAL(evalFiber); - + vm->fiber = evalFiber; + wrenPopRoot(vm); return PRIM_RUN_FIBER; } diff --git a/src/vm/wren_primitive.c b/src/vm/wren_primitive.c index 358534c4..1de8438c 100644 --- a/src/vm/wren_primitive.c +++ b/src/vm/wren_primitive.c @@ -2,85 +2,86 @@ #include -bool validateFn(WrenVM* vm, Value* args, int index, const char* argName) +// Validates that [value] is an integer within `[0, count)`. Also allows +// negative indices which map backwards from the end. Returns the valid positive +// index value. If invalid, reports an error and returns `UINT32_MAX`. +static uint32_t validateIndexValue(WrenVM* vm, uint32_t count, double value, + const char* argName) { - if (IS_FN(args[index]) || IS_CLOSURE(args[index])) return true; - - args[0] = wrenStringFormat(vm, "$ must be a function.", argName); - return false; -} - -bool validateNum(WrenVM* vm, Value* args, int index, const char* argName) -{ - if (IS_NUM(args[index])) return true; - - args[0] = wrenStringFormat(vm, "$ must be a number.", argName); - return false; -} - -bool validateIntValue(WrenVM* vm, Value* args, double value, - const char* argName) -{ - if (trunc(value) == value) return true; - - args[0] = wrenStringFormat(vm, "$ must be an integer.", argName); - return false; -} - -bool validateInt(WrenVM* vm, Value* args, int index, const char* argName) -{ - // Make sure it's a number first. - if (!validateNum(vm, args, index, argName)) return false; - - return validateIntValue(vm, args, AS_NUM(args[index]), argName); -} - -uint32_t validateIndexValue(WrenVM* vm, Value* args, uint32_t count, - double value, const char* argName) -{ - if (!validateIntValue(vm, args, value, argName)) return UINT32_MAX; - + if (!validateIntValue(vm, value, argName)) return UINT32_MAX; + // Negative indices count from the end. if (value < 0) value = count + value; - + // Check bounds. if (value >= 0 && value < count) return (uint32_t)value; - - args[0] = wrenStringFormat(vm, "$ out of bounds.", argName); + + vm->fiber->error = wrenStringFormat(vm, "$ out of bounds.", argName); return UINT32_MAX; } -bool validateKey(WrenVM* vm, Value* args, int index) +bool validateFn(WrenVM* vm, Value arg, const char* argName) +{ + if (IS_FN(arg) || IS_CLOSURE(arg)) return true; + + vm->fiber->error = wrenStringFormat(vm, "$ must be a function.", argName); + return false; +} + +bool validateNum(WrenVM* vm, Value arg, const char* argName) +{ + if (IS_NUM(arg)) return true; + + vm->fiber->error = wrenStringFormat(vm, "$ must be a number.", argName); + return false; +} + +bool validateIntValue(WrenVM* vm, double value, const char* argName) +{ + if (trunc(value) == value) return true; + + vm->fiber->error = wrenStringFormat(vm, "$ must be an integer.", argName); + return false; +} + +bool validateInt(WrenVM* vm, Value arg, const char* argName) +{ + // Make sure it's a number first. + if (!validateNum(vm, arg, argName)) return false; + + return validateIntValue(vm, AS_NUM(arg), argName); +} + +bool validateKey(WrenVM* vm, Value arg) { - Value arg = args[index]; if (IS_BOOL(arg) || IS_CLASS(arg) || IS_FIBER(arg) || IS_NULL(arg) || IS_NUM(arg) || IS_RANGE(arg) || IS_STRING(arg)) { return true; } - args[0] = CONST_STRING(vm, "Key must be a value type or fiber."); + vm->fiber->error = CONST_STRING(vm, "Key must be a value type or fiber."); return false; } -uint32_t validateIndex(WrenVM* vm, Value* args, uint32_t count, int arg, +uint32_t validateIndex(WrenVM* vm, Value arg, uint32_t count, const char* argName) { - if (!validateNum(vm, args, arg, argName)) return UINT32_MAX; + if (!validateNum(vm, arg, argName)) return UINT32_MAX; - return validateIndexValue(vm, args, count, AS_NUM(args[arg]), argName); + return validateIndexValue(vm, count, AS_NUM(arg), argName); } -bool validateString(WrenVM* vm, Value* args, int index, const char* argName) +bool validateString(WrenVM* vm, Value arg, const char* argName) { - if (IS_STRING(args[index])) return true; + if (IS_STRING(arg)) return true; - args[0] = wrenStringFormat(vm, "$ must be a string.", argName); + vm->fiber->error = wrenStringFormat(vm, "$ must be a string.", argName); return false; } -uint32_t calculateRange(WrenVM* vm, Value* args, ObjRange* range, - uint32_t* length, int* step) +uint32_t calculateRange(WrenVM* vm, ObjRange* range, uint32_t* length, + int* step) { *step = 0; @@ -92,13 +93,12 @@ uint32_t calculateRange(WrenVM* vm, Value* args, ObjRange* range, return 0; } - uint32_t from = validateIndexValue(vm, args, *length, range->from, - "Range start"); + uint32_t from = validateIndexValue(vm, *length, range->from, "Range start"); if (from == UINT32_MAX) return UINT32_MAX; // Bounds check the end manually to handle exclusive ranges. double value = range->to; - if (!validateIntValue(vm, args, value, "Range end")) return UINT32_MAX; + if (!validateIntValue(vm, value, "Range end")) return UINT32_MAX; // Negative indices count from the end. if (value < 0) value = *length + value; @@ -121,7 +121,7 @@ uint32_t calculateRange(WrenVM* vm, Value* args, ObjRange* range, // Check bounds. if (value < 0 || value >= *length) { - args[0] = CONST_STRING(vm, "Range end out of bounds."); + vm->fiber->error = CONST_STRING(vm, "Range end out of bounds."); return UINT32_MAX; } diff --git a/src/vm/wren_primitive.h b/src/vm/wren_primitive.h index 5d2b721d..2c58d541 100644 --- a/src/vm/wren_primitive.h +++ b/src/vm/wren_primitive.h @@ -32,47 +32,40 @@ #define RETURN_ERROR(msg) \ do { \ - args[0] = wrenStringFormat(vm, "$", msg); \ + vm->fiber->error = wrenStringFormat(vm, "$", msg); \ return PRIM_ERROR; \ } while (0); -// Validates that the given argument in [args] is a function. Returns true if -// it is. If not, reports an error and returns false. -bool validateFn(WrenVM* vm, Value* args, int index, const char* argName); +// Validates that the given [arg] is a function. Returns true if it is. If not, +// reports an error and returns false. +bool validateFn(WrenVM* vm, Value arg, const char* argName); -// Validates that the given argument in [args] is a Num. Returns true if it is. -// If not, reports an error and returns false. -bool validateNum(WrenVM* vm, Value* args, int index, const char* argName); +// Validates that the given [arg] is a Num. Returns true if it is. If not, +// reports an error and returns false. +bool validateNum(WrenVM* vm, Value arg, const char* argName); // Validates that [value] is an integer. Returns true if it is. If not, reports // an error and returns false. -bool validateIntValue(WrenVM* vm, Value* args, double value, - const char* argName); +bool validateIntValue(WrenVM* vm, double value, const char* argName); -// Validates that the given argument in [args] is an integer. Returns true if +// Validates that the given [arg] is an integer. Returns true if it is. If not, +// reports an error and returns false. +bool validateInt(WrenVM* vm, Value arg, const char* argName); + +// Validates that [arg] is a valid object for use as a map key. Returns true if // it is. If not, reports an error and returns false. -bool validateInt(WrenVM* vm, Value* args, int index, const char* argName); - -// Validates that [value] is an integer within `[0, count)`. Also allows -// negative indices which map backwards from the end. Returns the valid positive -// index value. If invalid, reports an error and returns `UINT32_MAX`. -uint32_t validateIndexValue(WrenVM* vm, Value* args, uint32_t count, - double value, const char* argName); - -// Validates that [key] is a valid object for use as a map key. Returns true if -// it is. If not, reports an error and returns false. -bool validateKey(WrenVM* vm, Value* args, int index); +bool validateKey(WrenVM* vm, Value arg); // Validates that the argument at [argIndex] is an integer within `[0, count)`. // Also allows negative indices which map backwards from the end. Returns the // valid positive index value. If invalid, reports an error and returns // `UINT32_MAX`. -uint32_t validateIndex(WrenVM* vm, Value* args, uint32_t count, int arg, +uint32_t validateIndex(WrenVM* vm, Value arg, uint32_t count, const char* argName); -// Validates that the given argument in [args] is a String. Returns true if it -// is. If not, reports an error and returns false. -bool validateString(WrenVM* vm, Value* args, int index, const char* argName); +// Validates that the given [arg] is a String. Returns true if it is. If not, +// reports an error and returns false. +bool validateString(WrenVM* vm, Value arg, const char* argName); // Given a [range] and the [length] of the object being operated on, determines // the series of elements that should be chosen from the underlying object. @@ -83,7 +76,7 @@ bool validateString(WrenVM* vm, Value* args, int index, const char* argName); // elements in the resulting sequence. [step] will be direction that the range // is going: `1` if the range is increasing from the start index or `-1` if the // range is decreasing. -uint32_t calculateRange(WrenVM* vm, Value* args, ObjRange* range, - uint32_t* length, int* step); +uint32_t calculateRange(WrenVM* vm, ObjRange* range, uint32_t* length, + int* step); #endif diff --git a/src/vm/wren_value.c b/src/vm/wren_value.c index 9a86b2b5..9397d5fc 100644 --- a/src/vm/wren_value.c +++ b/src/vm/wren_value.c @@ -166,7 +166,7 @@ void wrenResetFiber(WrenVM* vm, ObjFiber* fiber, Obj* fn) fiber->stackTop = fiber->stack; fiber->openUpvalues = NULL; fiber->caller = NULL; - fiber->error = NULL; + fiber->error = NULL_VAL; fiber->callerIsTrying = false; // Initialize the first call frame. @@ -922,7 +922,7 @@ static void markFiber(WrenVM* vm, ObjFiber* fiber) // The caller. wrenMarkObj(vm, (Obj*)fiber->caller); - wrenMarkObj(vm, (Obj*)fiber->error); + wrenMarkValue(vm, fiber->error); // Keep track of how much memory is still in use. vm->bytesAllocated += sizeof(ObjFiber); diff --git a/src/vm/wren_value.h b/src/vm/wren_value.h index 442180d2..2e554378 100644 --- a/src/vm/wren_value.h +++ b/src/vm/wren_value.h @@ -233,8 +233,8 @@ typedef struct sObjFiber struct sObjFiber* caller; // If the fiber failed because of a runtime error, this will contain the - // error message. Otherwise, it will be NULL. - ObjString* error; + // error object. Otherwise, it will be null. + Value error; // A unique-ish numeric ID for the fiber. Lets fibers be used as map keys. // Unique-ish since IDs may overflow and wrap around. diff --git a/src/vm/wren_vm.c b/src/vm/wren_vm.c index 855a3663..22ff6a22 100644 --- a/src/vm/wren_vm.c +++ b/src/vm/wren_vm.c @@ -292,9 +292,9 @@ static WrenForeignMethodFn findForeignMethod(WrenVM* vm, // Handles both foreign methods where [methodValue] is a string containing the // method's signature and Wren methods where [methodValue] is a function. // -// Returns an error string if the method is a foreign method that could not be -// found. Otherwise returns `NULL_VAL`. -static Value bindMethod(WrenVM* vm, int methodType, int symbol, +// Aborts the current fiber if the method is a foreign method that could not be +// found. +static void bindMethod(WrenVM* vm, int methodType, int symbol, ObjModule* module, ObjClass* classObj, Value methodValue) { const char* className = classObj->name->value; @@ -312,9 +312,10 @@ static Value bindMethod(WrenVM* vm, int methodType, int symbol, if (method.fn.foreign == NULL) { - return wrenStringFormat(vm, + vm->fiber->error = wrenStringFormat(vm, "Could not find foreign method '@' for class $ in module '$'.", methodValue, classObj->name->value, module->name->value); + return; } } else @@ -330,7 +331,6 @@ static Value bindMethod(WrenVM* vm, int methodType, int symbol, } wrenBindMethod(vm, classObj, symbol, method); - return NULL_VAL; } static void callForeign(WrenVM* vm, ObjFiber* fiber, @@ -353,41 +353,37 @@ static void callForeign(WrenVM* vm, ObjFiber* fiber, } } -// Puts [fiber] into a runtime failed state because of [error]. -// -// Returns the fiber that should receive the error or `NULL` if no fiber -// caught it. -static ObjFiber* runtimeError(WrenVM* vm, ObjFiber* fiber, Value error) +// Handles the current fiber having aborted because of an error. Switches to +// a new fiber if there is a fiber that will handle the error, otherwise, tells +// the VM to stop. +static void runtimeError(WrenVM* vm) { - ASSERT(fiber->error == NULL, "Can only fail once."); - - // Store the error in the fiber so it can be accessed later. - fiber->error = AS_STRING(error); - + ASSERT(!IS_NULL(vm->fiber->error), "Should only call this after an error."); + // Unhook the caller since we will never resume and return to it. - ObjFiber* caller = fiber->caller; - fiber->caller = NULL; - + ObjFiber* caller = vm->fiber->caller; + vm->fiber->caller = NULL; + // If the caller ran this fiber using "try", give it the error. - if (fiber->callerIsTrying) + if (vm->fiber->callerIsTrying) { // Make the caller's try method return the error message. - *(caller->stackTop - 1) = OBJ_VAL(fiber->error); + caller->stackTop[-1] = vm->fiber->error; vm->fiber = caller; - return caller; + return; } - + // If we got here, nothing caught the error, so show the stack trace. - wrenDebugPrintStackTrace(fiber); - return NULL; + wrenDebugPrintStackTrace(vm->fiber); + vm->fiber = NULL; } -// Creates a string containing an appropriate method not found error for a +// Aborts the current fiber with an appropriate method not found error for a // method with [symbol] on [classObj]. -static Value methodNotFound(WrenVM* vm, ObjClass* classObj, int symbol) +static void methodNotFound(WrenVM* vm, ObjClass* classObj, int symbol) { - return wrenStringFormat(vm, "@ does not implement '$'.", + vm->fiber->error = wrenStringFormat(vm, "@ does not implement '$'.", OBJ_VAL(classObj->name), vm->methodNames.data[symbol].buffer); } @@ -472,7 +468,8 @@ static Value importModule(WrenVM* vm, Value name) if (source == NULL) { // Couldn't load the module. - return wrenStringFormat(vm, "Could not find module '@'.", name); + vm->fiber->error = wrenStringFormat(vm, "Could not find module '@'.", name); + return NULL_VAL; } ObjFiber* moduleFiber = loadModule(vm, name, source); @@ -481,9 +478,7 @@ static Value importModule(WrenVM* vm, Value name) return OBJ_VAL(moduleFiber); } - -static bool importVariable(WrenVM* vm, Value moduleName, Value variableName, - Value* result) +static Value importVariable(WrenVM* vm, Value moduleName, Value variableName) { ObjModule* module = getModule(vm, moduleName); ASSERT(module != NULL, "Should only look up loaded modules."); @@ -496,14 +491,13 @@ static bool importVariable(WrenVM* vm, Value moduleName, Value variableName, // It's a runtime error if the imported variable does not exist. if (variableEntry != UINT32_MAX) { - *result = module->variables.data[variableEntry]; - return true; + return module->variables.data[variableEntry]; } - *result = wrenStringFormat(vm, + vm->fiber->error = wrenStringFormat(vm, "Could not find a variable named '@' in module '@'.", variableName, moduleName); - return false; + return NULL_VAL; } // Verifies that [superclassValue] is a valid object to inherit from. That @@ -600,35 +594,27 @@ static void bindForeignClass(WrenVM* vm, ObjClass* classObj, ObjModule* module) // // If [numFields] is -1, the class is a foreign class. The name and superclass // should be on top of the fiber's stack. After calling this, the top of the -// stack will contain either the new class or a string if a runtime error -// occurred. +// stack will contain the new class. // -// Returns false if the result is an error. -static bool createClass(WrenVM* vm, ObjFiber* fiber, int numFields, - ObjModule* module) +// Aborts the current fiber if an error occurs. +static void createClass(WrenVM* vm, int numFields, ObjModule* module) { // Pull the name and superclass off the stack. - Value name = fiber->stackTop[-2]; - Value superclass = fiber->stackTop[-1]; + Value name = vm->fiber->stackTop[-2]; + Value superclass = vm->fiber->stackTop[-1]; // We have two values on the stack and we are going to leave one, so discard // the other slot. - fiber->stackTop--; + vm->fiber->stackTop--; - Value error = validateSuperclass(vm, name, superclass, numFields); - if (!IS_NULL(error)) - { - fiber->stackTop[-1] = error; - return false; - } + vm->fiber->error = validateSuperclass(vm, name, superclass, numFields); + if (!IS_NULL(vm->fiber->error)) return; ObjClass* classObj = wrenNewClass(vm, AS_CLASS(superclass), numFields, AS_STRING(name)); - fiber->stackTop[-1] = OBJ_VAL(classObj); + vm->fiber->stackTop[-1] = OBJ_VAL(classObj); if (numFields == -1) bindForeignClass(vm, classObj, module); - - return true; } static void createForeign(WrenVM* vm, ObjFiber* fiber, Value* stack) @@ -719,15 +705,16 @@ static WrenInterpretResult runInterpreter(WrenVM* vm, register ObjFiber* fiber) // Terminates the current fiber with error string [error]. If another calling // fiber is willing to catch the error, transfers control to it, otherwise // exits the interpreter. - #define RUNTIME_ERROR(error) \ - do \ - { \ - STORE_FRAME(); \ - fiber = runtimeError(vm, fiber, error); \ - if (fiber == NULL) return WREN_RESULT_RUNTIME_ERROR; \ - LOAD_FRAME(); \ - DISPATCH(); \ - } \ + #define RUNTIME_ERROR() \ + do \ + { \ + STORE_FRAME(); \ + runtimeError(vm); \ + if (vm->fiber == NULL) return WREN_RESULT_RUNTIME_ERROR; \ + fiber = vm->fiber; \ + LOAD_FRAME(); \ + DISPATCH(); \ + } \ while (false) #if WREN_DEBUG_TRACE_INSTRUCTIONS @@ -901,7 +888,8 @@ static WrenInterpretResult runInterpreter(WrenVM* vm, register ObjFiber* fiber) if (symbol >= classObj->methods.count || (method = &classObj->methods.data[symbol])->type == METHOD_NONE) { - RUNTIME_ERROR(methodNotFound(vm, classObj, symbol)); + methodNotFound(vm, classObj, symbol); + RUNTIME_ERROR(); } switch (method->type) @@ -918,7 +906,7 @@ static WrenInterpretResult runInterpreter(WrenVM* vm, register ObjFiber* fiber) break; case PRIM_ERROR: - RUNTIME_ERROR(args[0]); + RUNTIME_ERROR(); case PRIM_CALL: STORE_FRAME(); @@ -930,10 +918,11 @@ static WrenInterpretResult runInterpreter(WrenVM* vm, register ObjFiber* fiber) STORE_FRAME(); // If we don't have a fiber to switch to, stop interpreting. - if (IS_NULL(args[0])) return WREN_RESULT_SUCCESS; + if (vm->fiber == NULL) return WREN_RESULT_SUCCESS; + if (!IS_NULL(vm->fiber->error)) RUNTIME_ERROR(); - fiber = AS_FIBER(args[0]); - vm->fiber = fiber; + fiber = vm->fiber; + LOAD_FRAME(); break; } @@ -1165,13 +1154,15 @@ static WrenInterpretResult runInterpreter(WrenVM* vm, register ObjFiber* fiber) CASE_CODE(CLASS): { - if (!createClass(vm, fiber, READ_BYTE(), NULL)) RUNTIME_ERROR(PEEK()); + createClass(vm, READ_BYTE(), NULL); + if (!IS_NULL(fiber->error)) RUNTIME_ERROR(); DISPATCH(); } CASE_CODE(FOREIGN_CLASS): { - if (!createClass(vm, fiber, -1, fn->module)) RUNTIME_ERROR(PEEK()); + createClass(vm, -1, fn->module); + if (!IS_NULL(fiber->error)) RUNTIME_ERROR(); DISPATCH(); } @@ -1181,9 +1172,8 @@ static WrenInterpretResult runInterpreter(WrenVM* vm, register ObjFiber* fiber) uint16_t symbol = READ_SHORT(); ObjClass* classObj = AS_CLASS(PEEK()); Value method = PEEK2(); - Value error = bindMethod(vm, instruction, symbol, fn->module, classObj, - method); - if (IS_STRING(error)) RUNTIME_ERROR(error); + bindMethod(vm, instruction, symbol, fn->module, classObj, method); + if (!IS_NULL(fiber->error)) RUNTIME_ERROR(); DROP(); DROP(); DISPATCH(); @@ -1194,8 +1184,7 @@ static WrenInterpretResult runInterpreter(WrenVM* vm, register ObjFiber* fiber) Value name = fn->constants[READ_SHORT()]; Value result = importModule(vm, name); - // If it returned a string, it was an error message. - if (IS_STRING(result)) RUNTIME_ERROR(result); + if (!IS_NULL(fiber->error)) RUNTIME_ERROR(); // Make a slot that the module's fiber can use to store its result in. // It ends up getting discarded, but CODE_RETURN expects to be able to @@ -1221,15 +1210,10 @@ static WrenInterpretResult runInterpreter(WrenVM* vm, register ObjFiber* fiber) { Value module = fn->constants[READ_SHORT()]; Value variable = fn->constants[READ_SHORT()]; - Value result; - if (importVariable(vm, module, variable, &result)) - { - PUSH(result); - } - else - { - RUNTIME_ERROR(result); - } + Value result = importVariable(vm, module, variable); + if (!IS_NULL(fiber->error)) RUNTIME_ERROR(); + + PUSH(result); DISPATCH(); } diff --git a/test/core/fiber/abort_not_string.wren b/test/core/fiber/abort_not_string.wren new file mode 100644 index 00000000..580c559e --- /dev/null +++ b/test/core/fiber/abort_not_string.wren @@ -0,0 +1,7 @@ +var fiber = Fiber.new { + Fiber.abort(123) +} + +System.print(fiber.try()) // expect: 123 +System.print(fiber.isDone) // expect: true +System.print(fiber.error) // expect: 123 diff --git a/test/core/fiber/abort_null.wren b/test/core/fiber/abort_null.wren new file mode 100644 index 00000000..d3b5ed37 --- /dev/null +++ b/test/core/fiber/abort_null.wren @@ -0,0 +1,9 @@ +var fiber = Fiber.new { + Fiber.abort(null) + System.print("get here") // expect: get here + Fiber.yield("value") +} + +System.print(fiber.try()) // expect: value +System.print(fiber.isDone) // expect: false +System.print(fiber.error) // expect: null diff --git a/test/core/fiber/abort_wrong_arg_type.wren b/test/core/fiber/abort_wrong_arg_type.wren deleted file mode 100644 index 40f4caa3..00000000 --- a/test/core/fiber/abort_wrong_arg_type.wren +++ /dev/null @@ -1 +0,0 @@ -Fiber.abort(123) // expect runtime error: Error message must be a string. diff --git a/test/core/fiber/transfer_error.wren b/test/core/fiber/transfer_error.wren new file mode 100644 index 00000000..dc8c47ba --- /dev/null +++ b/test/core/fiber/transfer_error.wren @@ -0,0 +1,15 @@ +var A = Fiber.new { + System.print("transferred to A") + B.transferError("error!") +} + +var B = Fiber.new { + System.print("started B") + A.transfer() + System.print("should not get here") +} + +B.try() +// expect: started B +// expect: transferred to A +System.print(B.error) // expect: error! diff --git a/test/core/fiber/transfer_error_not_string.wren b/test/core/fiber/transfer_error_not_string.wren new file mode 100644 index 00000000..0babc784 --- /dev/null +++ b/test/core/fiber/transfer_error_not_string.wren @@ -0,0 +1,10 @@ +var A = Fiber.new { + B.transferError(123) +} + +var B = Fiber.new { + A.transfer() +} + +B.try() +System.print(B.error) // expect: 123