[Webkit-unassigned] [Bug 149425] Implement the comma instruction in WebAssembly

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=149425

--- Comment #3 from Sukolsak Sakshuwong <sukolsak at gmail.com> ---
Thanks for the review!

(In reply to comment #2)
> Comment on attachment 261694 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=261694&action=review
> 
> > Source/JavaScriptCore/wasm/WASMFunctionCompiler.h:1052
> > +    void discard(int)
> 
> Shouldn't this be ContextExpression?

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: https://bugs.webkit.org/show_bug.cgi?id=149340#c4

> ContextExpression is a macro for "typename Context::Expression". 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.
>
> 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.)
>
> I could use Expression, of course. But that seems to introduce unnecessary indirectness. I use the same design as the JSC parser. For example,
>
> - In parser/SyntaxChecker.h, we have "int createBreakStatement(const JSTokenLocation&, int, int) { return StatementResult; }"
>
> - In parser/ASTBuilder.h, we have "StatementNode* createBreakStatement(const JSTokenLocation& location, const Identifier* ident, const JSTextPosition& start, const JSTextPosition& end)"

-- 
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/20150921/e80539c7/attachment.html>