<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#c7">Comment # 7</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:rniwa@webkit.org" title="Ryosuke Niwa <rniwa@webkit.org>"> <span class="fn">Ryosuke Niwa</span></a>
</span></b>
<pre>Comment on <span class=""><a href="attachment.cgi?id=253195&action=diff" name="attach_253195" title="Patch">attachment 253195</a> <a href="attachment.cgi?id=253195&action=edit" title="Patch">[details]</a></span>
Patch
View in context: <a href="https://bugs.webkit.org/attachment.cgi?id=253195&action=review">https://bugs.webkit.org/attachment.cgi?id=253195&action=review</a>
<span class="quote">> Source/JavaScriptCore/parser/ASTBuilder.h:372
> + ? m_sourceCode->subArrowExpression(info.arrowFunctionOffset, info.closeBraceOffset, info.bodyStartLine, info.bodyStartColumn)
> + : m_sourceCode->subExpression(info.openBraceOffset, info.closeBraceOffset, info.bodyStartLine, info.bodyStartColumn);</span >
Wrong indentation. ? and : should be exactly 4 spaces to the right from the beginning of "SourceCode source".
<span class="quote">> Source/JavaScriptCore/parser/ASTBuilder.h:375
> + info.body->setLoc(info.bodyStartLine, info.bodyEndLine, location.startOffset, location.lineStartOffset);</span >
Ditto. Indent the second line by exactly 4 spaces further to the right (12 total spaces).
<span class="quote">> Source/JavaScriptCore/parser/Lexer.cpp:1824
> + tokenData->line = savedlineNumber;
> + tokenData->offset = savedOffset;
> + tokenData->lineStartOffset = savedLineStartOffset;
> +
> + ASSERT(tokenData->offset >= tokenData->lineStartOffset);</span >
I don't think we want to shift() and then set offset back manually like this.
We should use peek() instead.
<span class="quote">> Source/JavaScriptCore/parser/Parser.cpp:605
> + setEndOfStatement();</span >
Why do we need this?
<span class="quote">> Source/JavaScriptCore/parser/Parser.cpp:1203
> + context.setEndOffset(result, m_lastTokenEndPosition.offset);</span >
What is this setEndOffset for?
<span class="quote">> Source/JavaScriptCore/parser/Parser.cpp:1260
> + // Consume the end of arrow function ,;]) and prevent
> + // parse this symbols by parseStatement as separate statement</span >
I don't understand this comment.
In general, we prefer adding assertions to document conditions rather than adding comments.
Is there a way to express this as an assertion instead?
<span class="quote">> Source/JavaScriptCore/parser/Parser.cpp:1341
> +#if ENABLE(ES6_ARROWFUNCTION_SYNTAX)
> + if (parseType == StandardFunctionParseType) {
> + next();
> + if (match(CLOSEBRACE)) {
> + unsigned endColumn = tokenColumn();
> + return context.createFunctionBody(startLocation, tokenLocation(), startColumn, endColumn, functionKeywordStart, functionNameStart, parametersStart, strictMode(), constructorKind);
> + }
> + }
> +#else</span >
It's not great to duplicate the code like this.
Since FunctionParseType is always define, we should just use that without wrapping in if-def.</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>