set_pmc_keyed_int delegates to set_pmc_keyed...?
Patrick R. Michaud
for Parrot, and I'm running into all sorts of interesting issues
when dealing with subclassing. (For those who aren't familiar with
Captures, a Capture is essentially a container object that has
both indexed (array) and keyed (hash) components, like Match object.)
Here's the latest...
Currently src/pmc/default.pmc has lots of functions like the
following:
/* Converts C<key> to a PMC key and calls C<set_integer_keyed()> with it
and C<value>. */
void set_integer_keyed_int (INTVAL key, INTVAL value) {
PMC* r_key = INT2KEY(INTERP, key);
DYNSELF.set_integer_keyed(r_key, value);
}
If I understand how this works, this means that any subclass of
ParrotObject that doesn't define its own set_integer_keyed_int vtable
entry is automatically re-dispatched to set_integer_keyed instead.
The same is true for others -- e.g., set_pmc_keyed_int is forwarded to
set_pmc_keyed, etc.
Well, okay, but that doesn't work if a compiler uses PIR to create
a subclass of a PMC class that makes a distinction between
keyed access and integer access. For example:
$P99 = subclass 'Capture', 'Match'
$P1 = new 'Match'
$P1['abc'] = 1 # store value in hash component
$P1[0] = 2 # store value in array component
Because 'Match' doesn't define its own set_integer_keyed_int
vtable entry, it ought to be inheriting the one from Capture.
But unfortunately, the default.pmc function above gets in the
way, and redispatches the keyed_int call as a keyed call, so that
the last instruction stores a 2 in the object's hash component instead
of its array component.
Here's a more complete example showing how inheritance isn't
working properly for a subclass:
$ cat z.pir
.sub main :main
$P0 = new .Capture # create Capture object
$P0['alpha'] = 1 # store value in hash component
$P0[0] = 2 # store value in array component
$I0 = elements $P0 # display size of array (should be 1)
print $I0
print "\n"
# create a 'Match' subclass of Capture
$P99 = subclass 'Capture', 'Match'
$P1 = new 'Match' # create Match object
$P1['alpha'] = 1 # store value in hash component
$P1[0] = 2 # store value in array component
$I1 = elements $P1 # display size of array (should be 1)
print $I1
print "\n"
.end
$ ./parrot z.pir
1
0
Any thoughts about how we should resolve this?
Pm
Leopold Toetsch
> Because 'Match' doesn't define its own set_integer_keyed_int
> vtable entry, it ought to be inheriting the one from Capture.
> But unfortunately, the default.pmc function above gets in the
> way, and redispatches the keyed_int call as a keyed call,
Class inheritance from PMCs is very static still (like PMC-only cases). I hope
that the :vtable patches will provide the base for a better solution. For
now, you can only implement the mssing _integer_keyed cases in Match so that
default isn't triggered. We could of course remove the defaults too, but that
would need a very complete set of these keyed vtables on all PMCs.
leo
Jonathan Worthington
> Class inheritance from PMCs is very static still (like PMC-only cases). I hope that the :vtable patches will provide the base for a better solution. For now, you can only implement the mssing _integer_keyed cases in Match so that default isn't triggered. We could of course remove the defaults too, but that would need a very complete set of these keyed vtables on all PMCs.
>
attribute specifiable on PMCs like "auto_keyed" (uh, somebody please
think of a less naff name) that generates missing keyed methods for
those PMCs that want them.
I discovered the same issue when implementing :vtable, and it can
produce some misleading error messages - you get a "no
get_integer_keyed" error when you know full well the code is doing a
get_integer_keyed_int.
Jonathan
Nicholas Clark
> Leopold Toetsch wrote:
> >Class inheritance from PMCs is very static still (like PMC-only cases). I
> >hope that the :vtable patches will provide the base for a better solution.
> >For now, you can only implement the mssing _integer_keyed cases in Match
> >so that default isn't triggered. We could of course remove the defaults
> >too, but that would need a very complete set of these keyed vtables on all
> >PMCs.
> >
> Or how about removing them from default.pmc and having an extra
> attribute specifiable on PMCs like "auto_keyed" (uh, somebody please
> think of a less naff name) that generates missing keyed methods for
> those PMCs that want them.
To me that feels like a hack. The current rather-too-static dispatch (to me)
seems to be the bug, and the thing that needs fixing.
Nicholas Clark
Jonathan Worthington
> To me that feels like a hack. The current rather-too-static dispatch (to me) seems to be the bug, and the thing that needs fixing.
>
can see, the problem comes up in building the v-table for the subclass.
The v-tables of the parent classes are not searched for any methods that
they implement when building the v-table for the subclass. Instead,
entries from the deleg_pmc PMC v-table (or the ParrotObject one) are
stuck in it's place.
So, in the attached patch I implemented searching v-tables of parent
classes. The idea is that if it finds a method that we'd have delegated
before but is implemented by a parent, it sticks the parent's method in
the v-table. (Well, kinda - what I really needed to do was check that a
PMC had over-ridden a method, but there is no v-table for the default
PMC, so I just used the Undef PMC's v-table for now - that's *wrong* I
know, but I just wanted an approximation to test the idea out with).
Anyway, I discovered two problems as a result of this. First is that a
subclass is always really an instance of the ParrotClass PMC, which uses
the PMC_data slot. However, Capture.pmc uses that for its data too (and
I guess the situation is the same for other PMCs). Obviously both PMCs
can't put their data in the same slot, so you just wind up with a segfault.
Second is that PGE segfaults now - somehow the exists_keyed of the Hash
PMC is getting called now where it wasn't before. But if it wasn't
before, then I'm not quite sure what *was* being called (well, it was
exists_keyed in deleg_pmc, but that appears to call exists_keyed on
attribute 0 (which looks me to mean whatever's in PMC_data, which would
be the array of parents?! I must have misunderstood something here...)
Either way, seems problem 1 is the real issue, and problem 2 is probably
just me being confused (though I'd love an explanation, from @leo ;-)).
Jonathan
Patrick R. Michaud
> Am Samstag, 4. November 2006 16:17 schrieb Patrick R. Michaud:
> > Because 'Match' doesn't define its own set_integer_keyed_int
> > vtable entry, it ought to be inheriting the one from Capture.
> > But unfortunately, the default.pmc function above gets in the
> > way, and redispatches the keyed_int call as a keyed call,
>
> Class inheritance from PMCs is very static still (like PMC-only cases). I hope
> that the :vtable patches will provide the base for a better solution. For
> now, you can only implement the mssing _integer_keyed cases in Match so that
> default isn't triggered.
I don't think that's possible, is it? Match is implemented as a
subclass of Capture, as in:
$P0 = subclass 'Capture', 'Match'
So, I can create the missing cases, but what do I put for the body
of the method to get to the corresponding method of Capture?
.namespace [ 'Match' ]
.sub set_integer_keyed_int :vtable
.param int key
.param int value
# ... how to do set_integer_keyed_int method of Capture?
.end
> We could of course remove the defaults too, but that
> would need a very complete set of these keyed vtables on all PMCs.
How many of these would there be? Doesn't this affect only those
classes that are built using ParrotObject ?
Pm
Leopold Toetsch
This is fixed now with r15111.
> Pm
leo
Leopold Toetsch
> I don't think that's possible, is it? Match is implemented as a
> subclass of Capture, as in:
>
> $P0 = subclass 'Capture', 'Match'
>
> So, I can create the missing cases, but what do I put for the body
> of the method to get to the corresponding method of Capture?
>
> .namespace [ 'Match' ]
> .sub set_integer_keyed_int :vtable
> .param int key
> .param int value
>
> # ... how to do set_integer_keyed_int method of Capture?
>
> .end
A subclass of a PMC delegates to that PMC (via deleg_pmc.pmc). The PMC is the
first attribute of that class named '__value'. Your code would look like:
.local pmc capt
capt = getattribute SELF, '__value'
capt[key] = value
But this is all clumsy, and might/should change.
Therefore I've ci'ed in r15111 another workaround in parrotobject.pmc, which
checks, if the parent isa PMC and in that case calls the deleg_pmc method
instead of the default.
leo
Patrick R. Michaud
Alas, this seems to work only for immediate subclasses of a PMC.
If we have a sub-subclass, then we're apparently back to the
same problem as before:
$ cat zz.pir
.sub main :main
$P0 = new .Capture # create Capture object
$P0['alpha'] = 1 # store value in hash component
$P0[0] = 2 # store value in array component
$I0 = elements $P0 # display size of array (should be 1)
print $I0
print "\n"
# create a 'Match' subclass of Capture
$P99 = subclass 'Capture', 'Match'
$P1 = new 'Match' # create Match object
$P1['alpha'] = 1 # store value in hash component
$P1[0] = 2 # store value in array component
$I1 = elements $P1 # display size of array (should be 1)
print $I1
print "\n"
# create a 'Exp' subclass of Match
$P99 = subclass 'Match', 'Exp'
$P2 = new 'Exp' # create Exp object
$P2['alpha'] = 1 # store value in hash component
$P2[0] = 2 # store value in array component
$I2 = elements $P2 # display size of array (should be 1)
print $I2
print "\n"
.end
$ ./parrot zz.pir
1
1
0
$
Looking at the above, it seems to me that the crux of the problem
(short of an overall saner design) is that deleg_pmc is occuring
after default.pmc. That seems backwards. Perhaps any deleg_pmc
methods should be taking place before falling back to the PMC defaults.
We also have a similar problem currently taking place with PMC
methods -- methods defined in a PMC aren't being properly inherited
or re-delegated in ParrotObject subclasses. For capture.pmc I've
put some workarounds for this into Capture's 'get_array' and 'get_hash'
methods (r15129), but it again points to something fundamentally
wrong with the way that method inheritance/delegation is being
handled in ParrotObjects.
Pm
Leopold Toetsch
> Alas, this seems to work only for immediate subclasses of a PMC.
> If we have a sub-subclass, then we're apparently back to the
> same problem as before:
I somehow thought that any object subclass would have a proper object vtable,
but as that is also inherited from the (already broken) parent vtable, this
of course doesn't work.
I've now removed [1] the default and the workaround for this one vtable call
as a test case and the impact seems to be very low: one String failure and
one explicit test for this case. It seems that these default vtable
redirections aren't very heavily used, and I'm inclined to go that direction,
i.e. remove the defualts. But that needs more testing by removing all such
keyed redirections and implementing a few keyed_int vtables in core PMCs.
leo
[1]
(pmc2c is ignoring unknown vtable names, thus the xxx_ ;)
Index: src/pmc/parrotobject.pmc
===================================================================
--- src/pmc/parrotobject.pmc (Revision 15139)
+++ src/pmc/parrotobject.pmc (Arbeitskopie)
@@ -368,7 +368,7 @@
}
}
- void set_integer_keyed_int (INTVAL key, INTVAL value) {
+ void xxx_set_integer_keyed_int (INTVAL key, INTVAL value) {
STRING *meth = CONST_STRING(interpreter, "__set_integer_keyed_int");
STRING *meth_v = CONST_STRING(interpreter, "set_integer_keyed_int");
PMC *sub = find_vtable_meth(interpreter, pmc, meth_v);
Index: src/pmc/default.pmc
===================================================================
--- src/pmc/default.pmc (Revision 15139)
+++ src/pmc/default.pmc (Arbeitskopie)
@@ -680,7 +680,7 @@
*/
- void set_integer_keyed_int (INTVAL key, INTVAL value) {
+ void xxx_set_integer_keyed_int (INTVAL key, INTVAL value) {