<html>
<head>
<base href="https://bugs.webkit.org/" />
</head>
<body>
<p>
<div>
<b><a class="bz_bug_link
bz_status_NEW "
title="NEW - [ES6]. Implement Annex B.3.3 function hoisting rules for eval"
href="https://bugs.webkit.org/show_bug.cgi?id=163208#c58">Comment # 58</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW - [ES6]. Implement Annex B.3.3 function hoisting rules for eval"
href="https://bugs.webkit.org/show_bug.cgi?id=163208">bug 163208</a>
from <span class="vcard"><a class="email" href="mailto:sbarati@apple.com" title="Saam Barati <sbarati@apple.com>"> <span class="fn">Saam Barati</span></a>
</span></b>
<pre>Comment on <span class=""><a href="attachment.cgi?id=302546&action=diff" name="attach_302546" title="Patch">attachment 302546</a> <a href="attachment.cgi?id=302546&action=edit" title="Patch">[details]</a></span>
Patch
View in context: <a href="https://bugs.webkit.org/attachment.cgi?id=302546&action=review">https://bugs.webkit.org/attachment.cgi?id=302546&action=review</a>
Mostly LGTM. Just a couple of questions and a recommendation.
<span class="quote">> Source/JavaScriptCore/bytecode/BytecodeList.json:156
> + { "name" : "op_resolve_scope_for_binding_func_decl_in_eval", "length" : 4 }</span >
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")
<span class="quote">> Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:1063
> + case ResolveScopeForBindingFuncDeclInEval:
> + compileResolveScopeForBindingFuncDeclInEval();
> + break;</span >
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.
<span class="quote">> Source/JavaScriptCore/interpreter/Interpreter.cpp:1148
> + for (int i = 0; i < numFunctions; ++i) {</span >
Why are there both "numFunctions" and "numHoistedFunctions". Isn't every function declaration in an eval a hoisting candidate?</pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are the assignee for the bug.</li>
</ul>
</body>
</html>