Last Comment Bug 697795 - Split up remaining TOK_* kinds whose meanings are conditioned on token.t_op
: Split up remaining TOK_* kinds whose meanings are conditioned on token.t_op
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla11
Assigned To: Jeff Walden [:Waldo] (remove +bmo to email)
:
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-10-27 12:52 PDT by Jeff Walden [:Waldo] (remove +bmo to email)
Modified: 2011-11-09 05:24 PST (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Split up TOK_RELOP (5.87 KB, patch)
2011-10-27 12:53 PDT, Jeff Walden [:Waldo] (remove +bmo to email)
cdleary: review+
Details | Diff | Splinter Review
Split up TOK_SHOP (6.37 KB, patch)
2011-10-27 12:56 PDT, Jeff Walden [:Waldo] (remove +bmo to email)
cdleary: review+
Details | Diff | Splinter Review
Split up TOK_DIVOP (4.45 KB, patch)
2011-10-27 12:57 PDT, Jeff Walden [:Waldo] (remove +bmo to email)
cdleary: review+
Details | Diff | Splinter Review
Split up TOK_PRIMARY (12.65 KB, patch)
2011-10-27 12:59 PDT, Jeff Walden [:Waldo] (remove +bmo to email)
cdleary: review+
Details | Diff | Splinter Review
Split up TOK_ASSIGN (17.28 KB, patch)
2011-10-27 13:04 PDT, Jeff Walden [:Waldo] (remove +bmo to email)
cdleary: review+
Details | Diff | Splinter Review

Description Jeff Walden [:Waldo] (remove +bmo to email) 2011-10-27 12:52:47 PDT
Following up on bug 697297, for the rest of the split-personality token types.
Comment 1 Jeff Walden [:Waldo] (remove +bmo to email) 2011-10-27 12:53:55 PDT
Created attachment 570057 [details] [diff] [review]
Split up TOK_RELOP
Comment 2 Jeff Walden [:Waldo] (remove +bmo to email) 2011-10-27 12:56:51 PDT
Created attachment 570058 [details] [diff] [review]
Split up TOK_SHOP

This also makes a < b < c < d < e not be a single-level parse node tree, as happened to the equality ops.  In retrospect I don't believe that was strictly necessary to do, but it's conceptually simpler, so I'm leaving it this way unless you object.

The <<= >>= >>>= ops grouped under TOK_ASSIGN, which are implicated in the code changed here, will be split up in a later patch.
Comment 3 Jeff Walden [:Waldo] (remove +bmo to email) 2011-10-27 12:57:57 PDT
Created attachment 570059 [details] [diff] [review]
Split up TOK_DIVOP
Comment 4 Jeff Walden [:Waldo] (remove +bmo to email) 2011-10-27 12:59:29 PDT
Created attachment 570061 [details] [diff] [review]
Split up TOK_PRIMARY

I wonder if the /* super */ comment was obsoleted by my changes to the reserved-word set, or if it bitrotted at some other time...
Comment 5 Jeff Walden [:Waldo] (remove +bmo to email) 2011-10-27 13:04:20 PDT
Created attachment 570062 [details] [diff] [review]
Split up TOK_ASSIGN

This is perhaps the nicest of the batch, because there are a bunch of places in variable declarations and such where we really only ever want '=', and never wanted '|=', '+=', and so on.  These places checked that the parse node sub-op was JSOP_NOP.  Now, TOK_ASSIGN only means '=', so those places don't need to check for the sub-op, and indeed they can even assert that the sub-op is JSOP_NOP.

The names for the +=, -=, etc. tokens are slightly awkward.  I stole my sense of self-approval for the names from <http://msdn.microsoft.com/en-us/library/bb155837.aspx>, so at least someone else thinks addassign and such aren't unreasonable names.  Feel free to suggest something better if you want.  (I did reject "pluseq" and such, despite my sounding them out that way in my head, because I don't think we want to associate "equality" with these operations, since they are in no way equality-related except in spelling.)
Comment 6 Chris Leary [:cdleary] (not checking bugmail) 2011-11-04 15:19:59 PDT
Comment on attachment 570061 [details] [diff] [review]
Split up TOK_PRIMARY

Review of attachment 570061 [details] [diff] [review]:
-----------------------------------------------------------------

I never liked TOK_PRIMARY.

::: js/src/frontend/FoldConstants.cpp
@@ +862,4 @@
>                  pn->become(pn1);
> +                if (pn->isKind(TOK_TRUE)) {
> +                    pn->setKind(TOK_FALSE);
> +                    pn->setOp(JSOP_FALSE);

Do we want a setKindAndOp if we're moving towards this pattern more?

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