[webkit-reviews] review granted: [Bug 177056] [DFG] Remove ToThis more aggressively : [Attachment 321048] Patch

[bugzilla-daemon at webkit.org]

Saam Barati <sbarati at apple.com> has granted Yusuke Suzuki
<utatane.tea at gmail.com>'s request for review:
Bug 177056: [DFG] Remove ToThis more aggressively
https://bugs.webkit.org/show_bug.cgi?id=177056

Attachment 321048: Patch

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




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

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

r=me

Can you also add a test where we change globalThisValue after we've already
compiled the code, that way we ensure that we're not trying to completely
constant fold the result of GetGlobalThis?

> Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h:174
> +	   bool allStructureIsScope = !valueForNode.m_structure.isClear();

let's name this: allStructuresAreScope or allStructuresAreJSScope

> Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h:188
> +	       if (!(type.isObject() && type.overridesToThis() &&
structure->classInfo()->methodTable.toThis ==
JSScope::info()->methodTable.toThis))

Why the three conditions? Doesn't (structure->classInfo()->methodTable.toThis
== JSScope::info()->methodTable.toThis) suffice for your needs?
I'd maybe just write this:
``allStructuresAreScope &= structure->classInfo()->methodTable.toThis ==
JSScope::info()->methodTable.toThis``