[gjs] js: Replace JS_ValueToInt32 and gjs_value_to_int64
- From: Philip Chimento <pchimento src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [gjs] js: Replace JS_ValueToInt32 and gjs_value_to_int64
- Date: Tue, 4 Oct 2016 23:26:18 +0000 (UTC)
commit 26f61014fc2fb84959f90005d7ef083e2adfbd75
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]
