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

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jeff Walden [:Waldo]]

Attachment: Patch

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.

Comment 1

[Jason Orendorff [:jorendorff]]

Oh, here's the patch I was looking for.

This looks great. I'll review today.

Comment 2

[Jason Orendorff [:jorendorff]]

Can't finish today. Tuesday.

Comment 3

[Jason Orendorff [:jorendorff]]

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

Comment 4

[Jeff Walden [:Waldo]]

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.

Comment 5

[Jeff Walden [:Waldo]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/377a1042074e

Comment 6

[Marco Bonardo [:mak]]

https://hg.mozilla.org/mozilla-central/rev/377a1042074e