Fix int.compareTo.

Fix int.compareTo.

In particular we want:
 0.compareTo(-0.0) => 1

Committed: https://code.google.com/p/dart/source/detail?r=9459

[Lasse Reichstein Nielsen, 2012-07-06 08:56:17]

LGTM with comments.

https://chromiumcodereview.appspot.com/10692099/diff/1/runtime/lib/integers.dart
File runtime/lib/integers.dart (right):

https://chromiumcodereview.appspot.com/10692099/diff/1/runtime/lib/integers.d...
runtime/lib/integers.dart:51: int bitAndFromInteger(int other) native
"Integer_bitAndFromInteger";
These are completely unspecified, even in the API docs, right?

https://chromiumcodereview.appspot.com/10692099/diff/1/runtime/lib/integers.d...
runtime/lib/integers.dart:98: int compareTo(Comparable other) {
Argh. Why did we not define Comparable to be generic, so num is Comparable<num>?
We only ever want to receive num values here. Later we do "this < other" and
int.operator< will/should throw on any non-num.
I think you should change the parameter to "num other". It's a valid subtype of
(Comparable)->int.

https://chromiumcodereview.appspot.com/10692099/diff/1/runtime/lib/integers.d...
runtime/lib/integers.dart:102: return -other.compareTo(this.toDouble());
I think that's wrong.
If this is, e.g., 1<<2000 (an int) this will make it compare equal to Infinity,
and so will 1<<2001, but those two are not equal, breaking transitivity.
Ditto for this being 1<<55+1 vs 1<<55+2 and the double 2^55.

I.e., only convert blindly to double if this is in the range 
[-2^53..2^53]. Above that, you lose precission.
That's also the range where doubles can have decimals, so outside that range,
you can convert the double to an int instead, but you have to rule out
Infinities/NaN first.

I.e., if other is a double, do:
  const MAX_EXACT_INT_TO_DOUBLE = ... value of 2^53 ...;
  const MIN_EXACT_INT_TO_DOUBLE = -MAX_EXACT_INT_TO_DOUBLE;
  double d = other;
  if (!d.isFinite()) {
    return d == double.NEGATIVE_INFINITY ? GREATER : LESS;
  }
  if (MIN_EXACT_INT_TO_DOUBLE <= this && 
      this <= MAX_EXACT_INT_TO_DOUBLE) {
    return -d.compareTo(this.toDouble());
  } else {
    // If abs(other) > MAX_EXACT_DOUBLE, then other has 
    // an integer value (no bits below the decimal point).
    other = d.toInt();
  }

https://chromiumcodereview.appspot.com/10692099/diff/1/tests/corelib/compare_...
File tests/corelib/compare_to_test.dart (right):

https://chromiumcodereview.appspot.com/10692099/diff/1/tests/corelib/compare_...
tests/corelib/compare_to_test.dart:7: // Test matrix:
You need bigger numbers. Try adding +/- 2^53, 2^53+1, 2^2000, 2^2000 + 1.
(I.e., integers that don't convert well to doubles, either by losing precision
or by becoming infinity).
Also have double numbers that aren't integers, preferably all of:
 min-non-zero: 5e-324
 max denormal: 2.225073858507201e-308
 min normal: 2.2250738585072014e-308
 max fraction: 0.9999999999999999
 one: 1.0
 min above 1: 1.0000000000000002
 max non-int: 4503599627370495.5
and for completeness, these numbers as both doubles and integers:
 Max exact int: 9007199254740992.0
 Max finite: 1.7976931348623157e+308
and the higher values integers.

You might want to make loops to do these comparisons. E.g., put all the values
into an array of arrays, in order with equal values in the same sub-array.

[floitsch, 2012-07-06 13:18:55]

Maybe PTAL.
The compare_to2_test needs changes in the way we handle relations between
integers and doubles.
I will try to fix this asap.

https://chromiumcodereview.appspot.com/10692099/diff/1/runtime/lib/integers.dart
File runtime/lib/integers.dart (right):

https://chromiumcodereview.appspot.com/10692099/diff/1/runtime/lib/integers.d...
runtime/lib/integers.dart:51: int bitAndFromInteger(int other) native
"Integer_bitAndFromInteger";
On 2012/07/06 08:56:17, Lasse Reichstein Nielsen wrote:
> These are completely unspecified, even in the API docs, right?
Yes. as discussed offline: we should maybe remove them.

https://chromiumcodereview.appspot.com/10692099/diff/1/runtime/lib/integers.d...
runtime/lib/integers.dart:98: int compareTo(Comparable other) {
On 2012/07/06 08:56:17, Lasse Reichstein Nielsen wrote:
> Argh. Why did we not define Comparable to be generic, so num is
Comparable<num>?
> We only ever want to receive num values here. Later we do "this < other" and
> int.operator< will/should throw on any non-num.
> I think you should change the parameter to "num other". It's a valid subtype
of
> (Comparable)->int.

Changing to num now, and adding generic type to comparable in a separate CL.

https://chromiumcodereview.appspot.com/10692099/diff/1/runtime/lib/integers.d...
runtime/lib/integers.dart:102: return -other.compareTo(this.toDouble());
On 2012/07/06 08:56:17, Lasse Reichstein Nielsen wrote:
> I think that's wrong.
> If this is, e.g., 1<<2000 (an int) this will make it compare equal to
Infinity,
> and so will 1<<2001, but those two are not equal, breaking transitivity.
> Ditto for this being 1<<55+1 vs 1<<55+2 and the double 2^55.
> 
> I.e., only convert blindly to double if this is in the range 
> [-2^53..2^53]. Above that, you lose precission.
> That's also the range where doubles can have decimals, so outside that range,
> you can convert the double to an int instead, but you have to rule out
> Infinities/NaN first.
> 
> I.e., if other is a double, do:
>   const MAX_EXACT_INT_TO_DOUBLE = ... value of 2^53 ...;
>   const MIN_EXACT_INT_TO_DOUBLE = -MAX_EXACT_INT_TO_DOUBLE;
>   double d = other;
>   if (!d.isFinite()) {
>     return d == double.NEGATIVE_INFINITY ? GREATER : LESS;
>   }
>   if (MIN_EXACT_INT_TO_DOUBLE <= this && 
>       this <= MAX_EXACT_INT_TO_DOUBLE) {
>     return -d.compareTo(this.toDouble());
>   } else {
>     // If abs(other) > MAX_EXACT_DOUBLE, then other has 
>     // an integer value (no bits below the decimal point).
>     other = d.toInt();
>   }

Done.

https://chromiumcodereview.appspot.com/10692099/diff/1/tests/corelib/compare_...
File tests/corelib/compare_to_test.dart (right):

https://chromiumcodereview.appspot.com/10692099/diff/1/tests/corelib/compare_...
tests/corelib/compare_to_test.dart:7: // Test matrix:
On 2012/07/06 08:56:17, Lasse Reichstein Nielsen wrote:
> You need bigger numbers. Try adding +/- 2^53, 2^53+1, 2^2000, 2^2000 + 1.
> (I.e., integers that don't convert well to doubles, either by losing precision
> or by becoming infinity).
> Also have double numbers that aren't integers, preferably all of:
>  min-non-zero: 5e-324
>  max denormal: 2.225073858507201e-308
>  min normal: 2.2250738585072014e-308
>  max fraction: 0.9999999999999999
>  one: 1.0
>  min above 1: 1.0000000000000002
>  max non-int: 4503599627370495.5
> and for completeness, these numbers as both doubles and integers:
>  Max exact int: 9007199254740992.0
>  Max finite: 1.7976931348623157e+308
> and the higher values integers.
> 
> You might want to make loops to do these comparisons. E.g., put all the values
> into an array of arrays, in order with equal values in the same sub-array.

Done.

[Lasse Reichstein Nielsen, 2012-07-09 07:38:00]

LGTM

https://chromiumcodereview.appspot.com/10692099/diff/3002/runtime/lib/integer...
File runtime/lib/integers.dart (right):

https://chromiumcodereview.appspot.com/10692099/diff/3002/runtime/lib/integer...
runtime/lib/integers.dart:113: return -d.compareTo(this.toDouble());
Please put parentheses around call. This looks like (even if it isn't) a call on
"-d".

https://chromiumcodereview.appspot.com/10692099/diff/3002/tests/corelib/compa...
File tests/corelib/compare_to2_test.dart (right):

https://chromiumcodereview.appspot.com/10692099/diff/3002/tests/corelib/compa...
tests/corelib/compare_to2_test.dart:63: mnan,
Should nan and mnan not be equal? If not, are nan and nan not equal?

https://chromiumcodereview.appspot.com/10692099/diff/3002/tests/corelib/compa...
tests/corelib/compare_to2_test.dart:77: print("($left).compareTo($right) failed
"
Can't we pass a message to Expect.equals instead of printing it manually?

[floitsch, 2012-07-09 07:56:31]

https://chromiumcodereview.appspot.com/10692099/diff/3002/runtime/lib/integer...
File runtime/lib/integers.dart (right):

https://chromiumcodereview.appspot.com/10692099/diff/3002/runtime/lib/integer...
runtime/lib/integers.dart:113: return -d.compareTo(this.toDouble());
On 2012/07/09 07:38:00, Lasse Reichstein Nielsen wrote:
> Please put parentheses around call. This looks like (even if it isn't) a call
on
> "-d".

Done.

https://chromiumcodereview.appspot.com/10692099/diff/3002/tests/corelib/compa...
File tests/corelib/compare_to2_test.dart (right):

https://chromiumcodereview.appspot.com/10692099/diff/3002/tests/corelib/compa...
tests/corelib/compare_to2_test.dart:63: mnan,
On 2012/07/09 07:38:00, Lasse Reichstein Nielsen wrote:
> Should nan and mnan not be equal? If not, are nan and nan not equal?

Done.

https://chromiumcodereview.appspot.com/10692099/diff/3002/tests/corelib/compa...
tests/corelib/compare_to2_test.dart:77: print("($left).compareTo($right) failed
"
On 2012/07/09 07:38:00, Lasse Reichstein Nielsen wrote:
> Can't we pass a message to Expect.equals instead of printing it manually?

Done.