[webkit-reviews] review granted: [Bug 238918] [JSC] DFG / FTL should be aware of JSString's String replacement : [Attachment 457004] Patch

[bugzilla-daemon at webkit.org]

Saam Barati <sbarati at apple.com> has granted Yusuke Suzuki <ysuzuki at apple.com>'s
request for review:
Bug 238918: [JSC] DFG / FTL should be aware of JSString's String replacement
https://bugs.webkit.org/show_bug.cgi?id=238918

Attachment 457004: Patch

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




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

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

Nice. r=me

> Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h:3795
> +	   ASSERT(node->arrayMode().type() != Array::String);

Can we put this in DFGValidate? That is the more canonical place to put such
things.

> Source/JavaScriptCore/dfg/DFGClobberize.h:1341
> +	   ASSERT(node->arrayMode().type() != Array::String);

ditto. I think just putting it in Validate is the right way

> Source/JavaScriptCore/dfg/DFGOperations.cpp:2524
> +    string->value(globalObject);

why not call JSRopeString->resolveRope here?

> Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:4775
> +	   ASSERT(m_node->arrayMode().type() != Array::String);

ditto about validation