<html>
    <head>
      <base href="https://bugs.webkit.org/" />
    </head>
    <body>
      <p>
        <div>
            <b><a class="bz_bug_link 
          bz_status_NEW "
   title="NEW - [JSC] Implement parsing of Async Functions"
   href="https://bugs.webkit.org/show_bug.cgi?id=161409#c19">Comment # 19</a>
              on <a class="bz_bug_link 
          bz_status_NEW "
   title="NEW - [JSC] Implement parsing of Async Functions"
   href="https://bugs.webkit.org/show_bug.cgi?id=161409">bug 161409</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=288384&amp;action=diff" name="attach_288384" title="Patch">attachment 288384</a> <a href="attachment.cgi?id=288384&amp;action=edit" title="Patch">[details]</a></span>
Patch

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

Could you hide async / await parsing feature behind compile time / run time flag?

<span class="quote">&gt; Source/JavaScriptCore/ChangeLog:7
&gt; +</span >

Could you note ChangeLog?

<span class="quote">&gt; Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:2980
&gt; +    ASSERT(func-&gt;metadata()-&gt;parseMode() == SourceParseMode::ArrowFunctionMode || func-&gt;metadata()-&gt;parseMode() == SourceParseMode::AsyncArrowFunctionMode);</span >

Let's use SourceParseModeSet here.

<span class="quote">&gt; Source/JavaScriptCore/parser/Parser.cpp:264
&gt; +        if (parseMode == SourceParseMode::GeneratorBodyMode || isAsyncFunctionBodyParseMode(parseMode))</span >

Let's use SourceParseModeSet even if there is only one candidate. See the reason why <a class="bz_bug_link 
          bz_status_REOPENED "
   title="REOPENED - [JSC] implement async functions proposal"
   href="show_bug.cgi?id=156147#c132">https://bugs.webkit.org/show_bug.cgi?id=156147#c132</a>.

