Bug 891107 - Part 7: Show information about value, type, function, and argument number in function related error messages in js-ctypes. r=jorendorff
☠☠ backed out by 65e48fb8295b ☠ ☠
authorTooru Fujisawa <arai_a@mac.com>
Fri, 07 Aug 2015 06:57:39 +0900
changeset 288442 ebd8c98e232911cfb61cf416d8466979a257b0e1
parent 288441 8fe18f2b8aa9240e032a935f9af3d99e180a9f84
child 288443 c47e7505bd94cec93f10756473bfa9b415ada737
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 7: Show information about value, type, function, and argument number in function related error messages in js-ctypes. r=jorendorff
js/src/ctypes/CTypes.cpp
js/src/ctypes/ctypes.msg
js/src/jit-test/tests/ctypes/function-definition.js
toolkit/components/ctypes/tests/unit/test_jsctypes.js
--- a/js/src/ctypes/CTypes.cpp
+++ b/js/src/ctypes/CTypes.cpp
@@ -1429,16 +1429,79 @@ FinalizerSizeError(JSContext* cx, Handle
     return false;
 
   JS_ReportErrorNumber(cx, GetErrorMessage, nullptr,
                        CTYPESMSG_FIN_SIZE_ERROR, funStr, valStr);
   return false;
 }
 
 static bool
