[Webkit-unassigned] [Bug 163208] [ES6]. Implement Annex B.3.3 function hoisting rules for eval

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=163208

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

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

Mostly LGTM. Just a couple of questions and a recommendation.

> Source/JavaScriptCore/bytecode/BytecodeList.json:156
> +            { "name" : "op_resolve_scope_for_binding_func_decl_in_eval", "length" : 4 }

Can we call this:
"op_resolve_scope_for_hoisting_func_decl_in_eval"
I think having the word "hoisting" in here will help everyone better understand what this opcode is doing.
(Also, can you change the various C++ functions that have "binding" in the name to "hoisting" or "hoist")

> Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:1063
> +        case ResolveScopeForBindingFuncDeclInEval:
> +            compileResolveScopeForBindingFuncDeclInEval();
> +            break;

You need to say that the FTL supports this opcode inside FTLCapabilities.cpp.
Please verify that this gets compiled in some of your tests, or add new tests that make it to the FTL.

> Source/JavaScriptCore/interpreter/Interpreter.cpp:1148
> +            for (int i = 0; i < numFunctions; ++i) {

Why are there both "numFunctions" and "numHoistedFunctions". Isn't every function declaration in an eval a hoisting candidate?

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20170312/08f0c494/attachment.html>