[webkit-reviews] review granted: [Bug 195901] [JSC] Generator should not create JSLexicalEnvironment if it is not necessary : [Attachment 365095] 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 195901: [JSC] Generator should not create JSLexicalEnvironment if it is not
necessary
https://bugs.webkit.org/show_bug.cgi?id=195901

Attachment 365095: Patch

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




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

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

> Source/JavaScriptCore/ChangeLog:11
> +	   are allocated in RAMification Basic test.

"RAMification Basic" => "RAMification's Basic"

> Source/JavaScriptCore/bytecode/BytecodeGeneratorification.cpp:286
> +	   rewriter.insertFragmentAfter(instruction,
[&](BytecodeRewriter::Fragment& fragment) {

style nit: I think space between "]" and "("

> Source/JavaScriptCore/bytecode/BytecodeGeneratorification.cpp:288
> +		   // Generator frame is not necessary. Let's remove it if we
can.

Let's rename this comment to say something like:
"This will cause us to put jsUndefined() into the generator frame's scope
value."

> Source/JavaScriptCore/bytecode/BytecodeUseDef.h:160
> +    USES(OpCreateGeneratorFrameEnvironment, scope)

Do we not need to use initialValue/SymbolTable here?

> Source/JavaScriptCore/bytecode/BytecodeUseDef.h:368
> +    USES(OpCreateGeneratorFrameEnvironment, dst)

This needs to be DEFS