[webkit-reviews] review granted: [Bug 226072] [JSC] Use DataIC for AccessCase : [Attachment 430426] Patch

[bugzilla-daemon at webkit.org]

Filip Pizlo <fpizlo at apple.com> has granted Yusuke Suzuki <ysuzuki at apple.com>'s
request for review:
Bug 226072: [JSC] Use DataIC for AccessCase
https://bugs.webkit.org/show_bug.cgi?id=226072

Attachment 430426: Patch

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




--- Comment #33 from Filip Pizlo <fpizlo at apple.com> ---
Comment on attachment 430426
  --> https://bugs.webkit.org/attachment.cgi?id=430426
Patch

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

Wow, what a patch.  R=me with mine and Saam's comments.

> Source/JavaScriptCore/bytecode/PolymorphicAccess.cpp:276
> +	   auto myCase = WTFMove(originalCasesToAdd[i]);

I don't like the use of auto right here.  I think this code will get read a
lot, and is subtle, so having this thing's type would be great here.

> Source/JavaScriptCore/jit/GCAwareJITStubRoutine.h:93
> +class AccessCaseJITStubRoutine : public GCAwareJITStubRoutine {

Is this used to represent a single access case, or a PolymorphicAccess-compiled
polymorphic thing with many cases?

If the latter, I suggest picking a different name.  Maybe
PolymorphicAccessJITStubRoutine.