<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#c77">Comment # 77</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:utatane.tea@gmail.com" title="Yusuke Suzuki <utatane.tea@gmail.com>"> <span class="fn">Yusuke Suzuki</span></a>
</span></b>
<pre>Comment on <span class=""><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)) {</span >
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 class="quote">> Source/JavaScriptCore/parser/Parser.cpp:593
> + failDueToUnexpectedToken();</span >
Why this is needed?
<span class="quote">> Source/JavaScriptCore/parser/Parser.cpp:615
> + next();</span >
I think `parseType` is always ArrowFunctionParseType. So this is not necessary.
<span class="quote">> Source/JavaScriptCore/parser/Parser.cpp:1226
> +#endif</span >
Drop here.
<span class="quote">> Source/JavaScriptCore/parser/Parser.cpp:1276
> +#endif</span >
Move this into parseSourceElements (at that time, do not loop.)
So, parseStatement's modification is dropped.
<span class="quote">> Source/JavaScriptCore/parser/Parser.cpp:1505
> + if (const SourceProviderCacheItem* cachedInfo = TreeBuilder::CanUseFunctionCache ? findCachedFunctionInfo(info.startFunctionOffset) : 0) {</span >
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 class="quote">> Source/JavaScriptCore/parser/Parser.cpp:1575
> + info.startFunctionOffset = m_token.m_data.offset;</span >
Because of this change, ArrowFunctionBodyBlock cache is never hit.
<span class="quote">> Source/JavaScriptCore/parser/Parser.cpp:1620
> + unsigned endFunctionOffset = location.startOffset;</span >
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 class="quote">> Source/JavaScriptCore/parser/Parser.cpp:1634
> + parameters.endFunctionStartOffset = endFunctionOffset;</span >
Use info.endFunctionOffset.
<span class="quote">> Source/JavaScriptCore/parser/Parser.cpp:2059
> + return parseArrowFunctionWrapper(context);</span >
Now, PrimaryExpression part is removed. I think `parseArrowFunctionWrapper` can be renamed to `parseArrowFunctionExpression`.
<span class="quote">> Source/JavaScriptCore/parser/Parser.cpp:2846
> + failIfFalse(match(IDENT) || match(OPENPAREN), "Expected an arrow function input parameter");</span >
This condition is guaranteed by `isArrowFunctionParameters()`. So this can be ASSERT.
<span class="quote">> Source/JavaScriptCore/parser/SourceCode.h:128
> + ASSERT(endArrowFunction >= 1);</span >
OK, now you updated `endArrowFunction`. So this assertion should be removed, correct?</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>