Last Comment Bug 701620 - Various parse node kind disambiguations
: Various parse node kind disambiguations
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: All All
: -- minor (vote)
: mozilla11
Assigned To: Jeff Walden [:Waldo] (remove +bmo to email)
:
Mentors:
http://www.youtube.com/watch?v=eWM2jo...
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-11-10 20:33 PST by Jeff Walden [:Waldo] (remove +bmo to email)
Modified: 2011-12-14 14:42 PST (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Extract PNK_DEFXMLNS from PNK_DEFAULT (2.41 KB, patch)
2011-11-10 20:34 PST, Jeff Walden [:Waldo] (remove +bmo to email)
jorendorff: review+
Details | Diff | Splinter Review
Extract PNK_ADD and PNK_SUB from PNK_PLUS and PNK_MINUS (7.05 KB, patch)
2011-11-10 20:35 PST, Jeff Walden [:Waldo] (remove +bmo to email)
jorendorff: review+
Details | Diff | Splinter Review
Extract PNK_FORIN from PNK_IN (5.00 KB, patch)
2011-11-10 20:38 PST, Jeff Walden [:Waldo] (remove +bmo to email)
jorendorff: review+
Details | Diff | Splinter Review
Rename PNK_DO to PNK_DOWHILE (3.14 KB, patch)
2011-11-10 20:39 PST, Jeff Walden [:Waldo] (remove +bmo to email)
jorendorff: review+
Details | Diff | Splinter Review
Split PNK_LC into PNK_STATEMENTLIST and PNK_XMLCURLYEXPR (17.99 KB, patch)
2011-11-11 16:06 PST, Jeff Walden [:Waldo] (remove +bmo to email)
jorendorff: review+
Details | Diff | Splinter Review

Description Jeff Walden [:Waldo] (remove +bmo to email) 2011-11-10 20:33:33 PST
Some parse node kinds have split personalities depending on context.  It would be conceptually simpler to just have distinct kinds in these cases.
Comment 1 Jeff Walden [:Waldo] (remove +bmo to email) 2011-11-10 20:34:51 PST
Created attachment 573731 [details] [diff] [review]
Extract PNK_DEFXMLNS from PNK_DEFAULT
Comment 2 Jeff Walden [:Waldo] (remove +bmo to email) 2011-11-10 20:35:37 PST
Created attachment 573732 [details] [diff] [review]
Extract PNK_ADD and PNK_SUB from PNK_PLUS and PNK_MINUS
Comment 3 Jeff Walden [:Waldo] (remove +bmo to email) 2011-11-10 20:38:05 PST
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.
Comment 4 Jeff Walden [:Waldo] (remove +bmo to email) 2011-11-10 20:39:43 PST
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.
Comment 5 Jeff Walden [:Waldo] (remove +bmo to email) 2011-11-11 16:06:11 PST
Created attachment 573951 [details] [diff] [review]
Split PNK_LC into PNK_STATEMENTLIST and PNK_XMLCURLYEXPR
Comment 6 Jason Orendorff [:jorendorff] 2011-11-28 11:09:13 PST
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.
Comment 7 Jeff Walden [:Waldo] (remove +bmo to email) 2011-11-28 12:34:45 PST
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.  :-)
Comment 9 Justin Dolske [:Dolske] 2011-12-04 18:45:05 PST
Let's test your knowledge, and see what you've learned so far... What color are the parse node kinds? PNNNNNNK!

Ahem. Sorry.
Comment 10 Jeff Walden [:Waldo] (remove +bmo to email) 2011-12-14 14:42:54 PST
/me smiles

Note You need to log in before you can comment on or make changes to this bug.