Closed
Bug 710941
Opened 13 years ago
Closed 13 years ago
Simplify parsing to the right of a dot in a property access
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla12
People
(Reporter: Waldo, Assigned: Waldo)
References
Details
Attachments
(1 file)
20.81 KB,
patch
|
jorendorff
:
review+
|
Details | Diff | Splinter Review |
Right now we reuse primary-expression parsing to parse in the |obj.<here>| and |obj..<here>| cases. This results in us having to propagate an |afterDot| argument through |primaryExpr| to cope, complicating control flow and making it difficult to read, and harder to understand. This also makes it harder to propagate property-is-a-PropertyName information from the tokenizer into parse nodes, and from there to the bytecode emitter. We should parse |obj.<here>| not using the same code that parses |<here>| alone. This adds lines of code overall, for the moment, although I argue it is much easier to understand than the previous, shorter code. It also has the benefit of not going to substantial effort to construct a parse node for <here> if it's just a plain old name and isn't followed by ::. (This is an adoption of simplicity from the existing non-E4X parsing path.) I may be able to get some back by subsequently semi-unifying |obj.<here>| parsing with |obj..<here>| parsing. But this is enough change here that an incremental patch makes sense, and in any case I think the simplicity pays for the line delta anyway. This is atop all the patches in bug 710932.
Attachment #581840 -
Flags: review?(jorendorff)
Comment 1•13 years ago
|
||
Oh, here's the patch I was looking for. This looks great. I'll review today.
Comment 2•13 years ago
|
||
Can't finish today. Tuesday.
Comment 3•13 years ago
|
||
Comment on attachment 581840 [details] [diff] [review] Patch You stopped short of rewriting the TOK_DBLDOT case in memberExpr, and then eliminating the afterDot parameter of primaryExpr entirely? I don't mean to look a gift refactoring in the mouth, mind. r=me
Attachment #581840 -
Flags: review?(jorendorff) → review+
Assignee | ||
Comment 4•13 years ago
|
||
Heh. At a certain level I remain aware that I'm doing this mostly to hasten the property storage split, so I'm wary of doing more work than necessary that exceeds the needs of that particular problem.
Assignee | ||
Comment 5•13 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/377a1042074e
Target Milestone: --- → mozilla12
Comment 6•13 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/377a1042074e
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•