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

[bugzilla-daemon at webkit.org]

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

--- Comment #80 from GSkachkov <gskachkov at gmail.com> ---
Comment on attachment 255426
  --> https://bugs.webkit.org/attachment.cgi?id=255426
Patch

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

>> Source/JavaScriptCore/parser/Parser.cpp:355
>> +    while (TreeStatement statement = parseStatement(context, directive, &directiveLiteralLength, functionParseType)) {
> 
> I think using `parseStatement` for ArrowFunctionBodyExpression case is not good.
> Now we have information about it as `functionParseType`.
> so instead of using parseArrowFunctionSingleExpressionBody inside parseStatement, just separate path here and use parseArrowFunctionSingleExpressionBody directly.

Done

>> Source/JavaScriptCore/parser/Parser.cpp:593
>> +        failDueToUnexpectedToken();
> 
> Why this is needed?

It is raise exception for following statement '=>{a}' 
This function can be run in several cases:
1)  from parseFunctionBody - correct path
2)  during reparsing  - correct path
3)  from parseSourceElements, when js has nor correct statement  '=>{ return a}'  - wrong path

>> Source/JavaScriptCore/parser/Parser.cpp:615
>> +        next();
> 
> I think `parseType` is always ArrowFunctionParseType. So this is not necessary.

In case of reparsing parseType there will be StandardFunctionParseType

>> Source/JavaScriptCore/parser/Parser.cpp:1226
>> +#endif
> 
> Drop here.

Done

>> Source/JavaScriptCore/parser/Parser.cpp:1276
>> +#endif
> 
> Move this into parseSourceElements (at that time, do not loop.)
> So, parseStatement's modification is dropped.

Done

>> Source/JavaScriptCore/parser/Parser.cpp:1505
>> +    if (const SourceProviderCacheItem* cachedInfo = TreeBuilder::CanUseFunctionCache ? findCachedFunctionInfo(info.startFunctionOffset) : 0) {
> 
> We lookup previously parsed function with info.startFunctionOffset key. But this value is later changed for ArrowFunctionBodyBlock case.
> So when the type is ArrowFunctionBodyBlock, cachedInfo is never hit.
> I've applied this patch to my workingcopy and log the searched `info.startFunctionOffset` and stored offset.
> 
> To test it, be carefull about the function's length. If the length is less than or equal to minimumFunctionLengthToCache, cachedInfo is not created.

I've moved up logic of the checking type of arrow function and setting the info.startFunctionOffset.

>> Source/JavaScriptCore/parser/Parser.cpp:1575
>> +            info.startFunctionOffset = m_token.m_data.offset;
> 
> Because of this change, ArrowFunctionBodyBlock cache is never hit.

Moved before looking up to cache

>> Source/JavaScriptCore/parser/Parser.cpp:1620
>> +    unsigned endFunctionOffset = location.startOffset;
> 
> Here, we just use location.startOffset. Is it OK for ArrowFunctionBodyExpression ended with line terminator? (we set info.endFunctionOffset = location.endOffset in the previous block).
> I think dropping this is better to keep consistency with info.endFunctionOffset.

There is no tocken line terminator. So I  end offset + 1 position of the tocken before line terminator.

>> Source/JavaScriptCore/parser/Parser.cpp:1634
>> +        parameters.endFunctionStartOffset = endFunctionOffset;
> 
> Use info.endFunctionOffset.

Done

>> Source/JavaScriptCore/parser/Parser.cpp:2059
>> +        return parseArrowFunctionWrapper(context);
> 
> Now, PrimaryExpression part is removed. I think `parseArrowFunctionWrapper` can be renamed to `parseArrowFunctionExpression`.

Done

>> Source/JavaScriptCore/parser/Parser.cpp:2846
>> +    failIfFalse(match(IDENT) || match(OPENPAREN), "Expected an arrow function input parameter");
> 
> This condition is guaranteed by `isArrowFunctionParameters()`. So this can be ASSERT.

Done.

>> Source/JavaScriptCore/parser/SourceCode.h:128
>> +        ASSERT(endArrowFunction >= 1);
> 
> OK, now you updated `endArrowFunction`. So this assertion should be removed, correct?

Correct. It always greater than startArrowFunction and 1. Done

-- 
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/20150624/4444d2af/attachment.html>