Bug 205416 – Teach the bytecode that arithmetic operations can return bigints

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Robin Morisset]

rdar://problem/57872219

[Robin Morisset]

Created attachment 386013 [details]
Patch

[Yusuke Suzuki]

Comment on attachment 386013 [details]
Patch

r=me

[Mark Lam]

Comment on attachment 386013 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=386013&action=review

> Source/JavaScriptCore/parser/ResultType.h:182
> +            return ResultType(TypeMaybeBigInt | TypeMaybeNumber);

Can you add a bigIntOrNumberType() constexpr function (like the ones above) for this and use that here?

> Source/JavaScriptCore/parser/ResultType.h:191
> +            return ResultType(TypeMaybeBigInt | TypeMaybeNumber);

Ditto.

[Robin Morisset]

Comment on attachment 386013 [details]
Patch

cq- until I apply Mark's suggestion

[Robin Morisset]

Created attachment 387829 [details]
Patch

Rebased + applied Mark's comment.

[Caio Lima]

Comment on attachment 387829 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=387829&action=review

> JSTests/stress/big-int-arithmetic-return-big-int.js:5
> +    return (1n*1n) / 1;

I think it would be good include other operations as well here.

> Source/JavaScriptCore/ChangeLog:8
> +        Add already has the correct ResultType, but previously Sub/Mult/Div/Mod/Pow/Negate were always claimed to return Number, and when BigInt is enabled they can also return BigInt.

nit: I think we need to wrap line

> Source/JavaScriptCore/parser/NodeConstructors.h:505
> +    // UnaryPlus is guaranteed to always return a number, never a BigInt.

It would be good add a link to spec here. (https://tc39.es/ecma262/#sec-unary-plus-operator-runtime-semantics-evaluation)

[Robin Morisset]

Thanks for the comments.

(In reply to Caio Lima from comment #6)
> Comment on attachment 387829 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=387829&action=review
> 
> > JSTests/stress/big-int-arithmetic-return-big-int.js:5
> > +    return (1n*1n) / 1;

Done.

> I think it would be good include other operations as well here.
> 
> > Source/JavaScriptCore/ChangeLog:8
> > +        Add already has the correct ResultType, but previously Sub/Mult/Div/Mod/Pow/Negate were always claimed to return Number, and when BigInt is enabled they can also return BigInt.
> 
> nit: I think we need to wrap line

Done.

> > Source/JavaScriptCore/parser/NodeConstructors.h:505
> > +    // UnaryPlus is guaranteed to always return a number, never a BigInt.
> 
> It would be good add a link to spec here.
> (https://tc39.es/ecma262/#sec-unary-plus-operator-runtime-semantics-
> evaluation)

Done.

[Robin Morisset]

Created attachment 387967 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 387967 [details]
Patch

Clearing flags on attachment: 387967

Committed r254716: <https://trac.webkit.org/changeset/254716>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/58661550>