Bug 891107 - Part 6: Show information about range and value in array index error messages in js-ctypes. r=jorendorff
authorTooru Fujisawa <arai_a@mac.com>
Fri, 07 Aug 2015 06:53:50 +0900
changeset 288494 daa58aad68585df2ae33b2432d17a5296cb150da
parent 288493 fc5171ce2a4b96c9f8b4a8f3c3a2c7074cd440c9
child 288495 1e8e3d17142899118207b146ed5b994127722ef3
push id30082
push userryanvm@gmail.com
push dateSun, 13 Mar 2016 23:08:35 +0000
treeherdermozilla-central@f0c0480732d3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjorendorff
bugs891107
milestone48.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 891107 - Part 6: Show information about range and value in array index error messages in js-ctypes. r=jorendorff
js/src/ctypes/CTypes.cpp
js/src/ctypes/ctypes.msg
js/src/jit-test/tests/ctypes/array-index.js
toolkit/components/ctypes/tests/unit/test_jsctypes.js
--- a/js/src/ctypes/CTypes.cpp
+++ b/js/src/ctypes/CTypes.cpp
@@ -1490,16 +1490,50 @@ IncompatibleThisType(JSContext* cx, cons
 
   JS_ReportErrorNumber(cx, GetErrorMessage, nullptr,
                        CTYPESMSG_INCOMPATIBLE_THIS_VAL,
                        funName, actualType, valStr);
   return false;
 }
 
 static bool
