GNOME.org
 

[gjs/wip/ptomato/mozjs31prep: 4/6] js: Replace JS_ValueToInt32 and gjs_value_to_int64

commit d7c6afd6533fe4e073c10cfab3ff6e7a3a37ee7f
Author: Philip Chimento <philip endlessm com>
Date:   Fri Sep 30 18:19:37 2016 -0700

    js: Replace JS_ValueToInt32 and gjs_value_to_int64
    
    Previously GJS behaved differently than the JavaScript language itself
    when marshalling arguments in and out of GObject introspection functions.
    GJS always used JS_ValueToInt32(), a deprecated function that dated from
    before the standardization of JavaScript, and had a hand-rolled 64-bit
    version to match the nonstandard behaviour, gjs_value_to_int64().
    
    This commit replaces those with JS::ToInt32() and JS::ToInt64(),
    respectively.
    
    Notable differences between the old and new behaviour are:
    
    - Floating-point numbers ending in 0.5 are rounded differently when
      marshalled into 32 or 64-bit signed integers. Previously they were
      rounded to floor(x + 0.5), now they are rounded to
      signum(x) * floor(abs(x)) as per the ECMA standard:
      http://www.ecma-international.org/ecma-262/7.0/index.html#sec-toint32
      Note that previously, GJS rounded according to the standard when
      converting to *unsigned* integers!
    
    - Objects whose number value is NaN (e.g, arrays of strings), would
      previously fail to convert, throwing a TypeError. Now they convert to
      0 when marshalled into 32 or 64-bit signed integers.
    
    Note that the new behaviour is the behaviour you got all along when using
    pure JavaScript, without any GObject introspection:
    
    gjs> let a = new Int32Array(2);
    gjs> a[0] = 10.5;
    10.5
    gjs> a[1] = ['this', 'is', 'fine'];
    this,is,fine
    gjs> a[0]
    10
    gjs> a[1]
    0
    
    Finally, this comments out a test altogether which relied on the
    fail-to-convert behaviour. There is not really any way to cause that
    behaviour on demand anymore, looking at
    http://www.ecma-international.org/ecma-262/7.0/index.html#sec-type-conversion
    until we upgrade to mozjs38, where we can make a Symbol fail to convert.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=742249

 NEWS                            |   34 +++++++++++++++++++++++++++++++++
 gi/arg.cpp                      |   14 ++++++------
 gi/value.cpp                    |   14 ++++++------
 gjs/context.cpp                 |   10 +++-----
 gjs/jsapi-util.cpp              |   40 +++++---------------------------------
 gjs/jsapi-util.h                |    3 +-
 installed-tests/js/testGDBus.js |   12 ++++++----
 modules/cairo-region.cpp        |    8 +++---
 8 files changed, 71 insertions(+), 64 deletions(-)
---
diff --git a/NEWS b/NEWS
index 330db9d..f24773e 100644
--- a/NEWS
+++ b/NEWS
@@ -1,3 +1,37 @@
+NEXT
+----
+- Backwards-incompatible change: we have changed the way certain JavaScript
+  values are marshalled into GObject introspection 32 or 64-bit signed integer
+  values, to match the ECMA standard. Here is the relevant section of the
+  standard:
+  http://www.ecma-international.org/ecma-262/7.0/index.html#sec-toint32
+  Notable differences between the old and new behaviour are:
+
+  * Floating-point numbers ending in 0.5 are rounded differently when
+    marshalled into 32 or 64-bit signed integers. Previously they were
+    rounded to floor(x + 0.5), now they are rounded to
+    signum(x) * floor(abs(x)) as per the ECMA standard:
+    http://www.ecma-international.org/ecma-262/7.0/index.html#sec-toint32
+    Note that previously, GJS rounded according to the standard when
+    converting to *unsigned* integers!
+
+  * Objects whose number value is NaN (e.g, arrays of strings), would
+    previously fail to convert, throwing a TypeError. Now they convert to
+    0 when marshalled into 32 or 64-bit signed integers.
+
+  Note that the new behaviour is the behaviour you got all along when using
+  pure JavaScript, without any GObject introspection:
+
+  gjs> let a = new Int32Array(2);
+  gjs> a[0] = 10.5;
+  10.5
+  gjs> a[1] = ['this', 'is', 'fine'];
+  this,is,fine
+  gjs> a[0]
+  10
+  gjs> a[1]
+  0
+
 Version 1.46.0
 --------------
 
