From: Jonathan Worthington <[EMAIL PROTECTED]>
Date: Tue, 26 Dec 2006 19:36:58 +0000
Nikolay Ananiev wrote:
> The attached patch fixes the problem. The code is taken from
> Parrot_call_sub (which works). But I don't know why it was not
> applied to the other two function. Maybe it's just a hack?
Perhaps, but I have applied it since it's consistent with
Parrot_call_sub. I'd like to poke into this a bit deeper . . .
I am leaving this ticket open until I or @other digs deeper into whether
there's a better answer to this, and we look into Parrot_call_method.
Thank you,
Jonathan
AFAICS, the old code failed because Parrot_switch_to_cs (which is what
normally sets ctx->contants in a sub call) is not given a chance to run
in this situation. Parrot_loadbc sets interp->code without doing any of
the other Parrot_switch_to_cs bookeeping. Sub:invoke subsequently finds
that sub->seg is already installed properly in interp->code, so it skips
calling Parrot_switch_to_cs, leaving ctx->contants uninitialized.
Dropping the interp->code assignment in Parrot_loadbc sorta works
(after telling RetContinuation:invoke not to panic if no returning code
segment), but causes other failures. Replacing the assignment with a
Parrot_switch_to_cs call fails immediately; this might be made to work,
but it would be setting the constants in the old context, just as
Nikolay's patch (and Parrot_call_sub) do; this seems wrong.
The attached patch changes Sub:invoke to call Parrot_switch_to_cs
unconditionally. This seems to work without breaking anything else, and
also allows all these other extracurricular assignments to
ctx->constants to go away. But there may be something cleaner still
(and the code works now), so I too will wait to hear from @other (I
suspect they are mostly on vacation -- ;-).
-- Bob Rogers
http://rgrjr.dyndns.org/
* src/pmc/sub.pmc:
+ (Sub:invoke): Do Parrot_switch_to_cs unconditionally. This fixes a bug
in embedding reported by Nikolay Ananiev (RT #41132).
* src/packfile.c:
+ (run_sub): Remove premature ctx->constants init.
+ (Parrot_switch_to_cs): Print a trace message only if really going
to a new segment.
* src/embed.c:
+ (Parrot_runcode): Remove premature ctx->constants init.
* src/extend.c:
+ (Parrot_call_sub): Ditto.
+ (Parrot_call_sub_ret_float): Ditto.
+ (Parrot_call_sub_ret_int): Ditto.
Diffs between last version checked in and current workfile(s):
Index: src/pmc/sub.pmc
===================================================================
--- src/pmc/sub.pmc (revision 16270)
+++ src/pmc/sub.pmc (working copy)
@@ -305,10 +305,8 @@
sub->lex_info);
VTABLE_set_pointer(INTERP, context->lex_pad, context);
}
- /* switch code segment if needed */
- if (INTERP->code != sub->seg) {
- Parrot_switch_to_cs(INTERP, sub->seg, 1);
- }
+ /* switch code segment */
+ Parrot_switch_to_cs(INTERP, sub->seg, 1);
if (PObj_get_FLAGS(ccont) & SUB_FLAG_TAILCALL) {
if (!(*pc == PARROT_OP_get_params_pc ||
(*pc == PARROT_OP_push_eh_ic &&
Index: src/packfile.c
===================================================================
--- src/packfile.c (revision 16270)
+++ src/packfile.c (working copy)
@@ -277,8 +277,6 @@
interp->run_core != PARROT_SLOW_CORE &&
interp->run_core != PARROT_FAST_CORE)
interp->run_core = PARROT_FAST_CORE;
- CONTEXT(interp->ctx)->constants =
- interp->code->const_table->constants;
retval = Parrot_runops_fromc_args(interp, sub_pmc, "P");
interp->run_core = old;
return retval;
@@ -2466,9 +2464,11 @@
/* compiling source code uses this function too,
* which gives misleading trace messages
*/
- if (really && Interp_trace_TEST(interp, PARROT_TRACE_SUB_CALL_FLAG)) {
- Interp *tracer = interp->debugger ?
- interp->debugger : interp;
+ if (really
+ && Interp_trace_TEST(interp, PARROT_TRACE_SUB_CALL_FLAG)
+ /* print only if it's a real switch. */
+ && cur_cs != new_cs) {
+ Interp *tracer = interp->debugger ? interp->debugger : interp;
PIO_eprintf(tracer, "*** switching to %s\n",
new_cs->base.name);
}
Index: src/embed.c
===================================================================
--- src/embed.c (revision 16270)
+++ src/embed.c (working copy)
@@ -801,8 +801,6 @@
main_sub = set_current_sub(interp);
}
CONTEXT(interp->ctx)->current_sub = NULL;
- CONTEXT(interp->ctx)->constants =
- interp->code->const_table->constants;
Parrot_runops_fromc_args(interp, main_sub, "vP", userargv);
}
Index: src/extend.c
===================================================================
--- src/extend.c (revision 16270)
+++ src/extend.c (working copy)
@@ -750,8 +750,6 @@
PARROT_CALLIN_START(interp);
va_start(ap, signature);
- CONTEXT(interp->ctx)->constants =
- PMC_sub(sub)->seg->const_table->constants;
result = Parrot_runops_fromc_arglist(interp, sub, signature, ap);
va_end(ap);
@@ -769,8 +767,6 @@
PARROT_CALLIN_START(interp);
va_start(ap, signature);
- CONTEXT(interp->ctx)->constants =
- PMC_sub(sub)->seg->const_table->constants;
result = Parrot_runops_fromc_arglist_reti(interp, sub, signature, ap);
va_end(ap);
@@ -788,8 +784,6 @@
PARROT_CALLIN_START(interp);
va_start(ap, signature);
- CONTEXT(interp->ctx)->constants =
- PMC_sub(sub)->seg->const_table->constants;
result = Parrot_runops_fromc_arglist_retf(interp, sub, signature, ap);
va_end(ap);
End of diffs.
