Removed some allocation and cruft from the parser
Created attachment 242857 [details] Patch
Comment on attachment 242857 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=242857&action=review r=me with one suggestion. > Source/JavaScriptCore/parser/SyntaxChecker.h:133 > + ExpressionType appendToCommaExpr(const JSTokenLocation&, ExpressionType& tail, ExpressionType next) { tail = next; return next; } I suggest changing this prototype to match the one in ASTBuilder (tail can be just ExpressionType). Also, you no longer need to assign "tail = next;" here since you've changed the parser to do the assign using the return value. Hence, this "tail" assignment is redundant.
> > Source/JavaScriptCore/parser/SyntaxChecker.h:133 > > + ExpressionType appendToCommaExpr(const JSTokenLocation&, ExpressionType& tail, ExpressionType next) { tail = next; return next; } > > I suggest changing this prototype to match the one in ASTBuilder (tail can > be just ExpressionType). Also, you no longer need to assign "tail = next;" > here since you've changed the parser to do the assign using the return > value. Hence, this "tail" assignment is redundant. You're right that I wrote this redundantly with its caller; I'll fix that. But the code will continue to need a distinct signature. This code is a bit evil. The ASTBuilder builds a valid linked list of comma nodes. But the SyntaxChecker actually builds an invalid list that constantly overwrites its head with its tail. The reason it does this is so that the parent expression of the comma list can statically check for errors caused by the value of the last node (which is the node that produces the comma expression's value). By overwriting the head with the tail, the syntax checker presents only the tail node of the comma list to its parent expression for checking.
For example, the program 'use strict'; ++(1, 2, 3, eval) is a syntax error.
(In reply to comment #3) > By overwriting the head with the tail, the syntax checker presents only the > tail node of the comma list to its parent expression for checking. Interesting. So tricky.
Committed r177001: <http://trac.webkit.org/changeset/177001>
EFL port has broken since r177001. Because EFL port treats build warning as error. This patch generated a below warning. ../../Source/JavaScriptCore/parser/Nodes.h:1104:11: error: direct base ‘JSC::ParserArenaFreeable’ inaccessible in ‘JSC::CommaNode’ due to ambiguity [-Werror] class CommaNode final : public ExpressionNode, public ParserArenaFreeable { ^
(In reply to comment #7) > EFL port has broken since r177001. Because EFL port treats build warning as > error. This patch generated a below warning. > > > ../../Source/JavaScriptCore/parser/Nodes.h:1104:11: error: direct base > ‘JSC::ParserArenaFreeable’ inaccessible in ‘JSC::CommaNode’ due to ambiguity > [-Werror] > class CommaNode final : public ExpressionNode, public > ParserArenaFreeable { > ^ I upload a fix to Bug 139428.