[webkit-reviews] review granted: [Bug 34019] Custom-written JavaScript parser : [Attachment 59603] Patch

[bugzilla-daemon at webkit.org]

Gavin Barraclough <barraclough at apple.com> has granted Oliver Hunt
<oliver at apple.com>'s request for review:
Bug 34019: Custom-written JavaScript parser
https://bugs.webkit.org/show_bug.cgi?id=34019

Attachment 59603: Patch
https://bugs.webkit.org/attachment.cgi?id=59603&action=review

------- Additional Comments from Gavin Barraclough <barraclough at apple.com>
comparing Identifer == char* in createGetterOrSetterProperty makes me a little
sad.

bool consume(int expected)

This looks a little screwy?

if (!result) {
    m_error = result;

is this what you really mean here? - if so, it's probably clearer written as:


if (!result) {
    m_error = false;

or is this inverted? - if so, I think you want


if (!result)
    fail();

?

 202 };
 203 int jsParse(JSGlobalData* globalData)

pls to be add a newline here.

 229 }
 230 
 231 
 232 bool JSParser::parseProgram()


pls to be remove a newline here. ;-D

 597	 consumeOrFail(CLOSEBRACE);
 598	 return context.createSwitchStatement(expr, firstClauses,
defaultClause, secondClauses, startLine, endLine);
 599 
 600 }

and again.

#define ENABLE_RECURSIVE_PARSE 1

will this go away completely in your next patch? - I strongly hope so.

r+, please please please to be removing the old parser asap.