[Webkit-unassigned] [Bug 144955] [ES6] Implement ES6 arrow function syntax. Parser of arrow function with execution as common function

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Jun 8 14:05:46 PDT 2015


https://bugs.webkit.org/show_bug.cgi?id=144955

--- Comment #55 from Yusuke Suzuki <utatane.tea at gmail.com> ---
Comment on attachment 254445
  --> https://bugs.webkit.org/attachment.cgi?id=254445
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=254445&action=review

Great.

> Source/JavaScriptCore/parser/Parser.cpp:607
> +    bool correctArrowFinish = isEndOfArrowFunction() || match(EOFTOK);

`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.

> Source/JavaScriptCore/parser/Parser.cpp:608
> +    failIfFalse(correctArrowFinish, "Expected a ';', ']', '}', ')', or ',' following a arrow function statement");

it seems refering to EOF and Line terminator is needed.

> Source/JavaScriptCore/parser/Parser.cpp:1257
> +        result = parseArrowFunctionStatement(context, parseType);

This `parseArrowFunctionStatement` name confuses me.
This function is used when

(e) `=> expr;`

part, right? If so, I think clearer name is encouraged.

> Source/JavaScriptCore/parser/Parser.cpp:1407
> +template <typename LexerType> template <class TreeBuilder> int Parser<LexerType>::parseArrowFunctionParameters(TreeBuilder& context, FunctionParseMode mode, ParserFunctionInfo<TreeBuilder>& info)

Why parseArrowFunctionParameters is needed in addition to `parseFunctionParameters`?
Is it possible just using `parseFunctionParameters`?

> Source/JavaScriptCore/parser/Parser.cpp:1585
> +        position.offset++;

Is it handled when <CR><LF> comes? Hm, this part looks a little bit weird.

> Source/JavaScriptCore/parser/Parser.cpp:1621
> +        failIfFalse(isEndOfArrowFunction(), "Expected the closing ';' ',' ']' ')' '}' after arrow function");

it seems refering to EOF and Line terminator is needed.

> Source/JavaScriptCore/parser/Parser.h:632
> +        if (match(EOFTOK))
> +            return isArrowFunction;
> +        

Is the above match(EOFTOK) check needed?

> Source/JavaScriptCore/parser/SourceCode.h:130
> +            UChar symbol = provider()->source()[endArrowFunction - 1];

I suggest adding `ASSERT(endArrowFunction >= 1);` if it's guaranteed.

> Source/JavaScriptCore/parser/SourceCode.h:131
> +            ASSERT_UNUSED(symbol, symbol == 13 || symbol == 10 || symbol == ',' || symbol == ';' || symbol == ')' || symbol == ']' || symbol == '}');

What are 13 and 10? Is it possible to use Lexer<...>::isLineTerminator? So are 0x2028/0x2029 is handled? (They are also line terminators.)

> Source/JavaScriptCore/parser/SourceProviderCacheItem.h:73
>          token.m_location.endOffset = closeBraceOffset + 1;

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?

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20150608/c1e0cc87/attachment-0001.html>


More information about the webkit-unassigned mailing list