[webkit-reviews] review granted: [Bug 221093] Support Ergonomic Brand Checks proposal (`#x in obj`) : [Attachment 428857] Patch

[bugzilla-daemon at webkit.org]

Caio Lima <ticaiolima at gmail.com> has granted Ross Kirsling
<ross.kirsling at sony.com>'s request for review:
Bug 221093: Support Ergonomic Brand Checks proposal (`#x in obj`)
https://bugs.webkit.org/show_bug.cgi?id=221093

Attachment 428857: Patch

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




--- Comment #9 from Caio Lima <ticaiolima at gmail.com> ---
Comment on attachment 428857
  --> https://bugs.webkit.org/attachment.cgi?id=428857
Patch

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

r=me with comments

> Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp:3220
> +	   }

I’d assert here if it’s a private method or accessor

> Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:8279
> +	       auto bytecode = currentInstruction->as<OpHasPrivateName>();

Could you add a FIXME to point to a bug where we could inline this operation?

> Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:8285
> +	       auto bytecode = currentInstruction->as<OpHasPrivateBrand>();

Ditto

> Source/JavaScriptCore/runtime/JSObjectInlines.h:702
> +    ASSERT(brand.isSymbol());

Could we assert here that it’s also a PrivateSymbol?

> JSTests/stress/private-in.js:58
> +  shouldThrowTypeError(() => SS.isSS(3));

It would be nice test cases where we evaluate a class more than once.