[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
Thu Apr 13 19:33:08 PDT 2017
https://bugs.webkit.org/show_bug.cgi?id=163208
--- Comment #69 from Saam Barati <sbarati at apple.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
Almost everything LGTM, just 1 fundamental question below:
> Source/JavaScriptCore/bytecode/UnlinkedEvalCodeBlock.h:60
> + void adoptFunctionHoistingCandidates(Vector<Identifier, 0, UnsafeVectorOverflow>& functionHoistingCandidates)
> + {
> + ASSERT(m_functionHoistingCandidates.isEmpty());
> + m_functionHoistingCandidates.swap(functionHoistingCandidates);
> + }
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); }
> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:771
> + if (!entry.value.isSloppyModeHoistingCandidate())
> + variables.append(Identifier::fromUid(m_vm, entry.key.get()));
> + else
> + 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.
> 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
> 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?
--
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/562d0cdf/attachment.html>
More information about the webkit-unassigned
mailing list