The attached patch consolidates most of the existing stack-unwinding
code into Continuation:invoke; previously, RetContinuation:invoke and
find_exception_handler also did stack-unwinding, and none of the three
did it quite the same way.
Here are the effects:
1. Improved code sharing, a prerequisite for improving semantics.
2. Actions are now triggered when a continuation is called, fixing
two TODO cases (and also two others which were looking for the buggy
behavior). Previously, this only worked for exception handling and
RetContinuation.
3. Actions are no longer triggered as a side-effect of discovering
that there are no active exception handlers, which is more in the spirit
of PDD23.
4. Variable names are now more consistent between
Continuation:invoke and RetContinuation:invoke (or what's left of it).
Questions:
1. I notice that parameter passing is handled by Continuation:invoke
but not by RetContinuation:invoke. Currently I have to use some
cumbersome "Am I a RetContinuation?" logic in what is supposed to be the
generic method in order to deal with this. It would be nicer if they
both passed parameters in the same way. Is there some reason this can't
be done?
2. The exception handling case is not as efficient as it could be
(quadratic handler search followed by re-searching for the handler's
stack entry). This would be helped by splitting the unwinding code into
a separate routine, and calling it from each of the invoke methods,
rather than trying to use inheritance. But I assume this will need to
be rewritten anyway when PDD23 is fully implemented. With that in mind,
is this OK for now?
3. Around src/exceptions.c:268 (unpatched) there is an assignment of
e->entry_type to NO_STACK_ENTRY_TYPE, after a comment explaining why.
The patch disables this assignment, with no ill effect. If the code is
still needed for some reason, it will have to be moved to
RetContinuation:invoke, but I don't have a test case to ensure that I've
done this right. Should I just chuck it?
Suggestions are cordially invited. If I hear no complaints by
tomorrow night, I will assume all answers are "yes," and commit this (or
something close to it).
-- Bob Rogers
http://rgrjr.dyndns.org/
Index: src/pmc/retcontinuation.pmc
===================================================================
--- src/pmc/retcontinuation.pmc (revision 14640)
+++ src/pmc/retcontinuation.pmc (working copy)
@@ -76,55 +76,19 @@
=item C<void *invoke(void *next)>
-Restores the "context" by calling the superclass's C<invoke()> and places
-the frame pointer in the stack frame cache.
+Transfers control to the calling context, and frees the current context.
=cut
*/
void* invoke (void* next) {
- Stack_Chunk_t *ctr_stack, *stack_now;
struct Parrot_cont * cc = PMC_cont(SELF);
- parrot_context_t *caller_ctx;
- struct PackFile_ByteCode *seg; /* bytecode segment */
- /*
- * unwind control stack
- */
- stack_now = CONTEXT(INTERP->ctx)->control_stack;
- caller_ctx = cc->to_ctx;
- ctr_stack = caller_ctx->control_stack;
- while (stack_now != ctr_stack) {
- if (!stack_now || !ctr_stack)
- internal_exception(1, "Control stack damaged");
- /*
- * this automagically runs all pushed action
- * handler during pop - see cleanup stuff
- * in stack_pop
- */
- (void)stack_pop(INTERP, &stack_now,
- NULL, NO_STACK_ENTRY_TYPE);
- }
+ parrot_context_t *from_ctx = cc->from_ctx;
+ struct PackFile_ByteCode * const seg = cc->seg;
- /* debug print before context is switched */
- if (Interp_trace_TEST(INTERP, PARROT_TRACE_SUB_CALL_FLAG)) {
- Interp *tracer;
-
- PMC *to_sub = caller_ctx->current_sub;
- PMC *from_sub = cc->from_ctx->current_sub;
-
- tracer = interpreter->debugger ?
- interpreter->debugger : interpreter;
- PIO_eprintf(tracer, "# Back in sub '%Ss' from '%Ss'\n",
- Parrot_full_sub_name(INTERP, to_sub),
- Parrot_full_sub_name(INTERP, from_sub));
- }
- CONTEXT(INTERP->ctx) = caller_ctx;
- INTERP->ctx.bp = caller_ctx->bp;
- INTERP->ctx.bp_ps = caller_ctx->bp_ps;
- next = cc->address;
- Parrot_free_context(INTERP, cc->from_ctx, 1);
- seg = cc->seg;
+ next = SUPER(next);
+ Parrot_free_context(INTERP, from_ctx, 1);
#ifdef NDEBUG
/* the continuation is dead - delete and destroy it */
mem_sys_free(cc);
@@ -147,7 +111,6 @@
*/
cc->to_ctx = NULL;
#endif
- INTERP->current_args = NULL;
if (INTERP->code != seg) {
Parrot_switch_to_cs(INTERP, seg, 1);
}
Index: src/pmc/continuation.pmc
===================================================================
--- src/pmc/continuation.pmc (revision 14640)
+++ src/pmc/continuation.pmc (working copy)
@@ -224,15 +224,25 @@
void* invoke (void* next) {
struct Parrot_cont * cc = PMC_cont(SELF);
- parrot_context_t *caller_ctx;
- parrot_context_t *ctx;
- opcode_t *pc;
+ Stack_Chunk_t *stack_target;
+ parrot_context_t *from_ctx = CONTEXT(INTERP->ctx);
+ parrot_context_t *to_ctx = cc->to_ctx;
+ opcode_t *pc = cc->address;
+ /* [bug: these should be 'isa' tests. -- rgr, 17-Sep-06.] */
+ int exception_continuation_p
+ = SELF->vtable->base_type == enum_class_Exception_Handler;
+ int ret_continuation_p
+ = SELF->vtable->base_type == enum_class_RetContinuation;
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
runloop 0 is for the return from the initial sub call. */
- && cc->runloop_id != 0) {
+ && cc->runloop_id != 0
+ /* since a RetContinuation [currently] only returns to the next
+ outer frame, exiting to the inner run loop does the right thing,
+ since it normally returns to the next outer runloop anyway. */
+ && ! ret_continuation_p) {
fprintf(stderr, "[oops; continuation %p of type %d "
"is trying to jump from runloop %d to runloop %d]\n",
SELF, (int) SELF->vtable->base_type,
@@ -242,34 +252,89 @@
if (Interp_debug_TEST(interpreter, PARROT_CTX_DESTROY_DEBUG_FLAG)) {
fprintf(stderr,
"[invoke cont %p, to_ctx %p, from_ctx %p (refs %d)]\n",
- SELF, cc->to_ctx, cc->from_ctx,
- (int) cc->from_ctx->ref_count);
+ SELF, to_ctx, from_ctx, (int) from_ctx->ref_count);
}
#endif
- if (! cc->to_ctx) {
+ if (! to_ctx) {
real_exception(interpreter, NULL, INVALID_OPERATION,
"Continuation invoked after deactivation.");
}
+
+ /* figure out how far to unwind. */
+ if (exception_continuation_p) {
+ /* if we are an exception handler, then we must pop ourself and
+ everything above. */
+ stack_target = from_ctx->control_stack;
+ while (stack_target
+ && UVal_pmc(((Stack_Entry_t
*)STACK_DATAP(stack_target))->entry) != SELF) {
+ stack_target = stack_target->prev;
+ }
+ if (! stack_target) {
+ internal_exception(ERROR_STACK_EMPTY,
+ "Can't find handler on control stack!");
+ }
+ /* move down one more to pop the EH itself. */
+ stack_target = stack_target->prev;
+ }
+ else {
+ stack_target = to_ctx->control_stack;
+ }
+
+ /*
+ * unwind control stack
+ */
+ while (from_ctx->control_stack != stack_target) {
+ PMC *cleanup_sub = NULL;
+ Stack_Entry_t *e;
+
+ if (! from_ctx->control_stack)
+ internal_exception(1, "Control stack damaged");
+ e = stack_entry(interpreter, from_ctx->control_stack, 0);
+ if (e->entry_type == STACK_ENTRY_ACTION) {
+ /*
+ * Disable automatic cleanup routine execution in stack_pop so
+ * that we can run the action subroutine manually. This is
+ * because we have to run the sub AFTER it has been popped,
lest
+ * a new error in the sub cause an infinite loop when invoking
+ * an error handler.
+ */
+ cleanup_sub = UVal_pmc(e->entry);
+ e->cleanup = STACK_CLEANUP_NULL;
+ }
+ (void)stack_pop(INTERP, &from_ctx->control_stack,
+ 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);
+ }
+ }
+
/* debug print before context is switched */
if (Interp_trace_TEST(INTERP, PARROT_TRACE_SUB_CALL_FLAG)) {
- PMC *sub = cc->to_ctx->current_sub;
+ PMC *sub = to_ctx->current_sub;
PIO_eprintf(INTERP, "# Back in sub '%Ss'\n",
Parrot_full_sub_name(INTERP, sub));
}
- caller_ctx = CONTEXT(INTERP->ctx);
+
/*
* set context
*/
- CONTEXT(INTERP->ctx) = ctx = cc->to_ctx;
- INTERP->ctx.bp = ctx->bp;
- INTERP->ctx.bp_ps = ctx->bp_ps;
+ CONTEXT(INTERP->ctx) = to_ctx;
+ INTERP->ctx.bp = to_ctx->bp;
+ INTERP->ctx.bp_ps = to_ctx->bp_ps;
+ if (ret_continuation_p) {
+ /* RetContinuation arg passing is handled elsewhere. */
+ return pc;
+ }
+
+ /* pass args */
if (cc->current_results) {
/* where caller wants result */
- ctx->current_results = cc->current_results;
+ to_ctx->current_results = cc->current_results;
}
- pc = cc->address;
- if (ctx->current_results && INTERP->current_args) {
+ if (to_ctx->current_results && INTERP->current_args) {
/*
* the register pointer is already switched back
* to the caller, therefore the registers of the
@@ -279,11 +344,13 @@
*/
Parrot_block_DOD(INTERP);
parrot_pass_args(INTERP,
- caller_ctx,
- ctx,
+ from_ctx,
+ to_ctx,
PARROT_OP_set_args_pc);
Parrot_unblock_DOD(INTERP);
}
+
+ /* switch segment */
INTERP->current_args = NULL;
if (INTERP->code != cc->seg) {
Parrot_switch_to_cs(INTERP, cc->seg, 1);
Index: src/exceptions.c
===================================================================
--- src/exceptions.c (revision 14640)
+++ src/exceptions.c (working copy)
@@ -226,33 +226,18 @@
STRING *message;
char *m;
int exit_status, print_location;
+ int depth = 0;
+ Stack_Entry_t * const e;
+
/* for now, we don't check the exception class and we don't
- * look for matching handlers
+ * look for matching handlers. [this is being redesigned anyway.]
*/
message = VTABLE_get_string_keyed_int(interpreter, exception, 0);
- do {
- PMC *cleanup_sub = NULL;
- Stack_Entry_t * const e = stack_entry(interpreter,
- CONTEXT(interpreter->ctx)->control_stack, 0);
-
- if (!e)
- break;
- if (e->entry_type == STACK_ENTRY_ACTION) {
- /*
- * Disable automatic cleanup routine execution in stack_pop so that
- * we can run the action subroutine manually with an INTVAL
argument
- * of 1. Note that we have to run the sub AFTER it has been
popped,
- * lest a new error in the sub cause an infinite loop.
- */
- cleanup_sub = UVal_pmc(e->entry);
- e->cleanup = STACK_CLEANUP_NULL;
- }
- (void)stack_pop(interpreter, &CONTEXT(interpreter->ctx)->control_stack,
- NULL, e->entry_type);
- if (cleanup_sub) {
- /* Now it's safe to run. */
- Parrot_runops_fromc_args(interpreter, cleanup_sub, "vI", 1);
- }
+ /* [TODO: replace quadratic search with something linear, hopefully without
+ trashing abstraction layers. -- rgr, 17-Sep-06.] */
+ while (e = stack_entry(interpreter,
+ CONTEXT(interpreter->ctx)->control_stack,
+ depth)) {
if (e->entry_type == STACK_ENTRY_PMC) {
/*
* During interpreter creation there is an initial context
@@ -265,14 +250,17 @@
*
* So invalidate entry_type.
*/
- e->entry_type = NO_STACK_ENTRY_TYPE;
+ /* [i think this is probably broken. -- rgr, 17-Sep-06.] */
+ /* e->entry_type = NO_STACK_ENTRY_TYPE; */
handler = UVal_pmc(e->entry);
if (handler && handler->vtable->base_type ==
enum_class_Exception_Handler) {
return handler;
}
}
- } while (1);
+ depth++;
+ }
+
/* flush interpreter output to get things printed in order */
PIO_flush(interpreter, PIO_STDOUT(interpreter));
PIO_flush(interpreter, PIO_STDERR(interpreter));
Index: t/pmc/exception.t
===================================================================
--- t/pmc/exception.t (revision 14640)
+++ t/pmc/exception.t (working copy)
@@ -433,11 +433,12 @@
.end
.sub action
- print "never\n"
+ print "unwind\n"
.end
CODE
main
foo
+unwind
back
OUTPUT
@@ -459,36 +460,17 @@
.end
.sub action
- print "never\n"
+ print "unwind\n"
.end
CODE
main
foo
+unwind
back
OUTPUT
-pir_output_is(<<'CODE', <<'OUTPUT', "register corruption - implicit P5");
+pir_output_is(<<'CODE', <<'OUTPUT', 'cleanup global: continuation');
.sub main :main
- null P0
- null P1
- null P2
- null P3
- I0 = 1
- I1 = 2
- push_eh coke
- $P0 = global 'no coke'
-coke:
- print_item I0
- print_item I1
- print_newline
-.end
-CODE
-1 2
-OUTPUT
-
-## this is broken; continuation calling does not execute actions when
unwinding.
-pir_output_is(<<'CODE', <<'OUTPUT', 'cleanup global: continuation', todo =>
'bug');
-.sub main :main
.local pmc outer, cont
outer = new String
outer = "Outer value\n"
@@ -735,13 +717,10 @@
.end
CODE
/^main
-at_exit, flag = 1
No exception handler/
OUTPUT
-# creating the Closure converts RetContinuation to Continuations
-# which dont trigger acction handlers
-pir_output_is(<<'CODE', <<'OUTPUT', "pushaction as closure", todo =>
'Continuation');
+pir_output_is(<<'CODE', <<'OUTPUT', "pushaction as closure");
.sub main :main
.local pmc a
.lex 'a', a
@@ -770,6 +749,38 @@
a = 42
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');
+.sub main :main
+ push_eh h
+ print "main\n"
+ .const .Sub at_exit = "exit_handler"
+ pushaction at_exit
+ $P1 = new .Exception
+ 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"
+.end
+
+.sub exit_handler :outer(main)
+ .param int flag
+ print_item "at_exit, flag ="
+ print_item flag
+ print_newline
+ $P2 = new .Exception
+ throw $P2
+ print "never 2\n"
+.end
+CODE
+/^main
+at_exit, flag = 1
+No exception handler/
+OUTPUT
+
pir_output_is(<<'CODE', <<'OUTPUT', "exit_handler via exit exception");
.sub main :main
.local pmc a
