FW: Can someone do a review of Pete's patch so that we can submit?

Review from Tomas

From: Tomas M.
Sent: Thursday, November 29, 2007 12:32 PM
To: John L. (DLR); IronRuby Team
Subject: RE: Can someone do a review of Pete’s patch so that we can
submit?

Few issues:

  •      Fixnum.Abs is incorrect: abs(Int32.MinValue) will blow up, it 
    

should overlflow to BigInteger.

  •      Why FloatOps.ToInteger doesn't just do try { return 
    

(int)self; } catch (…) {…} ? Flooring is actually incorrect:
(-0.4).to_i gives 0 in Ruby while -1 in IronRuby. A test for this is
apparently missing.

  •      Why FloatOps.Divmod doesn't use Math.IEEEReminder? There 
    

might be some precision peculiarity involved.

Tomas

Hi Tomas,

I was only focussing on Bignum methods. The other ones in Fixnum and
Float
were hacked together to support getting the specs to pass. I was going
to
look at Fixnum and Float next. Unfortunately, due to the coercion
functionality in Ruby, it is not possible to completely isolate any of
the
numeric types from each other to unit test.

Pete

From: [email protected]
[mailto:[email protected]] On Behalf Of John L. (DLR)
Sent: Thursday,29 November 29, 2007 22:13
To: [email protected]
Subject: [Ironruby-core] FW: Can someone do a review of Pete’s patch so
that
we can submit?

Review from Tomas

From: Tomas M.
Sent: Thursday, November 29, 2007 12:32 PM
To: John L. (DLR); IronRuby Team
Subject: RE: Can someone do a review of Pete’s patch so that we can
submit?

Few issues:

  •      Fixnum.Abs is incorrect: abs(Int32.MinValue) will blow up, it
    

should overlflow to BigInteger.

  •      Why FloatOps.ToInteger doesn't just do try { return 
    

(int)self; }
catch (.) {.} ? Flooring is actually incorrect: (-0.4).to_i gives 0 in
Ruby
while -1 in IronRuby. A test for this is apparently missing.

  •      Why FloatOps.Divmod doesn't use Math.IEEEReminder? There 
    

might be
some precision peculiarity involved.

Tomas

Sorry if my last mail on this subject sounded a bit defensive. Thanks
for
reviewing the patch I sent in. I am really pleased that there were no
serious issues with the code I wrote for the Bignum class and this gives
me
confidence to get on with the rest of the numeric classes.

Cheers,

Pete

From: [email protected]
[mailto:[email protected]] On Behalf Of John L. (DLR)
Sent: Thursday,29 November 29, 2007 22:13
To: [email protected]
Subject: [Ironruby-core] FW: Can someone do a review of Pete’s patch so
that
we can submit?

Review from Tomas

From: Tomas M.
Sent: Thursday, November 29, 2007 12:32 PM
To: John L. (DLR); IronRuby Team
Subject: RE: Can someone do a review of Pete’s patch so that we can
submit?

Few issues:

  •      Fixnum.Abs is incorrect: abs(Int32.MinValue) will blow up, it
    

should overlflow to BigInteger.

  •      Why FloatOps.ToInteger doesn't just do try { return 
    

(int)self; }
catch (.) {.} ? Flooring is actually incorrect: (-0.4).to_i gives 0 in
Ruby
while -1 in IronRuby. A test for this is apparently missing.

  •      Why FloatOps.Divmod doesn't use Math.IEEEReminder? There 
    

might be
some precision peculiarity involved.

Tomas