From a7ceab1b3c6ceefbe3db09499b60443fc71ebf00 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Thu, 8 Oct 2020 03:53:22 +0200 Subject: [PATCH 1/5] gjs: Add full (u)int64 support via BigInt usage SpiderMonkey 91 provides some APIs to use BigInt natively. This allows us to handle an open issue about handling of 64 bit (unsigned) integers, as native JS Number is not able to represent such big integers properly. Unfortunately, BigInt isn't fully retro-compatible with JS Numbers, and need specific conversions, so we have to handle this at GJS level so as not to break the old assumptions. As per this, as discussed upstream [1], the idea here is that when we're handling an (u)int64 we create a new Number wrapper object and we add to it a "bigInt" property that contains the BigInt value. However we only create a new BigInt value in case the number is out-of-range for Number, while we just use a getter otherwise (for coherency). Enabled various tests we already had, making them support both the case in which we're using the approximated value, and the new BigInt real value. Fixes: #271 [1] https://gitlab.gnome.org/GNOME/gjs/-/merge_requests/412#note_755269 --- gi/arg-inl.h | 23 ---- gi/arg.cpp | 9 +- gi/js-value-inl.h | 24 ++++ gi/value.cpp | 6 + gjs/atoms.h | 1 + gjs/jsapi-util.cpp | 53 +++++++ gjs/jsapi-util.h | 11 +- installed-tests/js/testGIMarshalling.js | 106 ++++++++------ installed-tests/js/testGObjectClass.js | 30 ++-- installed-tests/js/testRegress.js | 175 +++++++++++++++++------- modules/esm/console.js | 14 +- test/gjs-tests.cpp | 30 ++-- 12 files changed, 328 insertions(+), 154 deletions(-) diff --git a/gi/arg-inl.h b/gi/arg-inl.h index 930b75ae6..db6122f97 100644 --- a/gi/arg-inl.h +++ b/gi/arg-inl.h @@ -7,8 +7,6 @@ #include #include // for nullptr_t -#include -#include // for to_string #include #include @@ -171,27 +169,6 @@ template return val; } -// Implementation to store rounded (u)int64_t numbers into double - -template -[[nodiscard]] inline constexpr std::enable_if_t< - std::is_integral_v && (std::numeric_limits::max() > - std::numeric_limits::max()), - double> -gjs_arg_get_maybe_rounded(GIArgument* arg) { - BigT val = gjs_arg_get(arg); - - if (val < Gjs::min_safe_big_number() || - val > Gjs::max_safe_big_number()) { - g_warning( - "Value %s cannot be safely stored in a JS Number " - "and may be rounded", - std::to_string(val).c_str()); - } - - return static_cast(val); -} - template GJS_JSAPI_RETURN_CONVENTION inline bool gjs_arg_set_from_js_value( JSContext* cx, const JS::HandleValue& value, GArgument* arg, diff --git a/gi/arg.cpp b/gi/arg.cpp index 3d6b2efa4..973fa8ac6 100644 --- a/gi/arg.cpp +++ b/gi/arg.cpp @@ -2493,13 +2493,12 @@ bool gjs_value_from_g_argument(JSContext* context, break; case GI_TYPE_TAG_INT64: - value_p.setNumber(gjs_arg_get_maybe_rounded(arg)); - break; + return Gjs::js_value_set_bigint_number(context, value_p, + gjs_arg_get(arg)); case GI_TYPE_TAG_UINT64: - value_p.setNumber(gjs_arg_get_maybe_rounded(arg)); - break; - + return Gjs::js_value_set_bigint_number(context, value_p, + gjs_arg_get(arg)); case GI_TYPE_TAG_UINT16: value_p.setInt32(gjs_arg_get(arg)); break; diff --git a/gi/js-value-inl.h b/gi/js-value-inl.h index debfe1527..6e08126cb 100644 --- a/gi/js-value-inl.h +++ b/gi/js-value-inl.h @@ -10,6 +10,7 @@ #include // for isnan #include +#include // for to_string #include #include @@ -319,4 +320,27 @@ GJS_JSAPI_RETURN_CONVENTION inline bool js_value_to_c_checked( return true; } +template +GJS_JSAPI_RETURN_CONVENTION static bool js_value_set_bigint_number( + JSContext* cx, JS::MutableHandleValue value_p, T value) { + static_assert(std::is_same_v || std::is_same_v); + JS::BigInt* bigint = nullptr; + + if (value < min_safe_big_number() || value > max_safe_big_number()) + bigint = JS::NumberToBigInt(cx, value); + + if (!js_value_set_bigint_number(cx, value_p, bigint, value)) + return false; + + if (bigint) { + g_warning( + "Value %s cannot be safely stored in a JS Number and may be " + "rounded. You can use the '.bigInt' property to get the " + "non-rounded value.", + std::to_string(value).c_str()); + } + + return true; +} + } // namespace Gjs diff --git a/gi/value.cpp b/gi/value.cpp index 3571613fb..f7a464cb7 100644 --- a/gi/value.cpp +++ b/gi/value.cpp @@ -915,6 +915,12 @@ gjs_value_from_g_value_internal(JSContext *context, bool v; v = g_value_get_boolean(gvalue); value_p.setBoolean(!!v); + } else if (gtype == G_TYPE_INT64) { + return Gjs::js_value_set_bigint_number(context, value_p, + g_value_get_int64(gvalue)); + } else if (gtype == G_TYPE_UINT64) { + return Gjs::js_value_set_bigint_number(context, value_p, + g_value_get_uint64(gvalue)); } else if (g_type_is_a(gtype, G_TYPE_OBJECT) || g_type_is_a(gtype, G_TYPE_INTERFACE)) { return ObjectInstance::set_value_from_gobject( context, static_cast(g_value_get_object(gvalue)), diff --git a/gjs/atoms.h b/gjs/atoms.h index 955165017..8c372d8fc 100644 --- a/gjs/atoms.h +++ b/gjs/atoms.h @@ -18,6 +18,7 @@ class JSTracer; // clang-format off #define FOR_EACH_ATOM(macro) \ + macro(big_int, "bigInt") \ macro(cause, "cause") \ macro(code, "code") \ macro(column_number, "columnNumber") \ diff --git a/gjs/jsapi-util.cpp b/gjs/jsapi-util.cpp index aeb0e85ab..0999cc146 100644 --- a/gjs/jsapi-util.cpp +++ b/gjs/jsapi-util.cpp @@ -19,6 +19,7 @@ #include #include +#include #include #include #include @@ -31,6 +32,7 @@ #include // for DefaultHasher #include // for GetClass #include +#include #include #include // for BuildStackString #include @@ -615,3 +617,54 @@ const char* gjs_explain_gc_reason(JS::GCReason reason) { return reason_strings[size_t(reason) - size_t(JS::GCReason::FIRST_FIREFOX_REASON)]; } + +GJS_JSAPI_RETURN_CONVENTION +static bool number_bigint_getter(JSContext* cx, unsigned argc, JS::Value* vp) { + GJS_GET_THIS(cx, argc, vp, args, thisObj); + + JS::RootedValue v_number(cx); + if (!thisObj || !JS::ToPrimitive(cx, thisObj, JSTYPE_NUMBER, &v_number)) { + gjs_throw(cx, "Can't get primitive for non-number object"); + return false; + } + + g_assert(v_number.isNumber()); + args.rval().setBigInt(JS::NumberToBigInt(cx, v_number.toNumber())); + return true; +} + +namespace Gjs { + +GJS_JSAPI_RETURN_CONVENTION +bool js_value_set_bigint_number(JSContext* cx, JS::MutableHandleValue value_p, + JS::BigInt* bigint, double number) { + value_p.setNumber(number); + + JS::RootedObject value_object(cx); + if (!JS_ValueToObject(cx, value_p, &value_object)) + return false; + + JS::Rooted descr(cx); + JS::PropertyAttributes attrs{JS::PropertyAttribute::Enumerable}; + + if (bigint) { + descr = JS::PropertyDescriptor::Data(JS::BigIntValue(bigint), attrs); + } else { + // No need to duplicate the value in this case, just use a getter + JSFunction* getter = + JS_NewFunction(cx, &number_bigint_getter, 1, 0, "bigInt"); + descr = JS::PropertyDescriptor::Accessor(JS_GetFunctionObject(getter), + nullptr, attrs); + } + + const GjsAtoms& atoms = GjsContextPrivate::atoms(cx); + if (!JS_DefinePropertyById(cx, value_object, atoms.big_int(), descr)) { + gjs_throw(cx, "Can't define bigInt property on number"); + return false; + } + + value_p.setObject(*value_object); + return true; +} + +} // namespace Gjs diff --git a/gjs/jsapi-util.h b/gjs/jsapi-util.h index e06a215f2..601724b53 100644 --- a/gjs/jsapi-util.h +++ b/gjs/jsapi-util.h @@ -27,6 +27,7 @@ #include #include // for IgnoreGCPolicy #include +#include #include #include // for UniqueChars @@ -38,7 +39,7 @@ #endif namespace JS { -class CallArgs; +class BigInt; struct Dummy {}; using GTypeNotUint64 = @@ -594,6 +595,14 @@ bool gjs_object_require_converted_property(JSContext *context, JS::HandleId property_name, uint32_t *value); +namespace Gjs { + +GJS_JSAPI_RETURN_CONVENTION +bool js_value_set_bigint_number(JSContext* cx, JS::MutableHandleValue value_p, + JS::BigInt*, double number); + +} // namespace Gjs + [[nodiscard]] std::string gjs_debug_bigint(JS::BigInt* bi); [[nodiscard]] std::string gjs_debug_string(JSString* str); [[nodiscard]] std::string gjs_debug_symbol(JS::Symbol* const sym); diff --git a/installed-tests/js/testGIMarshalling.js b/installed-tests/js/testGIMarshalling.js index cc4176695..800eb0a95 100644 --- a/installed-tests/js/testGIMarshalling.js +++ b/installed-tests/js/testGIMarshalling.js @@ -104,6 +104,25 @@ function testContainerMarshalling(root, value, inoutValue, options = {}) { }); } +const Bit64Type = { + NONE: 0, + NUMBER: 1, + BIGINT: 2, +}; + +function getBit64Type(value) { + if (typeof value === 'bigint' || value instanceof BigInt) + return Bit64Type.BIGINT; + + if (typeof value === 'number' || value instanceof Number) { + if (BigInt(value) > BigInt(Number.MAX_SAFE_INTEGER) || + BigInt(value) < BigInt(Number.MIN_SAFE_INTEGER)) + return Bit64Type.NUMBER; + } + + return Bit64Type.NONE; +} + // Integer limits, defined without reference to GLib (because the GLib.MAXINT8 // etc. constants are also subject to marshalling) const Limits = { @@ -126,7 +145,7 @@ const Limits = { min: -(2 ** 63), max: 2 ** 63 - 1, umax: 2 ** 64 - 1, - bit64: true, // note: unsafe, values will not be accurate! + bit64: Bit64Type.NUMBER, // note: unsafe, values will not be accurate! }, short: {}, int: {}, @@ -140,6 +159,7 @@ const BigIntLimits = { min: -(2n ** 63n), max: 2n ** 63n - 1n, umax: 2n ** 64n - 1n, + bit64: Bit64Type.BIGINT, }, }; @@ -164,24 +184,28 @@ if (GLib.SIZEOF_SSIZE_T === 8) { // Sometimes tests pass if we are comparing two inaccurate values in JS with // each other. That's fine for now. Then we just have to suppress the warnings. -function warn64(is64bit, func, ...args) { - if (is64bit) { +function warn64(bit64Type, func, ...args) { + if (bit64Type) { GLib.test_expect_message('Gjs', GLib.LogLevelFlags.LEVEL_WARNING, '*cannot be safely stored*'); } const retval = func(...args); - if (is64bit) { + if (bit64Type) { GLib.test_assert_expected_messages_internal('Gjs', 'testGIMarshalling.js', 0, 'Ignore message'); } + + if (bit64Type === Bit64Type.BIGINT && retval instanceof Number) + return retval.bigInt; + return retval; } // Other times we compare an inaccurate value marshalled from JS into C, with an // accurate value in C. Those tests we have to skip. -function skip64(is64bit) { - if (is64bit) - pending('https://gitlab.gnome.org/GNOME/gjs/issues/271'); +function skipUnsafe64(bit64Type) { + if (bit64Type === Bit64Type.NUMBER) + pending('Int64 values can not be represented as Number\'s'); } describe('Boolean', function () { @@ -206,7 +230,7 @@ describe('Boolean', function () { }); describe('Integer', function () { - Object.entries(Limits).forEach(([type, {min, max, umax, bit64, utype = `u${type}`}]) => { + function createIntegerTests(type, utype, min, max, umax, bit64) { describe(`${type}-typed`, function () { it('marshals signed value as a return value', function () { expect(warn64(bit64, GIMarshallingTests[`${type}_return_max`])).toEqual(max); @@ -214,7 +238,7 @@ describe('Integer', function () { }); it('marshals signed value as an in parameter', function () { - skip64(bit64); + skipUnsafe64(bit64); expect(() => GIMarshallingTests[`${type}_in_max`](max)).not.toThrow(); expect(() => GIMarshallingTests[`${type}_in_min`](min)).not.toThrow(); }); @@ -225,9 +249,9 @@ describe('Integer', function () { }); it('marshals as an inout parameter', function () { - skip64(bit64); - expect(GIMarshallingTests[`${type}_inout_max_min`](max)).toEqual(min); - expect(GIMarshallingTests[`${type}_inout_min_max`](min)).toEqual(max); + skipUnsafe64(bit64); + expect(warn64(bit64, GIMarshallingTests[`${type}_inout_max_min`], max)).toEqual(min); + expect(warn64(bit64, GIMarshallingTests[`${type}_inout_min_max`], min)).toEqual(max); }); it('marshals unsigned value as a return value', function () { @@ -235,7 +259,7 @@ describe('Integer', function () { }); it('marshals unsigned value as an in parameter', function () { - skip64(bit64); + skipUnsafe64(bit64); expect(() => GIMarshallingTests[`${utype}_in`](umax)).not.toThrow(); }); @@ -244,26 +268,16 @@ describe('Integer', function () { }); it('marshals unsigned value as an inout parameter', function () { - skip64(bit64); + skipUnsafe64(bit64); expect(GIMarshallingTests[`${utype}_inout`](umax)).toEqual(0); }); }); - }); -}); - -describe('BigInt', function () { - Object.entries(BigIntLimits).forEach(([type, {min, max, umax, utype = `u${type}`}]) => { - describe(`${type}-typed`, function () { - it('marshals signed value as an in parameter', function () { - expect(() => GIMarshallingTests[`${type}_in_max`](max)).not.toThrow(); - expect(() => GIMarshallingTests[`${type}_in_min`](min)).not.toThrow(); - }); + } - it('marshals unsigned value as an in parameter', function () { - expect(() => GIMarshallingTests[`${utype}_in`](umax)).not.toThrow(); - }); - }); - }); + Object.entries(Limits).forEach(([type, {min, max, umax, bit64, utype = `u${type}`}]) => + createIntegerTests(type, utype, min, max, umax, bit64)); + Object.entries(BigIntLimits).forEach(([type, {min, max, umax, bit64, utype = `u${type}`}]) => + createIntegerTests(type, utype, min, max, umax, bit64)); }); describe('Floating point', function () { @@ -577,8 +591,12 @@ describe('GArray', function () { }); it('marshals int64s as a transfer-none return value', function () { - expect(warn64(true, GIMarshallingTests.garray_uint64_none_return)) + expect(warn64(Bit64Type.NUMBER, GIMarshallingTests.garray_uint64_none_return)) .toEqual([0, Limits.int64.umax]); + + const array = warn64(Bit64Type.BIGINT, GIMarshallingTests.garray_uint64_none_return); + expect(array).toEqual([0, Limits.int64.umax]); + expect(array[1].bigInt).toEqual(BigIntLimits.int64.umax); }); describe('of strings', function () { @@ -860,7 +878,10 @@ describe('GValue', function () { }); it('marshals as an int64 out parameter', function () { - expect(GIMarshallingTests.gvalue_int64_out()).toEqual(Limits.int64.max); + expect(warn64(Bit64Type.NUMBER, + GIMarshallingTests.gvalue_int64_out)).toEqual(Limits.int64.max); + expect(warn64(Bit64Type.BIGINT, + GIMarshallingTests.gvalue_int64_out)).toEqual(BigIntLimits.int64.max); }); it('marshals as a caller-allocated out parameter', function () { @@ -1831,6 +1852,9 @@ describe('Interface', function () { Limits.int64.max, Limits.int64.umax, ]); + expect(i3.stuff[0].bigInt).toEqual(BigIntLimits.int64.min); + expect(i3.stuff[1].bigInt).toEqual(BigIntLimits.int64.max); + expect(i3.stuff[2].bigInt).toEqual(BigIntLimits.int64.umax); }); }); @@ -1986,19 +2010,13 @@ describe('GObject properties', function () { it(`gets and sets a ${type} property`, function () { if (skip) pending(skip); + let bit64 = getBit64Type(value1); obj[`some_${type}`] = value1; - expect(obj[`some_${type}`]).toEqual(value1); - obj[`some_${type}`] = value2; - expect(obj[`some_${type}`]).toEqual(value2); - }); - } + expect(warn64(bit64, () => obj[`some_${type}`])).toEqual(value1); - function testPropertyGetSetBigInt(type, value1, value2) { - it(`gets and sets a ${type} property with a bigint`, function () { - obj[`some_${type}`] = value1; - expect(obj[`some_${type}`]).toEqual(Number(value1)); + bit64 = getBit64Type(value2); obj[`some_${type}`] = value2; - expect(obj[`some_${type}`]).toEqual(Number(value2)); + expect(warn64(bit64, () => obj[`some_${type}`])).toEqual(value2); }); } @@ -2011,9 +2029,9 @@ describe('GObject properties', function () { testPropertyGetSet('ulong', 42, 64); testPropertyGetSet('int64', 42, 64); testPropertyGetSet('int64', Number.MIN_SAFE_INTEGER, Number.MAX_SAFE_INTEGER); - testPropertyGetSetBigInt('int64', BigIntLimits.int64.min, BigIntLimits.int64.max); + testPropertyGetSet('int64', BigIntLimits.int64.min, BigIntLimits.int64.max); testPropertyGetSet('uint64', 42, 64); - testPropertyGetSetBigInt('uint64', BigIntLimits.int64.max, BigIntLimits.int64.umax); + testPropertyGetSet('uint64', BigIntLimits.int64.max, BigIntLimits.int64.umax); testPropertyGetSet('string', 'Gjs', 'is cool!'); it('get and sets out-of-range values throws', function () { @@ -2059,7 +2077,7 @@ describe('GObject properties', function () { new GIMarshallingTests.BoxedStruct({long_: 42})); testPropertyGetSet('boxed_glist', null, null); testPropertyGetSet('gvalue', 42, 'foo'); - testPropertyGetSetBigInt('gvalue', BigIntLimits.int64.umax, BigIntLimits.int64.min); + testPropertyGetSet('gvalue', BigIntLimits.int64.umax, BigIntLimits.int64.min); testPropertyGetSet('variant', new GLib.Variant('b', true), new GLib.Variant('s', 'hello')); testPropertyGetSet('variant', new GLib.Variant('x', BigIntLimits.int64.min), diff --git a/installed-tests/js/testGObjectClass.js b/installed-tests/js/testGObjectClass.js index 1e95e3a67..b14905893 100644 --- a/installed-tests/js/testGObjectClass.js +++ b/installed-tests/js/testGObjectClass.js @@ -267,6 +267,15 @@ const MyCustomInit = GObject.registerClass(class MyCustomInit extends GObject.Ob } }); +function expectWarn64(callable) { + GLib.test_expect_message('Gjs', GLib.LogLevelFlags.LEVEL_WARNING, + '*cannot be safely stored*'); + const ret = callable(); + GLib.test_assert_expected_messages_internal('Gjs', + 'testGIMarshalling.js', 0, 'Ignore message'); + return ret; +} + const NoName = GObject.registerClass(class extends GObject.Object {}); describe('GObject class with decorator', function () { @@ -489,12 +498,14 @@ describe('GObject class with decorator', function () { }, class PropInt64 extends GObject.Object {}); let int64 = GLib.MAXINT64_BIGINT - 5n; - let obj = new PropInt64({int64}); + let obj = expectWarn64(() => new PropInt64({int64})); expect(obj.int64).toEqual(Number(int64)); + expect(obj.int64.bigInt).toEqual(int64); int64 = GLib.MININT64_BIGINT + 555n; - obj = new PropInt64({int64}); + obj = expectWarn64(() => new PropInt64({int64})); expect(obj.int64).toEqual(Number(int64)); + expect(obj.int64.bigInt).toEqual(int64); }); it('can have a default int64 property', function () { @@ -508,8 +519,9 @@ describe('GObject class with decorator', function () { }, }, class PropDefaultInt64Init extends GObject.Object {}); - const obj = new PropInt64Init(); + const obj = expectWarn64(() => new PropInt64Init()); expect(obj.int64).toEqual(Number(defaultValue)); + expect(obj.int64.bigInt).toEqual(defaultValue); }); it('can have an uint64 property', function () { @@ -522,8 +534,9 @@ describe('GObject class with decorator', function () { }, class PropUint64 extends GObject.Object {}); const uint64 = GLib.MAXUINT64_BIGINT - 5n; - const obj = new PropUint64({uint64}); + const obj = expectWarn64(() => new PropUint64({uint64})); expect(obj.uint64).toEqual(Number(uint64)); + expect(obj.uint64.bigInt).toEqual(uint64); }); it('can have a default uint64 property', function () { @@ -536,8 +549,9 @@ describe('GObject class with decorator', function () { }, }, class PropDefaultUint64Init extends GObject.Object {}); - const obj = new PropUint64Init(); + const obj = expectWarn64(() => new PropUint64Init()); expect(obj.uint64).toEqual(Number(defaultValue)); + expect(obj.uint64.bigInt).toEqual(defaultValue); }); it('can override a property from the parent class', function () { @@ -1736,11 +1750,11 @@ describe('GObject class with int64 properties', function () { int64: GLib.MAXINT32, }); - expect(instance.int64).toBe(GLib.MAXINT32); + expect(instance.int64).toEqual(GLib.MAXINT32); instance.int64 = GLib.MAXINT32 + 1; - expect(instance.int64).toBe(GLib.MAXINT32 + 1); + expect(instance.int64).toEqual(GLib.MAXINT32 + 1); }); @@ -1749,6 +1763,6 @@ describe('GObject class with int64 properties', function () { int64: GLib.MAXINT32 + 1, }); - expect(instance.int64).toBe(GLib.MAXINT32 + 1); + expect(instance.int64).toEqual(GLib.MAXINT32 + 1); }); }); diff --git a/installed-tests/js/testRegress.js b/installed-tests/js/testRegress.js index 7e4f8bb8c..b2f31cea6 100644 --- a/installed-tests/js/testRegress.js +++ b/installed-tests/js/testRegress.js @@ -27,6 +27,24 @@ if (GLib.SIZEOF_SIZE_T === 8) if (GLib.SIZEOF_SSIZE_T === 8) bit64Types.push('ssize'); +const Limits = { + int64: { + min: expectWarn64(() => GLib.MININT64), + max: expectWarn64(() => GLib.MAXINT64), + }, + uint64: { + min: GLib.MINUINT64, + max: expectWarn64(() => GLib.MAXUINT64), + }, + ssize: {}, + size: {}, +}; + +if (bit64Types.includes('ssize')) + Object.assign(Limits.ssize, Limits.int64); +if (bit64Types.includes('size')) + Object.assign(Limits.size, Limits.uint64); + describe('Life, the Universe and Everything', function () { it('includes null return value', function () { expect(Regress.test_return_allow_none()).toBeNull(); @@ -43,15 +61,15 @@ describe('Life, the Universe and Everything', function () { [8, 16, 32, 64].forEach(bits => { it(`includes ${bits}-bit integers`, function () { const method = `test_int${bits}`; - expect(Regress[method](42)).toBe(42); - expect(Regress[method](-42)).toBe(-42); - expect(Regress[method](undefined)).toBe(0); - expect(Regress[method](42.42)).toBe(42); - expect(Regress[method](-42.42)).toBe(-42); + expect(Regress[method](42)).toEqual(42); + expect(Regress[method](-42)).toEqual(-42); + expect(Regress[method](undefined)).toEqual(0); + expect(Regress[method](42.42)).toEqual(42); + expect(Regress[method](-42.42)).toEqual(-42); if (bits >= 64) { - expect(Regress[method](42n)).toBe(42); - expect(Regress[method](-42n)).toBe(-42); + expect(Regress[method](42n)).toEqual(42); + expect(Regress[method](-42n)).toEqual(-42); } else { expect(() => Regress[method](42n)).toThrow(); expect(() => Regress[method](-42n)).toThrow(); @@ -60,9 +78,9 @@ describe('Life, the Universe and Everything', function () { it(`includes unsigned ${bits}-bit integers`, function () { const method = `test_uint${bits}`; - expect(Regress[method](42)).toBe(42); - expect(Regress[method](undefined)).toBe(0); - expect(Regress[method](42.42)).toBe(42); + expect(Regress[method](42)).toEqual(42); + expect(Regress[method](undefined)).toEqual(0); + expect(Regress[method](42.42)).toEqual(42); if (bits >= 64) expect(Regress[method](42n)).toEqual(42); @@ -74,22 +92,22 @@ describe('Life, the Universe and Everything', function () { ['short', 'int', 'long', 'ssize', 'float', 'double'].forEach(type => { it(`includes ${type}s`, function () { const method = `test_${type}`; - expect(Regress[method](42)).toBe(42); - expect(Regress[method](-42)).toBe(-42); + expect(Regress[method](42)).toEqual(42); + expect(Regress[method](-42)).toEqual(-42); if (['float', 'double'].includes(type)) { expect(Regress[method](undefined)).toBeNaN(); expect(Regress[method](42.42)).toBeCloseTo(42.42); expect(Regress[method](-42.42)).toBeCloseTo(-42.42); } else { - expect(Regress[method](undefined)).toBe(0); - expect(Regress[method](42.42)).toBe(42); - expect(Regress[method](-42.42)).toBe(-42); + expect(Regress[method](undefined)).toEqual(0); + expect(Regress[method](42.42)).toEqual(42); + expect(Regress[method](-42.42)).toEqual(-42); } if (bit64Types.includes(type)) { - expect(Regress[method](42n)).toBe(42); - expect(Regress[method](-42n)).toBe(-42); + expect(Regress[method](42n)).toEqual(42); + expect(Regress[method](-42n)).toEqual(-42); } else { expect(() => Regress[method](42n)).toThrow(); expect(() => Regress[method](-42n)).toThrow(); @@ -100,12 +118,12 @@ describe('Life, the Universe and Everything', function () { ['ushort', 'uint', 'ulong', 'size'].forEach(type => { it(`includes ${type}s`, function () { const method = `test_${type}`; - expect(Regress[method](42)).toBe(42); - expect(Regress[method](undefined)).toBe(0); - expect(Regress[method](42.42)).toBe(42); + expect(Regress[method](42)).toEqual(42); + expect(Regress[method](undefined)).toEqual(0); + expect(Regress[method](42.42)).toEqual(42); if (bit64Types.includes(type)) - expect(Regress[method](42n)).toBe(42); + expect(Regress[method](42n)).toEqual(42); else expect(() => Regress[method](42n)).toThrow(); }); @@ -127,16 +145,16 @@ describe('Life, the Universe and Everything', function () { describe('Infinity and NaN', function () { ['int8', 'int16', 'int32', 'int64', 'short', 'int', 'long', 'ssize'].forEach(type => { it(`converts to 0 for ${type}`, function () { - expect(Regress[`test_${type}`](Infinity)).toBe(0); - expect(Regress[`test_${type}`](-Infinity)).toBe(0); - expect(Regress[`test_${type}`](NaN)).toBe(0); + expect(Regress[`test_${type}`](Infinity)).toEqual(0); + expect(Regress[`test_${type}`](-Infinity)).toEqual(0); + expect(Regress[`test_${type}`](NaN)).toEqual(0); }); }); ['uint8', 'uint16', 'uint32', 'uint64', 'ushort', 'uint', 'ulong', 'size'].forEach(type => { it(`converts to 0 for ${type}`, function () { - expect(Regress[`test_${type}`](Infinity)).toBe(0); - expect(Regress[`test_${type}`](NaN)).toBe(0); + expect(Regress[`test_${type}`](Infinity)).toEqual(0); + expect(Regress[`test_${type}`](NaN)).toEqual(0); }); }); @@ -150,29 +168,90 @@ describe('Life, the Universe and Everything', function () { }); describe('(u)int64 numeric values', function () { - const minInt64 = -(2n ** 63n); - const maxInt64 = 2n ** 63n - 1n; - const maxUint64 = 2n ** 64n - 1n; - ['uint64', 'int64', 'long', 'ulong', 'size', 'ssize'].forEach(type => { - if (!bit64Types.includes(type)) + if (!bit64Types.includes(type)) { + pending('Not a big number'); return; + } const signed = ['int64', 'long', 'ssize'].includes(type); const limits = { - min: signed ? minInt64 : 0n, - max: signed ? maxInt64 : maxUint64, + min: signed ? Limits.int64.min : Limits.uint64.min, + max: signed ? Limits.int64.max : Limits.uint64.max, }; const testFunc = Regress[`test_${type}`]; + it(`have bigInt property for ${type}`, function () { + if (signed) { + expect(limits.min instanceof Number).toBeTruthy(); + expect(typeof limits.min.bigInt).toBe('bigint'); + + expect(testFunc(-53) instanceof Number).toBeTruthy(); + expect(typeof testFunc(-55).bigInt).toBe('bigint'); + + expect(testFunc(-53n) instanceof Number).toBeTruthy(); + expect(typeof testFunc(-55n).bigInt).toBe('bigint'); + } + + expect(limits.max instanceof Number).toBeTruthy(); + expect(typeof limits.max.bigInt).toBe('bigint'); + + expect(testFunc(53) instanceof Number).toBeTruthy(); + expect(typeof testFunc(55).bigInt).toBe('bigint'); + + expect(testFunc(53n) instanceof Number).toBeTruthy(); + expect(typeof testFunc(55n).bigInt).toBe('bigint'); + + let maxProp = `MAX${type.toUpperCase()}`; + if (maxProp in GLib) { + expect(limits.max).toBe(GLib[maxProp]); + expect(limits.max.bigInt).toBe(GLib[maxProp].bigInt); + } + + if (signed) { + let minProp = `MIN${type.toUpperCase()}`; + if (minProp in GLib) { + expect(limits.min).toBe(GLib[minProp]); + expect(limits.min.bigInt).toBe(GLib[minProp].bigInt); + } + } + }); + + it(`can be function arguments for ${type}`, function () { + expect(testFunc(53)).toEqual(53); + expect(testFunc(55).bigInt).toEqual(55n); + + expect(testFunc(53n)).toEqual(53); + expect(testFunc(55n).bigInt).toEqual(55n); + + if (signed) { + expect(testFunc(-53)).toEqual(-53); + expect(testFunc(-55).bigInt).toEqual(-55n); + + expect(testFunc(-53n)).toEqual(-53); + expect(testFunc(-55n).bigInt).toEqual(-55n); + } + }); + it(`can use numeric limits for ${type}`, function () { - expect(expectWarn64(() => testFunc(limits.max))) - .toEqual(Number(limits.max)); + expect(expectWarn64(() => testFunc(limits.max.bigInt))) + .toEqual(limits.max); + expect(expectWarn64(() => testFunc(limits.max.bigInt).bigInt)) + .toEqual(limits.max.bigInt); if (signed) { - expect(expectWarn64(() => testFunc(limits.min))) - .toEqual(Number(limits.min)); + expect(expectWarn64(() => testFunc(limits.min.bigInt))) + .toEqual(limits.min); + expect(expectWarn64(() => testFunc(limits.min.bigInt).bigInt)) + .toEqual(limits.min.bigInt); } }); + + it(`respect numeric limits for ${type}`, function () { + expect(() => testFunc(limits.max.bigInt + 1n)) + .toThrowError(/out of range/); + expect(() => testFunc(signed ? limits.min.bigInt - 1n : -1n)) + .toThrowError(/out of range/); + }); }); }); @@ -1221,23 +1300,25 @@ describe('Life, the Universe and Everything', function () { // See testCairo.js for a test of // Regress.TestObj::sig-with-foreign-struct. - xit('signal with int64 gets correct value', function (done) { + it('signal with int64 gets correct value', function (done) { o.connect('sig-with-int64-prop', (self, number) => { - expect(number).toEqual(GLib.MAXINT64); + expect(number).toEqual(Limits.int64.max); + expect(number.bigInt).toEqual(Limits.int64.max.bigInt); done(); - return GLib.MAXINT64; + return Limits.int64.max.bigInt; }); - o.emit_sig_with_int64(); - }).pend('https://gitlab.gnome.org/GNOME/gjs/issues/271'); + expectWarn64(() => o.emit_sig_with_int64()); + }); - xit('signal with uint64 gets correct value', function (done) { + it('signal with uint64 gets correct value', function (done) { o.connect('sig-with-uint64-prop', (self, number) => { - expect(number).toEqual(GLib.MAXUINT64); + expect(number).toEqual(Limits.uint64.max); + expect(number.bigInt).toEqual(Limits.uint64.max.bigInt); done(); - return GLib.MAXUINT64; + return Limits.uint64.max.bigInt; }); - o.emit_sig_with_uint64(); - }).pend('https://gitlab.gnome.org/GNOME/gjs/issues/271'); + expectWarn64(() => o.emit_sig_with_uint64()); + }); it('signal with array len parameter is not passed correct array and no length arg', function (done) { o.connect('sig-with-array-len-prop', (signalObj, signalArray, shouldBeUndefined) => { diff --git a/modules/esm/console.js b/modules/esm/console.js index 1532624d5..15978f692 100644 --- a/modules/esm/console.js +++ b/modules/esm/console.js @@ -265,7 +265,7 @@ class Console { * @returns {void} */ time(label) { - this.#timeLabels[label] = imports.gi.GLib.get_monotonic_time(); + this.#timeLabels[label] = imports.gi.GLib.get_monotonic_time().bigInt; } /** @@ -280,12 +280,14 @@ class Console { timeLog(label, ...data) { const startTime = this.#timeLabels[label]; - if (typeof startTime !== 'number') { + if (typeof startTime !== 'bigint') { this.#printer('reportWarning', [ `No time log found for label: '${label}'.`, ]); } else { - const durationMs = (imports.gi.GLib.get_monotonic_time() - startTime) / 1000; + const duration = Number( + imports.gi.GLib.get_monotonic_time().bigInt - startTime); + const durationMs = duration / 1000; const concat = `${label}: ${durationMs.toFixed(3)} ms`; data.unshift(concat); @@ -304,14 +306,16 @@ class Console { timeEnd(label) { const startTime = this.#timeLabels[label]; - if (typeof startTime !== 'number') { + if (typeof startTime !== 'bigint') { this.#printer('reportWarning', [ `No time log found for label: '${label}'.`, ]); } else { delete this.#timeLabels[label]; - const durationMs = (imports.gi.GLib.get_monotonic_time() - startTime) / 1000; + const duration = Number( + imports.gi.GLib.get_monotonic_time().bigInt - startTime); + const durationMs = duration / 1000; const concat = `${label}: ${durationMs.toFixed(3)} ms`; this.#printer('timeEnd', [concat]); diff --git a/test/gjs-tests.cpp b/test/gjs-tests.cpp index 06d96225f..d82e6c001 100644 --- a/test/gjs-tests.cpp +++ b/test/gjs-tests.cpp @@ -1096,35 +1096,23 @@ static void gjstest_test_args_set_get_unset() { random_unsigned_iface); } -static void gjstest_test_args_rounded_values() { +static void gjstest_test_args_big_int_values() { GIArgument arg = {0}; gjs_arg_set(&arg, std::numeric_limits::max()); - g_test_expect_message( - G_LOG_DOMAIN, G_LOG_LEVEL_WARNING, - "*cannot be safely stored in a JS Number and may be rounded"); - assert_equal(gjs_arg_get_maybe_rounded(&arg), - static_cast(gjs_arg_get(&arg))); - g_test_assert_expected_messages(); + assert_equal(gjs_arg_get(&arg), + std::numeric_limits::max()); gjs_arg_set(&arg, std::numeric_limits::min()); - g_test_expect_message( - G_LOG_DOMAIN, G_LOG_LEVEL_WARNING, - "*cannot be safely stored in a JS Number and may be rounded"); - assert_equal(gjs_arg_get_maybe_rounded(&arg), - static_cast(gjs_arg_get(&arg))); - g_test_assert_expected_messages(); + assert_equal(gjs_arg_get(&arg), + std::numeric_limits::min()); gjs_arg_set(&arg, std::numeric_limits::max()); - g_test_expect_message( - G_LOG_DOMAIN, G_LOG_LEVEL_WARNING, - "*cannot be safely stored in a JS Number and may be rounded"); - assert_equal(gjs_arg_get_maybe_rounded(&arg), - static_cast(gjs_arg_get(&arg))); - g_test_assert_expected_messages(); + assert_equal(gjs_arg_get(&arg), + std::numeric_limits::max()); gjs_arg_set(&arg, std::numeric_limits::min()); - assert_equal(gjs_arg_get_maybe_rounded(&arg), 0.0); + assert_equal(gjs_arg_get(&arg), 0LU); } static void gjstest_test_func_gjs_context_argv_array() { @@ -1230,7 +1218,7 @@ main(int argc, g_test_add_func("/gi/args/set-get-unset", gjstest_test_args_set_get_unset); g_test_add_func("/gi/args/rounded_values", - gjstest_test_args_rounded_values); + gjstest_test_args_big_int_values); g_test_add_func( "/gjs/context/eval-module-file/exit-code-omitted-warning", -- GitLab From 8d8bac9c87176148804245a8893f34cf9df972b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Wed, 4 Nov 2020 01:48:48 +0100 Subject: [PATCH 2/5] jsapi-util-args: Use js_value_to_c to convert numeric values As it will handles BigInt properly --- gjs/jsapi-util-args.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/gjs/jsapi-util-args.h b/gjs/jsapi-util-args.h index a114b06ed..9466b6d4c 100644 --- a/gjs/jsapi-util-args.h +++ b/gjs/jsapi-util-args.h @@ -16,12 +16,12 @@ #include #include -#include #include #include #include #include // for UniqueChars +#include "gi/js-value-inl.h" #include "gjs/jsapi-util.h" #include "gjs/macros.h" @@ -125,7 +125,7 @@ assign(JSContext *cx, throw g_strdup_printf("Wrong type for %c, got int32_t*", c); if (nullable) throw g_strdup("Invalid format string combination ?i"); - if (!JS::ToInt32(cx, value, ref)) + if (!Gjs::js_value_to_c(cx, value, ref)) throw g_strdup("Couldn't convert to integer"); } @@ -143,7 +143,7 @@ assign(JSContext *cx, throw g_strdup_printf("Wrong type for %c, got uint32_t*", c); if (nullable) throw g_strdup("Invalid format string combination ?u"); - if (!value.isNumber() || !JS::ToNumber(cx, value, &num)) + if (!value.isNumeric() || !Gjs::js_value_to_c(cx, value, &num)) throw g_strdup("Couldn't convert to unsigned integer"); if (num > G_MAXUINT32 || num < 0) throw g_strdup_printf("Value %f is out of range", num); @@ -162,7 +162,7 @@ assign(JSContext *cx, throw g_strdup_printf("Wrong type for %c, got int64_t*", c); if (nullable) throw g_strdup("Invalid format string combination ?t"); - if (!JS::ToInt64(cx, value, ref)) + if (!Gjs::js_value_to_c(cx, value, ref)) throw g_strdup("Couldn't convert to 64-bit integer"); } @@ -178,7 +178,7 @@ assign(JSContext *cx, throw g_strdup_printf("Wrong type for %c, got double*", c); if (nullable) throw g_strdup("Invalid format string combination ?f"); - if (!JS::ToNumber(cx, value, ref)) + if (!Gjs::js_value_to_c(cx, value, ref)) throw g_strdup("Couldn't convert to double"); } -- GitLab From f1298bbcac968721de325e0adea6379b4b9fed6e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Thu, 8 Oct 2020 04:54:56 +0200 Subject: [PATCH 3/5] jsapi-util: Support getting the BigInt value from a Number object If we pass a number object that contains the "bigInt" property that we've been adding (i.e. if we're passing a gjs-generated number for 64 bit integers), then use the actual value instead of the numeric value if the this value would be rounded otherwise. Add a test verifying this. --- gi/js-value-inl.h | 5 +++++ gjs/jsapi-util.cpp | 22 ++++++++++++++++++++++ gjs/jsapi-util.h | 3 +++ installed-tests/js/testRegress.js | 14 ++++++++++++++ 4 files changed, 44 insertions(+) diff --git a/gi/js-value-inl.h b/gi/js-value-inl.h index 6e08126cb..bc0aded43 100644 --- a/gi/js-value-inl.h +++ b/gi/js-value-inl.h @@ -257,6 +257,11 @@ GJS_JSAPI_RETURN_CONVENTION inline bool js_value_to_c_checked( bi = JS::NumberToBigInt(cx, number); if (!bi) return false; + } else if (value.isObject()) { + JS::RootedObject number_obj(cx, &value.toObject()); + bi = Gjs::bigint_number_to_bigint(cx, number_obj); + if (!bi) + return false; } if (bi) { diff --git a/gjs/jsapi-util.cpp b/gjs/jsapi-util.cpp index 0999cc146..df99a46a2 100644 --- a/gjs/jsapi-util.cpp +++ b/gjs/jsapi-util.cpp @@ -47,6 +47,7 @@ #include "gjs/context-private.h" #include "gjs/jsapi-util.h" #include "gjs/macros.h" +#include "util/log.h" static void throw_property_lookup_error(JSContext *cx, @@ -635,6 +636,27 @@ static bool number_bigint_getter(JSContext* cx, unsigned argc, JS::Value* vp) { namespace Gjs { +GJS_JSAPI_RETURN_CONVENTION +JS::BigInt* bigint_number_to_bigint(JSContext* cx, + JS::HandleObject number_obj) { + const GjsAtoms& atoms = GjsContextPrivate::atoms(cx); + JS::RootedValue bigint_prop_value(cx); + if (!JS_GetPropertyById(cx, number_obj, atoms.big_int(), + &bigint_prop_value)) + return nullptr; + + if (!bigint_prop_value.isBigInt()) { + gjs_throw(cx, "Handling a non numeric value"); + return nullptr; + } + + gjs_debug_marshal(GJS_DEBUG_GFUNCTION, + "Gjs Numeric value converted to BigInt %s", + gjs_debug_value(bigint_prop_value).c_str()); + + return bigint_prop_value.toBigInt(); +} + GJS_JSAPI_RETURN_CONVENTION bool js_value_set_bigint_number(JSContext* cx, JS::MutableHandleValue value_p, JS::BigInt* bigint, double number) { diff --git a/gjs/jsapi-util.h b/gjs/jsapi-util.h index 601724b53..557716e38 100644 --- a/gjs/jsapi-util.h +++ b/gjs/jsapi-util.h @@ -601,6 +601,9 @@ GJS_JSAPI_RETURN_CONVENTION bool js_value_set_bigint_number(JSContext* cx, JS::MutableHandleValue value_p, JS::BigInt*, double number); +GJS_JSAPI_RETURN_CONVENTION +JS::BigInt* bigint_number_to_bigint(JSContext*, JS::HandleObject number_obj); + } // namespace Gjs [[nodiscard]] std::string gjs_debug_bigint(JS::BigInt* bi); diff --git a/installed-tests/js/testRegress.js b/installed-tests/js/testRegress.js index b2f31cea6..f123b7f98 100644 --- a/installed-tests/js/testRegress.js +++ b/installed-tests/js/testRegress.js @@ -246,6 +246,20 @@ describe('Life, the Universe and Everything', function () { } }); + it(`convert gjs bigint number objects for ${type}`, function () { + expect(expectWarn64(() => testFunc(limits.max))) + .toEqual(limits.max); + expect(expectWarn64(() => testFunc(limits.max)).bigInt) + .toEqual(limits.max.bigInt); + + if (signed) { + expect(expectWarn64(() => testFunc(limits.min))) + .toEqual(limits.min); + expect(expectWarn64(() => testFunc(limits.min)).bigInt) + .toEqual(limits.min.bigInt); + } + }); + it(`respect numeric limits for ${type}`, function () { expect(() => testFunc(limits.max.bigInt + 1n)) .toThrowError(/out of range/); -- GitLab From dae8e32e472a56793bfb8ca5821fdebcd8199c63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Wed, 4 Nov 2020 17:31:17 +0100 Subject: [PATCH 4/5] jsapi-util: Add bigInt to the Number prototype By supporting BigInt for (u)int64 introspected values we broke an assumption for most of the cases, and so that the returned value is a primitive Number instead of a Number Object. This shouldn't really change things, but could break the case where strict equality was used. However, we can limit this to happen only to the not-safe-values, but still be consistent and provide a `bigInt` property for numbers. So basically the idea is to do this: Object.defineProperty(Number.prototype, 'bigInt', { get() { return BigInt(this); }, enumerable: true, configurable: false, }); While keep it only for the specific Number objects in case they are out of bounds for Number integers. In case the bigInt property is requested for a non-integer number we throw, although we could even just make it undefined as it normally was. As per this, revert the equality checks we did in tests, and adapt results to what is expected now. --- gjs/context.cpp | 5 +++ gjs/gjs_pch.hh | 1 + gjs/jsapi-util.cpp | 67 ++++++++++++++++++++++--------- gjs/jsapi-util.h | 6 ++- installed-tests/js/testRegress.js | 64 ++++++++++++++--------------- 5 files changed, 90 insertions(+), 53 deletions(-) diff --git a/gjs/context.cpp b/gjs/context.cpp index 587a244bb..6bff34bd6 100644 --- a/gjs/context.cpp +++ b/gjs/context.cpp @@ -738,6 +738,11 @@ GjsContextPrivate::GjsContextPrivate(JSContext* cx, GjsContext* public_context) gjs_log_exception(m_cx); g_error("Failed to define properties on global object"); } + + if (!Gjs::register_number_bigint_property(cx)) { + gjs_log_exception(m_cx); + g_error("Failed to define Number properties"); + } } JS::SetModuleResolveHook(rt, gjs_module_resolve); diff --git a/gjs/gjs_pch.hh b/gjs/gjs_pch.hh index 1f9818c0a..3b3fa5aaf 100644 --- a/gjs/gjs_pch.hh +++ b/gjs/gjs_pch.hh @@ -111,6 +111,7 @@ #include #include #include +#include #include #include #include diff --git a/gjs/jsapi-util.cpp b/gjs/jsapi-util.cpp index df99a46a2..74d8f057d 100644 --- a/gjs/jsapi-util.cpp +++ b/gjs/jsapi-util.cpp @@ -5,6 +5,7 @@ #include +#include // for fmod #include // for sscanf #include // for strlen @@ -621,21 +622,55 @@ const char* gjs_explain_gc_reason(JS::GCReason reason) { GJS_JSAPI_RETURN_CONVENTION static bool number_bigint_getter(JSContext* cx, unsigned argc, JS::Value* vp) { - GJS_GET_THIS(cx, argc, vp, args, thisObj); + JS::CallArgs args = JS::CallArgsFromVp(argc, vp); + auto const& thisv = args.thisv(); + double number; - JS::RootedValue v_number(cx); - if (!thisObj || !JS::ToPrimitive(cx, thisObj, JSTYPE_NUMBER, &v_number)) { - gjs_throw(cx, "Can't get primitive for non-number object"); + if (thisv.isNumber()) { + number = thisv.toNumber(); + } else { + JS::RootedObject obj(cx, &thisv.toObject()); + JS::RootedValue val(cx); + if (!JS::ToPrimitive(cx, obj, JSTYPE_NUMBER, &val)) { + gjs_throw(cx, "Can't get primitive for non-number object"); + return false; + } + number = val.toNumber(); + } + + if (fmod(number, 1.0) != 0) { + gjs_throw(cx, "Can't get BigInt for non-integer value %f", number); return false; } - g_assert(v_number.isNumber()); - args.rval().setBigInt(JS::NumberToBigInt(cx, v_number.toNumber())); + args.rval().setBigInt(JS::NumberToBigInt(cx, number)); return true; } namespace Gjs { +GJS_JSAPI_RETURN_CONVENTION +bool register_number_bigint_property(JSContext* cx) { + JS::RootedObject number_proto(cx); + + if (!JS_GetClassPrototype(cx, JSProto_Number, &number_proto)) + return false; + + JS::PropertyAttributes attrs{JS::PropertyAttribute::Enumerable}; + JSFunction* getter = + JS_NewFunction(cx, &number_bigint_getter, 1, 0, "bigInt"); + JS::Rooted descr( + cx, JS::PropertyDescriptor::Accessor(JS_GetFunctionObject(getter), + nullptr, attrs)); + const GjsAtoms& atoms = GjsContextPrivate::atoms(cx); + if (!JS_DefinePropertyById(cx, number_proto, atoms.big_int(), descr)) { + gjs_throw(cx, "Can't define bigInt property on Number"); + return false; + } + + return true; +} + GJS_JSAPI_RETURN_CONVENTION JS::BigInt* bigint_number_to_bigint(JSContext* cx, JS::HandleObject number_obj) { @@ -662,25 +697,17 @@ bool js_value_set_bigint_number(JSContext* cx, JS::MutableHandleValue value_p, JS::BigInt* bigint, double number) { value_p.setNumber(number); + if (!bigint) + return true; + JS::RootedObject value_object(cx); if (!JS_ValueToObject(cx, value_p, &value_object)) return false; - JS::Rooted descr(cx); - JS::PropertyAttributes attrs{JS::PropertyAttribute::Enumerable}; - - if (bigint) { - descr = JS::PropertyDescriptor::Data(JS::BigIntValue(bigint), attrs); - } else { - // No need to duplicate the value in this case, just use a getter - JSFunction* getter = - JS_NewFunction(cx, &number_bigint_getter, 1, 0, "bigInt"); - descr = JS::PropertyDescriptor::Accessor(JS_GetFunctionObject(getter), - nullptr, attrs); - } - + JS::RootedValue bigint_value(cx, JS::BigIntValue(bigint)); const GjsAtoms& atoms = GjsContextPrivate::atoms(cx); - if (!JS_DefinePropertyById(cx, value_object, atoms.big_int(), descr)) { + if (!JS_DefinePropertyById(cx, value_object, atoms.big_int(), bigint_value, + GJS_MODULE_PROP_FLAGS | JSPROP_READONLY)) { gjs_throw(cx, "Can't define bigInt property on number"); return false; } diff --git a/gjs/jsapi-util.h b/gjs/jsapi-util.h index 557716e38..9d70fc225 100644 --- a/gjs/jsapi-util.h +++ b/gjs/jsapi-util.h @@ -27,7 +27,7 @@ #include #include // for IgnoreGCPolicy #include -#include +#include // for JSPROP_ENUMERATE, JSPROP_PERMANENT #include #include // for UniqueChars @@ -40,6 +40,7 @@ namespace JS { class BigInt; +class CallArgs; struct Dummy {}; using GTypeNotUint64 = @@ -597,6 +598,9 @@ bool gjs_object_require_converted_property(JSContext *context, namespace Gjs { +GJS_JSAPI_RETURN_CONVENTION +bool register_number_bigint_property(JSContext* cx); + GJS_JSAPI_RETURN_CONVENTION bool js_value_set_bigint_number(JSContext* cx, JS::MutableHandleValue value_p, JS::BigInt*, double number); diff --git a/installed-tests/js/testRegress.js b/installed-tests/js/testRegress.js index f123b7f98..157d1b776 100644 --- a/installed-tests/js/testRegress.js +++ b/installed-tests/js/testRegress.js @@ -61,15 +61,15 @@ describe('Life, the Universe and Everything', function () { [8, 16, 32, 64].forEach(bits => { it(`includes ${bits}-bit integers`, function () { const method = `test_int${bits}`; - expect(Regress[method](42)).toEqual(42); - expect(Regress[method](-42)).toEqual(-42); - expect(Regress[method](undefined)).toEqual(0); - expect(Regress[method](42.42)).toEqual(42); - expect(Regress[method](-42.42)).toEqual(-42); + expect(Regress[method](42)).toBe(42); + expect(Regress[method](-42)).toBe(-42); + expect(Regress[method](undefined)).toBe(0); + expect(Regress[method](42.42)).toBe(42); + expect(Regress[method](-42.42)).toBe(-42); if (bits >= 64) { - expect(Regress[method](42n)).toEqual(42); - expect(Regress[method](-42n)).toEqual(-42); + expect(Regress[method](42n)).toBe(42); + expect(Regress[method](-42n)).toBe(-42); } else { expect(() => Regress[method](42n)).toThrow(); expect(() => Regress[method](-42n)).toThrow(); @@ -78,9 +78,9 @@ describe('Life, the Universe and Everything', function () { it(`includes unsigned ${bits}-bit integers`, function () { const method = `test_uint${bits}`; - expect(Regress[method](42)).toEqual(42); - expect(Regress[method](undefined)).toEqual(0); - expect(Regress[method](42.42)).toEqual(42); + expect(Regress[method](42)).toBe(42); + expect(Regress[method](undefined)).toBe(0); + expect(Regress[method](42.42)).toBe(42); if (bits >= 64) expect(Regress[method](42n)).toEqual(42); @@ -92,22 +92,22 @@ describe('Life, the Universe and Everything', function () { ['short', 'int', 'long', 'ssize', 'float', 'double'].forEach(type => { it(`includes ${type}s`, function () { const method = `test_${type}`; - expect(Regress[method](42)).toEqual(42); - expect(Regress[method](-42)).toEqual(-42); + expect(Regress[method](42)).toBe(42); + expect(Regress[method](-42)).toBe(-42); if (['float', 'double'].includes(type)) { expect(Regress[method](undefined)).toBeNaN(); expect(Regress[method](42.42)).toBeCloseTo(42.42); expect(Regress[method](-42.42)).toBeCloseTo(-42.42); } else { - expect(Regress[method](undefined)).toEqual(0); - expect(Regress[method](42.42)).toEqual(42); - expect(Regress[method](-42.42)).toEqual(-42); + expect(Regress[method](undefined)).toBe(0); + expect(Regress[method](42.42)).toBe(42); + expect(Regress[method](-42.42)).toBe(-42); } if (bit64Types.includes(type)) { - expect(Regress[method](42n)).toEqual(42); - expect(Regress[method](-42n)).toEqual(-42); + expect(Regress[method](42n)).toBe(42); + expect(Regress[method](-42n)).toBe(-42); } else { expect(() => Regress[method](42n)).toThrow(); expect(() => Regress[method](-42n)).toThrow(); @@ -118,9 +118,9 @@ describe('Life, the Universe and Everything', function () { ['ushort', 'uint', 'ulong', 'size'].forEach(type => { it(`includes ${type}s`, function () { const method = `test_${type}`; - expect(Regress[method](42)).toEqual(42); - expect(Regress[method](undefined)).toEqual(0); - expect(Regress[method](42.42)).toEqual(42); + expect(Regress[method](42)).toBe(42); + expect(Regress[method](undefined)).toBe(0); + expect(Regress[method](42.42)).toBe(42); if (bit64Types.includes(type)) expect(Regress[method](42n)).toEqual(42); @@ -185,20 +185,20 @@ describe('Life, the Universe and Everything', function () { expect(limits.min instanceof Number).toBeTruthy(); expect(typeof limits.min.bigInt).toBe('bigint'); - expect(testFunc(-53) instanceof Number).toBeTruthy(); + expect(typeof testFunc(-53)).toBe('number'); expect(typeof testFunc(-55).bigInt).toBe('bigint'); - expect(testFunc(-53n) instanceof Number).toBeTruthy(); + expect(typeof testFunc(-53n)).toBe('number'); expect(typeof testFunc(-55n).bigInt).toBe('bigint'); } expect(limits.max instanceof Number).toBeTruthy(); expect(typeof limits.max.bigInt).toBe('bigint'); - expect(testFunc(53) instanceof Number).toBeTruthy(); + expect(typeof testFunc(53)).toBe('number'); expect(typeof testFunc(55).bigInt).toBe('bigint'); - expect(testFunc(53n) instanceof Number).toBeTruthy(); + expect(typeof testFunc(53n)).toBe('number'); expect(typeof testFunc(55n).bigInt).toBe('bigint'); let maxProp = `MAX${type.toUpperCase()}`; @@ -217,18 +217,18 @@ describe('Life, the Universe and Everything', function () { }); it(`can be function arguments for ${type}`, function () { - expect(testFunc(53)).toEqual(53); - expect(testFunc(55).bigInt).toEqual(55n); + expect(testFunc(53)).toBe(53); + expect(testFunc(55).bigInt).toBe(55n); - expect(testFunc(53n)).toEqual(53); - expect(testFunc(55n).bigInt).toEqual(55n); + expect(testFunc(53n)).toBe(53); + expect(testFunc(55n).bigInt).toBe(55n); if (signed) { - expect(testFunc(-53)).toEqual(-53); - expect(testFunc(-55).bigInt).toEqual(-55n); + expect(testFunc(-53)).toBe(-53); + expect(testFunc(-55).bigInt).toBe(-55n); - expect(testFunc(-53n)).toEqual(-53); - expect(testFunc(-55n).bigInt).toEqual(-55n); + expect(testFunc(-53n)).toBe(-53); + expect(testFunc(-55n).bigInt).toBe(-55n); } }); -- GitLab From bcd4db6b055a610f8df859e7c039335130fb842d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Wed, 4 Nov 2020 17:45:43 +0100 Subject: [PATCH 5/5] testBigInt: Add a separate test case for basic BigInt tests --- installed-tests/js/meson.build | 1 + installed-tests/js/testBigInt.js | 128 +++++++++++++++++++++++++++++++ 2 files changed, 129 insertions(+) create mode 100644 installed-tests/js/testBigInt.js diff --git a/installed-tests/js/meson.build b/installed-tests/js/meson.build index 4d7b3aadc..42b74e11f 100644 --- a/installed-tests/js/meson.build +++ b/installed-tests/js/meson.build @@ -113,6 +113,7 @@ subdir('libgjstesttools') jasmine_tests = [ 'self', + 'BigInt', 'ByteArray', 'Exceptions', 'Format', diff --git a/installed-tests/js/testBigInt.js b/installed-tests/js/testBigInt.js new file mode 100644 index 000000000..777950d3c --- /dev/null +++ b/installed-tests/js/testBigInt.js @@ -0,0 +1,128 @@ +// SPDX-License-Identifier: MIT OR LGPL-2.0-or-later +// SPDX-FileCopyrightText: 2020 Marco Trevisan + +const {GLib, Regress} = imports.gi; + +function expectWarn64(callable) { + GLib.test_expect_message('Gjs', GLib.LogLevelFlags.LEVEL_WARNING, + '*cannot be safely stored*'); + const ret = callable(); + GLib.test_assert_expected_messages_internal('Gjs', + 'testBigInt.js', 0, 'Ignore message'); + return ret; +} + +const Limits = { + int64: { + min: expectWarn64(() => GLib.MININT64), + max: expectWarn64(() => GLib.MAXINT64), + }, + uint64: { + min: GLib.MINUINT64, + max: expectWarn64(() => GLib.MAXUINT64), + }, + ssize: {}, + size: {}, +}; + +const bit64Types = ['uint64', 'int64']; +if (GLib.SIZEOF_LONG === 8) + bit64Types.push('long', 'ulong'); +if (GLib.SIZEOF_SIZE_T === 8) + bit64Types.push('size'); +if (GLib.SIZEOF_SSIZE_T === 8) + bit64Types.push('ssize'); + +if (bit64Types.includes('ssize')) + Object.assign(Limits.ssize, Limits.int64); +if (bit64Types.includes('size')) + Object.assign(Limits.size, Limits.uint64); + +describe('Number integer values have bigInt property', function () { + it('for Number prototype', function () { + expect(Number.prototype.hasOwnProperty('bigInt')).toBeTruthy(); + }); + + it('and cannot be changed', function () { + expect(() => + Object.defineProperty(Number.prototype, 'bigInt', {value: 'foo'})) + .toThrowError(/non-configurable/); + + expect(() => + Object.defineProperty(Limits.int64.max, 'bigInt', {value: 'foo'})) + .toThrowError(/non-configurable/); + }); + + it('and cannot be deleted', function () { + delete Limits.int64.max.bigInt; + expect(Limits.int64.max.hasOwnProperty('bigInt')).toBeTruthy(); + + delete Limits.int64.max.bigInt; + expect(Limits.int64.max.hasOwnProperty('bigInt')).toBeTruthy(); + }); + + it('for primitive numbers', function () { + let num = 12345; + expect(num).toBe(12345); + expect(typeof num).toBe('number'); + expect(num.bigInt).toBe(12345n); + expect(typeof num.bigInt).toBe('bigint'); + }); + + it('contains actual value on big numbers', function () { + let bigValue = Limits.int64.max; + expect(bigValue).toEqual(Limits.int64.max); + expect(bigValue instanceof Number).toBeTruthy(); + expect(bigValue.bigInt).toBe(9223372036854775807n); + expect(typeof bigValue.bigInt).toBe('bigint'); + }); + + [8, 16, 32, 64].forEach(bits => { + [true, false].forEach(sign => { + it(`for ${bits}-bit ${sign ? 'signed' : 'unsigned'} integers`, function () { + const func = Regress[`test_${sign ? '' : 'u'}int${bits}`]; + expect(func(123)).toBe(123); + expect(typeof func(123)).toBe('number'); + + if (sign) { + expect(func(-123).bigInt).toBe(-123n); + expect(typeof func(-123).bigInt).toBe('bigint'); + } else { + expect(() => func(-123).bigInt).toThrowError(/out of range/); + } + }); + }); + }); +}); + +describe('Number non-integer values throws on bigInt property', function () { + it('for primitive numbers', function () { + let num = 12345.6; + expect(num).toBe(12345.6); + expect(typeof num).toBe('number'); + expect(() => num.bigInt).toThrowError(/non-integer/); + }); + + it('for number objects', function () { + let num = new Number(12345.6); // eslint-disable-line no-new-wrappers + expect(num).toEqual(12345.6); + expect(num instanceof Number).toBeTruthy(); + expect(() => num.bigInt).toThrowError(/non-integer/); + }); + + it('for introspected numbers', function () { + let double = GLib.random_double_range(0.1, 0.9); + expect(typeof double).toBe('number'); + expect(double.hasOwnProperty('bigInt')).toBeFalsy(); + expect(() => double.bigInt).toThrowError(/non-integer/); + }); + + ['float', 'double'].forEach(type => { + it(`for return ${type}`, function () { + const func = Regress[`test_${type}`]; + expect(typeof func(12345.67)).toBe('number'); + expect(func(12345.67).hasOwnProperty('bigInt')).toBeFalsy(); + expect(() => func(12345.67).bigInt).toThrowError(/non-integer/); + }); + }); +}); -- GitLab