<html>
    <head>
      <base href="https://bugs.webkit.org/" />
    </head>
    <body>
      <p>
        <div>
            <b><a class="bz_bug_link 
          bz_status_NEW "
   title="NEW - Implement the comma instruction in WebAssembly"
   href="https://bugs.webkit.org/show_bug.cgi?id=149425#c3">Comment # 3</a>
              on <a class="bz_bug_link 
          bz_status_NEW "
   title="NEW - Implement the comma instruction in WebAssembly"
   href="https://bugs.webkit.org/show_bug.cgi?id=149425">bug 149425</a>
              from <span class="vcard"><a class="email" href="mailto:sukolsak&#64;gmail.com" title="Sukolsak Sakshuwong &lt;sukolsak&#64;gmail.com&gt;"> <span class="fn">Sukolsak Sakshuwong</span></a>
</span></b>
        <pre>Thanks for the review!

(In reply to <a href="show_bug.cgi?id=149425#c2">comment #2</a>)
<span class="quote">&gt; Comment on <span class=""><a href="attachment.cgi?id=261694&amp;action=diff" name="attach_261694" title="Patch">attachment 261694</a> <a href="attachment.cgi?id=261694&amp;action=edit" title="Patch">[details]</a></span>
&gt; Patch
&gt; 
&gt; View in context:
&gt; <a href="https://bugs.webkit.org/attachment.cgi?id=261694&amp;action=review">https://bugs.webkit.org/attachment.cgi?id=261694&amp;action=review</a>
&gt; 
&gt; &gt; Source/JavaScriptCore/wasm/WASMFunctionCompiler.h:1052
&gt; &gt; +    void discard(int)
&gt; 
&gt; Shouldn't this be ContextExpression?</span >

It could be Expression, but I think it would introduce indirectness and break the pattern that we've been using. Here is my answer to Mark's similar question: <a class="bz_bug_link 
          bz_status_RESOLVED  bz_closed"
   title="RESOLVED FIXED - Implement type conversion instructions in WebAssembly"
   href="show_bug.cgi?id=149340#c4">https://bugs.webkit.org/show_bug.cgi?id=149340#c4</a>

<span class="quote">&gt; ContextExpression is a macro for &quot;typename Context::Expression&quot;. It is only used in the parser. Methods in the parser are templatized and take Context as the template parameter. The current possible Contexts are WASMFunctionSyntaxChecker WASMFunctionCompiler. In the future, we will have WASMFunctionLLVMIRGenerator.</span >
&gt;
<span class="quote">&gt; In the context of WASMFunctionSyntaxChecker or WASMFunctionCompiler, Expression is just a typedef of int, because we never use it. In the context of WASMFunctionLLVMIRGenerator, Expression will be a typedef of FTL::LValue (or LLVMValueRef.)</span >
&gt;
<span class="quote">&gt; I could use Expression, of course. But that seems to introduce unnecessary indirectness. I use the same design as the JSC parser. For example,</span >
&gt;
<span class="quote">&gt; - In parser/SyntaxChecker.h, we have &quot;int createBreakStatement(const JSTokenLocation&amp;, int, int) { return StatementResult; }&quot;</span >
&gt;
<span class="quote">&gt; - In parser/ASTBuilder.h, we have &quot;StatementNode* createBreakStatement(const JSTokenLocation&amp; location, const Identifier* ident, const JSTextPosition&amp; start, const JSTextPosition&amp; end)&quot;</span ></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>