+FunctionArgumentLengthMismatch(JSContext* cx,
+                               unsigned expectedCount, unsigned actualCount,
+                               HandleObject funObj, HandleObject typeObj,
+                               bool isVariadic)
+{
+  AutoString funSource;
+  JSAutoByteString funBytes;
+  Value slot = JS_GetReservedSlot(funObj, SLOT_REFERENT);
+  if (!slot.isUndefined() && Library::IsLibrary(&slot.toObject())) {
+    BuildFunctionTypeSource(cx, funObj, funSource);
+  } else {
+    BuildFunctionTypeSource(cx, typeObj, funSource);
+  }
+  const char* funStr = EncodeLatin1(cx, funSource, funBytes);
+  if (!funStr)
+    return false;
+
+  char expectedCountStr[16];
+  JS_snprintf(expectedCountStr, 16, "%u", expectedCount);
+  char actualCountStr[16];
+  JS_snprintf(actualCountStr, 16, "%u", actualCount);
+
+  const char* variadicStr = isVariadic ? " or more": "";
+
+  JS_ReportErrorNumber(cx, GetErrorMessage, nullptr,
+                       CTYPESMSG_ARG_COUNT_MISMATCH,
+                       funStr, expectedCountStr, variadicStr, actualCountStr);
+  return false;
+}
+
+static bool
+FunctionArgumentTypeError(JSContext* cx,
+                          uint32_t index, Value type, const char* reason)
+{
+  RootedValue val(cx, type);
+  JSAutoByteString valBytes;
+  const char* valStr = CTypesToSourceForError(cx, val, valBytes);
+  if (!valStr)
+    return false;
+
+  char indexStr[16];
+  JS_snprintf(indexStr, 16, "%u", index + 1);
+
+  JS_ReportErrorNumber(cx, GetErrorMessage, nullptr,
+                       CTYPESMSG_ARG_TYPE_ERROR, indexStr, reason, valStr);
+  return false;
+}
+
+static bool
+FunctionReturnTypeError(JSContext* cx, Value type, const char* reason)
+{
+  RootedValue val(cx, type);
+  JSAutoByteString valBytes;
+  const char* valStr = CTypesToSourceForError(cx, val, valBytes);
+  if (!valStr)
+    return false;
+
+  JS_ReportErrorNumber(cx, GetErrorMessage, nullptr,
+                       CTYPESMSG_RET_TYPE_ERROR, reason, valStr);
+  return false;
+}
+
+static bool
 IncompatibleCallee(JSContext* cx, const char* funName, HandleObject actualObj)
 {
   JSAutoByteString valBytes;
   RootedValue val(cx, ObjectValue(*actualObj));
   const char* valStr = CTypesToSourceForError(cx, val, valBytes);
   if (!valStr)
     return false;
 
@@ -1607,16 +1670,32 @@ TypeOverflow(JSContext* cx, const char* 
   if (!valStr)
     return false;
 
   JS_ReportErrorNumber(cx, GetErrorMessage, nullptr,
                        CTYPESMSG_TYPE_OVERFLOW, valStr, expected);
   return false;
 }
 
+static bool
+VariadicArgumentTypeError(JSContext* cx, uint32_t index, HandleValue actual)
+{
+  JSAutoByteString valBytes;
+  const char* valStr = CTypesToSourceForError(cx, actual, valBytes);
+  if (!valStr)
+    return false;
+
+  char indexStr[16];
+  JS_snprintf(indexStr, 16, "%u", index + 1);
+
+  JS_ReportErrorNumber(cx, GetErrorMessage, nullptr,
+                       CTYPESMSG_VARG_TYPE_ERROR, indexStr, valStr);
+  return false;
+}
+
 static JSObject*
 InitCTypeClass(JSContext* cx, HandleObject ctypesObj)
 {
   JSFunction* fun = JS_DefineFunction(cx, ctypesObj, "CType", ConstructAbstract, 0,
                                       CTYPESCTOR_FLAGS);
   if (!fun)
     return nullptr;
 
@@ -4022,17 +4101,18 @@ CType::ConstructData(JSContext* cx,
   // An instance 'd' of a CData object of type 't' has:
   //   * [[Class]] "CData"
   //   * __proto__ === t.prototype
   switch (GetTypeCode(obj)) {
   case TYPE_void_t:
     JS_ReportError(cx, "cannot construct from void_t");
     return false;
   case TYPE_function:
-    JS_ReportError(cx, "cannot construct from FunctionType; use FunctionType.ptr instead");
+    JS_ReportErrorNumber(cx, GetErrorMessage, nullptr,
+                         CTYPESMSG_FUNCTION_CONSTRUCT);
     return false;
   case TYPE_pointer:
     return PointerType::ConstructData(cx, obj, args);
   case TYPE_array:
     return ArrayType::ConstructData(cx, obj, args);
   case TYPE_struct:
     return StructType::ConstructData(cx, obj, args);
   default:
@@ -6277,70 +6357,70 @@ GetABI(JSContext* cx, Value abiType, ffi
 #endif
   case INVALID_ABI:
     break;
   }
   return false;
 }
 
 static JSObject*
-PrepareType(JSContext* cx, Value type)
+PrepareType(JSContext* cx, uint32_t index, HandleValue type)
 {
   if (type.isPrimitive() || !CType::IsCType(type.toObjectOrNull())) {
-    JS_ReportError(cx, "not a ctypes type");
+    FunctionArgumentTypeError(cx, index, type, "is not a ctypes type");
     return nullptr;
   }
 
   JSObject* result = type.toObjectOrNull();
   TypeCode typeCode = CType::GetTypeCode(result);
 
   if (typeCode == TYPE_array) {
     // convert array argument types to pointers, just like C.
     // ImplicitConvert will do the same, when passing an array as data.
     RootedObject baseType(cx, ArrayType::GetBaseType(result));
     result = PointerType::CreateInternal(cx, baseType);
     if (!result)
       return nullptr;
 
   } else if (typeCode == TYPE_void_t || typeCode == TYPE_function) {
     // disallow void or function argument types
-    JS_ReportError(cx, "Cannot have void or function argument type");
+    FunctionArgumentTypeError(cx, index, type, "cannot be void or function");
     return nullptr;
   }
 
   if (!CType::IsSizeDefined(result)) {
-    JS_ReportError(cx, "Argument type must have defined size");
+    FunctionArgumentTypeError(cx, index, type, "must have defined size");
     return nullptr;
   }
 
   // libffi cannot pass types of zero size by value.
   MOZ_ASSERT(CType::GetSize(result) != 0);
 
   return result;
 }
 
 static JSObject*
 PrepareReturnType(JSContext* cx, Value type)
 {
   if (type.isPrimitive() || !CType::IsCType(type.toObjectOrNull())) {
-    JS_ReportError(cx, "not a ctypes type");
+    FunctionReturnTypeError(cx, type, "is not a ctypes type");
     return nullptr;
   }
 
   JSObject* result = type.toObjectOrNull();
   TypeCode typeCode = CType::GetTypeCode(result);
 
   // Arrays and functions can never be return types.
   if (typeCode == TYPE_array || typeCode == TYPE_function) {
-    JS_ReportError(cx, "Return type cannot be an array or function");
+    FunctionReturnTypeError(cx, type, "cannot be an array or function");
     return nullptr;
   }
 
   if (typeCode != TYPE_void_t && !CType::IsSizeDefined(result)) {
-    JS_ReportError(cx, "Return type must have defined size");
+    FunctionReturnTypeError(cx, type, "must have defined size");
     return nullptr;
   }
 
   // libffi cannot pass types of zero size by value.
   MOZ_ASSERT(typeCode == TYPE_void_t || CType::GetSize(result) != 0);
 
   return result;
 }
@@ -6498,17 +6578,17 @@ CreateFunctionInfo(JSContext* cx,
       if (GetABICode(fninfo->mABI) != ABI_DEFAULT) {
         JS_ReportError(cx, "Variadic functions must use the __cdecl calling "
                        "convention");
         return false;
       }
       break;
     }
 
-    JSObject* argType = PrepareType(cx, args[i]);
+    JSObject* argType = PrepareType(cx, i, args[i]);
     if (!argType)
       return false;
 
     ffi_type* ffiType = CType::GetFFIType(cx, argType);
     if (!ffiType)
       return false;
 
     fninfo->mArgTypes.infallibleAppend(argType);
@@ -6713,18 +6793,18 @@ FunctionType::Call(JSContext* cx,
                                 "non-FunctionType pointer", args.calleev());
   }
 
   FunctionInfo* fninfo = GetFunctionInfo(typeObj);
   uint32_t argcFixed = fninfo->mArgTypes.length();
 
   if ((!fninfo->mIsVariadic && args.length() != argcFixed) ||
       (fninfo->mIsVariadic && args.length() < argcFixed)) {
-    JS_ReportError(cx, "Number of arguments does not match declaration");
-    return false;
+    return FunctionArgumentLengthMismatch(cx, argcFixed, args.length(),
+                                          obj, typeObj, fninfo->mIsVariadic);
   }
 
   // Check if we have a Library object. If we do, make sure it's open.
   Value slot = JS_GetReservedSlot(obj, SLOT_REFERENT);
   if (!slot.isUndefined() && Library::IsLibrary(&slot.toObject())) {
     PRLibrary* library = Library::GetLibrary(&slot.toObject());
     if (!library) {
       JS_ReportError(cx, "library is not open");
@@ -6754,27 +6834,35 @@ FunctionType::Call(JSContext* cx,
     RootedObject obj(cx);  // Could reuse obj instead of declaring a second
     RootedObject type(cx); // RootedObject, but readability would suffer.
 
     for (uint32_t i = argcFixed; i < args.length(); ++i) {
       if (args[i].isPrimitive() ||
           !CData::IsCData(obj = &args[i].toObject())) {
         // Since we know nothing about the CTypes of the ... arguments,
         // they absolutely must be CData objects already.
-        JS_ReportError(cx, "argument %d of type %s is not a CData object",
-                       i, InformalValueTypeName(args[i]));
+        return VariadicArgumentTypeError(cx, i, args[i]);
+      }
+      type = CData::GetCType(obj);
+      if (!type) {
+        // These functions report their own errors.
         return false;
       }
-      if (!(type = CData::GetCType(obj)) ||
-          !(type = PrepareType(cx, ObjectValue(*type))) ||
-          // Relying on ImplicitConvert only for the limited purpose of
-          // converting one CType to another (e.g., T[] to T*).
-          !ConvertArgument(cx, obj, i, args[i], type, &values[i], &strings) ||
-          !(fninfo->mFFITypes[i] = CType::GetFFIType(cx, type))) {
-        // These functions report their own errors.
+      RootedValue typeVal(cx, ObjectValue(*type));
+      type = PrepareType(cx, i, typeVal);
+      if (!type) {
+        return false;
+      }
+      // Relying on ImplicitConvert only for the limited purpose of
+      // converting one CType to another (e.g., T[] to T*).
+      if (!ConvertArgument(cx, obj, i, args[i], type, &values[i], &strings)) {
+        return false;
+      }
+      fninfo->mFFITypes[i] = CType::GetFFIType(cx, type);
+      if (!fninfo->mFFITypes[i]) {
         return false;
       }
     }
     if (!PrepareCIF(cx, fninfo))
       return false;
   }
 
   // initialize a pointer to an appropriate location, for storing the result
--- a/js/src/ctypes/ctypes.msg
+++ b/js/src/ctypes/ctypes.msg
@@ -43,8 +43,15 @@ MSG_DEF(CTYPESMSG_INCOMPATIBLE_CALLEE,2,
 MSG_DEF(CTYPESMSG_INCOMPATIBLE_THIS,2, JSEXN_TYPEERR, "{0} called on incompatible {1}")
 MSG_DEF(CTYPESMSG_INCOMPATIBLE_THIS_TYPE,2, JSEXN_TYPEERR, "{0} called on {1}")
 MSG_DEF(CTYPESMSG_INCOMPATIBLE_THIS_VAL,3, JSEXN_TYPEERR, "{0} called on {1}, got {2}")
 MSG_DEF(CTYPESMSG_WRONG_ARG_LENGTH,3, JSEXN_TYPEERR, "{0} takes {1} argument{2}")
 
 /* overflow */
 MSG_DEF(CTYPESMSG_SIZE_OVERFLOW, 2, JSEXN_RANGEERR, "{0} does not fit in {1}")
 MSG_DEF(CTYPESMSG_TYPE_OVERFLOW, 2, JSEXN_RANGEERR, "{0} does not fit in the type {1}")
+
+/* function */
+MSG_DEF(CTYPESMSG_ARG_COUNT_MISMATCH,4, JSEXN_RANGEERR, "number of arguments does not match declaration of {0} (expected {1}{2}, got {3})")
+MSG_DEF(CTYPESMSG_ARG_TYPE_ERROR,3, JSEXN_TYPEERR, "the type of argument {0} {1} (got {2})")
+MSG_DEF(CTYPESMSG_FUNCTION_CONSTRUCT,0, JSEXN_TYPEERR, "cannot construct from FunctionType; use FunctionType.ptr instead")
+MSG_DEF(CTYPESMSG_RET_TYPE_ERROR,2, JSEXN_TYPEERR, "return type {0} (got {1})")
+MSG_DEF(CTYPESMSG_VARG_TYPE_ERROR,2, JSEXN_TYPEERR, "variadic argument {0} must be a CData object (got {1})")
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/ctypes/function-definition.js
@@ -0,0 +1,47 @@
+load(libdir + 'asserts.js');
+
+function test() {
+  assertRangeErrorMessage(() => { ctypes.FunctionType(ctypes.default_abi, ctypes.int32_t, []).ptr(x=>1)(1); },
+                         "number of arguments does not match declaration of ctypes.FunctionType(ctypes.default_abi, ctypes.int32_t) (expected 0, got 1)");
+
+  assertTypeErrorMessage(() => { ctypes.FunctionType(ctypes.default_abi, ctypes.int32_t, [1]); },
+                         "the type of argument 1 is not a ctypes type (got the number 1)");
+  assertTypeErrorMessage(() => { ctypes.FunctionType(ctypes.default_abi, ctypes.int32_t, [ctypes.void_t]); },
+                         "the type of argument 1 cannot be void or function (got ctypes.void)");
+  assertTypeErrorMessage(() => { ctypes.FunctionType(ctypes.default_abi, ctypes.int32_t, [ctypes.FunctionType(ctypes.default_abi, ctypes.int32_t, [])]); },
+                         "the type of argument 1 cannot be void or function (got ctypes.FunctionType(ctypes.default_abi, ctypes.int32_t))");
+  assertTypeErrorMessage(() => { ctypes.FunctionType(ctypes.default_abi, ctypes.int32_t, [ctypes.StructType("a")]); },
+                         "the type of argument 1 must have defined size (got ctypes.StructType(\"a\"))");
+
+  assertTypeErrorMessage(() => { ctypes.FunctionType(ctypes.default_abi, ctypes.int32_t, [])(); },
+                         "cannot construct from FunctionType; use FunctionType.ptr instead");
+
+  assertTypeErrorMessage(() => { ctypes.FunctionType(ctypes.default_abi, 1, []); },
+                         "return type is not a ctypes type (got the number 1)");
+  assertTypeErrorMessage(() => { ctypes.FunctionType(ctypes.default_abi, ctypes.int32_t.array(), []); },
+                         "return type cannot be an array or function (got ctypes.int32_t.array())");
+  assertTypeErrorMessage(() => { ctypes.FunctionType(ctypes.default_abi, ctypes.FunctionType(ctypes.default_abi, ctypes.int32_t, []), []); },
+                         "return type cannot be an array or function (got ctypes.FunctionType(ctypes.default_abi, ctypes.int32_t))");
+  assertTypeErrorMessage(() => { ctypes.FunctionType(ctypes.default_abi, ctypes.StructType("a"), []); },
+                         "return type must have defined size (got ctypes.StructType(\"a\"))");
+
+  let lib;
+  try {
+    lib = ctypes.open(ctypes.libraryName("c"));
+  } catch (e) {
+  }
+  if (!lib)
+    return;
+
+  let func = lib.declare("hypot",
+                         ctypes.default_abi,
+                         ctypes.double,
+                         ctypes.double, "...");
+  assertRangeErrorMessage(() => { func(); },
+                          "number of arguments does not match declaration of double hypot(double, ...) (expected 1 or more, got 0)");
+  assertTypeErrorMessage(() => { func(1, 2); },
+                          "variadic argument 2 must be a CData object (got the number 2)");
+}
+
+if (typeof ctypes === "object")
+  test();
--- a/toolkit/components/ctypes/tests/unit/test_jsctypes.js
+++ b/toolkit/components/ctypes/tests/unit/test_jsctypes.js
@@ -1861,29 +1861,29 @@ function run_FunctionType_tests() {
     ctypes.FunctionType(ctypes.default_abi, ctypes.void_t),
     ctypes.FunctionType(ctypes.default_abi, ctypes.void_t, [ ctypes.int32_t ]),
     [ "abi", "returnType", "argTypes", "isVariadic" ],
     undefined, undefined, undefined, undefined);
 
   do_check_throws(function() { ctypes.FunctionType(); }, TypeError);
   do_check_throws(function() {
     ctypes.FunctionType(ctypes.default_abi, ctypes.void_t, [ ctypes.void_t ]);
-  }, Error);
+  }, TypeError);
   do_check_throws(function() {
     ctypes.FunctionType(ctypes.default_abi, ctypes.void_t, [ ctypes.void_t ], 5);
   }, TypeError);
   do_check_throws(function() {
     ctypes.FunctionType(ctypes.default_abi, ctypes.void_t, ctypes.void_t);
   }, TypeError);
   do_check_throws(function() {
     ctypes.FunctionType(ctypes.default_abi, ctypes.void_t, null);
   }, TypeError);
   do_check_throws(function() {
     ctypes.FunctionType(ctypes.default_abi, ctypes.int32_t());
-  }, Error);
+  }, TypeError);
   do_check_throws(function() {
     ctypes.FunctionType(ctypes.void_t, ctypes.void_t);
   }, Error);
 
   let g_t = ctypes.StructType("g_t", [{ a: ctypes.int32_t }, { b: ctypes.double }]);
   let g = g_t(1, 2);
 
   let f_t = ctypes.FunctionType(ctypes.default_abi, g_t);
@@ -1903,17 +1903,17 @@ function run_FunctionType_tests() {
   do_check_eq(fp_t.name, name);
   do_check_eq(fp_t.size, ctypes.uintptr_t.size);
 
   do_check_eq(fp_t.toString(), "type " + name);
   do_check_eq(fp_t.toSource(),
     "ctypes.FunctionType(ctypes.default_abi, g_t).ptr");
 
   // Check that constructing a FunctionType CData directly throws.
-  do_check_throws(function() { f_t(); }, Error);
+  do_check_throws(function() { f_t(); }, TypeError);
 
   // Test ExplicitConvert.
   let f = fp_t();
   do_check_throws(function() { f.value; }, TypeError);
   do_check_eq(ptrValue(f), 0);
   f = fp_t(5);
   do_check_eq(ptrValue(f), 5);
   f = fp_t(ctypes.UInt64(10));
@@ -2239,17 +2239,17 @@ function run_cast_tests() {
 
 function run_void_tests(library) {
   let test_void_t = library.declare("test_void_t_cdecl", ctypes.default_abi, ctypes.void_t);
   do_check_eq(test_void_t(), undefined);
 
   // Test that library.declare throws with void function args.
   do_check_throws(function() {
     library.declare("test_void_t_cdecl", ctypes.default_abi, ctypes.void_t, ctypes.void_t);
-  }, Error);
+  }, TypeError);
 
   if ("winLastError" in ctypes) {
     test_void_t = library.declare("test_void_t_stdcall", ctypes.stdcall_abi, ctypes.void_t);
     do_check_eq(test_void_t(), undefined);
 
     // Check that WINAPI symbol lookup for a regular stdcall function fails on
     // Win32 (it's all the same on Win64 though).
     if (ctypes.voidptr_t.size == 4) {