<html>
<head>
<base href="https://bugs.webkit.org/" />
</head>
<body><span class="vcard"><a class="email" href="mailto:sbarati@apple.com" title="Saam Barati <sbarati@apple.com>"> <span class="fn">Saam Barati</span></a>
</span> changed
<a class="bz_bug_link
bz_status_NEW "
title="NEW - [ES6] Arrow function syntax. Arrow function should support the destructuring parameters."
href="https://bugs.webkit.org/show_bug.cgi?id=146934">bug 146934</a>
<br>
<table border="1" cellspacing="0" cellpadding="8">
<tr>
<th>What</th>
<th>Removed</th>
<th>Added</th>
</tr>
<tr>
<td style="text-align:right;">Attachment #268951 Flags</td>
<td>review?, commit-queue?
</td>
<td>review+, commit-queue-
</td>
</tr></table>
<p>
<div>
<b><a class="bz_bug_link
bz_status_NEW "
title="NEW - [ES6] Arrow function syntax. Arrow function should support the destructuring parameters."
href="https://bugs.webkit.org/show_bug.cgi?id=146934#c9">Comment # 9</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW - [ES6] Arrow function syntax. Arrow function should support the destructuring parameters."
href="https://bugs.webkit.org/show_bug.cgi?id=146934">bug 146934</a>
from <span class="vcard"><a class="email" href="mailto:sbarati@apple.com" title="Saam Barati <sbarati@apple.com>"> <span class="fn">Saam Barati</span></a>
</span></b>
<pre>Comment on <span class=""><a href="attachment.cgi?id=268951&action=diff" name="attach_268951" title="Patch">attachment 268951</a> <a href="attachment.cgi?id=268951&action=edit" title="Patch">[details]</a></span>
Patch
View in context: <a href="https://bugs.webkit.org/attachment.cgi?id=268951&action=review">https://bugs.webkit.org/attachment.cgi?id=268951&action=review</a>
<span class="quote">> Source/JavaScriptCore/parser/Parser.cpp:377
> +bool Parser<LexerType>::isArrowFunctionParameters()</span >
This looks good to me, just some really small nits on how to write the function:
<span class="quote">> Source/JavaScriptCore/parser/Parser.cpp:382
> +
> + if (match(EOFTOK) || (!match(OPENPAREN) && !match(IDENT)))
> + return isArrowFunction;</span >
I would write this like:
```
if (match(EOFTOK))
return false;
bool isOpenParen = match(OPENPAREN);
bool isIdent = match(IDENT);
if (!isOpenParen && !isIdent)
return false;
```
Then, below, you can use these booleans instead of performing the match(...) twice.
I say this because I think that LLVM will CSE this, but lets take matters into our own hand.
<span class="quote">> Source/JavaScriptCore/parser/Parser.cpp:386
> + if (match(IDENT)) {</span >
Then this could be: "if (isIdent) { next(); isArrowFunction = match(ARROWFUNCTION) }
<span class="quote">> Source/JavaScriptCore/parser/Parser.cpp:388
> + } else if (consume(OPENPAREN)) {</span >
Then I would change this to a pure else clause.
Then, I would have the first two lines be:
RELEASE_ASSERT(isOpenParen); // your check at the top of the function guarantees it, so lets not pretend it isn't true.
next();
<span class="quote">> Source/JavaScriptCore/parser/Parser.cpp:390
> + isArrowFunction = consume(CLOSEPAREN) && match(ARROWFUNCTION);</span >
I would write this as:
if {
next();
isArrowFunction = match(ARROWFUNCTION);
}
<span class="quote">> Source/JavaScriptCore/parser/Parser.cpp:393
> + // We make fake scope, otherwise parseFormalParameters will add variable to current scope that lead to errors</span >
style: indentation.</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>