[webkit-reviews] review granted: [Bug 184327] [ESNext][BigInt] Implement support for "%" operation : [Attachment 341487] Patch

[bugzilla-daemon at webkit.org]

Yusuke Suzuki <utatane.tea at gmail.com> has granted Caio Lima
<ticaiolima at gmail.com>'s request for review:
Bug 184327: [ESNext][BigInt] Implement support for "%" operation
https://bugs.webkit.org/show_bug.cgi?id=184327

Attachment 341487: Patch

https://bugs.webkit.org/attachment.cgi?id=341487&action=review




--- Comment #4 from Yusuke Suzuki <utatane.tea at gmail.com> ---
Comment on attachment 341487
  --> https://bugs.webkit.org/attachment.cgi?id=341487
Patch

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

r=me with comments.

> Source/JavaScriptCore/runtime/CommonSlowPaths.cpp:572
> +	       JSValue result(JSBigInt::remainder(exec,
WTF::get<JSBigInt*>(leftNumeric), WTF::get<JSBigInt*>(rightNumeric)));

Keep the narrower type (JSBigInt rather than JSValue).

JSBigInt* result = JSBigInt::remainder(exec, WTF::get<JSBigInt*>(leftNumeric),
WTF::get<JSBigInt*>(rightNumeric)));

> Source/JavaScriptCore/runtime/JSBigInt.cpp:1018
>      if (isZero())
>	   return this;

Typically, we call `setSign()` and `rightTrim()`. But what prevents us from
creating `negative zero`?
Please review the use of rightTrim(), and if we have a bug, could you fix it
with a test?

> JSTests/ChangeLog:14
> +	   * stress/big-int-mod-memory-stress.js: Added.
> +	   * stress/big-int-mod-to-primitive-precedence.js: Added.
> +	   * stress/big-int-mod-to-primitive.js: Added.
> +	   * stress/big-int-mod-type-error.js: Added.
> +	   * stress/big-int-mod-wrapped-value.js: Added.
> +	   * stress/big-int-mod.js: Added.

Add tests involving JIT tiers. Especially, prevent constant folding by using
`noInline` and arguments.