Update range errors to agree on the numbers.

sdk/lib/_internal/js_runtime/lib/native_typed_data.dart

Index: sdk/lib/_internal/js_runtime/lib/native_typed_data.dart
diff --git a/sdk/lib/_internal/js_runtime/lib/native_typed_data.dart b/sdk/lib/_internal/js_runtime/lib/native_typed_data.dart
index d134c6db81766fb4a62ebd0f0d3d10207c0f48b9..2b5cabd783745145236603a98543440e44ed75e0 100644
--- a/sdk/lib/_internal/js_runtime/lib/native_typed_data.dart
+++ b/sdk/lib/_internal/js_runtime/lib/native_typed_data.dart
@@ -148,39 +148,10 @@ class NativeFloat32x4List
 
   int get elementSizeInBytes => Float32x4List.BYTES_PER_ELEMENT;
 
-  void _invalidIndex(int index, int length) {
-    if (index < 0 || index >= length) {
-      if (length == this.length) {
-        throw new RangeError.index(index, this);
-      }
-      throw new RangeError.range(index, 0, length - 1);
-    } else {
-      throw new ArgumentError('Invalid list index $index');
-    }
-  }
-
-  void _checkIndex(int index, int length) {
-    if (JS('bool', '(# >>> 0 != #)', index, index) || index >= length) {
-      _invalidIndex(index, length);
-    }
-  }
-
-  int _checkSublistArguments(int start, int end, int length) {
-    // For `sublist` the [start] and [end] indices are allowed to be equal to
-    // [length]. However, [_checkIndex] only allows indices in the range
-    // 0 .. length - 1. We therefore increment the [length] argument by one
-    // for the [_checkIndex] checks.
-    _checkIndex(start, length + 1);
-    if (end == null) return length;
-    _checkIndex(end, length + 1);
-    if (start > end) throw new RangeError.range(start, 0, end);
-    return end;
-  }
-
   int get length => _storage.length ~/ 4;
 
   Float32x4 operator[](int index) {
-    _checkIndex(index, length);
+    _checkValidIndex(index, this);
     double _x = _storage[(index * 4) + 0];
     double _y = _storage[(index * 4) + 1];
     double _z = _storage[(index * 4) + 2];
@@ -189,7 +160,7 @@ class NativeFloat32x4List
   }
 
   void operator[]=(int index, Float32x4 value) {
-    _checkIndex(index, length);
+    _checkValidIndex(index, this);
     _storage[(index * 4) + 0] = value.x;
     _storage[(index * 4) + 1] = value.y;
     _storage[(index * 4) + 2] = value.z;
@@ -197,7 +168,7 @@ class NativeFloat32x4List
   }
 
   List<Float32x4> sublist(int start, [int end]) {
-    end = _checkSublistArguments(start, end, length);
+    end = _checkValidRange(start, end, this);
     return new NativeFloat32x4List._externalStorage(
         _storage.sublist(start * 4, end * 4));
   }
@@ -257,40 +228,10 @@ class NativeInt32x4List
 
   int get elementSizeInBytes => Int32x4List.BYTES_PER_ELEMENT;
 
-  void _invalidIndex(int index, int length) {
-    if (index < 0 || index >= length) {
-      if (length == this.length) {
-        throw new RangeError.index(index, this);
-      }
-      throw new RangeError.range(index, 0, length - 1);
-    } else {
-      throw new ArgumentError('Invalid list index $index');
-    }
-  }
-
-  void _checkIndex(int index, int length) {
-    if (JS('bool', '(# >>> 0 != #)', index, index)
-        || JS('bool', '# >= #', index, length)) {
-      _invalidIndex(index, length);
-    }
-  }
-
-  int _checkSublistArguments(int start, int end, int length) {
-    // For `sublist` the [start] and [end] indices are allowed to be equal to
-    // [length]. However, [_checkIndex] only allows indices in the range
-    // 0 .. length - 1. We therefore increment the [length] argument by one
-    // for the [_checkIndex] checks.
-    _checkIndex(start, length + 1);
-    if (end == null) return length;
-    _checkIndex(end, length + 1);
-    if (start > end) throw new RangeError.range(start, 0, end);
-    return end;
-  }
-
   int get length => _storage.length ~/ 4;
 
   Int32x4 operator[](int index) {
-    _checkIndex(index, length);
+    _checkValidIndex(index, this);
     int _x = _storage[(index * 4) + 0];
     int _y = _storage[(index * 4) + 1];
     int _z = _storage[(index * 4) + 2];
@@ -299,7 +240,7 @@ class NativeInt32x4List
   }
 
   void operator[]=(int index, Int32x4 value) {
-    _checkIndex(index, length);
+    _checkValidIndex(index, this);
     _storage[(index * 4) + 0] = value.x;
     _storage[(index * 4) + 1] = value.y;
     _storage[(index * 4) + 2] = value.z;
@@ -307,7 +248,7 @@ class NativeInt32x4List
   }
 
   List<Int32x4> sublist(int start, [int end]) {
-    end = _checkSublistArguments(start, end, length);
+    end = _checkValidRange(start, end, this);
     return new NativeInt32x4List._externalStorage(
         _storage.sublist(start * 4, end * 4));
   }
