Better messages for optimized index errors.

Typed list, Array and String indexer methods detect index errors in their own optimized way but now defer to common code to diagnose the error.  This code is shared with the optimized 'ioore' helper, which makes the error message consistent between optimized and unoptimized code.

The main reason for doing this is to ensure that the error messages have as much information as possible to help debug off-line using captured error messages.

BUG=
R=lrn@google.com

Committed: 0d4361f

Reverted: 82215d3

Review URL: https://codereview.chromium.org//1180713003.

Author: rakudrama

sdk/lib/_internal/compiler/js_lib/interceptors.dart

@@ -20,6 +20,7 @@ import 'dart:_js_helper' show allMatchesInStringUnchecked,
                               checkNum,
                               checkString,
                               defineProperty,
+                              diagnoseIndexError,
                               getRuntimeType,
                               initNativeDispatch,
                               initNativeDispatchFlag,

sdk/lib/_internal/compiler/js_lib/js_array.dart

@@ -575,21 +575,28 @@ class JSArray<E> extends Interceptor implements List<E>, JSIndexable {
 
   void set length(int newLength) {
     checkGrowable('set length');
-    if (newLength is !int) throw new ArgumentError(newLength);
-    if (newLength < 0) throw new RangeError.value(newLength);
+    if (newLength is !int) {
+      throw new ArgumentError.value(newLength, 'newLength');
+    }
+    // TODO(sra): Remove this test and let JavaScript throw an error.
+    if (newLength < 0) {
+      throw new RangeError.range(newLength, 0, null, 'newLength');
+    }
+    // JavaScript with throw a RangeError for numbers that are too big. The
+    // message does not contain the value.
     JS('void', r'#.length = #', this, newLength);
   }
 
   E operator [](int index) {
-    if (index is !int) throw new ArgumentError(index);
-    if (index >= length || index < 0) throw new RangeError.value(index);
+    if (index is !int) throw diagnoseIndexError(this, index);
+    if (index >= length || index < 0) throw diagnoseIndexError(this, index);
     return JS('var', '#[#]', this, index);
   }
 
   void operator []=(int index, E value) {
     checkMutable('indexed set');
-    if (index is !int) throw new ArgumentError(index);
-    if (index >= length || index < 0) throw new RangeError.value(index);
+    if (index is !int) throw diagnoseIndexError(this, index);
+    if (index >= length || index < 0) throw diagnoseIndexError(this, index);
     JS('void', r'#[#] = #', this, index, value);
   }
 

sdk/lib/_internal/compiler/js_lib/js_helper.dart

@@ -1388,23 +1388,41 @@ class JsCache {
  * for example, if a non-integer index is given to an optimized
  * indexed access.
  */
+@NoInline()
 iae(argument) {
   throw _argumentError(argument);
 }
 
 /**
- * Called by generated code to throw an index-out-of-range exception,
- * for example, if a bounds check fails in an optimized indexed
- * access.  This may also be called when the index is not an integer, in
- * which case it throws an illegal-argument exception instead, like
- * [iae], or when the receiver is null.
+ * Called by generated code to throw an index-out-of-range exception, for
+ * example, if a bounds check fails in an optimized indexed access.  This may
+ * also be called when the index is not an integer, in which case it throws an
+ * illegal-argument exception instead, like [iae], or when the receiver is null.
  */
+@NoInline()
 ioore(receiver, index) {
   if (receiver == null) receiver.length; // Force a NoSuchMethodError.
-  if (index is !int) iae(index);
-  throw new RangeError.value(index);
+  throw diagnoseIndexError(receiver, index);
+}
+
+/**
+ * Diagnoses an indexing error. Returns the ArgumentError or RangeError that
+ * describes the problem.
+ */
+@NoInline()
+Error diagnoseIndexError(indexable, index) {
+  if (index is !int) return new ArgumentError.value(index, 'index');
+  int length = indexable.length;
+  // The following returns the same error that would be thrown by calling
+  // [RangeError.checkValidIndex] with no optional parameters provided.
+  if (index < 0 || index >= length) {
+    return new RangeError.index(index, indexable, 'index', null, length);
+  }
+  // The above should always match, but if it does not, use the following.
+  return new RangeError.value(index, 'index');
 }
 
+
 stringLastIndexOfUnchecked(receiver, element, start)
   => JS('int', r'#.lastIndexOf(#, #)', receiver, element, start);
 
@@ -2015,10 +2033,15 @@ unwrapException(ex) {
       return new StackOverflowError();
     }
 
-    // In general, a RangeError is thrown when trying to pass a number
-    // as an argument to a function that does not allow a range that
-    // includes that number.
-    return saveStackTrace(new ArgumentError());
+    // In general, a RangeError is thrown when trying to pass a number as an
+    // argument to a function that does not allow a range that includes that
+    // number. Translate to a Dart ArgumentError with the same message.
+    // TODO(sra): Translate to RangeError.
+    String message = tryStringifyException(ex);
+    if (message is String) {
+      message = JS('String', r'#.replace(/^RangeError:\s*/, "")', message);
+    }
+    return saveStackTrace(new ArgumentError(message));
   }
 
   // Check for the Firefox specific stack overflow signal.
@@ -2036,6 +2059,20 @@ unwrapException(ex) {
   return ex;
 }
 
+String tryStringifyException(ex) {
+  // Since this function is called from [unwrapException] which is called from
+  // code injected into a catch-clause, use JavaScript try-catch to avoid a
+  // potential loop if stringifying crashes.
+  return JS('String|Null', r'''
+    (function(ex) {
+      try {
+        return String(ex);
+      } catch (e) {}
+      return null;
+    })(#)
+    ''', ex);
+}
+
 /**
  * Called by generated code to fetch the stack trace from an
  * exception. Should never return null.

sdk/lib/_internal/compiler/js_lib/js_string.dart

@@ -14,9 +14,9 @@ class JSString extends Interceptor implements String, JSIndexable {
   const JSString();
 
   int codeUnitAt(int index) {
-    if (index is !int) throw new ArgumentError(index);
-    if (index < 0) throw new RangeError.value(index);
-    if (index >= length) throw new RangeError.value(index);
+    if (index is !int) throw diagnoseIndexError(this, index);
+    if (index < 0) throw diagnoseIndexError(this, index);
+    if (index >= length) throw diagnoseIndexError(this, index);
     return JS('JSUInt31', r'#.charCodeAt(#)', this, index);
   }
 
@@ -44,7 +44,7 @@ class JSString extends Interceptor implements String, JSIndexable {
   }
 
   String operator +(String other) {
-    if (other is !String) throw new ArgumentError(other);
+    if (other is !String) throw new ArgumentError.value(other);
     return JS('String', r'# + #', this, other);
   }
 
@@ -469,8 +469,8 @@ class JSString extends Interceptor implements String, JSIndexable {
   int get length => JS('int', r'#.length', this);
 
   String operator [](int index) {
-    if (index is !int) throw new ArgumentError(index);
-    if (index >= length || index < 0) throw new RangeError.value(index);
+    if (index is !int) throw diagnoseIndexError(this, index);
+    if (index >= length || index < 0) throw diagnoseIndexError(this, index);
     return JS('String', '#[#]', this, index);
   }
 }

sdk/lib/_internal/compiler/js_lib/native_typed_data.dart

@@ -11,8 +11,9 @@ library dart.typed_data.implementation;
 import 'dart:collection';
 import 'dart:_internal';
 import 'dart:_interceptors' show JSIndexable, JSUInt32, JSUInt31;
-import 'dart:_js_helper'
-show Creates, JavaScriptIndexingBehavior, JSName, Native, Null, Returns;
+import 'dart:_js_helper' show
+    Creates, JavaScriptIndexingBehavior, JSName, Native, Null, Returns,
+    diagnoseIndexError;
 import 'dart:_foreign_helper' show JS;
 import 'dart:math' as Math;
 
@@ -445,35 +446,34 @@ class NativeTypedData implements TypedData {
   @JSName('BYTES_PER_ELEMENT')
   final int elementSizeInBytes;
 
-  void _invalidIndex(int index, int length) {
-    if (index < 0 || index >= length) {
-      if (this is List) {
-        var list = this;  // Typed as dynamic to avoid warning.
-        if (length == list.length) {
-          throw new RangeError.index(index, this);
-        }
-      }
-      throw new RangeError.range(index, 0, length - 1);
+  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) {
+    if (position is !int) {
+      throw new ArgumentError.value(position, null, 'Invalid list position');
     } else {
-      throw new ArgumentError('Invalid list index $index');
+      throw new RangeError.range(position, 0, length);
     }
   }
 
-  void _checkIndex(int index, int length) {
-    if (JS('bool', '(# >>> 0) !== #', index, index) ||
-        JS('int', '#', index) >= length) {  // 'int' guaranteed by above test.
-      _invalidIndex(index, length);
+  void _checkPosition(int position, int length) {
+    if (JS('bool', '(# >>> 0) !== #', position, position) ||
+        JS('int', '#', position) > length) {  // 'int' guaranteed by above test.
+      _invalidPosition(position, 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);
+    // [length].
+    _checkPosition(start, length);
     if (end == null) return length;
-    _checkIndex(end, length + 1);
+    _checkPosition(end, length);
     if (start > end) throw new RangeError.range(start, 0, end);
     return end;
   }
@@ -862,8 +862,8 @@ abstract class NativeTypedArray extends NativeTypedData
   void _setRangeFast(int start, int end,
       NativeTypedArray source, int skipCount) {
     int targetLength = this.length;
-    _checkIndex(start, targetLength + 1);
-    _checkIndex(end, targetLength + 1);
+    _checkPosition(start, targetLength);
+    _checkPosition(end, targetLength);
     if (start > end) throw new RangeError.range(start, 0, end);
     int count = end - start;
 

sdk/lib/core/errors.dart

@@ -282,7 +282,8 @@ class RangeError extends ArgumentError {
   static void checkValidIndex(int index, var indexable,
                               [String name, int length, String message]) {
     if (length == null) length = indexable.length;
-    if (index < 0 || index >= length) {
+    // Comparing with `0` as receiver produces better dart2js type inference.
+    if (0 > index || index >= length) {
       if (name == null) name = "index";
       throw new RangeError.index(index, indexable, name, message, length);
     }