[webkit-reviews] review denied: [Bug 190799] [ESNext][BigInt] Implement support for "**" : [Attachment 369121] Patch

[bugzilla-daemon at webkit.org]

Saam Barati <sbarati at apple.com> has denied Caio Lima <ticaiolima at gmail.com>'s
request for review:
Bug 190799: [ESNext][BigInt] Implement support for "**"
https://bugs.webkit.org/show_bug.cgi?id=190799

Attachment 369121: Patch

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




--- Comment #10 from Saam Barati <sbarati at apple.com> ---
Comment on attachment 369121
  --> https://bugs.webkit.org/attachment.cgi?id=369121
Patch

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

LGTM overall. Some bugs I believe that are similar to ValueMod patch

> Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h:862
> +	   if (childX && childY && childX.isNumber() && childY.isNumber()) {
> +	       if (!node->isBinaryUseKind(BigIntUse))
> +		   didFoldClobberWorld();
> +	       setConstant(node,
jsDoubleNumber(operationMathPow(childX.asNumber(), childY.asNumber())));
> +	       break;
> +	   }

same comment as I had in ValueMod patch. I believe you need to ensure that the
types of X and Y are valid given BigInt.

> Source/JavaScriptCore/dfg/DFGOperations.cpp:545
> +    if (WTF::holds_alternative<JSBigInt*>(leftNumeric) ||
WTF::holds_alternative<JSBigInt*>(rightNumeric)) {
> +	   if (WTF::holds_alternative<JSBigInt*>(leftNumeric) &&
WTF::holds_alternative<JSBigInt*>(rightNumeric))
> +	       RELEASE_AND_RETURN(scope,
JSValue::encode(JSBigInt::exponentiate(exec, WTF::get<JSBigInt*>(leftNumeric),
WTF::get<JSBigInt*>(rightNumeric))));

Not related to this patch, but maybe we can make this type of pattern easier to
read since it's so common in all the operations. Maybe this can be a followup.
But if we had something like:
bool doBigIntMath(variant a, variant b, doMath lambda, throw lambda or throw
message)

It returns false if both are *not* BigInt. Returns true otherwise. Throws an
exception if 1 of 2 are big int, otherwise, returns lambda