@@ -366,52 +307,23 @@ class NativeFloat64x2List
 
   int get elementSizeInBytes => Float64x2List.BYTES_PER_ELEMENT;
 
-  void _invalidIndex(int index, int length) {
-    if (index < 0 || index >= length) {
-      if (length == this.length) {
-        throw new RangeError.index(index, this);
-      }
-      throw new RangeError.range(index, 0, length - 1);
-    } else {
-      throw new ArgumentError('Invalid list index $index');
-    }
-  }
-
-  void _checkIndex(int index, int length) {
-    if (JS('bool', '(# >>> 0 != #)', index, index) || index >= length) {
-      _invalidIndex(index, length);
-    }
-  }
-
-  int _checkSublistArguments(int start, int end, int length) {
-    // For `sublist` the [start] and [end] indices are allowed to be equal to
-    // [length]. However, [_checkIndex] only allows indices in the range
-    // 0 .. length - 1. We therefore increment the [length] argument by one
-    // for the [_checkIndex] checks.
-    _checkIndex(start, length + 1);
-    if (end == null) return length;
-    _checkIndex(end, length + 1);
-    if (start > end) throw new RangeError.range(start, 0, end);
-    return end;
-  }
-
   int get length => _storage.length ~/ 2;
 
   Float64x2 operator[](int index) {
-    _checkIndex(index, length);
+    _checkValidIndex(index, this);
     double _x = _storage[(index * 2) + 0];
     double _y = _storage[(index * 2) + 1];
     return new Float64x2(_x, _y);
   }
 
   void operator[]=(int index, Float64x2 value) {
-    _checkIndex(index, length);
+    _checkValidIndex(index, this);
     _storage[(index * 2) + 0] = value.x;
     _storage[(index * 2) + 1] = value.y;
   }
 
   List<Float64x2> sublist(int start, [int end]) {
-    end = _checkSublistArguments(start, end, length);
+    end = _checkValidRange(start, end, this);
     return new NativeFloat64x2List._externalStorage(
         _storage.sublist(start * 2, end * 2));
   }
