<html>
<head>
<base href="https://bugs.webkit.org/" />
</head>
<body><span class="vcard"><a class="email" href="mailto:gskachkov@gmail.com" title="GSkachkov <gskachkov@gmail.com>"> <span class="fn">GSkachkov</span></a>
</span> changed
<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>
<br>
<table border="1" cellspacing="0" cellpadding="8">
<tr>
<th>What</th>
<th>Removed</th>
<th>Added</th>
</tr>
<tr>
<td style="text-align:right;">Attachment #255191 is obsolete</td>
<td>1
</td>
<td>
</td>
</tr></table>
<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#c70">Comment # 70</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:gskachkov@gmail.com" title="GSkachkov <gskachkov@gmail.com>"> <span class="fn">GSkachkov</span></a>
</span></b>
<pre>Comment on <span class=""><a href="attachment.cgi?id=255191&action=diff" name="attach_255191" title="Patch">attachment 255191</a> <a href="attachment.cgi?id=255191&action=edit" title="Patch">[details]</a></span>
Patch
View in context: <a href="https://bugs.webkit.org/attachment.cgi?id=255191&action=review">https://bugs.webkit.org/attachment.cgi?id=255191&action=review</a>
<span class="quote">>> 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.</span >
Done
<span class="quote">>> Source/JavaScriptCore/parser/Parser.cpp:1270
>> + result = parseArrowFunctionExpression(context, parseType);
>
> Saam's pointing is reasonable. Could you rename it to "parseArrowFunctionSingleExpressionBody"?</span >
Done.
<span class="quote">>> Source/JavaScriptCore/parser/Parser.cpp:1447
>> + bool isClassConstructor;
>
> Let's drop this line.</span >
Done
<span class="quote">>> 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?</span >
Done
<span class="quote">>> Source/JavaScriptCore/parser/Parser.cpp:1478
>> + isClassConstructor = constructorKind != ConstructorKind::None;
>
> This always override the previous one. And I think this line is not necessary.</span >
Done
<span class="quote">>> Source/JavaScriptCore/parser/Parser.cpp:1487
>> + isClassConstructor = false;
>
> I think this line is not necessary. Instead, let's insert `ASSERT(constructorKind == ConstructorKind::None);` (correct?)</span >
Done
<span class="quote">>> Source/JavaScriptCore/parser/Parser.cpp:1495
>> + constructorKind = isClassConstructor ? constructorKind : ConstructorKind::None;
>
> I think this line is not necessary.</span >
Done
<span class="quote">>> Source/JavaScriptCore/parser/Parser.cpp:1610
>> + info.isEndByTerminator = true;
>
> OK, they are initialized in struct definition with C++11 initialization form.</span >
Done
<span class="quote">>> 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.</span >
So I've added initializing of this paramters in the struct definition
<span class="quote">>> Source/JavaScriptCore/parser/ParserFunctionInfo.h:41
>> + unsigned startFunctionOffset = 0;
>
> Let's exchange this member's order.
> unsigned startFunctionOffset = 0;
> unsigned endFunctionOffset = 0;</span >
Done
<span class="quote">>> Source/JavaScriptCore/parser/ParserFunctionInfo.h:49
>> + FunctionBodyType functionBodyType = StandardFunctionBodyBlock;
>
> They are initialized with C++ initialization form.</span >
Done
<span class="quote">>> Source/JavaScriptCore/parser/SourceProviderCacheItem.h:39
>> + unsigned endFunctionOffset;
>
> Since now we have endFunctionEndOffset, endFunctionStartOffset sounds better.</span >
Done
<span class="quote">>> 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 };</span >
Done</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>