diff --git a/Makefile b/Makefile index abb079f5..74f68129 100644 --- a/Makefile +++ b/Makefile @@ -76,8 +76,7 @@ clean: $(V) rm -rf lib # Run the tests against the debug build of Wren. -test: debug - $(V) $(MAKE) -f util/wren.mk MODE=debug api_test +test: api_test debug $(V) ./util/test.py $(suite) benchmark: release @@ -92,6 +91,10 @@ unit_test: $(V) $(MAKE) -f util/wren.mk MODE=debug unit_test $(V) ./build/debug/test/unit_wrend +# Build API tests. +api_test: + $(V) $(MAKE) -f util/wren.mk MODE=debug api_test + # Generate the Wren site. docs: mkdir -p build @@ -113,4 +116,4 @@ gh-pages: docs amalgamation: src/include/wren.h src/vm/*.h src/vm/*.c ./util/generate_amalgamation.py > build/wren.c -.PHONY: all amalgamation benchmark builtin clean debug docs gh-pages release test vm watchdocs ci ci_32 ci_64 +.PHONY: all amalgamation api_test benchmark builtin clean debug docs gh-pages release test vm watchdocs ci ci_32 ci_64 diff --git a/src/vm/wren_vm.c b/src/vm/wren_vm.c index d1a7784b..4c657d37 100644 --- a/src/vm/wren_vm.c +++ b/src/vm/wren_vm.c @@ -268,8 +268,8 @@ static ObjUpvalue* captureUpvalue(WrenVM* vm, ObjFiber* fiber, Value* local) return createdUpvalue; } -// Closes any open upvates that have been created for stack slots at [last] and -// above. +// Closes any open upvalues that have been created for stack slots at [last] +// and above. static void closeUpvalues(ObjFiber* fiber, Value* last) { while (fiber->openUpvalues != NULL && @@ -370,8 +370,7 @@ static void bindMethod(WrenVM* vm, int methodType, int symbol, static void callForeign(WrenVM* vm, ObjFiber* fiber, WrenForeignMethodFn foreign, int numArgs) { - // Save the current state so we can restore it when done. - Value* apiStack = vm->apiStack; + ASSERT(vm->apiStack == NULL, "Cannot already be in foreign call."); vm->apiStack = fiber->stackTop - numArgs; foreign(vm); @@ -379,7 +378,8 @@ static void callForeign(WrenVM* vm, ObjFiber* fiber, // Discard the stack slots for the arguments and temporaries but leave one // for the result. fiber->stackTop = vm->apiStack + 1; - vm->apiStack = apiStack; + + vm->apiStack = NULL; } // Handles the current fiber having aborted because of an error. @@ -624,13 +624,12 @@ 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. - Value* oldApiStack = vm->apiStack; + ASSERT(vm->apiStack == NULL, "Cannot already be in foreign call."); vm->apiStack = stack; method->as.foreign(vm); - vm->apiStack = oldApiStack; - // TODO: Check that allocateForeign was called. + vm->apiStack = NULL; } void wrenFinalizeForeign(WrenVM* vm, ObjForeign* foreign) @@ -1379,12 +1378,24 @@ WrenInterpretResult wrenCall(WrenVM* vm, WrenHandle* method) ASSERT(vm->fiber->stackTop - vm->fiber->stack >= closure->fn->arity, "Stack must have enough arguments for method."); + // Clear the API stack. Now that wrenCall() has control, we no longer need + // it. We use this being non-null to tell if re-entrant calls to foreign + // methods are happening, so it's important to clear it out now so that you + // can call foreign methods from within calls to wrenCall(). + vm->apiStack = NULL; + // Discard any extra temporary slots. We take for granted that the stub // function has exactly one slot for each argument. vm->fiber->stackTop = &vm->fiber->stack[closure->fn->maxSlots]; wrenCallFunction(vm, vm->fiber, closure, 0); - return runInterpreter(vm, vm->fiber); + WrenInterpretResult result = runInterpreter(vm, vm->fiber); + + // If the call didn't abort, then set up the API stack to point to the + // beginning of the stack so the host can access the call's return value. + if (vm->fiber != NULL) vm->apiStack = vm->fiber->stack; + + return result; } WrenHandle* wrenMakeHandle(WrenVM* vm, Value value) diff --git a/test/api/call.c b/test/api/call.c index f8d5c7d7..3b0cd295 100644 --- a/test/api/call.c +++ b/test/api/call.c @@ -66,6 +66,7 @@ void callRunTests(WrenVM* vm) wrenEnsureSlots(vm, 1); wrenSetSlotHandle(vm, 0, callClass); wrenCall(vm, getValue); + printf("slots after call: %d\n", wrenGetSlotCount(vm)); WrenHandle* value = wrenGetSlotHandle(vm, 0); // Different argument types. diff --git a/test/api/call.wren b/test/api/call.wren index cf4f2017..9306c7cc 100644 --- a/test/api/call.wren +++ b/test/api/call.wren @@ -48,6 +48,7 @@ class Call { // expect: subscript 1 2 // expect: subscript set 1 2 3 +// expect: slots after call: 1 // expect: two true false // expect: two 1.2 3.4 // expect: two string another diff --git a/test/api/call_calls_foreign.c b/test/api/call_calls_foreign.c new file mode 100644 index 00000000..b2768ee8 --- /dev/null +++ b/test/api/call_calls_foreign.c @@ -0,0 +1,44 @@ +#include +#include + +#include "wren.h" + +static void api(WrenVM *vm) { + // Grow the slot array. This should trigger the stack to be moved. + wrenEnsureSlots(vm, 10); + wrenSetSlotNewList(vm, 0); + + for (int i = 1; i < 10; i++) + { + wrenSetSlotDouble(vm, i, i); + wrenInsertInList(vm, 0, -1, i); + } +} + +WrenForeignMethodFn callCallsForeignBindMethod(const char* signature) +{ + if (strcmp(signature, "static CallCallsForeign.api()") == 0) return api; + + return NULL; +} + +void callCallsForeignRunTests(WrenVM* vm) +{ + wrenEnsureSlots(vm, 1); + wrenGetVariable(vm, "./test/api/call_calls_foreign", "CallCallsForeign", 0); + WrenHandle* apiClass = wrenGetSlotHandle(vm, 0); + WrenHandle *call = wrenMakeCallHandle(vm, "call(_)"); + + wrenEnsureSlots(vm, 2); + wrenSetSlotHandle(vm, 0, apiClass); + wrenSetSlotString(vm, 1, "parameter"); + + printf("slots before %d\n", wrenGetSlotCount(vm)); + wrenCall(vm, call); + + // We should have a single slot count for the return. + printf("slots after %d\n", wrenGetSlotCount(vm)); + + wrenReleaseHandle(vm, call); + wrenReleaseHandle(vm, apiClass); +} diff --git a/test/api/call_calls_foreign.h b/test/api/call_calls_foreign.h new file mode 100644 index 00000000..2bb3f1bf --- /dev/null +++ b/test/api/call_calls_foreign.h @@ -0,0 +1,4 @@ +#include "wren.h" + +WrenForeignMethodFn callCallsForeignBindMethod(const char* signature); +void callCallsForeignRunTests(WrenVM* vm); diff --git a/test/api/call_calls_foreign.wren b/test/api/call_calls_foreign.wren new file mode 100644 index 00000000..88020188 --- /dev/null +++ b/test/api/call_calls_foreign.wren @@ -0,0 +1,17 @@ +// Regression test for https://github.com/munificent/wren/issues/510. +// +// Tests that re-entrant API calls are handled correctly. The host uses +// `wrenCall()` to invoke `CallCallsForeign.call()`. That in turn calls +// `CallCallsForeign.api()`, which goes back through the API. +class CallCallsForeign { + foreign static api() + + static call(param) { + System.print(api()) + // expect: slots before 2 + // expect: [1, 2, 3, 4, 5, 6, 7, 8, 9] + System.print(param) // expect: parameter + // expect: slots after 1 + return "result" + } +} diff --git a/test/api/main.c b/test/api/main.c index 4065d73f..ee810702 100644 --- a/test/api/main.c +++ b/test/api/main.c @@ -6,6 +6,7 @@ #include "benchmark.h" #include "call.h" +#include "call_calls_foreign.h" #include "call_wren_call_root.h" #include "error.h" #include "get_variable.h" @@ -42,6 +43,9 @@ static WrenForeignMethodFn bindForeignMethod( method = benchmarkBindMethod(fullName); if (method != NULL) return method; + method = callCallsForeignBindMethod(fullName); + if (method != NULL) return method; + method = errorBindMethod(fullName); if (method != NULL) return method; @@ -102,6 +106,10 @@ static void afterLoad(WrenVM* vm) { callRunTests(vm); } + else if (strstr(testName, "/call_calls_foreign.wren") != NULL) + { + callCallsForeignRunTests(vm); + } else if (strstr(testName, "/call_wren_call_root.wren") != NULL) { callWrenCallRootRunTests(vm); diff --git a/test/api/reset_stack_after_call_abort.c b/test/api/reset_stack_after_call_abort.c index a4d09af3..8a85cd7f 100644 --- a/test/api/reset_stack_after_call_abort.c +++ b/test/api/reset_stack_after_call_abort.c @@ -10,7 +10,7 @@ void resetStackAfterCallAbortRunTests(WrenVM* vm) WrenHandle* testClass = wrenGetSlotHandle(vm, 0); WrenHandle* abortFiber = wrenMakeCallHandle(vm, "abortFiber()"); - WrenHandle* afterConstruct = wrenMakeCallHandle(vm, "afterAbort(_,_)"); + WrenHandle* afterAbort = wrenMakeCallHandle(vm, "afterAbort(_,_)"); wrenEnsureSlots(vm, 1); wrenSetSlotHandle(vm, 0, testClass); @@ -20,9 +20,9 @@ void resetStackAfterCallAbortRunTests(WrenVM* vm) wrenSetSlotHandle(vm, 0, testClass); wrenSetSlotDouble(vm, 1, 1.0); wrenSetSlotDouble(vm, 2, 2.0); - wrenCall(vm, afterConstruct); + wrenCall(vm, afterAbort); wrenReleaseHandle(vm, testClass); wrenReleaseHandle(vm, abortFiber); - wrenReleaseHandle(vm, afterConstruct); + wrenReleaseHandle(vm, afterAbort); } diff --git a/util/xcode/wren.xcodeproj/project.pbxproj b/util/xcode/wren.xcodeproj/project.pbxproj index 44755f9d..9aabedcb 100644 --- a/util/xcode/wren.xcodeproj/project.pbxproj +++ b/util/xcode/wren.xcodeproj/project.pbxproj @@ -35,6 +35,7 @@ 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 */; }; + 2993ED1D2111FD6D000F0D49 /* call_calls_foreign.c in Sources */ = {isa = PBXBuildFile; fileRef = 2993ED1C2111FD6D000F0D49 /* call_calls_foreign.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 */; }; @@ -152,6 +153,8 @@ 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 = ""; }; + 2993ED1B2111FD6D000F0D49 /* call_calls_foreign.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = call_calls_foreign.h; path = ../../test/api/call_calls_foreign.h; sourceTree = ""; }; + 2993ED1C2111FD6D000F0D49 /* call_calls_foreign.c */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.c; name = call_calls_foreign.c; path = ../../test/api/call_calls_foreign.c; 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 = ""; }; @@ -340,9 +343,10 @@ children = ( 29932D4F1C20D8C900099DEE /* benchmark.c */, 29932D501C20D8C900099DEE /* benchmark.h */, + 2993ED1C2111FD6D000F0D49 /* call_calls_foreign.c */, + 2993ED1B2111FD6D000F0D49 /* call_calls_foreign.h */, 29F3D08021039A630082DD88 /* call_wren_call_root.c */, 29F3D07F21039A630082DD88 /* call_wren_call_root.h */, - 29D009A61B7E3993000CE58C /* main.c */, 293D46941BB43F9900200083 /* call.c */, 293D46951BB43F9900200083 /* call.h */, 29AD965F1D0A57F800C4DFE7 /* error.c */, @@ -355,6 +359,7 @@ 29D009AD1B7E39A8000CE58C /* handle.h */, 29932D521C210F8D00099DEE /* lists.c */, 29932D531C210F8D00099DEE /* lists.h */, + 29D009A61B7E3993000CE58C /* main.c */, 29C946961C88F14F00B4A4F3 /* new_vm.c */, 29C946971C88F14F00B4A4F3 /* new_vm.h */, 29D880641DC8ECF600025364 /* reset_stack_after_call_abort.c */, @@ -527,6 +532,7 @@ 29D24DB21E82C0A2006618CC /* user_data.c in Sources */, 29DC14A61BBA3017008A8274 /* wren_primitive.c in Sources */, 29DC14A71BBA301A008A8274 /* wren_utils.c in Sources */, + 2993ED1D2111FD6D000F0D49 /* call_calls_foreign.c in Sources */, 29DC14A81BBA301D008A8274 /* wren_value.c in Sources */, 29A4273B1BDBE435001E6E22 /* wren_opt_random.wren.inc in Sources */, 29D025E61C19CD1000A3BB28 /* os.wren.inc in Sources */,