Basic implementation of BigInt, BigInt64Array and BigUint64Array

[psilva261]

Hi!

Here's an implementation for BigInt and Big(U)Int64Array. I thought it could be useful because BigInt cannot be directly translated with Babel but also requires then JSBI. So the numeric token can then be a BigInt. I'm really not 100% sure about this or whether it's better to have a separate token.BIGINT. On the other hand Big(U)Int64Array take 64 bit integers and then there are the arbitrary precision bigints ....n. (At least it's not much code.) Also in the builtin_typedarrays.go there are now additional switch statements to check input for bigint arrays.

Philip

[dop251]

Hi. Thanks for submitting. Not ideal timing as I was working on upgrading the tests for the last couple of weeks which has resulted in a lot of fixes all over the place. I have merged the changes into your branch (see https://github.com/dop251/goja/tree/upgrade-tests-bigint).

As a general comment I think it's a good starting point, however you should always at least read the specification before implementing a new feature. It does not have to be followed exactly as sometimes there is a better, more efficient way to implement something in go, but you must only deviate from the specification if the resulting behavior is exactly the same and there are no side effects (like different order of execution which can be observed by the code). Also, you should try to keep the standard operations as specified. This will save a lot of effort adding new features going forward. I made this mistake myself and dealing with consequences now 😢

For example, you should have implemented ToBigInt and used it where appropriate (for example in bigInt64Array.toRaw). This probably could have removed the need for type switches in TypedArray.prototype methods (although I haven't looked close enough, but these type switches don't look right to me).

Anyway, let me know if you're willing to continue working on this. Otherwise I can take over (can't tell when though...)

[psilva261]

Thanks for looking over this and the combined branch. I should have tested the order related tests before excluding them (I falsely assumed that wouldn't be a regression 🤦‍♂️) At least from the old test suite the whole sec-multiplicative-* also passes on master. It should be quite doable to use the ToBigInt properly and to backtrack where the wrong ordering comes from.

Yeah, the switches look wrong, part of the reason they are there is that the test suite requires SyntaxErrors there. So I thought builtinin_typedarray.go would be a better place to throw these but maybe this solves by itself with the other changes. I'll create an updated pull request soon'ish

[abemedia]

Any update on this?

[vcastellm]

Why is this closed?