<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#c55">Comment # 55</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=254445&action=diff" name="attach_254445" title="Patch">attachment 254445</a> <a href="attachment.cgi?id=254445&action=edit" title="Patch">[details]</a></span>
Patch
View in context: <a href="https://bugs.webkit.org/attachment.cgi?id=254445&action=review">https://bugs.webkit.org/attachment.cgi?id=254445&action=review</a>
Great.
<span class="quote">> Source/JavaScriptCore/parser/Parser.cpp:607
> + bool correctArrowFinish = isEndOfArrowFunction() || match(EOFTOK);</span >
`match(EOFTOK)` is contained in `isEndOfArrowFunction()`. So I think `match(EOFTOK)` is not needed. Correct?
If so, we can change it to `failIfFalse(isEndOfArrowFunction(), "...");` by merging the following check.
<span class="quote">> Source/JavaScriptCore/parser/Parser.cpp:608
> + failIfFalse(correctArrowFinish, "Expected a ';', ']', '}', ')', or ',' following a arrow function statement");</span >
it seems refering to EOF and Line terminator is needed.
<span class="quote">> Source/JavaScriptCore/parser/Parser.cpp:1257
> + result = parseArrowFunctionStatement(context, parseType);</span >
This `parseArrowFunctionStatement` name confuses me.
This function is used when
(e) `=> expr;`
part, right? If so, I think clearer name is encouraged.
<span class="quote">> Source/JavaScriptCore/parser/Parser.cpp:1407
> +template <typename LexerType> template <class TreeBuilder> int Parser<LexerType>::parseArrowFunctionParameters(TreeBuilder& context, FunctionParseMode mode, ParserFunctionInfo<TreeBuilder>& info)</span >
Why parseArrowFunctionParameters is needed in addition to `parseFunctionParameters`?
Is it possible just using `parseFunctionParameters`?
<span class="quote">> Source/JavaScriptCore/parser/Parser.cpp:1585
> + position.offset++;</span >
Is it handled when <CR><LF> comes? Hm, this part looks a little bit weird.
<span class="quote">> Source/JavaScriptCore/parser/Parser.cpp:1621
> + failIfFalse(isEndOfArrowFunction(), "Expected the closing ';' ',' ']' ')' '}' after arrow function");</span >
it seems refering to EOF and Line terminator is needed.
<span class="quote">> Source/JavaScriptCore/parser/Parser.h:632
> + if (match(EOFTOK))
> + return isArrowFunction;
> + </span >
Is the above match(EOFTOK) check needed?
<span class="quote">> Source/JavaScriptCore/parser/SourceCode.h:130
> + UChar symbol = provider()->source()[endArrowFunction - 1];</span >
I suggest adding `ASSERT(endArrowFunction >= 1);` if it's guaranteed.
<span class="quote">> Source/JavaScriptCore/parser/SourceCode.h:131
> + ASSERT_UNUSED(symbol, symbol == 13 || symbol == 10 || symbol == ',' || symbol == ';' || symbol == ')' || symbol == ']' || symbol == '}');</span >
What are 13 and 10? Is it possible to use Lexer<...>::isLineTerminator? So are 0x2028/0x2029 is handled? (They are also line terminators.)
<span class="quote">> Source/JavaScriptCore/parser/SourceProviderCacheItem.h:73
> token.m_location.endOffset = closeBraceOffset + 1;</span >
Is this `+1` OK for non `CLOSEBRACE` token? For example, If the arrow function expression is splitted by previous line terminators, I think (almost) arbitrary tokens can come here. 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>