Re: set_pmc_keyed_int delegates to set_pmc_keyed...?
On Sun, Nov 05, 2006 at 05:41:12PM +0100, Leopold Toetsch wrote: Am Sonntag, 5. November 2006 15:22 schrieb Patrick R. Michaud: 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. 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
Re: set_pmc_keyed_int delegates to set_pmc_keyed...?
Am Montag, 6. November 2006 21:54 schrieb 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:
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) {
PMC* r_key = INT2KEY(INTERP, key);
DYNSELF.set_integer_keyed(r_key, value);
}
Re: set_pmc_keyed_int delegates to set_pmc_keyed...?
On Sat, Nov 04, 2006 at 05:18:22PM +0100, Leopold Toetsch wrote: 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
Re: set_pmc_keyed_int delegates to set_pmc_keyed...?
Am Samstag, 4. November 2006 16:17 schrieb Patrick R. Michaud: Any thoughts about how we should resolve this? This is fixed now with r15111. Pm leo
Re: set_pmc_keyed_int delegates to set_pmc_keyed...?
Am Sonntag, 5. November 2006 15:22 schrieb Patrick R. Michaud: 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
Re: set_pmc_keyed_int delegates to set_pmc_keyed...?
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. We could of course remove the defaults too, but that would need a very complete set of these keyed vtables on all PMCs. leo
Re: set_pmc_keyed_int delegates to set_pmc_keyed...?
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. 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
Re: set_pmc_keyed_int delegates to set_pmc_keyed...?
On Fri, Nov 03, 2006 at 04:28:41PM -0800, Jonathan Worthington wrote: 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
Re: set_pmc_keyed_int delegates to set_pmc_keyed...?
Nicholas Clark wrote:
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.
Yeah, you're right. So, I decided to try and address this. From what I
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
Index: src/objects.c
===
--- src/objects.c (revision 15108)
+++ src/objects.c (working copy)
@@ -290,21 +290,41 @@
#endif
}
else if (full) {
-/*
- * the method doesn't exist; put in the deleg_pmc vtable,
- * but only if ParrotObject hasn't overridden the method
- */
-if (((void **)delegate_vtable)[i] == ((void**)object_vtable)[i]) {
-if (ro_vtable)
-((void **)ro_vtable)[i] = ((void**)deleg_pmc_vtable)[i];
-((void **)vtable)[i] = ((void**)deleg_pmc_vtable)[i];
+/* See if a parent PMC has already provided an implementation of
+ * the method, and in that case put that in the v-table slot.
+ * Otherwise, stick in a delegate PMC entry or, if ParrotObject
+ * overrides it, a ParrotObject v-table entry. */
+int num_parents = VTABLE_elements(interpreter, class-vtable-mro);
+int p;
+VTABLE* const default_vtable =
+interpreter-vtables[enum_class_Undef]; /* XXX want
enum_class_default, but abstract. */
+void* parent_meth = NULL;
+for (p = 1; p num_parents parent_meth == NULL; p++) {
+PMC* cur_parent = VTABLE_get_pmc_keyed_int(interpreter,
+class-vtable-mro, p);
+VTABLE* parent_vtable = cur_parent-vtable;
+if (((void **)parent_vtable)[i] != ((void**)default_vtable)[i])
+parent_meth = ((void **)parent_vtable)[i];
}
-else {
+
+if (((void **)delegate_vtable)[i] != ((void**)object_vtable)[i]) {
+/* Overridden in ParrotObject. */
((void **)vtable)[i] = ((void**)object_vtable)[i];
if (ro_vtable)
((void **)ro_vtable)[i] = ((void**)ro_object_vtable)[i];
-
}
+else if (parent_meth != NULL) {
+/* We found a parent method. Use that.*/
+if (ro_vtable)
+((void **)ro_vtable)[i] = parent_meth;
+((void **)vtable)[i] = parent_meth;
+}
+else {
+/* Not overridden in ParrotObject. */
+if (ro_vtable)
+((void **)ro_vtable)[i] = ((void**)deleg_pmc_vtable)[i];
+((void **)vtable)[i] = ((void**)deleg_pmc_vtable)[i];
+}
}
}
}
