Various parse node kind disambiguations

RESOLVED FIXED in mozilla11

Status

()

Core
JavaScript Engine
--
minor
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: Waldo, Assigned: Waldo)

Tracking

Trunk
mozilla11
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(5 attachments)

Some parse node kinds have split personalities depending on context.  It would be conceptually simpler to just have distinct kinds in these cases.
Created attachment 573731 [details] [diff] [review]
Extract PNK_DEFXMLNS from PNK_DEFAULT
Attachment #573731 - Flags: review?(cdleary)
Created attachment 573732 [details] [diff] [review]
Extract PNK_ADD and PNK_SUB from PNK_PLUS and PNK_MINUS
Attachment #573732 - Flags: review?(cdleary)
Created attachment 573733 [details] [diff] [review]
Extract PNK_FORIN from PNK_IN

There's at least an argument that in [x for (y in z)], the for node shouldn't be PNK_FORIN but should be different.  I haven't quite seen value in doing it, so for now there is this reuse.
Attachment #573733 - Flags: review?(cdleary)
Created attachment 573734 [details] [diff] [review]
Rename PNK_DO to PNK_DOWHILE

I think of them as do-while loops, not do-loops, so I find PNK_DOWHILE more readable than PNK_DO.  No code change, just a name change.
Attachment #573734 - Flags: review?(cdleary)
Created attachment 573951 [details] [diff] [review]
Split PNK_LC into PNK_STATEMENTLIST and PNK_XMLCURLYEXPR
Attachment #573951 - Flags: review?(cdleary)
Attachment #573731 - Flags: review?(cdleary) → review?(jorendorff)
Attachment #573732 - Flags: review?(cdleary) → review?(jorendorff)
Attachment #573733 - Flags: review?(cdleary) → review?(jorendorff)
Attachment #573734 - Flags: review?(cdleary) → review?(jorendorff)
Attachment #573951 - Flags: review?(cdleary) → review?(jorendorff)
Attachment #573731 - Flags: review?(jorendorff) → review+
Attachment #573732 - Flags: review?(jorendorff) → review+
Attachment #573733 - Flags: review?(jorendorff) → review+
Attachment #573734 - Flags: review?(jorendorff) → review+
Comment on attachment 573951 [details] [diff] [review]
Split PNK_LC into PNK_STATEMENTLIST and PNK_XMLCURLYEXPR

>+      case PNK_XMLCURLYEXPR:
>+#if JS_HAS_XML_SUPPORT
>+        JS_ASSERT(pn->isArity(PN_UNARY));
>+        if (!EmitTree(cx, bce, pn->pn_kid))
>+            return JS_FALSE;
>+        if (Emit1(cx, bce, pn->getOp()) < 0)
>+            return JS_FALSE;
>+#else
>+        JS_NOT_REACHED("XML curly expression without XML support");
>+#endif
>+        break;

Can you just #if JS_HAS_XML_SUPPORT around the whole case, including the break?

All this is great stuff and long overdue.

If you feel like renaming PNK_{RC,LB,RB,LP,RP} please go for it. They are totally cryptic.
Attachment #573951 - Flags: review?(jorendorff) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/9e33bed54f9a
https://hg.mozilla.org/integration/mozilla-inbound/rev/6ae5203fe3e1
https://hg.mozilla.org/integration/mozilla-inbound/rev/1030be78cb51
https://hg.mozilla.org/integration/mozilla-inbound/rev/c7099e04ec39
https://hg.mozilla.org/integration/mozilla-inbound/rev/db3280d6199d

I do indeed feel like it.  But it's not as simple as that, yet, because of kind overloading at least for PNK_RB/PNK_RC.  I don't know about the others.  But I do indeed plan to get to it, over time.  :-)
Target Milestone: --- → mozilla11
https://hg.mozilla.org/mozilla-central/rev/9e33bed54f9a
https://hg.mozilla.org/mozilla-central/rev/6ae5203fe3e1
https://hg.mozilla.org/mozilla-central/rev/1030be78cb51
https://hg.mozilla.org/mozilla-central/rev/c7099e04ec39
https://hg.mozilla.org/mozilla-central/rev/db3280d6199d
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Let's test your knowledge, and see what you've learned so far... What color are the parse node kinds? PNNNNNNK!

Ahem. Sorry.
/me smiles
You need to log in before you can comment on or make changes to this bug.