Skip to content

Commit

Permalink
coro: refactor return code. problem: wrong continuation
Browse files Browse the repository at this point in the history
let Parrot_sub_continuation_rewind_environment return from_sub
for continuation.invoke.
let yield carry a 2nd state: 2 for .return
fix .return() mostly [GH# 1106]
remove unneeded empty ctx check in coroutine.invoke back from coro

changed t/pmc/coroutine.t to match the new implementation.
Fixed: 9, 13, 14, 15, 16
Remaining todo:
-  2: wrong returncc cont to label, regression (also syn_7)
- 14: wrong cont
  • Loading branch information
Reini Urban committed Oct 23, 2014
1 parent 8fc1305 commit 5fb0926
Show file tree
Hide file tree
Showing 6 changed files with 100 additions and 67 deletions.
4 changes: 3 additions & 1 deletion ChangeLog
Expand Up @@ -32,7 +32,9 @@
Removed defunct Parrot_ascii_typetable, unused since 2010.
+ Protect callmethod from an empty object, missed by fixup_subs
immediate. #1024
+ Add Couroutine reset and autoreset methods. #564
+ Add Coroutine reset and autoreset methods. #564
+ Fixed Coroutine return and tests, with one returncc regression
and one n_regs_used for autoreset bug left. #1106
+ Add -t8 trace flag for Coroutine states
+ Disabled trace flags -t4 and -t8 with --optimize. #1105
+ Harmonize parrot usage messages
Expand Down
3 changes: 2 additions & 1 deletion include/parrot/sub.h
Expand Up @@ -201,7 +201,8 @@ void Parrot_sub_continuation_check(PARROT_INTERP, ARGIN(const PMC *pmc))
__attribute__nonnull__(1)
__attribute__nonnull__(2);

void Parrot_sub_continuation_rewind_environment(PARROT_INTERP,
PARROT_CAN_RETURN_NULL
PMC * Parrot_sub_continuation_rewind_environment(PARROT_INTERP,
ARGIN(PMC *pmc))
__attribute__nonnull__(1)
__attribute__nonnull__(2);
Expand Down
32 changes: 29 additions & 3 deletions src/pmc/continuation.pmc
Expand Up @@ -22,6 +22,7 @@ more information.

#include "parrot/oplib/ops.h"
#include "pmc/pmc_sub.h"
#include "pmc/pmc_coroutine.h"

/*

Expand Down Expand Up @@ -286,17 +287,42 @@ destination to continue execution.
*/

VTABLE opcode_t *invoke(void *next) :no_wb {
UNUSED(next)
PMC * const from_obj = Parrot_pcc_get_signature(INTERP, CURRENT_CONTEXT(INTERP));
opcode_t *pc;
PackFile_ByteCode *seg;
PMC *from_sub;
PMC * const from_obj = Parrot_pcc_get_signature(INTERP, CURRENT_CONTEXT(INTERP));
UNUSED(next)

GET_ATTR_seg(INTERP, SELF, seg);
GET_ATTR_address(INTERP, SELF, pc);
SET_ATTR_invoked(INTERP, SELF, 1);

Parrot_sub_continuation_check(INTERP, SELF);
Parrot_sub_continuation_rewind_environment(INTERP, SELF);
from_sub = Parrot_sub_continuation_rewind_environment(INTERP, SELF);

/* if return from a coro */
if (from_sub && from_sub->vtable->base_type == enum_class_Coroutine) {
INTVAL autoreset;
GETATTR_Coroutine_autoreset(INTERP, from_sub, autoreset);
if (autoreset) {
#ifndef NDEBUG
if (Interp_trace_TEST(INTERP, PARROT_TRACE_SUB_CALL_FLAG))
Parrot_io_eprintf(INTERP, "# Coroutine autoreset '%Ss'\n",
Parrot_sub_full_sub_name(INTERP, from_sub));
#endif
SETATTR_Coroutine_ctx(INTERP, from_sub, PMCNULL);
}
else {
#ifndef NDEBUG
if (Interp_trace_TEST(INTERP, PARROT_TRACE_SUB_CALL_FLAG))
Parrot_io_eprintf(INTERP, "# Coroutine no autoreset '%Ss'\n",
Parrot_sub_full_sub_name(INTERP, from_sub));
#endif
}
GETATTR_Coroutine_address(INTERP, from_sub, pc);
SETATTR_Coroutine_yield(INTERP, from_sub, 2);
pc = VTABLE_invoke(INTERP, from_sub, pc);
}

if (!PMC_IS_NULL(from_obj))
Parrot_pcc_set_signature(INTERP, CURRENT_CONTEXT(INTERP), from_obj);
Expand Down
39 changes: 19 additions & 20 deletions src/pmc/coroutine.pmc
Expand Up @@ -48,13 +48,15 @@ static void print_sub_name(PARROT_INTERP, ARGIN(PMC *sub_pmc))
#ifdef NDEBUG
# define TRACE_CORO(s)
# define TRACE_CORO_1(s, arg)
# define TRACE_CORO_d(s, arg)
#else
# define TRACE_CORO(s) \
if (Interp_trace_TEST(interp, PARROT_TRACE_CORO_STATE_FLAG)) \
fprintf(stderr, (s))
# define TRACE_CORO_1(s, arg) \
if (Interp_trace_TEST(interp, PARROT_TRACE_CORO_STATE_FLAG)) \
fprintf(stderr, (s), (arg))
# define TRACE_CORO_d(s, arg) TRACE_CORO_1(s, arg)
#endif


Expand All @@ -81,7 +83,7 @@ print_sub_name(PARROT_INTERP, ARGIN(PMC *sub_pmc))
GETATTR_Coroutine_ctx(interp, sub_pmc, ctx);
GETATTR_Coroutine_yield(interp, sub_pmc, yield);

if (!yield && (PObj_get_FLAGS(sub_pmc) & SUB_FLAG_CORO_FF))
if (yield != 1 && (PObj_get_FLAGS(sub_pmc) & SUB_FLAG_CORO_FF))
Parrot_io_eprintf(tracer, "# %s coroutine '%Ss'",
"returning from",
Parrot_sub_full_sub_name(interp, sub_pmc));
Expand Down Expand Up @@ -226,8 +228,9 @@ yields are exhausted and the coro is dead.
PMC *ctx;
opcode_t *dest;
PackFile_ByteCode *wanted_seg;
PMC * const signature = Parrot_pcc_get_signature(INTERP,
CURRENT_CONTEXT(INTERP));
PMC * const caller_ctx = CURRENT_CONTEXT(INTERP);
PMC * const signature = Parrot_pcc_get_signature(INTERP,
caller_ctx);

#ifndef NDEBUG
if (Interp_trace_TEST(INTERP, PARROT_TRACE_SUB_CALL_FLAG))
Expand All @@ -240,7 +243,6 @@ yields are exhausted and the coro is dead.
size_t start_offs;
const UINTVAL *n_regs_used;
PMC *lex_info;
PMC * const caller_ctx = CURRENT_CONTEXT(INTERP);
PMC *ccont = INTERP->current_cont;

PARROT_ASSERT(!PMC_IS_NULL(ccont));
Expand All @@ -255,6 +257,8 @@ yields are exhausted and the coro is dead.
ctx = Parrot_pmc_new(INTERP, enum_class_CallContext);
TRACE_CORO("# - coro: first ctx\n");
Parrot_pcc_set_context(INTERP, ctx);
TRACE_CORO_d("# - coro: first ctx %p\n", caller_ctx);
TRACE_CORO_d("# - coro: to_ctx %p\n", ctx);

GET_ATTR_n_regs_used(INTERP, SELF, n_regs_used);
Parrot_pcc_allocate_registers(INTERP, ctx, n_regs_used);
Expand All @@ -267,6 +271,7 @@ yields are exhausted and the coro is dead.

Parrot_pcc_set_sub(INTERP, ctx, SELF);
Parrot_pcc_set_continuation(INTERP, ctx, ccont);
TRACE_CORO_d("# - coro: cont %p\n", ccont);

INTERP->current_cont = PMCNULL;

Expand Down Expand Up @@ -297,6 +302,9 @@ yields are exhausted and the coro is dead.
GET_ATTR_ctx(INTERP, SELF, ctx);
ccont = Parrot_pcc_get_continuation(INTERP, ctx);
TRACE_CORO("# - coro: !ff\n");
TRACE_CORO_d("# - coro: cont %p\n", ccont);
TRACE_CORO_d("# - coro: ctx %p\n", caller_ctx);
TRACE_CORO_d("# - coro: to_ctx %p\n", ctx);
PObj_get_FLAGS(SELF) |= SUB_FLAG_CORO_FF;

GET_ATTR_seg(INTERP, SELF, seg);
Expand All @@ -306,13 +314,13 @@ yields are exhausted and the coro is dead.
SET_ATTR_caller_seg(INTERP, SELF, INTERP->code);

/* and the recent call context */
SETATTR_Continuation_to_ctx(INTERP, ccont, CURRENT_CONTEXT(INTERP));
Parrot_pcc_set_caller_ctx(INTERP, ctx, CURRENT_CONTEXT(INTERP));
SETATTR_Continuation_to_ctx(INTERP, ccont, caller_ctx);
Parrot_pcc_set_caller_ctx(INTERP, ctx, caller_ctx);

/* set context to coroutine context */
Parrot_pcc_set_context(INTERP, ctx);
}
else { /* FF: yield or returncc from the Coro back */
else { /* FF: yield 1 or returncc 2 back from the Coro */
INTVAL yield;
PMC *ccont, *to_ctx;
PackFile_ByteCode *caller_seg;
Expand All @@ -329,23 +337,14 @@ yields are exhausted and the coro is dead.
ccont = Parrot_pcc_get_continuation(INTERP, ctx);

GETATTR_Continuation_to_ctx(INTERP, ccont, to_ctx);
TRACE_CORO_d("# - coro: cont %p\n", ccont);
TRACE_CORO_d("# - coro: ctx %p\n", ctx);
TRACE_CORO_d("# - coro: to_ctx %p\n", to_ctx);
PObj_get_FLAGS(SELF) &= ~SUB_FLAG_CORO_FF;
GET_ATTR_caller_seg(INTERP, SELF, caller_seg);

GET_ATTR_caller_seg(INTERP, SELF, caller_seg);
/* switch back to last remembered code seg and context */
wanted_seg = caller_seg;

if (PMC_IS_NULL(to_ctx)) {
/* This still isn't quite right, but it beats segfaulting. See
the "Call an exited coroutine" case in t/pmc/coroutine.t; the
problem is that the defunct coroutine yields up one more
result before we get here. -- rgr, 7-Oct-06.
* This may be unneeded after the yield fix, see TT #1003
*/
Parrot_ex_throw_from_c_args(INTERP, NULL, EXCEPTION_INVALID_OPERATION,
"Cannot resume dead coroutine. Invalid context");
}

Parrot_pcc_set_context(INTERP, to_ctx);
}

Expand Down
32 changes: 7 additions & 25 deletions src/sub.c
Expand Up @@ -478,7 +478,7 @@ Parrot_sub_continuation_check(PARROT_INTERP, ARGIN(const PMC *pmc))

/*
=item C<void Parrot_sub_continuation_rewind_environment(PARROT_INTERP, PMC
=item C<PMC * Parrot_sub_continuation_rewind_environment(PARROT_INTERP, PMC
*pmc)>
Restores the appropriate context for the continuation.
Expand All @@ -487,37 +487,17 @@ Restores the appropriate context for the continuation.
*/

void
PARROT_CAN_RETURN_NULL
PMC *
Parrot_sub_continuation_rewind_environment(PARROT_INTERP, ARGIN(PMC *pmc))
{
ASSERT_ARGS(Parrot_sub_continuation_rewind_environment)

PMC * const ctx = CURRENT_CONTEXT(interp);
PMC * const to_ctx = PARROT_CONTINUATION(pmc)->to_ctx;
PMC * const ctx = CURRENT_CONTEXT(interp);
PMC * to_ctx = PARROT_CONTINUATION(pmc)->to_ctx;
PMC * const sig = Parrot_pcc_get_signature(interp, ctx);
PMC * const from_sub = Parrot_pcc_get_sub(interp, ctx);

/* A yield could not bring us here */
if (from_sub && from_sub->vtable->base_type == enum_class_Coroutine) {
INTVAL autoreset;
GETATTR_Coroutine_autoreset(interp, from_sub, autoreset);
if (autoreset) {
#ifndef NDEBUG
if (Interp_trace_TEST(interp, PARROT_TRACE_SUB_CALL_FLAG))
Parrot_io_eprintf(interp, "# Coroutine autoreset '%Ss'\n",
Parrot_sub_full_sub_name(interp, from_sub));
#endif
SETATTR_Coroutine_ctx(interp, from_sub, PMCNULL);
}
#ifndef NDEBUG
else {
if (Interp_trace_TEST(interp, PARROT_TRACE_SUB_CALL_FLAG))
Parrot_io_eprintf(interp, "# Coroutine no autoreset '%Ss'\n",
Parrot_sub_full_sub_name(interp, from_sub));
}
#endif
}

#ifndef NDEBUG
/* debug print before context is switched */
if (Interp_trace_TEST(interp, PARROT_TRACE_SUB_CALL_FLAG)) {
Expand All @@ -530,9 +510,11 @@ Parrot_sub_continuation_rewind_environment(PARROT_INTERP, ARGIN(PMC *pmc))
/* set context */
Parrot_pcc_set_context(interp, to_ctx);
Parrot_pcc_set_signature(interp, to_ctx, sig);
return from_sub;
}



/*
=item C<void * Parrot_get_sub_pmc_from_subclass(PARROT_INTERP, PMC *subclass)>
Expand Down

0 comments on commit 5fb0926

Please sign in to comment.