[webkit-reviews] review granted: [Bug 175470] Support the with keyword in DFG : [Attachment 318054] Patch with test and fixing a bug

[bugzilla-daemon at webkit.org]

Saam Barati <sbarati at apple.com> has granted Robin Morisset
<rmorisset at apple.com>'s request for review:
Bug 175470: Support the with keyword in DFG
https://bugs.webkit.org/show_bug.cgi?id=175470

Attachment 318054: Patch with test and fixing a bug

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




--- Comment #10 from Saam Barati <sbarati at apple.com> ---
Comment on attachment 318054
  --> https://bugs.webkit.org/attachment.cgi?id=318054
Patch with test and fixing a bug

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

r=me still

> Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h:2068
> +	   // We cannot use the more precise withScopeStructure() here because
it is a LazyProperty and may not yet be allocated.

Style nit: No need for this comment IMO. If you kept, I also wouldn't say
"cannot", I'd just say, "we don't", since we could fix how withScopeStructure
is allocated during JSGlobalObject creation.

> Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h:2069
> +	   forNode(node).setType(m_graph, SpecObject);

You want SpecObjectOther here since that's more precise than SpecObject

> JSTests/stress/with.js:1
> +for (var i = 0; i < 10000; ++i) {

You need to re-run prepare-Changelog so it can modify the Changelog entry
inside JSTests/