Fix int.compareTo.

runtime/lib/integers.dart

 // Copyright (c) 2012, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 // TODO(srdjan): fix limitations.
 // - shift amount must be a Smi.
 class IntegerImplementation {
   factory IntegerImplementation._uninstantiable() {
     throw const UnsupportedOperationException(
         "IntegerImplementation can only be allocated by the VM");
@@ skipping 30 matching lines @@
   }
   int operator |(int other) {
     return other.bitOrFromInteger(this);
   }
   int operator ^(int other) {
     return other.bitXorFromInteger(this);
   }
   num remainder(num other) {
     return other.remainderFromInteger(this);
   }
   int bitAndFromInteger(int other) native "Integer_bitAndFromInteger";

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

These are completely unspecified, even in the API docs, right?

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

Yes. as discussed offline: we should maybe remove them.

   int bitOrFromInteger(int other) native "Integer_bitOrFromInteger";
   int bitXorFromInteger(int other) native "Integer_bitXorFromInteger";
   int addFromInteger(int other) native "Integer_addFromInteger";
   int subFromInteger(int other) native "Integer_subFromInteger";
   int mulFromInteger(int other) native "Integer_mulFromInteger";
   int truncDivFromInteger(int other) native "Integer_truncDivFromInteger";
   int moduloFromInteger(int other) native "Integer_moduloFromInteger";
   int remainderFromInteger(int other) {
     return other - (other ~/ this) * this;
   }
@@ skipping 26 matching lines @@
   bool equalToInteger(int other) native "Integer_equalToInteger";
   int abs() {
     return this < 0 ? -this : this;
   }
   bool isEven() { return ((this & 1) === 0); }
   bool isOdd() { return !isEven(); }
   bool isNaN() { return false; }
   bool isNegative() { return this < 0; }
   bool isInfinite() { return false; }
 
   int compareTo(Comparable other) {

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

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.

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

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

     final int EQUAL = 0, LESS = -1, GREATER = 1;
+    if (other is double) {
+      // Let the double implementation deal with -0.0 and NaN.
+      return -other.compareTo(this.toDouble());

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

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();
  }

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

Done.

+    }
     if (this < other) {
       return LESS;
     } else if (this > other) {
       return GREATER;
-    } else if (this == other) {
+    } else {
       return EQUAL;
-    } else {
-      // Other is NaN.
-      return LESS;
     }
   }
 
   int round() { return this; }
   int floor() { return this; }
   int ceil() { return this; }
   int truncate() { return this; }
 
   int toInt() { return this; }
   double toDouble() { return new Double.fromInteger(this); }
@@ skipping 100 matching lines @@
     }
   }
   int shlFromInt(int other) {
     throw const OutOfMemoryException();
   }
 
   int pow(int exponent) {
     throw "Bigint.pow not implemented";
   }
 }