The default bug view has changed. See this FAQ.

Simplify parsing to the right of a dot in a property access

RESOLVED FIXED in mozilla12



JavaScript Engine
5 years ago
5 years ago


(Reporter: Waldo, Assigned: Waldo)



Firefox Tracking Flags

(Not tracked)



(1 attachment)

Created attachment 581840 [details] [diff] [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)
Oh, here's the patch I was looking for.

This looks great. I'll review today.
Can't finish today. Tuesday.
Comment on attachment 581840 [details] [diff] [review]

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.

Attachment #581840 - Flags: review?(jorendorff) → review+

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.
Target Milestone: --- → mozilla12
Last Resolved: 5 years ago
Resolution: --- → FIXED
Depends on: 721322
You need to log in before you can comment on or make changes to this bug.