Almost two weeks ago, I had what I thought was a clever idea for eliminating the continuation barrier from action invocation: Simply call the action using the original continuation instead of creating a new RetContinuation. The original continuation, I reasoned, should be re-entrant after having the dynamic environment partially restored.
Unfortunately, it's not that easy. The trick is that you need to pass values from the original context, not the one that is exiting. And exception handling is, well, exceptional: The values come from C code, and not any context. So I had to add a 'payload' slot to struct Parrot_cont in order to remember the exception; I had been hoping to avoid extra state. Furthermore, there is no way for the vtable method of a superclass to abandon the rest of the subclass method, so I used some flag-setting kludgery to hack around that [1]. And it doesn't even quite work; in r15040, I get the following additional test failures: Failed Test Stat Wstat Total Fail Failed List of Failed ------------------------------------------------------------------------- t/compilers/imcc/syn/pcc.t 1 256 21 1 4.76% 8 * t/library/mime_base64.t 1 256 552 1104 200.00% 1-552 t/op/gc.t 1 256 22 1 4.55% 13 t/pmc/eval.t 1 256 21 1 4.76% 19 * t/pmc/resizablestringarray.t 1 256 173 326 188.44% 11-173 (The ones marked with "*" also have some failures in vanilla r15040.) So what I want to know is: Do you think this approach is worth pursuing? I would certainly make it work before committing it, and would also want to make the flag-setting hack prettier (if I couldn't eliminate it altogether). There's also some "proxy" stuff I think I can get rid of. So it wouldn't be quite so ugly. I just want to know if you think it is worth the trouble. Or, I could sit tight and wait for a better way to return values from C, since that is what the handler-calling code needs to do, before or after PDD23 . . . -- Bob Rogers http://rgrjr.dyndns.org/ [1] I also merged Error_Handler:invoke into Continuation:invoke; it actually reduces the code volume, but flattens the abstraction, which doesn't feel quite right. But the point of the existing Error_Handler:invoke is to return values from C, which ought to be handled generally anyway. And I assume EH can go away after PDD23, replaced with a general Continuation. In fact, it may even be possible to get rid of it now.
Index: src/pmc/retcontinuation.pmc =================================================================== --- src/pmc/retcontinuation.pmc (revision 15040) +++ src/pmc/retcontinuation.pmc (working copy) @@ -88,6 +88,11 @@ struct PackFile_ByteCode * const seg = cc->seg; next = SUPER(next); + if (PObj_flag_TEST(private1, SELF)) { + PObj_flag_CLEAR(private1, SELF); + return next; + } + Parrot_free_context(INTERP, from_ctx, 1); #ifdef NDEBUG /* the continuation is dead - delete and destroy it */ Index: src/pmc/continuation.pmc =================================================================== --- src/pmc/continuation.pmc (revision 15040) +++ src/pmc/continuation.pmc (working copy) @@ -25,7 +25,60 @@ #include "parrot/oplib/ops.h" #include <assert.h> +void * +Parrot_invoke_with_continuation_args(Parrot_Interp interpreter, + PMC *sub, PMC *continuation, + const char* sig, ...) +{ + opcode_t *dest; + parrot_context_t *old_ctx = CONTEXT(interpreter->ctx); + parrot_context_t *orig_from_ctx = PMC_cont(continuation)->from_ctx; + /* Mark ourself so that RetContinuation:invoke (if continuation is a + RetContinuation) doesn't immediately destroy the returning context; + we will need it to complete arg passing. */ + PObj_flag_SET(private1, continuation); + + if (continuation->vtable->base_type == enum_class_Exception_Handler) { + /* create a new continuation, since Exception_Handler can't be recycled + this way. */ + PMC *eh = continuation; + continuation = new_ret_continuation_pmc(interpreter, NULL); + PMC_cont(continuation)->payload = eh; + } + + interpreter->current_cont = continuation; + interpreter->current_object = NULL; + dest = VTABLE_invoke(interpreter, sub, NULL); + if (!dest) + internal_exception(1, "Subroutine returned a NULL address"); + /* restore the original from_ctx (Sub:invoke bashes it). */ + PMC_cont(continuation)->from_ctx = orig_from_ctx; + + /* process args, if any. */ + if (sig && *sig) { + va_list args; + + va_start(args, sig); + dest = parrot_pass_args_fromc(interpreter, sig, dest, old_ctx, args); + va_end(args); + } + return dest; +} + +/* [copied from exception.pmc src. -- rgr, 22-Oct-06.] */ +static opcode_t * +pass_exception_args(Interp *interpreter, const char *sig, + opcode_t *dest, parrot_context_t * old_ctx, ...) +{ + va_list ap; + void *next; + va_start(ap, old_ctx); + next = parrot_pass_args_to_result(interpreter, sig, dest, old_ctx, ap); + va_end(ap); + return next; +} + /* =back @@ -89,6 +142,8 @@ mark_context(INTERP, cc->to_ctx); if (cc->dynamic_state) mark_stack(INTERP, cc->dynamic_state); + if (cc->payload) + pobject_lives(INTERP, (PObj *)cc->payload); } /* @@ -136,6 +191,7 @@ cc = new_continuation(INTERP, cc_self); cc->runloop_id = cc_self->runloop_id; cc->dynamic_state = cc_self->dynamic_state; + cc->payload = cc_self->payload; PMC_struct_val(ret) = cc; PMC_pmc_val(ret) = PMC_pmc_val(SELF); return ret; @@ -240,6 +296,26 @@ int ret_continuation_p = SELF->vtable->base_type == enum_class_RetContinuation; + /* if we are a proxy for another continuation, unpack its guts so we use + * those instead. + */ + if (cc->payload + && cc->payload->vtable->base_type != enum_class_Exception) { + PMC *payload_continuation = cc->payload; + + cc = PMC_cont(payload_continuation); + stack_target = cc->dynamic_state; + to_ctx = cc->to_ctx; + from_ctx = cc->from_ctx; + pc = cc->address; + exception_continuation_p + = payload_continuation->vtable->base_type == enum_class_Exception_Handler; + ret_continuation_p + = payload_continuation->vtable->base_type == enum_class_RetContinuation; + /* prevent further processing. */ + PObj_flag_SET(private1, SELF); + } + if (interpreter->current_runloop_id != cc->runloop_id /* it's ok if we are exiting to "runloop 0"; there is no such runloop, but the only continuation that thinks it came from @@ -310,9 +386,11 @@ (void)stack_pop(INTERP, &interpreter->dynamic_env, NULL, NO_STACK_ENTRY_TYPE); if (cleanup_sub) { - /* Now it's safe to run. */ - Parrot_runops_fromc_args(interpreter, cleanup_sub, - "vI", exception_continuation_p); + /* Now it's safe to run. Set up the call to the action sub, + using SELF as the continuation. */ + pc = Parrot_invoke_with_continuation_args(interpreter, cleanup_sub, SELF, + "I", exception_continuation_p); + return pc; } /* Keep corresponding_target in sync. If stack_delta is negative, @@ -356,7 +434,18 @@ /* where caller wants result */ to_ctx->current_results = cc->current_results; } - if (to_ctx->current_results && INTERP->current_args) { + if (! to_ctx->current_results) { + /* no values to return. */ + } + else if (exception_continuation_p) { + /* return specialized exception values. */ + PMC *exception = cc->payload; + STRING *message = VTABLE_get_string_keyed_int(INTERP, exception, 0); + + pc = pass_exception_args(interpreter, "PS", pc, + from_ctx, exception, message); + } + else if (INTERP->current_args) { /* * the register pointer is already switched back * to the caller, therefore the registers of the Index: src/pmc/exception_handler.pmc =================================================================== --- src/pmc/exception_handler.pmc (revision 15040) +++ src/pmc/exception_handler.pmc (working copy) @@ -81,32 +81,6 @@ PObj_custom_mark_CLEAR(result); return result; } - - void* invoke (void* ex) { - struct Parrot_cont * cc = PMC_cont(SELF); - PMC *exception = ex; - parrot_context_t *ex_ctx; - void *next = NULL; - opcode_t *results; - /* COMPAT: PMC *p5 = REG_PMC(5);*/ - - assert(cc->to_ctx == cc->from_ctx); - results = cc->current_results; - /* clear all results, so that continuation.invoke - * doesn't pass any args #' - */ - cc->to_ctx->current_results = cc->current_results= NULL; - ex_ctx = CONTEXT(INTERP->ctx); - next = SUPER(next); - if (results) { - STRING *message = VTABLE_get_string_keyed_int(INTERP, - exception, 0); - assert(next == results); - next = pass_exception_args(interpreter, "PS", next, - ex_ctx, exception, message); - } - return next; - } } /* Index: src/exceptions.c =================================================================== --- src/exceptions.c (revision 15040) +++ src/exceptions.c (working copy) @@ -395,7 +395,8 @@ if (!handler) return NULL; /* put the handler aka continuation ctx in the interpreter */ - address = VTABLE_invoke(interpreter, handler, exception); + PMC_cont(handler)->payload = exception; + address = VTABLE_invoke(interpreter, handler, NULL); /* address = VTABLE_get_pointer(interpreter, handler); */ if (PObj_get_FLAGS(handler) & SUB_FLAG_C_HANDLER) { /* its a C exception handler */ @@ -426,7 +427,8 @@ if (exception->vtable->base_type != enum_class_Exception) PANIC("Illegal rethrow"); handler = find_exception_handler(interpreter, exception); - address = VTABLE_invoke(interpreter, handler, exception); + PMC_cont(handler)->payload = exception; + address = VTABLE_invoke(interpreter, handler, NULL); /* return the address of the handler */ return address; } Index: src/sub.c =================================================================== --- src/sub.c (revision 15040) +++ src/sub.c (working copy) @@ -143,6 +143,7 @@ cc->to_ctx = to_ctx; cc->from_ctx = CONTEXT(interp->ctx); cc->dynamic_state = NULL; + cc->payload = NULL; cc->runloop_id = 0; CONTEXT(interp->ctx)->ref_count++; if (to) { @@ -176,6 +177,7 @@ cc->to_ctx = CONTEXT(interp->ctx); cc->from_ctx = NULL; /* filled in during a call */ cc->dynamic_state = NULL; + cc->payload = NULL; cc->runloop_id = 0; cc->seg = interp->code; cc->current_results = NULL; Index: include/parrot/sub.h =================================================================== --- include/parrot/sub.h (revision 15040) +++ include/parrot/sub.h (working copy) @@ -116,6 +116,7 @@ opcode_t *address; /* start of bytecode, addr to continue */ struct Parrot_Context *to_ctx; /* pointer to dest context */ struct Stack_Chunk *dynamic_state; /* dest dynamic state */ + PMC *payload; /* optional "payload"; see Exception_Handler */ /* a Continuation keeps the from_ctx alive */ struct Parrot_Context *from_ctx; /* sub, this cont is returning from */ opcode_t *current_results; /* ptr into code with get_results opcode Index: t/pmc/exception.t =================================================================== --- t/pmc/exception.t (revision 15040) +++ t/pmc/exception.t (working copy) @@ -582,21 +582,25 @@ No exception handler/ OUTPUT -# exception handlers are still run in an inferior runloop, which messes up -# nonlocal exit from within handlers. -pir_output_like(<<'CODE', <<'OUTPUT', "pushaction: error while handling error", todo => 'runloop shenanigans'); +pir_output_is(<<'CODE', <<'OUTPUT', "pushaction: error while handling error"); .sub main :main push_eh h print "main\n" .const .Sub at_exit = "exit_handler" pushaction at_exit $P1 = new .Exception + $P1 = new .Exception + $P1['_message'] = 'outer error' throw $P1 print "never 1\n" h: ## this is never actually reached, because exit_handler throws an unhandled ## exception before the handler is entered. - print "in outer handler\n" + .local pmc exception + .get_results (exception, $S0) + print "in outer handler, caught " + print exception + print "\n" .end .sub exit_handler :outer(main) @@ -605,13 +609,15 @@ print_item flag print_newline $P2 = new .Exception + $P2 = new .Exception + $P2['_message'] = 'handler error' throw $P2 print "never 2\n" .end CODE -/^main +main at_exit, flag = 1 -No exception handler/ +in outer handler, caught handler error OUTPUT pir_output_is(<<'CODE', <<'OUTPUT', "exit_handler via exit exception");