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

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Apr 14 12:01:01 PDT 2017


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

--- Comment #71 from GSkachkov <gskachkov at gmail.com> ---
Comment on attachment 306399
  --> https://bugs.webkit.org/attachment.cgi?id=306399
Patch

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

>> Source/JavaScriptCore/bytecode/UnlinkedEvalCodeBlock.h:60
>> +    }
> 
> Style nit: You can use C++ move semantics here and do something like:
> adopt(Vector<T>&& v) { ASSERT(m_v.isEmpty()); m_v = WTFMove(v); }

Done

>> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:771
>> +            hoistedFunctions.append(Identifier::fromUid(m_vm, entry.key.get()));
> 
> Style nit:
> I think this is easier to read if you flip the if and else and remove the "!" on the if's condition.

Done

>> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:2225
>> +            ASSERT_UNUSED(checkResult.get(), checkResult.get() != nullptr);
> 
> no need for ASSERT_UNUSED here, can just be a regular ASSERT.
> ASSERT_UNUSED is only for variables, not temp expressions

I've just remove ASSERT, not sure if it necessary at this place

>> Source/JavaScriptCore/parser/Parser.h:1201
>> +            bool isSloppyModeHoistingCandidate = !strictMode() && closestParentOrdinaryFunctionNonLexicalScope()->isEvalContext();
> 
> This can only be true if m_statementDepth == 1, however, isn't this just a top level function if m_statementDepth == 1? Why is this needed here then?

Ohhh, you are right.I think it should be just false

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20170414/02eaf230/attachment.html>


More information about the webkit-unassigned mailing list