Looking at LargeIntegers (I'm 64 bit, so these are big): { 1152921504606846977 digitCompare: 1152921504606846977. 1152921504606846977 digitCompare: 1152921504606846978. 1152921504606846978 digitCompare: 1152921504606846977. } "#(0 1 1)" { 1249 digitCompare: 1249. 1249 digitCompare: 1250. 1250 digitCompare: 1249. } #(1 1 1) 
(keyboard failure)
On Sun, Oct 28, 2018 at 3:41 PM Chris Cunningham <[hidden email]> wrote:
} "#(1 1 1)" The method is actively different for SmallIntegers and LargeIntegers. I'm not sure how much this matters  besides the fact that it is lying around waiting for someone to use it. For comparisons of integers, the callers check to see if one (or more) of the numbers are negative, then handles the issues here. It is also currently used in DateAndTime class methods (#nowWithOffset: [deprecated] and #now:offset:), which is probably safe because now is always after the epoch. And we hopefully wouldn't use #now:withOffset: for preepoch times. Is this worth fixing? cbc 
On Sun, Oct 28, 2018 at 03:48:58PM 0700, Chris Cunningham wrote:
> (keyboard failure) > On Sun, Oct 28, 2018 at 3:41 PM Chris Cunningham <[hidden email]> > wrote: > > > Looking at LargeIntegers (I'm 64 bit, so these are big): > > { > > 1152921504606846977 digitCompare: 1152921504606846977. > > 1152921504606846977 digitCompare: 1152921504606846978. > > 1152921504606846978 digitCompare: 1152921504606846977. > > } "#(0 1 1)" > > > > { > > 1249 digitCompare: 1249. > > 1249 digitCompare: 1250. > > 1250 digitCompare: 1249 > > > } "#(1 1 1)" > > The method is actively different for SmallIntegers and LargeIntegers. > > I'm not sure how much this matters  besides the fact that it is lying > around waiting for someone to use it. For comparisons of integers, the > callers check to see if one (or more) of the numbers are negative, then > handles the issues here. I would say that this is crying out for some unit tests :) > > It is also currently used in DateAndTime class methods (#nowWithOffset: > [deprecated] and #now:offset:), which is probably safe because now is > always after the epoch. And we hopefully wouldn't use #now:withOffset: for > preepoch times. > > Is this worth fixing? > Definitely, yes. Dave 
On Sun, Oct 28, 2018 at 4:54 PM David T. Lewis <[hidden email]> wrote: On Sun, Oct 28, 2018 at 03:48:58PM 0700, Chris Cunningham wrote: Yes, just need to figure out what the right results are.
Good. Then this starts in LargeIntegerPlugin>>primDigitCompare: with this nice comment: "shortcut: aSmallInteger has to be smaller in Magnitude as aLargeInteger" This has been there at least since 2004... cross posting to the VM Dev list (since it should be handled there first). I'd be happy to replicate the right answer in the fallback code, and craft tests, once the right behavior is explained. Thanks, cbc

In reply to this post by cbc
Hi Chris,
> On Oct 28, 2018, at 3:41 PM, Chris Cunningham <[hidden email]> wrote: > > Looking at LargeIntegers (I'm 64 bit, so these are big): > { > 1152921504606846977 digitCompare: 1152921504606846977. > 1152921504606846977 digitCompare: 1152921504606846978. > 1152921504606846978 digitCompare: 1152921504606846977. > } "#(0 1 1)" > > { > 1249 digitCompare: 1249. > 1249 digitCompare: 1250. > 1250 digitCompare: 1249. > } #(1 1 1) this is correct. The primitive is supposed to answer 1, 0 or 1 depending on whether the (receiver digitAt: n) is <, =, or > the (argument digitAt: n) where n is either the first digit at which the receiver and argument differ or the last digit. Since digitAt: does not answer the 2’s complement bitanded SmallIntegers are not actually inconsistent 1 digitAt: 1 => 1 1 digitAt: 2 => 0 1 digitAt: 1 => 1 1 digitAt: 2 => 0 SmallInteger minVal  1 digitAt: Smalltalk wordSize => 16 (64bits) 64 (32bits) SmallInteger maxVal + 1 digitAt: Smalltalk wordSize => 16 (64bits) 64 (32bits) So the method needs a) a really good comment and b) a warning that this is private to the Integer hierarchy implementation and not for general use. It looks to me like the use in DateAndTime is a hack that works because LastClockValue is always +ve. _,,,^..^,,,_ (phone) 
> On Oct 29, 2018, at 11:46 AM, Eliot Miranda <[hidden email]> wrote: > > Hi Chris, > >> On Oct 28, 2018, at 3:41 PM, Chris Cunningham <[hidden email]> wrote: >> >> Looking at LargeIntegers (I'm 64 bit, so these are big): >> { >> 1152921504606846977 digitCompare: 1152921504606846977. >> 1152921504606846977 digitCompare: 1152921504606846978. >> 1152921504606846978 digitCompare: 1152921504606846977. >> } "#(0 1 1)" >> >> { >> 1249 digitCompare: 1249. >> 1249 digitCompare: 1250. >> 1250 digitCompare: 1249. >> } #(1 1 1) > > this is correct. The primitive is supposed to answer 1, 0 or 1 depending on whether the (receiver digitAt: n) is <, =, or > the (argument digitAt: n) where n is either the first digit at which the receiver and argument differ or the last digit. Since digitAt: does not answer the 2’s complement bitanded SmallIntegers are not actually inconsistent > > 1 digitAt: 1 => 1 > 1 digitAt: 2 => 0 > 1 digitAt: 1 => 1 > 1 digitAt: 2 => 0 > > SmallInteger minVal  1 digitAt: Smalltalk wordSize => 16 (64bits) 64 (32bits) > SmallInteger maxVal + 1 digitAt: Smalltalk wordSize => 16 (64bits) 64 (32bits) or more clearly: (SmallInteger minVal digitCompare: SmallInteger maxVal + 1) = 0 As the comment says, digitCompare: compares the magnitudes, not the 2’s complement representations. > > So the method needs a) a really good comment and b) a warning that this is private to the Integer hierarchy implementation and not for general use. > > It looks to me like the use in DateAndTime is a hack that works because LastClockValue is always +ve. > > _,,,^..^,,,_ (phone) 
In reply to this post by cbc
Hi Chris,
> On Oct 28, 2018, at 3:41 PM, Chris Cunningham <[hidden email]> wrote: > > Looking at LargeIntegers (I'm 64 bit, so these are big): > { > 1152921504606846977 digitCompare: 1152921504606846977. > 1152921504606846977 digitCompare: 1152921504606846978. > 1152921504606846978 digitCompare: 1152921504606846977. > } "#(0 1 1)" > > { > 1249 digitCompare: 1249. > 1249 digitCompare: 1250. > 1250 digitCompare: 1249. > } #(1 1 1) I do apologize. You’re quite right; the primitive is broken for SmallInteger. It should be the case that (1 digitCompare: 1) = 0 since ((1 to: 8) collect: [:i 1 digitAt: i ]) = #(1 0 0 0 0 0 0 0) and: [(1 to: 8) collect: [:i 1 digitAt: i ]) = #(1 0 0 0 0 0 0 0)] I’ll fix this ASAP :) 
Free forum by Nabble  Edit this page 