Miscellaneous parser/scanner cleanups

RESOLVED FIXED in mozilla11

Status

()

Core
JavaScript Engine
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: Waldo, Assigned: Waldo)

Tracking

Trunk
mozilla11
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

Created attachment 569421 [details] [diff] [review]
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.
Attachment #569421 - Flags: review?(cdleary)
Created attachment 569422 [details] [diff] [review]
Remove nearly all initializers from the TokenKind enum
Attachment #569422 - Flags: review?(cdleary)
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?
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.
Attachment #569421 - Flags: review?(cdleary) → review+
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?
Attachment #569422 - Flags: review?(cdleary) → review+
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.
Target Milestone: --- → mozilla11
https://hg.mozilla.org/mozilla-central/rev/f0afc4c9fd0c
https://hg.mozilla.org/mozilla-central/rev/9b6b6ee628f8
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.