<span class="quote">&gt; Source/JavaScriptCore/parser/Parser.cpp:269
&gt; +        if (oneOfSourceParseModes&lt;SourceParseMode::ArrowFunctionMode, SourceParseMode::AsyncArrowFunctionMode&gt;(parseMode) &amp;&amp; !hasError()) {</span >

Could you change this oneOfSourceParseModes to one done in <a href="https://trac.webkit.org/changeset/201523">https://trac.webkit.org/changeset/201523</a> (SourceParseModeSet) ?

<span class="quote">&gt; Source/JavaScriptCore/parser/Parser.cpp:315
&gt; +    if (oneOfSourceParseModes&lt;SourceParseMode::GeneratorWrapperFunctionMode&gt;(parseMode) || isAsyncFunctionWrapperParseMode(parseMode)) {</span >

Ditto

<span class="quote">&gt; Source/JavaScriptCore/parser/Parser.cpp:559
&gt; +    info.parameterCount = 4; // generator, state, value, resume mode</span >

We no longer need this code since createGeneratorParameters automatically set info.parameterCount.
And keep in mind that the updated generator takes 5th argument, &#64;generatorFrame.

<span class="quote">&gt; Source/JavaScriptCore/parser/Parser.cpp:629
&gt; +        // FIXME: These branch causes 1% regression in octane code-load jquery.</span >

Since we already solved this, let's drop it.

<span class="quote">&gt; Source/JavaScriptCore/parser/Parser.cpp:1748
&gt; +        // FIXME: These branch causes 1% regression in octane code-load jquery.</span >

Ditto.

<span class="quote">&gt; Source/JavaScriptCore/parser/Parser.cpp:1946
&gt; +    RELEASE_ASSERT(!(oneOfSourceParseModes&lt;SourceParseMode::ProgramMode, SourceParseMode::ModuleAnalyzeMode, SourceParseMode::ModuleEvaluateMode&gt;(mode)));</span >

Ditto

<span class="quote">&gt; Source/JavaScriptCore/parser/Parser.cpp:1950
&gt; +    if (UNLIKELY((oneOfSourceParseModes&lt;SourceParseMode::ArrowFunctionMode, SourceParseMode::AsyncArrowFunctionMode&gt;(mode)))) {</span >

Ditto.

<span class="quote">&gt; Source/JavaScriptCore/parser/Parser.cpp:2086
&gt; +            if (UNLIKELY((oneOfSourceParseModes&lt;SourceParseMode::ArrowFunctionMode, SourceParseMode::AsyncArrowFunctionMode&gt;(mode))))</span >

Ditto.

<span class="quote">&gt; Source/JavaScriptCore/parser/Parser.cpp:2129
&gt; +    if (UNLIKELY((oneOfSourceParseModes&lt;SourceParseMode::ArrowFunctionMode, SourceParseMode::AsyncArrowFunctionMode&gt;(mode)))) {</span >

Ditto.

<span class="quote">&gt; Source/JavaScriptCore/parser/Parser.cpp:2179
&gt; +        if (!(functionDefinitionType == FunctionDefinitionType::Expression &amp;&amp; oneOfSourceParseModes&lt;SourceParseMode::NormalFunctionMode, SourceParseMode::AsyncFunctionMode&gt;(mode)))</span >

Ditto.

<span class="quote">&gt; Source/JavaScriptCore/parser/Parser.cpp:2294
&gt; +        RELEASE_ASSERT((oneOfSourceParseModes&lt;SourceParseMode::NormalFunctionMode, SourceParseMode::MethodMode, SourceParseMode::ArrowFunctionMode, SourceParseMode::GeneratorBodyMode, SourceParseMode::GeneratorWrapperFunctionMode&gt;(mode)) || isAsyncFunctionWrapperParseMode(mode));</span >

Ditto.

<span class="quote">&gt; Source/JavaScriptCore/parser/Parser.cpp:2722
&gt; +        // FIXME: These branch causes 1% regression in octane code-load jquery.</span >

Since it is already solved, we can drop this FIXME.

<span class="quote">&gt; Source/JavaScriptCore/parser/Parser.cpp:3353
&gt; +    // FIXME: These branch causes 1% regression in octane code-load jquery.</span >

Ditto.

<span class="quote">&gt; Source/JavaScriptCore/parser/Parser.cpp:3512
&gt; +    return context.createYield(location, argument, delegate, divotStart, argumentStart, lastTokenEndPosition());</span >

How about using AwaitNode here and emit the similar code in code generation phase?

<span class="quote">&gt; Source/JavaScriptCore/parser/Parser.cpp:4117
&gt; +        // FIXME: These branch causes 1% regression in octane code-load jquery.</span >

Ditto.

<span class="quote">&gt; Source/JavaScriptCore/parser/ParserModes.h:76
&gt; +static ALWAYS_INLINE bool oneOfSourceParseModes(SourceParseMode mode)</span >

Change to SourceParseModeSet version.

<span class="quote">&gt; Source/JavaScriptCore/parser/ParserModes.h:83
&gt; +    return oneOfSourceParseModes&lt;</span >

Ditto.

<span class="quote">&gt; Source/JavaScriptCore/parser/ParserModes.h:100
&gt; +    return oneOfSourceParseModes&lt;</span >

Ditto.

<span class="quote">&gt; Source/JavaScriptCore/parser/ParserModes.h:110
&gt; +    return oneOfSourceParseModes&lt;</span >

Ditto.

<span class="quote">&gt; Source/JavaScriptCore/parser/ParserModes.h:117
&gt; +    return oneOfSourceParseModes&lt;</span >

Ditto.

<span class="quote">&gt; Source/JavaScriptCore/parser/ParserModes.h:125
&gt; +    return oneOfSourceParseModes&lt;</span >

Ditto.

<span class="quote">&gt; Source/JavaScriptCore/parser/ParserModes.h:132
&gt; +    return oneOfSourceParseModes&lt;</span >

Ditto.

<span class="quote">&gt; Source/JavaScriptCore/parser/ParserModes.h:142
&gt; +    return oneOfSourceParseModes&lt;</span >

Ditto.

<span class="quote">&gt; Source/JavaScriptCore/parser/ParserModes.h:149
&gt; +    return oneOfSourceParseModes&lt;SourceParseMode::ProgramMode&gt;(parseMode);</span >

Ditto.</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>