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");

Reply via email to