diff --git a/gi/arg.cpp b/gi/arg.cpp
index 295e720..78e47de 100644
--- a/gi/arg.cpp
+++ b/gi/arg.cpp
@@ -1171,7 +1171,7 @@ gjs_value_to_g_argument(JSContext      *context,
 
     case GI_TYPE_TAG_INT8: {
         gint32 i;
-        if (!JS_ValueToInt32(context, value, &i))
+        if (!JS::ToInt32(context, value, &i))
             wrong = true;
         if (i > G_MAXINT8 || i < G_MININT8)
             out_of_range = true;
@@ -1189,7 +1189,7 @@ gjs_value_to_g_argument(JSContext      *context,
     }
     case GI_TYPE_TAG_INT16: {
         gint32 i;
-        if (!JS_ValueToInt32(context, value, &i))
+        if (!JS::ToInt32(context, value, &i))
             wrong = true;
         if (i > G_MAXINT16 || i < G_MININT16)
             out_of_range = true;
@@ -1208,7 +1208,7 @@ gjs_value_to_g_argument(JSContext      *context,
     }
 
     case GI_TYPE_TAG_INT32:
-        if (!JS_ValueToInt32(context, value, &arg->v_int))
+        if (!JS::ToInt32(context, value, &arg->v_int))
             wrong = true;
         break;
 
@@ -1583,9 +1583,9 @@ gjs_value_to_g_argument(JSContext      *context,
 
             } else if (value.isNumber()) {
                 if (interface_type == GI_INFO_TYPE_ENUM) {
-                    gint64 value_int64;
+                    int64_t value_int64;
 
-                    if (!gjs_value_to_int64 (context, value, &value_int64))
+                    if (!JS::ToInt64(context, value, &value_int64))
                         wrong = true;
                     else if (!_gjs_enum_value_is_valid(context, (GIEnumInfo *)interface_info, value_int64))
                         wrong = true;
@@ -1593,9 +1593,9 @@ gjs_value_to_g_argument(JSContext      *context,
                         arg->v_int = _gjs_enum_to_int ((GIEnumInfo *)interface_info, value_int64);
 
                 } else if (interface_type == GI_INFO_TYPE_FLAGS) {
-                    gint64 value_int64;
+                    int64_t value_int64;
 
-                    if (!gjs_value_to_int64 (context, value, &value_int64))
+                    if (!JS::ToInt64(context, value, &value_int64))
                         wrong = true;
                     else if (!_gjs_flags_value_is_valid(context, gtype, value_int64))
                         wrong = true;
diff --git a/gi/value.cpp b/gi/value.cpp
index 514f8ce..16eb8e7 100644
--- a/gi/value.cpp
+++ b/gi/value.cpp
@@ -412,7 +412,7 @@ gjs_value_to_g_value_internal(JSContext    *context,
         }
     } else if (gtype == G_TYPE_CHAR) {
         gint32 i;
-        if (JS_ValueToInt32(context, value, &i) && i >= SCHAR_MIN && i <= SCHAR_MAX) {
+        if (JS::ToInt32(context, value, &i) && i >= SCHAR_MIN && i <= SCHAR_MAX) {
             g_value_set_schar(gvalue, (signed char)i);
         } else {
             gjs_throw(context,
@@ -432,7 +432,7 @@ gjs_value_to_g_value_internal(JSContext    *context,
         }
     } else if (gtype == G_TYPE_INT) {
         gint32 i;
-        if (JS_ValueToInt32(context, value, &i)) {
+        if (JS::ToInt32(context, value, &i)) {
             g_value_set_int(gvalue, i);
         } else {
             gjs_throw(context,
@@ -627,9 +627,9 @@ gjs_value_to_g_value_internal(JSContext    *context,
 
         g_value_set_variant (gvalue, variant);
     } else if (g_type_is_a(gtype, G_TYPE_ENUM)) {
-        gint64 value_int64;
+        int64_t value_int64;
 
-        if (gjs_value_to_int64 (context, value, &value_int64)) {
+        if (JS::ToInt64(context, value, &value_int64)) {
             GEnumValue *v;
             gpointer gtype_class = g_type_class_ref(gtype);
 
@@ -653,9 +653,9 @@ gjs_value_to_g_value_internal(JSContext    *context,
             return false;
         }
     } else if (g_type_is_a(gtype, G_TYPE_FLAGS)) {
-        gint64 value_int64;
+        int64_t value_int64;
 
-        if (gjs_value_to_int64 (context, value, &value_int64)) {
+        if (JS::ToInt64(context, value, &value_int64)) {
             if (!_gjs_flags_value_is_valid(context, gtype, value_int64))
                 return false;
 
@@ -717,7 +717,7 @@ gjs_value_to_g_value_internal(JSContext    *context,
          * e.g. ClutterUnit.
          */
         gint32 i;
-        if (JS_ValueToInt32(context, value, &i)) {
+        if (JS::ToInt32(context, value, &i)) {
             GValue int_value = { 0, };
             g_value_init(&int_value, G_TYPE_INT);
             g_value_set_int(&int_value, i);
diff --git a/gjs/context.cpp b/gjs/context.cpp
index 1954d86..68837aa 100644
--- a/gjs/context.cpp
+++ b/gjs/context.cpp
@@ -652,12 +652,10 @@ gjs_context_eval(GjsContext   *js_context,
 
     if (exit_status_p) {
         if (retval.isInt32()) {
-            int code;
-            if (JS_ValueToInt32(js_context->context, retval, &code)) {
-                gjs_debug(GJS_DEBUG_CONTEXT,
-                          "Script returned integer code %d", code);
-                *exit_status_p = code;
-            }
+            int code = retval.toInt32();
+            gjs_debug(GJS_DEBUG_CONTEXT,
+                      "Script returned integer code %d", code);
+            *exit_status_p = code;
         } else {
             /* Assume success if no integer was returned */
             *exit_status_p = 0;
diff --git a/gjs/jsapi-util.cpp b/gjs/jsapi-util.cpp
index 1d075af..6d6942c 100644
--- a/gjs/jsapi-util.cpp
+++ b/gjs/jsapi-util.cpp
@@ -817,46 +817,18 @@ gjs_get_type_name(JS::Value value)
  * gjs_value_to_int64:
  * @context: the Javascript context object
  * @val: Javascript value to convert
- * @gint64: location to store the return value
+ * @result: location to store the return value
  *
  * Converts a Javascript value into the nearest 64 bit signed value.
  *
- * This function behaves indentically for rounding to JS_ValueToInt32(), which
- * means that it rounds (0.5 toward positive infinity) rather than doing
- * a C-style truncation to 0. If we change to using JS::ToInt32() then this
- * should be changed to match.
- *
- * Return value: If the javascript value converted to a number (see
- *   JS::ToNumber()) is NaN, or outside the range of 64-bit signed numbers,
- *   fails and sets an exception. Otherwise returns the value rounded to the
- *   nearest 64-bit integer. Like JS::ToInt32(), undefined throws, but
- *   null => 0, false => 0, true => 1.
+ * Deprecated: Use JS::ToInt64() instead.
  */
 bool
 gjs_value_to_int64(JSContext      *context,
                    const JS::Value val,
                    gint64         *result)
 {
-    if (val.isInt32()) {
-        *result = val.toInt32();
-        return true;
-    } else {
-        double value_double;
-        if (!JS::ToNumber(context, val, &value_double))
-            return false;
-
-        if (isnan(value_double) ||
-            value_double < G_MININT64 ||
-            value_double > G_MAXINT64) {
-
-            gjs_throw(context,
-                      "Value is not a valid 64-bit integer");
-            return false;
-        }
-
-        *result = (gint64)(value_double + 0.5);
-        return true;
-    }
+    return JS::ToInt64(context, val, (int64_t *) result);
 }
 
 static bool
@@ -996,7 +968,7 @@ gjs_parse_args_valist (JSContext  *context,
         }
             break;
         case 'i': {
-            if (!JS_ValueToInt32(context, js_value, (gint32*) arg_location)) {
+            if (!JS::ToInt32(context, js_value, (int32_t *) arg_location)) {
                 /* Our error message is going to be more useful */
                 JS_ClearPendingException(context);
                 arg_error_message = "Couldn't convert to integer";
@@ -1017,7 +989,7 @@ gjs_parse_args_valist (JSContext  *context,
         }
             break;
         case 't': {
-            if (!gjs_value_to_int64(context, js_value, (gint64*) arg_location)) {
+            if (!JS::ToInt64(context, js_value, (int64_t *) arg_location)) {
                 /* Our error message is going to be more useful */
                 JS_ClearPendingException(context);
                 arg_error_message = "Couldn't convert to 64-bit integer";
@@ -1082,7 +1054,7 @@ gjs_parse_args_valist (JSContext  *context,
  * F: A string, converted into "filename encoding" (i.e. active locale)
  * i: A number, will be converted to a C "gint32"
  * u: A number, converted into a C "guint32"
- * t: A 64-bit number, converted into a C "gint64" by way of gjs_value_to_int64()
+ * t: A 64-bit number, converted into a C "gint64"
  * o: A JavaScript object, as a "JSObject *"
  *
  * If the first character in the format string is a '!', then JS is allowed
diff --git a/gjs/jsapi-util.h b/gjs/jsapi-util.h
index ef7b216..23244ea 100644
--- a/gjs/jsapi-util.h
+++ b/gjs/jsapi-util.h
@@ -406,7 +406,8 @@ const char* gjs_get_type_name                (JS::Value        value);
 
 bool        gjs_value_to_int64               (JSContext       *context,
                                               const JS::Value  val,
-                                              gint64          *result);
+                                              gint64          *result)
+G_GNUC_DEPRECATED_FOR(JS::ToInt64);
 
 bool        gjs_parse_args                   (JSContext  *context,
                                               const char *function_name,
diff --git a/installed-tests/js/testGDBus.js b/installed-tests/js/testGDBus.js
index 4e5c8ec..4a083fa 100644
--- a/installed-tests/js/testGDBus.js
+++ b/installed-tests/js/testGDBus.js
@@ -492,8 +492,10 @@ function testMultipleArrayOut() {
     JSUnit.assertNull(theExcp);
 }
 
-/* We are returning an array but the signature says it's an integer,
- * so this should fail
+/* COMPAT: This test should test what happens when a TypeError is thrown during
+ * argument marshalling, but conversions don't throw TypeErrors anymore, so we
+ * can't test that ... until we upgrade to mozjs38 which has Symbols. Converting
+ * a Symbol to an int32 or string will throw a TypeError.
  */
 function testArrayOutBadSig() {
     let loop = GLib.MainLoop.new(null, false);
@@ -506,8 +508,8 @@ function testArrayOutBadSig() {
     });
 
     loop.run();
-    JSUnit.assertNull(theResult);
-    JSUnit.assertNotNull(theExcp);
+    // JSUnit.assertNull(theResult);
+    // JSUnit.assertNotNull(theExcp);
 }
 
 function testAsyncImplementation() {
@@ -591,7 +593,7 @@ function testDictSignatures() {
     JSUnit.assertNotNull(theResult);
 
     // verify the fractional part was dropped off int
-    JSUnit.assertEquals(11, theResult['anInteger'].deep_unpack());
+    JSUnit.assertEquals(10, theResult['anInteger'].deep_unpack());
 
     // and not dropped off a double
     JSUnit.assertEquals(10.5, theResult['aDoubleBeforeAndAfter'].deep_unpack());
diff --git a/modules/cairo-region.cpp b/modules/cairo-region.cpp
index 1745f17..1a6ec48 100644
--- a/modules/cairo-region.cpp
+++ b/modules/cairo-region.cpp
@@ -121,22 +121,22 @@ fill_rectangle(JSContext *context, JSObject *obj,
 
     if (!gjs_object_get_property_const(context, obj, GJS_STRING_X, &val))
         return false;
-    if (!JS_ValueToInt32(context, val, &rect->x))
+    if (!JS::ToInt32(context, val, &rect->x))
         return false;
 
     if (!gjs_object_get_property_const(context, obj, GJS_STRING_Y, &val))
         return false;
-    if (!JS_ValueToInt32(context, val, &rect->y))
+    if (!JS::ToInt32(context, val, &rect->y))
         return false;
 
     if (!gjs_object_get_property_const(context, obj, GJS_STRING_WIDTH, &val))
         return false;
-    if (!JS_ValueToInt32(context, val, &rect->width))
+    if (!JS::ToInt32(context, val, &rect->width))
         return false;
 
     if (!gjs_object_get_property_const(context, obj, GJS_STRING_HEIGHT, &val))
         return false;
-    if (!JS_ValueToInt32(context, val, &rect->height))
+    if (!JS::ToInt32(context, val, &rect->height))
         return false;
 
     return true;
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]