Skip to content

Add full (u)int64 support via BigInt usage

Marco Trevisan requested to merge 3v1n0/gjs:bigint-support into master

While there was some work going on for this in !412 (closed), that was not relying in the mozjs API but rather based on string support. Now, given that mozjs supports BigInt natively and that some refactors on args landed, I took the opportunity to look at this. So I hope I didn't overstep on anything.

As per spidermonkey 78.2.0, it 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've to handle this at gjs level not to break the old assumptions.

As per this, as discussed upstream, the idea here is that when we're handling an (u)int64 we create a new Number 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

So, as per what we discussed, when a value can't be represented via a normal Number we do:

let retval = Number(notRepresentableNumber);
retval.bigInt = actualValueAsBigInt;
return retval;

However, this could be improved imho so that we don't have the problem of building Number object's when not needed, but still being consistent in having the bigInt property set.

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.

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 bound 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.

CI would pass, however we're failing in building images right now (I wasn't able to understand why), so last commits need to be adjusted, however we need to build the image using this mozjs: https://github.com/ptomato/mozjs/pull/2

TODO

Missing APIs

  • BigInt::isValidNumber()
  • BigInt::toNumber() [throws if above doesn't work]
  • Value::asBigInt() [throws if not possible] as done in !524 (2541bd46)
  • BigInt::holdsSigned() not sure this is possible, but given we can create a bigInt from both UInt and Int values, would be convenient to figure out if the BigInt value we got is signed or not (as per !524 (d02eaa14))
  • BigInt::fits<IntType>() to check if the given BigInt can be represented by the given type (or maybe this can be handled as part of isValidNumber(), throwing different errors when out of range). As a BigInt value can be bigger than MAX(U)INT64 and so we need a way to figure it as done in !524 (4c4f82c4) via JS.
Edited by Philip Chimento

Merge request reports