+InvalidIndexError(JSContext* cx, HandleValue val)
+{
+  JSAutoByteString idBytes;
+  const char* indexStr = CTypesToSourceForError(cx, val, idBytes);
+  if (!indexStr)
+    return false;
+
+  JS_ReportErrorNumber(cx, GetErrorMessage, nullptr,
+                       CTYPESMSG_INVALID_INDEX, indexStr);
+  return false;
+}
+
+static bool
+InvalidIndexError(JSContext* cx, HandleId id)
+{
+  RootedValue idVal(cx, IdToValue(id));
+  return InvalidIndexError(cx, idVal);
+}
+
+static bool
+InvalidIndexRangeError(JSContext* cx, size_t index, size_t length)
+{
+  char indexStr[16];
+  JS_snprintf(indexStr, 16, "%u", index);
+
+  char lengthStr[16];
+  JS_snprintf(lengthStr, 16, "%u", length);
+
+  JS_ReportErrorNumber(cx, GetErrorMessage, nullptr,
+                       CTYPESMSG_INVALID_RANGE, indexStr, lengthStr);
+  return false;
+}
+
+static bool
 NonPrimitiveError(JSContext* cx, HandleObject typeObj)
 {
   MOZ_ASSERT(CType::IsCType(typeObj));
 
   AutoString typeSource;
   JSAutoByteString typeBytes;
   BuildTypeSource(cx, typeObj, true, typeSource);
   const char* typeStr = EncodeLatin1(cx, typeSource, typeBytes);
@@ -5371,19 +5405,21 @@ ArrayType::Getter(JSContext* cx, HandleO
     return true;
   bool dummy2;
   if (!ok && JSID_IS_STRING(idval) &&
       !StringToInteger(cx, JSID_TO_STRING(idval), &dummy, &dummy2)) {
     // String either isn't a number, or doesn't fit in size_t.
     // Chances are it's a regular property lookup, so return.
     return true;
   }
-  if (!ok || index >= length) {
-    JS_ReportError(cx, "invalid index");
-    return false;
+  if (!ok) {
+    return InvalidIndexError(cx, idval);
+  }
+  if (index >= length) {
+    return InvalidIndexRangeError(cx, index, length);
   }
 
   RootedObject baseType(cx, GetBaseType(typeObj));
   size_t elementSize = CType::GetSize(baseType);
   char* data = static_cast<char*>(CData::GetData(obj)) + elementSize * index;
   return ConvertToJS(cx, baseType, obj, data, false, false, vp);
 }
 
@@ -5412,19 +5448,21 @@ ArrayType::Setter(JSContext* cx, HandleO
     return true;
   bool dummy2;
   if (!ok && JSID_IS_STRING(idval) &&
       !StringToInteger(cx, JSID_TO_STRING(idval), &dummy, &dummy2)) {
     // String either isn't a number, or doesn't fit in size_t.
     // Chances are it's a regular property lookup, so return.
     return result.succeed();
   }
-  if (!ok || index >= length) {
-    JS_ReportError(cx, "invalid index");
-    return false;
+  if (!ok) {
+    return InvalidIndexError(cx, idval);
+  }
+  if (index >= length) {
+    return InvalidIndexRangeError(cx, index, length);
   }
 
   RootedObject baseType(cx, GetBaseType(typeObj));
   size_t elementSize = CType::GetSize(baseType);
   char* data = static_cast<char*>(CData::GetData(obj)) + elementSize * index;
   if (!ImplicitConvert(cx, vp, baseType, data, ConversionType::Setter,
                        nullptr, nullptr, 0, typeObj, index))
     return false;
@@ -5464,20 +5502,21 @@ ArrayType::AddressOfElement(JSContext* c
   if (!result)
     return false;
 
   args.rval().setObject(*result);
 
   // Convert the index to a size_t and bounds-check it.
   size_t index;
   size_t length = GetLength(typeObj);
-  if (!jsvalToSize(cx, args[0], false, &index) ||
-      index >= length) {
-    JS_ReportError(cx, "invalid index");
-    return false;
+  if (!jsvalToSize(cx, args[0], false, &index)) {
+    return InvalidIndexError(cx, args[0]);
+  }
+  if (index >= length) {
+    return InvalidIndexRangeError(cx, index, length);
   }
 
   // Manually set the pointer inside the object, so we skip the conversion step.
   void** data = static_cast<void**>(CData::GetData(result));
   size_t elementSize = CType::GetSize(baseType);
   *data = static_cast<char*>(CData::GetData(obj)) + elementSize * index;
   return true;
 }
--- a/js/src/ctypes/ctypes.msg
+++ b/js/src/ctypes/ctypes.msg
@@ -19,16 +19,18 @@ MSG_DEF(CTYPESMSG_CONV_ERROR_RET,2, JSEX
 MSG_DEF(CTYPESMSG_CONV_ERROR_SET,2, JSEXN_TYPEERR, "can't convert {0} to the type {1}")
 MSG_DEF(CTYPESMSG_CONV_ERROR_STRUCT,5, JSEXN_TYPEERR, "can't convert {0} to the '{1}' field ({2}) of {3}{4}")
 MSG_DEF(CTYPESMSG_NON_PRIMITIVE, 1, JSEXN_TYPEERR, ".value only works on character and numeric types, not `{0}`")
 MSG_DEF(CTYPESMSG_TYPE_ERROR,    2, JSEXN_TYPEERR, "expected {0}, got {1}")
 
 /* array */
 MSG_DEF(CTYPESMSG_ARRAY_MISMATCH,4, JSEXN_TYPEERR, "length of {0} does not match to the length of the type {1} (expected {2}, got {3})")
 MSG_DEF(CTYPESMSG_ARRAY_OVERFLOW,4, JSEXN_TYPEERR, "length of {0} does not fit in the length of the type {1} (expected {2} or lower, got {3})")
+MSG_DEF(CTYPESMSG_INVALID_INDEX, 1, JSEXN_TYPEERR, "{0} is not a valid array index")
+MSG_DEF(CTYPESMSG_INVALID_RANGE, 2, JSEXN_RANGEERR, "array index {0} is out of bounds for array of length {1}")
 
 /* struct */
 MSG_DEF(CTYPESMSG_FIELD_MISMATCH,5, JSEXN_TYPEERR, "property count of {0} does not match to field count of the type {1} (expected {2}, got {3}){4}")
 MSG_DEF(CTYPESMSG_PROP_NONSTRING,3, JSEXN_TYPEERR, "property name {0} of {1} is not a string{2}")
 
 /* data finalizer */
 MSG_DEF(CTYPESMSG_EMPTY_FIN,     1, JSEXN_TYPEERR, "attempting to convert an empty CDataFinalizer{0}")
 MSG_DEF(CTYPESMSG_EMPTY_FIN_CALL,1, JSEXN_TYPEERR, "{0} called on empty CDataFinalizer")
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/ctypes/array-index.js
@@ -0,0 +1,29 @@
+load(libdir + 'asserts.js');
+
+function test() {
+  let a = ctypes.int32_t.array(10)();
+  assertTypeErrorMessage(() => { let x = a[-1]; },
+                          "the string \"-1\" is not a valid array index");
+  assertTypeErrorMessage(() => { a[-1] = 1; },
+                          "the string \"-1\" is not a valid array index");
+  assertTypeErrorMessage(() => { a.addressOfElement(-1); },
+                          "the number -1 is not a valid array index");
+
+  assertRangeErrorMessage(() => { let x = a[10]; },
+                          "array index 10 is out of bounds for array of length 10");
+  assertRangeErrorMessage(() => { a[10] = 1; },
+                          "array index 10 is out of bounds for array of length 10");
+  assertRangeErrorMessage(() => { a.addressOfElement(10); },
+                          "array index 10 is out of bounds for array of length 10");
+
+  let obj = {
+    toSource() {
+      throw 1;
+    }
+  };
+  assertTypeErrorMessage(() => { a.addressOfElement(obj); },
+                          "<<error converting value to string>> is not a valid array index");
+}
+
+if (typeof ctypes === "object")
+  test();
--- a/toolkit/components/ctypes/tests/unit/test_jsctypes.js
+++ b/toolkit/components/ctypes/tests/unit/test_jsctypes.js
@@ -2011,19 +2011,19 @@ function run_ArrayType_tests() {
   do_check_throws(function() { ctypes.int32_t.array().array(); }, Error);
 
   let a = new a_t();
   do_check_eq(a[0].a, 0);
   do_check_eq(a[0].b, 0);
   a[0] = g;
   do_check_eq(a[0].a, 1);
   do_check_eq(a[0].b, 2);
-  do_check_throws(function() { a[-1]; }, Error);
+  do_check_throws(function() { a[-1]; }, TypeError);
   do_check_eq(a[9].a, 0);
-  do_check_throws(function() { a[10]; }, Error);
+  do_check_throws(function() { a[10]; }, RangeError);
 
   do_check_eq(a[ctypes.Int64(0)].a, 1);
   do_check_eq(a[ctypes.UInt64(0)].b, 2);
 
   let a_p = a.addressOfElement(0);
   do_check_true(a_p.constructor.targetType === g_t);
   do_check_true(a_p.constructor === g_t.ptr);
   do_check_eq(a_p.contents.a, a[0].a);