Bug 891107 - Part 6: Show information about range and value in array index error messages in js-ctypes. r=jorendorff
☠☠ backed out by 69104d84a781 ☠ ☠
authorTooru Fujisawa <arai_a@mac.com>
Fri, 07 Aug 2015 06:53:50 +0900
changeset 288441 8fe18f2b8aa9240e032a935f9af3d99e180a9f84
parent 288440 13e045fff28d32c498821dc0d3f094140f1f1a36
child 288442 ebd8c98e232911cfb61cf416d8466979a257b0e1
push id30079
push userryanvm@gmail.com
push dateSat, 12 Mar 2016 20:24:19 +0000
treeherdermozilla-central@d1d47ba19ce9 [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);