Re: [perl #59630] [BUG] Complex subtraction fails for subclasses of Complex

2008-10-07 Thread Patrick R. Michaud
Just for the record, I went ahead and added a version of my
test script in this ticket to the test suite (t/pmc/complex.t).

(The test still fails as of r31755, let me know if it should 
be marked 'todo'.)

Thanks,

Pm


Re: [perl #59630] [BUG] Complex subtraction fails for subclasses of Complex

2008-10-06 Thread Patrick R. Michaud
On Mon, Oct 06, 2008 at 06:05:20AM -0700, NotFound via RT wrote:
 I've done some work in this problem. The attached patch is a way to make
 the examples work but I think this is not the way to go, or a lot of
 functions in a lot of pmc will need changes.
 [...]

I agree this really isn't the way to go.  

Also, I'm very curious about the use of VTABLE_morph here -- 
indeed, if we just look at the Integer PMC class we see some
oddities.  For example, here is the code for adding and
subtracting an Integer PMC and a Complex PMC:

/* src/pmc/integer.pmc:341 */
MULTI PMC *add(Complex value, PMC *dest) {
const INTVAL a = SELF.get_integer();
dest = pmc_new(INTERP, VTABLE_type(interp, value));

VTABLE_set_number_native(INTERP, dest,
a + VTABLE_get_number_keyed_int(INTERP, value, 0));
VTABLE_set_number_keyed_int(INTERP, dest, 1,
VTABLE_get_number_keyed_int(INTERP, value, 1));

return dest;
}


/* src/pmc/integer.pmc:452 */
MULTI PMC *subtract(Complex value, PMC *dest) {
const INTVAL a = SELF.get_integer();
if (dest)
VTABLE_morph(INTERP, dest, value-vtable-base_type);
else
dest = pmc_new(INTERP, VTABLE_type(INTERP, value));

VTABLE_set_number_native(INTERP, dest,
a - VTABLE_get_number_keyed_int(INTERP, value, 0));
VTABLE_set_number_keyed_int(INTERP, dest, 1,
-VTABLE_get_number_keyed_int(INTERP, value, 1));

return dest;
}


The add operation always creates a new PMC and returns it
(never using the Cdest PMC passed in), but the subtract 
operation tries to reuse an existing Cdest PMC if it's 
not null.  (Note:  it actually tests for NULL and not PMCNULL, 
which also seems odd.)

Seems like it should be one way or the other, but in the new
MMD code I'm not sure which.

Pm