Skip to content

Conversation

@terurou
Copy link
Member

@terurou terurou commented Sep 1, 2019

@terurou terurou changed the title Js bigint add js.lib.BigInt Sep 1, 2019
@RealyUniqueName RealyUniqueName added this to the Release 4.0 milestone Sep 1, 2019
@RealyUniqueName
Copy link
Member

This is out of scope of this PR, but if it get merged js.Syntax should probably have a static method, which will instantiate bigint literals because:

A BigInt is created by appending n to the end of an integer literal — 10n — or by calling the function BigInt()


@see <https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/BigInt>
**/
abstract BigInt(_BigInt) from _BigInt to _BigInt {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it should be @:coreType instead of an underlying extern class because BigInt is a primitive type:

BigInt is slated to become the first new primitive type added to JavaScript since Symbol in ES2015.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And then all bodies of @:op methods should be removed leaving declarations only. E.g.

@:op(-A) static inline function neg(a:BigInt):BigInt;

Copy link
Member Author

@terurou terurou Sep 2, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to use @:coretype, but it didn't work.
http://try-haxe.mrcdk.com/#091Cf

Haxe converts a++ to a = a + 1. But BigInt + Int doesn't work...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In JS, var a = 1n; a++; works fine.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Increment/decrement could still have bodies even with @:coreType

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Umm..., increment works fine with untyped __js__, but it doesn't work with Syntax.code().

http://try-haxe.mrcdk.com/#6BB44
http://try-haxe.mrcdk.com/#6B8e6

Is it correct? Should I use untyped __js__?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm... Yeah, looks like it's the only option for now.

Copy link
Member

@RealyUniqueName RealyUniqueName Sep 2, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Simn any idea how to make @:op(A++) function():BigInt; not to inline into BigInt(1)++?

return Syntax.code("({0} ^ {1})", a, b);
}

@:op(A << B) static inline function lshift(a:BigInt, b:BigInt):BigInt {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure bit shift ops require a BigInt as the right hand argument?

Copy link
Member Author

@terurou terurou Sep 1, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. In V8(Chrome and Node.js),

> 1n << 1
Uncaught TypeError: Cannot mix BigInt and other types, use explicit conversions

But Firefox accepts int.

Copy link
Member Author

@terurou terurou Sep 5, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I find a mistake of sample code... (It is already fixed.)

It works fine.

> 1n << 1n
2n

It doesn't work.

> 1n << 1
Thrown:
TypeError: Cannot mix BigInt and other types, use explicit conversions

@RealyUniqueName RealyUniqueName modified the milestones: Release 4.0, Backlog Sep 1, 2019
@haxiomic
Copy link
Member

haxiomic commented Sep 3, 2019

@terurou, what do you think about using @:native rather than js.Syntax? The benefit is we'd no longer need to have the if( arg == null ) checks

(same as #8633 (comment))

@terurou
Copy link
Member Author

terurou commented Sep 3, 2019

@haxiomic I tried @:native.
http://try-haxe.mrcdk.com/#B0c66

This is not good... I don't need null.

var a = new BigInt(1);
a.toLocaleString();
BigInt.prototype.toLocaleString.call(BigInt(1),null,null);

@haxiomic
Copy link
Member

haxiomic commented Sep 3, 2019

@terurou thanks for testing, I see what's happening. I guess the null checks are the best approach then

Edit: more discussion #8633 (comment)

@Simn
Copy link
Member

Simn commented Feb 17, 2020

What's the status here?

@RealyUniqueName
Copy link
Member

This PR still needs some work.
Let's try it for 4.1.

@RealyUniqueName RealyUniqueName self-assigned this Feb 17, 2020
@RealyUniqueName RealyUniqueName modified the milestones: Backlog, Release 4.1 Feb 17, 2020
@Simn Simn modified the milestones: Release 4.1, Release 4.2 Apr 29, 2020
@RealyUniqueName RealyUniqueName modified the milestones: Release 4.2, Backlog Jun 30, 2020
@Simn Simn removed this from the Backlog milestone Mar 24, 2023
@Simn
Copy link
Member

Simn commented Mar 25, 2023

At this point, I suppose it's best to close this particular PR. It would require someone to look into this more, and I don't see that happening.

@Simn Simn closed this Mar 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants