697179 - Miscellaneous parser/scanner cleanups

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jeff Walden [:Waldo]]

Attachment: Extract ParseNode::append from ParseNode::newBinaryOrAppend, for slightly more readable code

These two patches are a total grab-bag of unrelated stuff, but I need a bug to dump them under as they're not quite worthy of drivebying.

Comment 1

[Jeff Walden [:Waldo]]

Attachment: Remove nearly all initializers from the TokenKind enum

Comment 2

[Nicholas Nethercote [inactive]]

Comment on attachment 569422 [details] [diff] [review]
Remove nearly all initializers from the TokenKind enum

Review of attachment 569422 [details] [diff] [review]:
-----------------------------------------------------------------

At various times in the past I've found the initializers in this table very helpful for working out which TokenKind a particular integer (e.g. printed out with debugging printfs for profiling purposes) represents.  Do we really need to remove them?

Comment 3

[Jeff Walden [:Waldo]]

Initializing token kinds as happens now can make for large diffs when token kinds are changed, as I'm currently doing in an effort to simplify tokenizing and parsing code.  Having initializers for all of them makes it easy to make a mistake and duplicate something.  Such mistakes are pernicious and tricky to debug.

If what you want is a way to correlate token kinds with names, would a helper function returning a const char* naming it do the trick?  That could be easily printf'd and similar.

Comment 4

[Chris Leary [:cdleary] (not checking bugmail)]

Comment on attachment 569422 [details] [diff] [review]
Remove nearly all initializers from the TokenKind enum

Review of attachment 569422 [details] [diff] [review]:
-----------------------------------------------------------------

I'm okay with it on maintainability grounds. We can add a TokenKindToString function under #ifdef DEBUG for debugging purposes if we want, or put one value per line?

Comment 5

[Jeff Walden [:Waldo]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f0afc4c9fd0c
https://hg.mozilla.org/integration/mozilla-inbound/rev/9b6b6ee628f8 (with a debug TokenKindToString function, and also with one value per line for good measure)

The latter change cries out for a .tbl file, to be sure (although there are a couple things repeating like this does that not-repeating can't).  I filed bug 700719 for that.

Comment 6

[Marco Bonardo [:mak]]

https://hg.mozilla.org/mozilla-central/rev/f0afc4c9fd0c
https://hg.mozilla.org/mozilla-central/rev/9b6b6ee628f8