[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 #60 from GSkachkov <gskachkov at gmail.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

>> 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")

Fixed

>> Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:1063
>> +            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.

Fixed
I added RELEASE_ASSERT_.. and receive following tests failed:    stress/eval-func-decl-block-with-remove.js.ftl-eager
    stress/eval-func-decl-block-with-remove.js.ftl-eager-no-cjit
    stress/eval-func-decl-block-with-var-and-remove.js.ftl-eager
    stress/eval-func-decl-block-with-var-and-remove.js.ftl-eager-no-cjit
    stress/eval-func-decl-block-with-var-sinthesize.js.ftl-eager
    stress/eval-func-decl-block-with-var-sinthesize.js.ftl-eager-no-cjit
    stress/eval-func-decl-in-eval-within-block-with-let.js.ftl-eager
    stress/eval-func-decl-in-eval-within-block-with-let.js.ftl-eager-no-cjit
    stress/eval-func-decl-in-eval-within-with-scope.js.default
    stress/eval-func-decl-in-eval-within-with-scope.js.ftl-eager
    stress/eval-func-decl-in-eval-within-with-scope.js.ftl-eager-no-cjit
    stress/eval-func-decl-in-eval-within-with-scope.js.ftl-no-cjit-no-inline-validate
    stress/eval-func-decl-in-eval-within-with-scope.js.ftl-no-cjit-no-put-stack-validate
    stress/eval-func-decl-in-eval-within-with-scope.js.ftl-no-cjit-validate-sampling-profiler
    stress/eval-func-decl-within-eval-with-reassign-to-var.js.ftl-eager
    stress/eval-func-decl-within-eval-with-reassign-to-var.js.ftl-eager-no-cjit
    stress/eval-func-decl-within-eval-without-reassign-to-let.js.ftl-eager
    stress/eval-func-decl-within-eval-without-reassign-to-let.js.ftl-eager-no-cjit
So I think it covered by Tests

>> 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?

We have topLevel function that is list of the declared functions, and list of the function names from codeBlockes, that later will be bounded to function. 
I've renamed to correspond this difference. Could you please look if it more clear?

-- 
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/20170320/d1726981/attachment.html>