@@ -446,37 +358,20 @@ class NativeTypedData implements TypedData {
   @JSName('BYTES_PER_ELEMENT')
   final int elementSizeInBytes;
 
-  void _checkIndex(int index, int length) {
-    if (JS('bool', '(# >>> 0) !== #', index, index) ||
-        JS('int', '#', index) >= length) {  // 'int' guaranteed by above test.
-      throw diagnoseIndexError(this, index);
-    }
-  }
-
-  void _invalidPosition(int position, int length) {
+  void _invalidPosition(int position, int length, String name) {
     if (position is !int) {
-      throw new ArgumentError.value(position, null, 'Invalid list position');
+      throw new ArgumentError.value(position, name, 'Invalid list position');
     } else {
-      throw new RangeError.range(position, 0, length);
+      throw new RangeError.range(position, 0, length, name);
     }
   }
 
-  void _checkPosition(int position, int length) {
+  void _checkPosition(int position, int length, String name) {
     if (JS('bool', '(# >>> 0) !== #', position, position) ||
         JS('int', '#', position) > length) {  // 'int' guaranteed by above test.
-      _invalidPosition(position, length);
+      _invalidPosition(position, length, name);
     }
   }
-
-  int _checkSublistArguments(int start, int end, int length) {
-    // For `sublist` the [start] and [end] indices are allowed to be equal to
-    // [length].
-    _checkPosition(start, length);
-    if (end == null) return length;
-    _checkPosition(end, length);
-    if (start > end) throw new RangeError.range(start, 0, end);
-    return end;
-  }
 }
 
 
@@ -862,8 +757,8 @@ abstract class NativeTypedArray extends NativeTypedData
   void _setRangeFast(int start, int end,
       NativeTypedArray source, int skipCount) {
     int targetLength = this.length;
-    _checkPosition(start, targetLength);
-    _checkPosition(end, targetLength);
+    _checkPosition(start, targetLength, "start");
+    _checkPosition(end, targetLength, "end");
     if (start > end) throw new RangeError.range(start, 0, end);
     int count = end - start;
 
@@ -888,12 +783,12 @@ abstract class NativeTypedArrayOfDouble
         with ListMixin<double>, FixedLengthListMixin<double> {
 
   num operator[](int index) {
-    _checkIndex(index, length);
+    _checkValidIndex(index, this);
     return JS('num', '#[#]', this, index);
   }
 
   void operator[]=(int index, num value) {
-    _checkIndex(index, length);
+    _checkValidIndex(index, this);
     JS('void', '#[#] = #', this, index, value);
   }
 
@@ -916,7 +811,7 @@ abstract class NativeTypedArrayOfInt
   // types
 
   void operator[]=(int index, int value) {
-    _checkIndex(index, length);
+    _checkValidIndex(index, this);
     JS('void', '#[#] = #', this, index, value);
   }
 
@@ -952,7 +847,7 @@ class NativeFloat32List
   Type get runtimeType => Float32List;
 
   List<double> sublist(int start, [int end]) {
-    end = _checkSublistArguments(start, end, length);
+    end = _checkValidRange(start, end, this);
     var source = JS('NativeFloat32List', '#.subarray(#, #)', this, start, end);
     return _create1(source);
   }
@@ -989,7 +884,7 @@ class NativeFloat64List
   Type get runtimeType => Float64List;
 
   List<double> sublist(int start, [int end]) {
-    end = _checkSublistArguments(start, end, length);
+    end = _checkValidRange(start, end, this);
     var source = JS('NativeFloat64List', '#.subarray(#, #)', this, start, end);
     return _create1(source);
   }
@@ -1026,12 +921,12 @@ class NativeInt16List
   Type get runtimeType => Int16List;
 
   int operator[](int index) {
-    _checkIndex(index, length);
+    _checkValidIndex(index, this);
     return JS('int', '#[#]', this, index);
   }
 
   List<int> sublist(int start, [int end]) {
-    end = _checkSublistArguments(start, end, length);
+    end = _checkValidRange(start, end, this);
     var source = JS('NativeInt16List', '#.subarray(#, #)', this, start, end);
     return _create1(source);
   }
@@ -1066,12 +961,12 @@ class NativeInt32List extends NativeTypedArrayOfInt implements Int32List {
   Type get runtimeType => Int32List;
 
   int operator[](int index) {
-    _checkIndex(index, length);
+    _checkValidIndex(index, this);
     return JS('int', '#[#]', this, index);
   }
 
   List<int> sublist(int start, [int end]) {
-    end = _checkSublistArguments(start, end, length);
+    end = _checkValidRange(start, end, this);
     var source = JS('NativeInt32List', '#.subarray(#, #)', this, start, end);
     return _create1(source);
   }
@@ -1106,12 +1001,12 @@ class NativeInt8List extends NativeTypedArrayOfInt implements Int8List {
   Type get runtimeType => Int8List;
 
   int operator[](int index) {
-    _checkIndex(index, length);
+    _checkValidIndex(index, this);
     return JS('int', '#[#]', this, index);
   }
 
   List<int> sublist(int start, [int end]) {
-    end = _checkSublistArguments(start, end, length);
+    end = _checkValidRange(start, end, this);
     var source = JS('NativeInt8List', '#.subarray(#, #)', this, start, end);
     return _create1(source);
   }
@@ -1146,12 +1041,12 @@ class NativeUint16List extends NativeTypedArrayOfInt implements Uint16List {
   Type get runtimeType => Uint16List;
 
   int operator[](int index) {
-    _checkIndex(index, length);
+    _checkValidIndex(index, this);
     return JS('JSUInt31', '#[#]', this, index);
   }
 
   List<int> sublist(int start, [int end]) {
-    end = _checkSublistArguments(start, end, length);
+    end = _checkValidRange(start, end, this);
     var source = JS('NativeUint16List', '#.subarray(#, #)', this, start, end);
     return _create1(source);
   }
@@ -1186,12 +1081,12 @@ class NativeUint32List extends NativeTypedArrayOfInt implements Uint32List {
   Type get runtimeType => Uint32List;
 
   int operator[](int index) {
-    _checkIndex(index, length);
+    _checkValidIndex(index, this);
     return JS('JSUInt32', '#[#]', this, index);
   }
 
   List<int> sublist(int start, [int end]) {
-    end = _checkSublistArguments(start, end, length);
+    end = _checkValidRange(start, end, this);
     var source = JS('NativeUint32List', '#.subarray(#, #)', this, start, end);
     return _create1(source);
   }
@@ -1230,12 +1125,12 @@ class NativeUint8ClampedList
   int get length => JS('JSUInt32', '#.length', this);
 
   int operator[](int index) {
-    _checkIndex(index, length);
+    _checkValidIndex(index, this);
     return JS('JSUInt31', '#[#]', this, index);
   }
 
   List<int> sublist(int start, [int end]) {
-    end = _checkSublistArguments(start, end, length);
+    end = _checkValidRange(start, end, this);
     var source = JS('NativeUint8ClampedList', '#.subarray(#, #)',
         this, start, end);
     return _create1(source);
@@ -1278,12 +1173,12 @@ class NativeUint8List extends NativeTypedArrayOfInt implements Uint8List {
   int get length => JS('JSUInt32', '#.length', this);
 
   int operator[](int index) {
-    _checkIndex(index, length);
+    _checkValidIndex(index, this);
     return JS('JSUInt31', '#[#]', this, index);
   }
 
   List<int> sublist(int start, [int end]) {
-    end = _checkSublistArguments(start, end, length);
+    end = _checkValidRange(start, end, this);
     var source = JS('NativeUint8List', '#.subarray(#, #)', this, start, end);
     return _create1(source);
   }
@@ -2006,3 +1901,29 @@ class NativeFloat64x2 implements Float64x2 {
       return new NativeFloat64x2._doubles(Math.sqrt(x), Math.sqrt(y));
   }
 }
+
+/// Checks that the value is a Uint32. If not, it's not valid as an array
+/// index or offset.
+void _checkValidArrayIndex(int index, String name) {
+  if (JS('bool', '(# >>> 0 != #)', index, index)) {
+    throw new ArgumentError.value(index, "index", "Not a valid list index");
+  }
+}
+
+void _checkValidIndex(int index, List list) {

[sra1, 2015/09/08 17:49:20]

You must be careful using code from true native classes like Float32List and
fake ones like Float32x4List. Not only is 'list' polymorphic, it is polymorphic
between intercepted classes and 'plain Dart' classes so any method calls with
require a two-level dispatch with a getInterceptor call, which is going to harm
performance.

This is why _checkIndex is a *member* on the native-only hierarchy - to avoid
any possibility of a getInterceptor call in the indexer. The difference between
passing something as a regular parameter and as 'this' is that in the 'this'
case the interceptor has already been computed as teh call site or earlier.
(Hopefully the only getInterceptor call has been hoisted out of the loop that
calls the indexer).

In addition to being a member function, the original form of _checkIndex was
written so that the actual checks are inlined into the many copies of
operator[], but nothing else is.

The 'throw diagnoseIndexError(this, index)' achieves several things:

1. Since diagnoseIndexError returns the Error object, the JS call stack has no
extra frames (other than _checkIndex if it is not inlined, e.g. in checked
mode). This is nice since you don't have to skip a lot of frames to get to your
program.

2. dart2js is much more conservative when inlining into throw expressions - it
is going to be slow so it might as well be small. diagnoseIndexError is not
duplicated by inlining, making the program smaller. That is where all the
unminifiable strings are.

3. since diagnoseIndexError is on the failure path, it can tolerate polymorphic
call sites and getInterceptor calls that are not acceptable on the fast path.

The x4 classes were unnecessarily different because some of the code was copied
from the VM implementations. I support making them more regular.

Can you make wider use of the 'throw diagnoseIndexError' pattern?
Would the VM benefit from never having to optimize the cold path?

+  _checkValidArrayIndex(index, "index");
+  // Above call ensures index is a non-negative integer.
+  if (JS('int', '#', index) >= list.length) {
+    throw new RangeError.index(index, list);
+  }
+}
+
+int _checkValidRange(int start, int end, List list) {

[floitsch, 2015/09/08 10:54:18]

I'm afraid that this will get polymorphic in JS.
accessing list.length on different arrays could then get expensive.

[Lasse Reichstein Nielsen, 2015/09/08 11:38:03]

So passing the length as a parameter would be better?

[floitsch, 2015/09/08 13:53:25]

Yes. That's what I would do.

+  _checkValidArrayIndex(start, "start");
+  if (end == null) {
+    end = list.length;
+  } else {
+    _checkValidArrayIndex(end, "end");
+  }
+  return RangeError.checkValidRange(start, end, list.length);
+}