[webkit-reviews] review granted: [Bug 179000] [ESNext][BigInt] Implement BigInt literals and JSBigInt : [Attachment 329078] Patch for landing

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Caio Lima <ticaiolima at gmail.com>'s
request for review:
Bug 179000: [ESNext][BigInt] Implement BigInt literals and JSBigInt
https://bugs.webkit.org/show_bug.cgi?id=179000

Attachment 329078: Patch for landing

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




--- Comment #56 from Darin Adler <darin at apple.com> ---
Comment on attachment 329078
  --> https://bugs.webkit.org/attachment.cgi?id=329078
Patch for landing

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

> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h:1019
> +	   struct BigIntEntryHash {
> +	       static unsigned hash(BigIntMapEntry key)
> +	       {
> +		   return computeHash(key.first->existingSymbolAwareHash(),
key.second);
> +	       }
> +	       
> +	       static bool equal(BigIntMapEntry a, BigIntMapEntry b)
> +	       {
> +		   return a == b;
> +	       }
> +	       
> +	       static const bool safeToCompareToEmptyOrDeleted = true;
> +	   };

Now that I look at this again, I believe the default hash for pairs will do the
right thing without us coding anything here. We just not specify
BigIntEntryHash and let the default do its thing. We could also remove the
Hasher.h include if we do that.

> Source/JavaScriptCore/parser/Lexer.h:181
> +    ALWAYS_INLINE Variant<double, const Identifier*> parseHex();
> +    ALWAYS_INLINE std::optional<Variant<double, const Identifier*>>
parseBinary();
> +    ALWAYS_INLINE std::optional<Variant<double, const Identifier*>>
parseOctal();
> +    ALWAYS_INLINE std::optional<Variant<double, const Identifier*>>
parseDecimal();

Another way to make the code tighter is to do this:

    using NumberParseResult = Variant<double, const Identifier*>;

Then you can just use that instead of writing out the Variant type each time.
Like:

    ALWAYS_INLINE NumberParseResult parseHex();
    ALWAYS_INLINE std::optional<NumberParseResult> parseBinary();

And:

    template<typename T> ALWAYS_INLINE auto Lexer<T>::parseBinary() ->
std::optional<NumberParseResult>

> Source/JavaScriptCore/parser/Nodes.h:338
> +	   bool isBigInt() const override { return true; }

This can be final instead of override.