<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 ES6 arrow function syntax"
href="https://bugs.webkit.org/show_bug.cgi?id=140855#c70">Comment # 70</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW - [ES6] Implement ES6 arrow function syntax"
href="https://bugs.webkit.org/show_bug.cgi?id=140855">bug 140855</a>
from <span class="vcard"><a class="email" href="mailto:gskachkov@gmail.com" title="GSkachkov <gskachkov@gmail.com>"> <span class="fn">GSkachkov</span></a>
</span></b>
<pre>Comment on <span class="bz_obsolete"><a href="attachment.cgi?id=249968&action=diff" name="attach_249968" title="Patch">attachment 249968</a> <a href="attachment.cgi?id=249968&action=edit" title="Patch">[details]</a></span>
Patch
View in context: <a href="https://bugs.webkit.org/attachment.cgi?id=249968&action=review">https://bugs.webkit.org/attachment.cgi?id=249968&action=review</a>
<span class="quote">>> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:1768
>> + instructions().append(thisRegister()->index());
>
> Are we emitting a TDZ check if an arrow function is used inside a derived class' constructor before super() is called?
> e.g. new (class extends (class {}) { () => this; super(); }) should throw a ReferenceError instead of crashing.
>
> Please add a stress test too.</span >
I've added test Source/JavaScriptCore/tests/stress/arrowfunction-tdz.js.
<span class="quote">>> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:1772
>> +RegisterID* BytecodeGenerator::emitThisNodeBytecode(RegisterID* dst, JSTextPosition position)
>
> How is this code different at all from what's in ThisNode::emitBytecode?
> I don't think we should duplicate this much code in two places.</span >
Removed duplication.
<span class="quote">>> Source/JavaScriptCore/jit/JITOperations.cpp:47
>> +#endif
>
> We should probably be checking this inside JSArrowFunction.h</span >
Done.
<span class="quote">>> Source/JavaScriptCore/jit/JITOperations.h:121
>> +#endif
>
> I don't think we need to if-def op codes like this. e.g. op_check_tdz is used only when class syntax is enabled but we don't if-def it.</span >
removed if-def
<span class="quote">>> Source/JavaScriptCore/parser/Parser.cpp:598
>> + bool correcArrowFinish = isEndOfArrowFunction() || match(EOFTOK);
>
> Typo: correc*t*. How is this condition different from autoSemiColon()?
> It seems that we need to document the difference one way or another.</span >
I'll take a look deeper in autoSemiColon(), possible this functions both do the same.
<span class="quote">>> Source/JavaScriptCore/parser/Parser.cpp:1198
>> + bool handled = false;
>
> This variable is unnecessary.</span >
Done
<span class="quote">>> Source/JavaScriptCore/parser/Parser.cpp:1206
>> + handled = true;
>
> We should just call context.setEndOffset and return here instead.</span >
Done
<span class="quote">>> Source/JavaScriptCore/parser/Parser.cpp:1457
>> + if (parseType == ArrowFunctionParseType) {
>
> We should either use switch here or assert that parseType == ArrowFunctionParseType if the previous if evaluated to false.
> Also, can we do a refactoring to put the original code into a separate helper function in a separate patch so that the diff here is readable?
> As is, it's hard to review...</span >
I'll try to refactor to increase patch readability this weekend.
<span class="quote">>> Source/JavaScriptCore/parser/Parser.cpp:1528
>> m_token = cachedInfo->closeBraceToken();
>
> Can't we just add endFunctionToken to cacheInfo instead?</span >
Introduced new function with name endFunctionToken instead of closeBraceToken() & endArrowFunctionToken
<span class="quote">>> Source/JavaScriptCore/parser/Parser.cpp:1544
>> + next();
>
> !? Why not (parseType != ArrowFunctionParseType || info.arrowFunctionBodyType == ArrowFunctionBodyBlock)?</span >
Fixed
<span class="quote">>> Source/JavaScriptCore/parser/Parser.cpp:1629
>> + } else
>
> It looks like these two conditions can be combined into parseType == ArrowFunctionParseType && info.arrowFunctionBodyType != ArrowFunctionBodyBlock
> to run the code in else since the statement in the inner if and the outer else are identical.</span >
Done</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>