<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#c48">Comment # 48</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:saambarati1@gmail.com" title="Saam Barati <saambarati1@gmail.com>"> <span class="fn">Saam Barati</span></a>
</span></b>
<pre>Comment on <span class=""><a href="attachment.cgi?id=254239&action=diff" name="attach_254239" title="Patch">attachment 254239</a> <a href="attachment.cgi?id=254239&action=edit" title="Patch">[details]</a></span>
Patch
View in context: <a href="https://bugs.webkit.org/attachment.cgi?id=254239&action=review">https://bugs.webkit.org/attachment.cgi?id=254239&action=review</a>
<span class="quote">>> Source/JavaScriptCore/parser/Parser.cpp:2027
>> + doParseConditionalExpression = !isArrowFunctionEmptyParamterList();
>
> In isArrowFunctionEmptyPararmeterList, we create SavePoint. So I think the above `createSavevPoint` and SavePoint are duplicate.
> Could you change to the implementation that `isArrowFunctionEmptyParamterList` always restore the state?
> With that implementation, the above SavePoint is not necessary.</span >
I agree with Yusuke here, the implementation should always restore the save point. It's weird
for a function that tests this to sometimes restore and sometimes not depending on if it found
a match. You can have that function always restore the save point and then ASSERT that
the assumption is held. Also, the structuring of these two if statements is weird to me, we know
for sure one or the other will execute. I'd structure it such that if we match an empty arrow function
then we early return and then remove the doParseConditionalExpression. Like so:
if (emptyArrow) {
ASSERT (...)
Blah
return ...
}
TreeExpression lhs = parseConditionalExpression(...)
...
<span class="quote">> Source/JavaScriptCore/tests/stress/arrowfunction-syntax-error.js:9
> + var errorSyntaxError = false;</span >
Should we be doing something with the "statement" parameter?
<span class="quote">> Source/JavaScriptCore/tests/stress/arrowfunction-syntax-error.js:27
> +testCase(forceRaiseException('debug(=>x+1)'), true, "Error: No exception during parse wrong statement '=>{}'");</span >
Also add a test for this syntax error: "x, y => ..."
(And maybe other cases, I'm not super familiar with the grammar on arrow functions.)</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>