[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

GSkachkov <gskachkov at gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #255191|1                           |0
        is obsolete|                            |

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

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

>> Source/JavaScriptCore/ChangeLog:106
>> +        (JSC::SyntaxChecker::setFunctionNameStart):
> 
> Ah, when using `Tools/Scripts/webkit-patch upload --update-changelogs` and the ChangeLog is significantly different from the previous version, it appends new logs.
> So let's drop the previous one manually.

Done

>> Source/JavaScriptCore/parser/Parser.cpp:1270
>> +        result = parseArrowFunctionExpression(context, parseType);
> 
> Saam's pointing is reasonable. Could you rename it to "parseArrowFunctionSingleExpressionBody"?

Done.

>> Source/JavaScriptCore/parser/Parser.cpp:1447
>> +    bool isClassConstructor;
> 
> Let's drop this line.

Done

>> Source/JavaScriptCore/parser/Parser.cpp:1472
>> +        isClassConstructor = mode == MethodMode && info.name && *info.name == m_vm->propertyNames->constructor;
> 
> It seems this line is not necessary, right?

Done

>> Source/JavaScriptCore/parser/Parser.cpp:1478
>> +        isClassConstructor = constructorKind != ConstructorKind::None;
> 
> This always override the previous one. And I think this line is not necessary.

Done

>> Source/JavaScriptCore/parser/Parser.cpp:1487
>> +        isClassConstructor = false;
> 
> I think this line is not necessary. Instead, let's insert `ASSERT(constructorKind == ConstructorKind::None);` (correct?)

Done

>> Source/JavaScriptCore/parser/Parser.cpp:1495
>> +    constructorKind = isClassConstructor ? constructorKind : ConstructorKind::None;
> 
> I think this line is not necessary.

Done

>> Source/JavaScriptCore/parser/Parser.cpp:1610
>> +        info.isEndByTerminator = true;
> 
> OK, they are initialized in struct definition with C++11 initialization form.

Done

>> Source/JavaScriptCore/parser/Parser.cpp:1635
>> +            parameters.isPrevTerminator = m_lexer->prevTerminator();
> 
> These fields are not initialized if parseType is not ArrowFunctionParseType.
> Since they are primimtive types (unsigned int etc.), use of uninitialized values causes undefined behavior in C++.
> 
> Let's take the either way
> 1. initialize them in the struct definition with C++11 initialization form.
> 2. initialize them before this if-branch.

So I've added initializing of this paramters in the struct definition

>> Source/JavaScriptCore/parser/ParserFunctionInfo.h:41
>> +    unsigned startFunctionOffset = 0;
> 
> Let's exchange this member's order.
>     unsigned startFunctionOffset = 0;
>     unsigned endFunctionOffset = 0;

Done

>> Source/JavaScriptCore/parser/ParserFunctionInfo.h:49
>> +    FunctionBodyType functionBodyType = StandardFunctionBodyBlock;
> 
> They are initialized with C++ initialization form.

Done

>> Source/JavaScriptCore/parser/SourceProviderCacheItem.h:39
>> +    unsigned endFunctionOffset;
> 
> Since now we have endFunctionEndOffset, endFunctionStartOffset sounds better.

Done

>> Source/JavaScriptCore/parser/SourceProviderCacheItem.h:50
>> +    bool isPrevTerminator;
> 
> Since the above 3 fields not set if the function is standard function, it becomes undefined values in C++.
> Let's use C++11 initialization.
> 
>     bool isBodyArrowExpression { false };
>     JSTokenType tokenType { CLOSEBRACE };
>     bool isPrevTerminator { false };

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/20150623/66c2a6a0/attachment-0001.html>