add MMD bug in Integer PMC

[Will Coleda]

On Fri, Oct 30, 2009 at 1:04 PM, Andrew Whitworth <wknight8111 at gmail.com> wrote:
> This is the offending code: from src/pmc/integer.pmc:
>
> MULTI PMC *add(DEFAULT value, PMC *dest) {
>        dest = pmc_new(INTERP, VTABLE_type(interp, value));
>
>        VTABLE_set_number_native(INTERP, dest,
>                SELF.get_integer() + VTABLE_get_number(INTERP, value));
>        return dest;
>    }
>
> I suggest that the middle line there be changed to:
>
> INTVAL myvalue = SELF.get_integer();
> INTVAL othervalue = VTABLE_get_integer(INTERP, value);
> VTABLE_set_integer_native(INTERP, dest, myvalue + othervalue);
>
> Has these benefits:
>
> 1) The result type is whatever type the second value is, so it
> respects subclassing rules for HLLs
> 2) Subclasses of Integer, or integral types need only define the
> get_integer and set_integer_native VTABLEs to interact with an
> Integer, as we should expect
> 3) Integer gets to act on INTVALs in a consistent way.
>
> I do not see any advantage to having the Integer.add VTABLE use the
> set_number_native and get_number VTABLEs of the destination PMC. If
> another PMC type wants to be able to interact with Integer, it should
> define the set_integer_native and get_integer VTABLEs.
>
>
> --Andrew Whitworth
>
>
>
> On Fri, Oct 30, 2009 at 12:14 PM, Jonathan Leto <jaleto at gmail.com> wrote:
>> Howdy,
>>
>> I think this is wrong and has been a long-standing bug in our code:
>>
>> The Integer PMC, in the default case of it's add MMD, treats values as
>> Floats and returns a Float
>>
>> I would like to treat it as such and write a failing test for it, fix
>> the bug, and then change the behavior. Is there any reason not to do
>> this?
> _______________________________________________
> http://lists.parrot.org/mailman/listinfo/parrot-dev
>

>From a HLL perspective, your proposal is like having

1 + 3.4 == 4

and

3.4 + 1 = 4.4

I'd rather keep the defaults the way they are, and let HLLs override
them to force integer-based math if that's what they want, much like
partcl currently overrides /
(http://github.com/partcl/partcl/blob/master/src/pmc/tclint.pmc#L42).
(but it's the *only* case where I'm overriding the built in arithmetic
of the core types.)

If you change the behavior of add here (and presumably the rest of
math based vtables), partcl, and probably many other languages
(including rakudo), are going to have to override them all to make
them work the way core does today.

This may simply be part of the "auto conversion of core types is
surprising" thread.

-- 
Will "Coke" Coleda