<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#c91">Comment # 91</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:utatane.tea&#64;gmail.com" title="Yusuke Suzuki &lt;utatane.tea&#64;gmail.com&gt;"> <span class="fn">Yusuke Suzuki</span></a>
</span></b>
        <pre>Comment on <span class=""><a href="attachment.cgi?id=255593&amp;action=diff" name="attach_255593" title="Patch">attachment 255593</a> <a href="attachment.cgi?id=255593&amp;action=edit" title="Patch">[details]</a></span>
Patch

View in context: <a href="https://bugs.webkit.org/attachment.cgi?id=255593&amp;action=review">https://bugs.webkit.org/attachment.cgi?id=255593&amp;action=review</a>

I think only a few nits remains. Code becomes very simple, nice.
Thank you for your contributions. Sorry for troubling you by a long time review.

<span class="quote">&gt; Source/JavaScriptCore/parser/Parser.cpp:364
&gt; +        </span >

I think propagateError() is needed before `return sourceElements`.

<span class="quote">&gt; Source/JavaScriptCore/parser/Parser.cpp:1529
&gt; +        endLocation.lineStartOffset = cachedInfo-&gt;lastTockenLineStartOffset;</span >

Looks nice.

<span class="quote">&gt; Source/JavaScriptCore/parser/Parser.cpp:1570
&gt; +        }</span >

OK, looks nice.

<span class="quote">&gt; Source/JavaScriptCore/parser/Parser.cpp:1588
&gt; +        info.startFunctionOffset = m_token.m_data.offset;</span >

Still I don't understand why this line is needed.
`info.startFunctionOffset` is already adjusted in the
`info.startFunctionOffset = (info.functionBodyType == ArrowFunctionBodyBlock) ? m_token.m_data.offset : info.arrowFunctionOffset;`
part, right?

When setting info.startFunctionOffset previously, we already considered functionBodyType.
When block body functions come, we set `{`'s offset. And when expr body functions come, we set `=&gt;`'s offset.
So this resetting is already done.

Anyway, if info.startFunctionOffset is modified after looking up the cacheInfo, cacheInfo's key becomes different.
So if this adjustment is needed, this need to be done before `if (const SourceProviderCacheItem* cachedInfo = TreeBuilder::CanUseFunctionCache ? findCachedFunctionInfo(info.startFunctionOffset) : 0) {` part. Reaching here, we already looked up the cache with the previous info.startFunctionOffset.

<span class="quote">&gt; Source/JavaScriptCore/parser/Parser.cpp:1628
&gt; +#endif</span >

Very nice :D</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>