[squeak-dev] What should Integer>>digitCompare: return?

[Chris Cunningham]

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)

[Chris Cunningham]

(keyboard failure)

Post by Chris Cunningham
{
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.

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
pre-epoch times.

Is this worth fixing?

-cbc

[David T. Lewis]

Post by Chris Cunningham
(keyboard failure)

Post by Chris Cunningham
{
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 :-)

Post by Chris Cunningham
[deprecated] and #now:offset:), which is probably safe because now is
always after the epoch. And we hopefully wouldn't use #now:withOffset: for
pre-epoch times.
Is this worth fixing?

Definitely, yes.

Dave

[Chris Cunningham]

Post by David T. Lewis

Post by Chris Cunningham
(keyboard failure)
On Sun, Oct 28, 2018 at 3:41 PM Chris Cunningham <

Post by Chris Cunningham
{
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 :-)
Yes, just need to figure out what the right results are.

Post by Chris Cunningham
[deprecated] and #now:offset:), which is probably safe because now is

for

Post by Chris Cunningham
pre-epoch times.
Is this worth fixing?

Definitely, yes.

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

Post by David T. Lewis
Dave

[Eliot Miranda]

Hi Chris,

Post by Chris Cunningham
{
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 bit-anded 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 (64-bits) 64 (32-bits)
SmallInteger maxVal + 1 digitAt: Smalltalk wordSize => 16 (64-bits) 64 (32-bits)

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.

_,,,^..^,

[Eliot Miranda]

Post by Eliot Miranda
Hi Chris,

Post by Chris Cunningham
{
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 bit-anded 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 (64-bits) 64 (32-bits)
SmallInteger maxVal + 1 digitAt: Smalltalk wordSize => 16 (64-bits) 64 (32-bits)

or more clearly:

(SmallInteger minVal digitCompare: SmallInteger maxVal + 1) = 0

As the comment says, digitCompare: compares the magnitudes, not the 2’s complement representations.

Post by Eliot Miranda
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 LastCloc

[Levente Uzonyi]

Post by Eliot Miranda
Hi Chris,

Post by Chris Cunningham
{
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 bit-anded 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 (64-bits) 64 (32-bits)
SmallInteger maxVal + 1 digitAt: Smalltalk wordSize => 16 (64-bits) 64 (32-bits)
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.

I think DateAndTime >> #now:offset:'s comment gives a very good reason why
#digitCompare: is used there. It doesn't go into details though.

Levente

Post by Eliot Miranda
_,,,^..^,,,_ (phone)

[Eliot Miranda]

Hi Levente,

Post by Eliot Miranda

Post by Eliot Miranda
Hi Chris,

Post by Chris Cunningham
{
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 bit-anded SmallIntegers are not actually inconsistent

Post by Eliot Miranda
-1 digitAt: 1 => 1
-1 digitAt: 2 => 0
1 digitAt: 1 => 1
1 digitAt: 2 => 0
SmallInteger minVal - 1 digitAt: Smalltalk wordSize => 16 (64-bits) 64

(32-bits)

Post by Eliot Miranda
SmallInteger maxVal + 1 digitAt: Smalltalk wordSize => 16 (64-bits) 64

(32-bits)

Post by Eliot Miranda
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.

Post by Eliot Miranda
It looks to me like the use in DateAndTime is a hack that works because

LastClockValue is always +ve.
I think DateAndTime >> #now:offset:'s comment gives a very good reason why
#digitCompare: is used there. It doesn't go into details though.

Right. The comment states: "Ensure that consecutive sends of this method
return increasing values, by adding small values to the nanosecond part of
the created object. The next few lines are assumed to be executed
atomically - having no suspension points.". Clever, Bert!

[Eliot Miranda]

Hi Chris,

Post by Chris Cunningham
{
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 t