<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. Parser of arrow function with execution as common function"
href="https://bugs.webkit.org/show_bug.cgi?id=144955#c80">Comment # 80</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW - [ES6] Implement ES6 arrow function syntax. Parser of arrow function with execution as common function"
href="https://bugs.webkit.org/show_bug.cgi?id=144955">bug 144955</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=255426&action=diff" name="attach_255426" title="Patch">attachment 255426</a> <a href="attachment.cgi?id=255426&action=edit" title="Patch">[details]</a></span>
Patch
View in context: <a href="https://bugs.webkit.org/attachment.cgi?id=255426&action=review">https://bugs.webkit.org/attachment.cgi?id=255426&action=review</a>
<span class="quote">>> Source/JavaScriptCore/parser/Parser.cpp:355
>> + while (TreeStatement statement = parseStatement(context, directive, &directiveLiteralLength, functionParseType)) {
>
> I think using `parseStatement` for ArrowFunctionBodyExpression case is not good.
> Now we have information about it as `functionParseType`.
> so instead of using parseArrowFunctionSingleExpressionBody inside parseStatement, just separate path here and use parseArrowFunctionSingleExpressionBody directly.</span >
Done
<span class="quote">>> Source/JavaScriptCore/parser/Parser.cpp:593
>> + failDueToUnexpectedToken();
>
> Why this is needed?</span >
It is raise exception for following statement '=>{a}'
This function can be run in several cases:
1) from parseFunctionBody - correct path
2) during reparsing - correct path
3) from parseSourceElements, when js has nor correct statement '=>{ return a}' - wrong path
<span class="quote">>> Source/JavaScriptCore/parser/Parser.cpp:615
>> + next();
>
> I think `parseType` is always ArrowFunctionParseType. So this is not necessary.</span >
In case of reparsing parseType there will be StandardFunctionParseType
<span class="quote">>> Source/JavaScriptCore/parser/Parser.cpp:1226
>> +#endif
>
> Drop here.</span >
Done
<span class="quote">>> Source/JavaScriptCore/parser/Parser.cpp:1276
>> +#endif
>
> Move this into parseSourceElements (at that time, do not loop.)
> So, parseStatement's modification is dropped.</span >
Done
<span class="quote">>> Source/JavaScriptCore/parser/Parser.cpp:1505
>> + if (const SourceProviderCacheItem* cachedInfo = TreeBuilder::CanUseFunctionCache ? findCachedFunctionInfo(info.startFunctionOffset) : 0) {
>
> We lookup previously parsed function with info.startFunctionOffset key. But this value is later changed for ArrowFunctionBodyBlock case.
> So when the type is ArrowFunctionBodyBlock, cachedInfo is never hit.
> I've applied this patch to my workingcopy and log the searched `info.startFunctionOffset` and stored offset.
>
> To test it, be carefull about the function's length. If the length is less than or equal to minimumFunctionLengthToCache, cachedInfo is not created.</span >
I've moved up logic of the checking type of arrow function and setting the info.startFunctionOffset.
<span class="quote">>> Source/JavaScriptCore/parser/Parser.cpp:1575
>> + info.startFunctionOffset = m_token.m_data.offset;
>
> Because of this change, ArrowFunctionBodyBlock cache is never hit.</span >
Moved before looking up to cache
<span class="quote">>> Source/JavaScriptCore/parser/Parser.cpp:1620
>> + unsigned endFunctionOffset = location.startOffset;
>
> Here, we just use location.startOffset. Is it OK for ArrowFunctionBodyExpression ended with line terminator? (we set info.endFunctionOffset = location.endOffset in the previous block).
> I think dropping this is better to keep consistency with info.endFunctionOffset.</span >
There is no tocken line terminator. So I end offset + 1 position of the tocken before line terminator.
<span class="quote">>> Source/JavaScriptCore/parser/Parser.cpp:1634
>> + parameters.endFunctionStartOffset = endFunctionOffset;
>
> Use info.endFunctionOffset.</span >
Done
<span class="quote">>> Source/JavaScriptCore/parser/Parser.cpp:2059
>> + return parseArrowFunctionWrapper(context);
>
> Now, PrimaryExpression part is removed. I think `parseArrowFunctionWrapper` can be renamed to `parseArrowFunctionExpression`.</span >
Done
<span class="quote">>> Source/JavaScriptCore/parser/Parser.cpp:2846
>> + failIfFalse(match(IDENT) || match(OPENPAREN), "Expected an arrow function input parameter");
>
> This condition is guaranteed by `isArrowFunctionParameters()`. So this can be ASSERT.</span >
Done.
<span class="quote">>> Source/JavaScriptCore/parser/SourceCode.h:128
>> + ASSERT(endArrowFunction >= 1);
>
> OK, now you updated `endArrowFunction`. So this assertion should be removed, correct?</span >
Correct. It always greater than startArrowFunction and 1. 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>