Leopold Toetsch writes:
> + if (arg->vtable->base_type == enum_class_Key) {
> + while (arg) {
> + UINTVAL flags = PObj_get_FLAGS(arg);
> + if (flags & KEY_register_FLAG) {
> + INTVAL n = PMC_int_val(arg);
> + if (flags & KEY_integer_FLAG)
> + REG_INT(n) = BP_REG_INT(bp, n);
> + else if (flags & KEY_pmc_FLAG)
> + REG_PMC(n) = BP_REG_PMC(bp, n);
> + else if (flags & KEY_string_FLAG)
> + REG_STR(n) = BP_REG_STR(bp, n);
> + }
> + arg = key_next(interpreter, arg);
> + }
> + }
Ack! I don't think that's going to do it. What if the
get_integer_keyed method were:
.namespace ["Foo"]
.sub __get_integer_keyed
.param pmc key
$S0 = "bar"
print "Key = "
print key
print "\n"
.return(0)
.end
The first line overwrites the register that was there before, and you
get:
Key = foo
Key = bar
(Assuming that $S0 was coincidentally mapped to the same register...
which it was)
Even more subtle and no less wrong.
This patch solves the problem completely, but I'm wary of putting it in
because it's so evil ugly. Is it safe to do this?
Index: src/inter_run.c
===================================================================
RCS file: /cvs/public/parrot/src/inter_run.c,v
retrieving revision 1.20
diff -u -u -r1.20 inter_run.c
--- src/inter_run.c 22 Nov 2004 12:07:22 -0000 1.20
+++ src/inter_run.c 22 Nov 2004 12:51:02 -0000
@@ -202,7 +202,7 @@
STRING *meth, const char* sig, va_list ap)
{
opcode_t offset, *dest;
- struct parrot_regs_t *bp;
+ struct parrot_regs_t *bp, *old_bp;
int next[4], count[4];
int i;
PMC *ret_c;
@@ -284,30 +284,21 @@
break;
case 'P': /* REG_PMC */
arg = va_arg(ap, PMC*);
+
+ /* Deal with key arguments, which might reference registers */
+ if (arg->vtable->base_type == enum_class_Key) {
+ /* XXX EVIL! Making this work in a non-evil way requires
+ * a major rethinking in how this function operates... */
+ old_bp = interpreter->ctx.bp;
+ interpreter->ctx.bp = bp;
+ arg = VTABLE_clone(interpreter, arg);
+ interpreter->ctx.bp = old_bp;
+ }
+
if (next[2] == 16)
VTABLE_set_pmc_keyed_int(interpreter, p3, i++, arg);
else
REG_PMC(next[2]++) = arg;
- /*
- * if this is a Key PMC with registers, pass on these
- * registers.
- * XXX make a distinct 'K' signature ?
- */
- if (arg->vtable->base_type == enum_class_Key) {
- while (arg) {
- UINTVAL flags = PObj_get_FLAGS(arg);
- if (flags & KEY_register_FLAG) {
- INTVAL n = PMC_int_val(arg);
- if (flags & KEY_integer_FLAG)
- REG_INT(n) = BP_REG_INT(bp, n);
- else if (flags & KEY_pmc_FLAG)
- REG_PMC(n) = BP_REG_PMC(bp, n);
- else if (flags & KEY_string_FLAG)
- REG_STR(n) = BP_REG_STR(bp, n);
- }
- arg = key_next(interpreter, arg);
- }
- }
break;
case 'N': /* REG_NUM */
if (next[3] == 16)
