Closed
Bug 889584
Opened 11 years ago
Closed 11 years ago
Four trivial parser cleanups
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla25
People
(Reporter: jorendorff, Assigned: jorendorff)
Details
Attachments
(4 files)
Part 1 - Put all Parser::statement() parsing code in the switch statement, and make each case return
2.04 KB,
patch
|
Waldo
:
review+
|
Details | Diff | Splinter Review |
640 bytes,
patch
|
Waldo
:
review+
|
Details | Diff | Splinter Review |
4.32 KB,
patch
|
Waldo
:
review+
|
Details | Diff | Splinter Review |
868 bytes,
patch
|
Waldo
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Assignee | ||
Comment 1•11 years ago
|
||
As discussed, leaving the semicolon code after the switch statement is lame; let's get rid of that, even if it means adding a /* FALL THROUGH */. Now each kind of statement has its own dedicated parsing code that parses the whole thing.
Assignee: general → jorendorff
Attachment #770427 -
Flags: review?(jwalden+bmo)
Assignee | ||
Comment 2•11 years ago
|
||
A bad comment is better than none? This is all I know. More detailed info welcome.
Attachment #770428 -
Flags: review?(jwalden+bmo)
Assignee | ||
Comment 3•11 years ago
|
||
The two specializations are basically the same code; no point keeping two copies.
Attachment #770429 -
Flags: review?(jwalden+bmo)
Assignee | ||
Updated•11 years ago
|
Attachment #770429 -
Attachment description: Un-specialize Parser::expr() → Part 3 - Un-specialize Parser::expr()
Assignee | ||
Comment 4•11 years ago
|
||
FullParseHandler::newFunctionDefinition() already initializes that field to NULL. And SyntaxParseHandler::setFunctionBody() is a no-op.
Attachment #770431 -
Flags: review?(jwalden+bmo)
Comment 5•11 years ago
|
||
Comment on attachment 770427 [details] [diff] [review] Part 1 - Put all Parser::statement() parsing code in the switch statement, and make each case return Review of attachment 770427 [details] [diff] [review]: ----------------------------------------------------------------- Glorious.
Attachment #770427 -
Flags: review?(jwalden+bmo) → review+
Comment 6•11 years ago
|
||
Comment on attachment 770428 [details] [diff] [review] Part 2 - comment withStatement() Review of attachment 770428 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/frontend/Parser.cpp @@ +4460,5 @@ > ParseNode * > Parser<FullParseHandler>::withStatement() > { > + // test262/ch12/12.10/12.10-0-1.js fails if we try to parse with-statements > + // in syntax-parse mode. File a bug on this, note it here?
Attachment #770428 -
Flags: review?(jwalden+bmo) → review+
Comment 7•11 years ago
|
||
Comment on attachment 770429 [details] [diff] [review] Part 3 - Un-specialize Parser::expr() Review of attachment 770429 [details] [diff] [review]: ----------------------------------------------------------------- Most excellent. ::: js/src/frontend/Parser.cpp @@ +4858,2 @@ > if (pn && tokenStream.matchToken(TOK_COMMA)) { > + Node seq = handler.newList(PNK_COMMA, pn); Don't you need |if (!seq) return null();|?
Attachment #770429 -
Flags: review?(jwalden+bmo) → review+
Updated•11 years ago
|
Attachment #770431 -
Flags: review?(jwalden+bmo) → review+
Assignee | ||
Comment 8•11 years ago
|
||
(In reply to Jeff Walden [:Waldo] (remove +bmo to email) from comment #6) > > + // test262/ch12/12.10/12.10-0-1.js fails if we try to parse with-statements > > + // in syntax-parse mode. > > File a bug on this, note it here? - // in syntax-parse mode. + // in syntax-parse mode. See bug 892583. > Don't you need |if (!seq) return null();|? Quite. Thanks.
Comment 9•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/cfc87e23ddef https://hg.mozilla.org/mozilla-central/rev/7c0333e51fbc https://hg.mozilla.org/mozilla-central/rev/e6efea0c05e8 https://hg.mozilla.org/mozilla-central/rev/5b6f72fecbec
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
You need to log in
before you can comment on or make changes to this bug.
Description
•