Bug 883333, part 6 - Further forStatement cleanup. r=Waldo.
authorJason Orendorff <jorendorff@mozilla.com>
Fri, 21 Jun 2013 08:17:59 -0500
changeset 147461 92e82546fccac0ccbdd90d6f4bbaffea40d2468e
parent 147460 f8f6e7caa54cb3f6a0c1f807c7b3317c2edf0c42
child 147462 2c7f6627fbd01d45a21dc8f0ea2838cd6017b9b6
push id2697
push userbbajaj@mozilla.com
push dateMon, 05 Aug 2013 18:49:53 +0000
treeherdermozilla-beta@dfec938c7b63 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersWaldo
bugs883333
milestone24.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 883333, part 6 - Further forStatement cleanup. r=Waldo.
js/src/frontend/FullParseHandler.h
js/src/frontend/Parser.cpp
--- a/js/src/frontend/FullParseHandler.h
+++ b/js/src/frontend/FullParseHandler.h
@@ -215,19 +215,27 @@ class FullParseHandler
         return new_<BinaryNode>(PNK_DOWHILE, JSOP_NOP, pos, body, cond);
     }
 
     ParseNode *newWhileStatement(uint32_t begin, ParseNode *cond, ParseNode *body) {
         TokenPos pos = TokenPos::make(begin, body->pn_pos.end);
         return new_<BinaryNode>(PNK_WHILE, JSOP_NOP, pos, cond, body);
     }
 
-    ParseNode *newForStatement(uint32_t begin) {
-        return new_<BinaryNode>(PNK_FOR, JSOP_NOP, TokenPos::make(begin, begin + 1),
-                                (ParseNode *) NULL, (ParseNode *) NULL);
+    ParseNode *newForStatement(uint32_t begin, ParseNode *forHead, ParseNode *body,
+                               unsigned iflags)
+    {
+        /* A FOR node is binary, left is loop control and right is the body. */
+        JSOp op = forHead->isKind(PNK_FORIN) ? JSOP_ITER : JSOP_NOP;
+        BinaryNode *pn = new_<BinaryNode>(PNK_FOR, op, TokenPos::make(begin, body->pn_pos.end),
+                                          forHead, body);
+        if (!pn)
+            return null();
+        pn->pn_iflags = iflags;
+        return pn;
     }
 
     ParseNode *newForHead(bool isForInOrOf, ParseNode *pn1, ParseNode *pn2, ParseNode *pn3,
                           const TokenPos &pos)
     {
         ParseNodeKind kind = isForInOrOf ? PNK_FORIN : PNK_FORHEAD;
         return new_<TernaryNode>(kind, JSOP_NOP, pn1, pn2, pn3, pos);
     }
--- a/js/src/frontend/Parser.cpp
+++ b/js/src/frontend/Parser.cpp
@@ -3718,27 +3718,27 @@ Parser<ParseHandler>::matchInOrOf(bool *
         tokenStream.ungetToken();
     }
     return false;
 }
 
 template <>
 bool
 Parser<FullParseHandler>::isValidForStatementLHS(ParseNode *pn1, JSVersion version,
-                                                 bool forDecl, bool forEach, bool forOf)
-{
-    if (forDecl) {
+                                                 bool isForDecl, bool isForEach, bool isForOf)
+{
+    if (isForDecl) {
         if (pn1->pn_count > 1)
             return false;
         if (pn1->isOp(JSOP_DEFCONST))
             return false;
 #if JS_HAS_DESTRUCTURING
         // In JS 1.7 only, for (var [K, V] in EXPR) has a special meaning.
         // Hence all other destructuring decls are banned there.
-        if (version == JSVERSION_1_7 && !forEach && !forOf) {
+        if (version == JSVERSION_1_7 && !isForEach && !isForOf) {
             ParseNode *lhs = pn1->pn_head;
             if (lhs->isKind(PNK_ASSIGN))
                 lhs = lhs->pn_left;
 
             if (lhs->isKind(PNK_OBJECT))
                 return false;
             if (lhs->isKind(PNK_ARRAY) && lhs->pn_count != 2)
                 return false;
@@ -3754,17 +3754,17 @@ Parser<FullParseHandler>::isValidForStat
       case PNK_ELEM:
         return true;
 
 #if JS_HAS_DESTRUCTURING
       case PNK_ARRAY:
       case PNK_OBJECT:
         // In JS 1.7 only, for ([K, V] in EXPR) has a special meaning.
         // Hence all other destructuring left-hand sides are banned there.
-        if (version == JSVERSION_1_7 && !forEach && !forOf)
+        if (version == JSVERSION_1_7 && !isForEach && !isForOf)
             return pn1->isKind(PNK_ARRAY) && pn1->pn_count == 2;
         return true;
 #endif
 
       default:
         return false;
     }
 }
@@ -3786,26 +3786,25 @@ Parser<FullParseHandler>::forStatement()
         if (tokenStream.currentToken().name() == context->names().each) {
             iflags = JSITER_FOREACH;
             isForEach = true;
         } else {
             tokenStream.ungetToken();
         }
     }
 
-    TokenPos lp_pos = pos();
     MUST_MATCH_TOKEN(TOK_LP, JSMSG_PAREN_AFTER_FOR);
 
     /*
      * True if we have 'for (var/let/const ...)', except in the oddball case
      * where 'let' begins a let-expression in 'for (let (...) ...)'.
      */
-    bool forDecl = false;
-
-    /* Non-null when forDecl is true for a 'for (let ...)' statement. */
+    bool isForDecl = false;
+
+    /* Non-null when isForDecl is true for a 'for (let ...)' statement. */
     RootedStaticBlockObject blockObj(context);
 
     /* Set to 'x' in 'for (x ;... ;...)' or 'for (x in ...)'. */
     ParseNode *pn1;
 
     {
         TokenKind tt = tokenStream.peekToken(TSF_OPERAND);
         if (tt == TOK_SEMI) {
@@ -3821,95 +3820,101 @@ Parser<FullParseHandler>::forStatement()
              * part of a for/in loop.
              *
              * A side effect of this restriction is that (unparenthesized)
              * expressions involving an 'in' operator are illegal in the init
              * clause of an ordinary for loop.
              */
             pc->parsingForInit = true;
             if (tt == TOK_VAR || tt == TOK_CONST) {
-                forDecl = true;
+                isForDecl = true;
                 tokenStream.consumeKnownToken(tt);
                 pn1 = variables(tt == TOK_VAR ? PNK_VAR : PNK_CONST);
             }
 #if JS_HAS_BLOCK_SCOPE
             else if (tt == TOK_LET) {
                 handler.disableSyntaxParser();
                 (void) tokenStream.getToken();
                 if (tokenStream.peekToken() == TOK_LP) {
                     pn1 = letBlock(LetExpresion);
                 } else {
-                    forDecl = true;
+                    isForDecl = true;
                     blockObj = StaticBlockObject::create(context);
                     if (!blockObj)
                         return null();
                     pn1 = variables(PNK_LET, NULL, blockObj, DontHoistVars);
                 }
             }
 #endif
             else {
                 pn1 = expr();
             }
             pc->parsingForInit = false;
             if (!pn1)
                 return null();
         }
     }
 
-    JS_ASSERT_IF(forDecl, pn1->isArity(PN_LIST));
-    JS_ASSERT(!!blockObj == (forDecl && pn1->isOp(JSOP_NOP)));
-
-    ParseNode *pn = handler.newForStatement(begin);
-    if (!pn)
-        return null();
-
-    /* If non-null, the parent that should be returned instead of pn. */
-    ParseNode *forParent = NULL;
+    JS_ASSERT_IF(isForDecl, pn1->isArity(PN_LIST));
+    JS_ASSERT(!!blockObj == (isForDecl && pn1->isOp(JSOP_NOP)));
+
+    // The form 'for (let <vars>; <expr2>; <expr3>) <stmt>' generates an
+    // implicit block even if stmt is not a BlockStatement.
+    // If the loop has that exact form, then:
+    // - forLetImpliedBlock is the node for the implicit block scope.
+    // - forLetDecl is the node for the decl 'let <vars>'.
+    // Otherwise both are null.
+    ParseNode *forLetImpliedBlock = NULL;
+    ParseNode *forLetDecl = NULL;
+
+    // If non-null, the node for the decl 'var v = expr1' in the weirdo form
+    // 'for (var v = expr1 in expr2) stmt'.
+    ParseNode *hoistedVar = NULL;
 
     /*
      * We can be sure that it's a for/in loop if there's still an 'in'
      * keyword here, even if JavaScript recognizes 'in' as an operator,
      * as we've excluded 'in' from being parsed in RelExpr by setting
      * pc->parsingForInit.
      */
     StmtInfoPC letStmt(context); /* used if blockObj != NULL. */
-    ParseNode *pn2, *pn3;      /* forHead->pn_kid1 and pn_kid2. */
-    bool forOf;
-    bool isForInOrOf = pn1 && matchInOrOf(&forOf);
+    ParseNode *pn2, *pn3;      /* forHead->pn_kid2 and pn_kid3. */
+    bool isForOf;
+    bool isForInOrOf = pn1 && matchInOrOf(&isForOf);
     if (isForInOrOf) {
         /*
          * Parse the rest of the for/in or for/of head.
          *
          * Here pn1 is everything to the left of 'in' or 'of'. At the end of
          * this block, pn1 is a decl or NULL, pn2 is the assignment target that
          * receives the enumeration value each iteration, and pn3 is the rhs of
          * 'in'.
          */
         forStmt.type = STMT_FOR_IN_LOOP;
 
         /* Set iflags and rule out invalid combinations. */
-        if (forOf && isForEach) {
+        if (isForOf && isForEach) {
             report(ParseError, false, null(), JSMSG_BAD_FOR_EACH_LOOP);
             return null();
         }
-        iflags |= (forOf ? JSITER_FOR_OF : JSITER_ENUMERATE);
+        iflags |= (isForOf ? JSITER_FOR_OF : JSITER_ENUMERATE);
 
         /* Check that the left side of the 'in' or 'of' is valid. */
-        if (!isValidForStatementLHS(pn1, versionNumber(), forDecl, isForEach, forOf)) {
+        if (!isValidForStatementLHS(pn1, versionNumber(), isForDecl, isForEach, isForOf)) {
             report(ParseError, false, pn1, JSMSG_BAD_FOR_LEFTSIDE);
             return null();
         }
 
         /*
          * After the following if-else, pn2 will point to the name or
          * destructuring pattern on in's left. pn1 will point to the decl, if
          * any, else NULL. Note that the "declaration with initializer" case
          * rewrites the loop-head, moving the decl and setting pn1 to NULL.
          */
-        if (forDecl) {
+        if (isForDecl) {
             pn2 = pn1->pn_head;
             if ((pn2->isKind(PNK_NAME) && pn2->maybeExpr())
 #if JS_HAS_DESTRUCTURING
                 || pn2->isKind(PNK_ASSIGN)
 #endif
                 )
             {
                 /*
@@ -3921,19 +3926,17 @@ Parser<FullParseHandler>::forStatement()
                  */
 #if JS_HAS_BLOCK_SCOPE
                 if (blockObj) {
                     report(ParseError, false, pn2, JSMSG_INVALID_FOR_IN_INIT);
                     return null();
                 }
 #endif /* JS_HAS_BLOCK_SCOPE */
 
-                ParseNode *pnseq = handler.newList(PNK_SEQ, pn1);
-                if (!pnseq)
-                    return null();
+                hoistedVar = pn1;
 
                 /*
                  * All of 'var x = i' is hoisted above 'for (x in o)'.
                  *
                  * Request JSOP_POP here since the var is for a simple
                  * name (it is not a destructuring binding's left-hand
                  * side) and it has an initializer.
                  */
@@ -3942,19 +3945,16 @@ Parser<FullParseHandler>::forStatement()
 
 #if JS_HAS_DESTRUCTURING
                 if (pn2->isKind(PNK_ASSIGN)) {
                     pn2 = pn2->pn_left;
                     JS_ASSERT(pn2->isKind(PNK_ARRAY) || pn2->isKind(PNK_OBJECT) ||
                               pn2->isKind(PNK_NAME));
                 }
 #endif
-                pnseq->pn_pos.begin = pn->pn_pos.begin;
-                pnseq->append(pn);
-                forParent = pnseq;
             }
         } else {
             /* Not a declaration. */
             JS_ASSERT(!blockObj);
             pn2 = pn1;
             pn1 = NULL;
 
             if (!setAssignmentLhsOps(pn2, JSOP_NOP))
@@ -3976,17 +3976,17 @@ Parser<FullParseHandler>::forStatement()
             if (!block)
                 return null();
             letStmt.isForLetBlock = true;
             block->pn_expr = pn1;
             block->pn_pos = pn1->pn_pos;
             pn1 = block;
         }
 
-        if (forDecl) {
+        if (isForDecl) {
             /*
              * pn2 is part of a declaration. Make a copy that can be passed to
              * EmitAssignment. Take care to do this after PushLetScope.
              */
             pn2 = cloneLeftHandSide(pn2);
             if (!pn2)
                 return null();
         }
@@ -4004,47 +4004,42 @@ Parser<FullParseHandler>::forStatement()
 
           case PNK_ARRAY:
           case PNK_OBJECT:
             if (versionNumber() == JSVERSION_1_7) {
                 /*
                  * Destructuring for-in requires [key, value] enumeration
                  * in JS1.7.
                  */
-                if (!isForEach && !forOf)
+                if (!isForEach && !isForOf)
                     iflags |= JSITER_FOREACH | JSITER_KEYVALUE;
             }
             break;
 #endif
 
           default:;
         }
     } else {
         if (isForEach) {
-            report(ParseError, false, pn, JSMSG_BAD_FOR_EACH_LOOP);
+            reportWithOffset(ParseError, false, begin, JSMSG_BAD_FOR_EACH_LOOP);
             return null();
         }
 
         if (blockObj) {
             /*
              * Desugar 'for (let A; B; C) D' into 'let (A) { for (; B; C) D }'
              * to induce the correct scoping for A.
              */
-            ParseNode *block = pushLetScope(blockObj, &letStmt);
-            if (!block)
+            forLetImpliedBlock = pushLetScope(blockObj, &letStmt);
+            if (!forLetImpliedBlock)
                 return null();
             letStmt.isForLetBlock = true;
 
-            ParseNode *let = handler.newBinary(PNK_LET, pn1, block);
-            if (!let)
-                return null();
-
+            forLetDecl = pn1;
             pn1 = NULL;
-            block->pn_expr = pn;
-            forParent = let;
         }
 
         /* Parse the loop condition or null into pn2. */
         MUST_MATCH_TOKEN(TOK_SEMI, JSMSG_SEMI_AFTER_FOR_INIT);
         if (tokenStream.peekToken(TSF_OPERAND) == TOK_SEMI) {
             pn2 = NULL;
         } else {
             pn2 = expr();
@@ -4058,46 +4053,56 @@ Parser<FullParseHandler>::forStatement()
             pn3 = NULL;
         } else {
             pn3 = expr();
             if (!pn3)
                 return null();
         }
     }
 
-    TokenPos headPos = TokenPos::make(lp_pos.begin, pos().end);
+    MUST_MATCH_TOKEN(TOK_RP, JSMSG_PAREN_AFTER_FOR_CTRL);
+
+    TokenPos headPos = TokenPos::make(begin, pos().end);
     ParseNode *forHead = handler.newForHead(isForInOrOf, pn1, pn2, pn3, headPos);
     if (!forHead)
         return null();
 
-    MUST_MATCH_TOKEN(TOK_RP, JSMSG_PAREN_AFTER_FOR_CTRL);
-
     /* Parse the loop body. */
     ParseNode *body = statement();
     if (!body)
         return null();
 
-    /* A FOR node is binary, left is loop control and right is the body. */
-    pn->setOp(isForInOrOf ? JSOP_ITER : JSOP_NOP);
-    pn->pn_left = forHead;
-    pn->pn_right = body;
-    pn->pn_iflags = iflags;
-    pn->pn_pos.end = body->pn_pos.end;
-
-    if (forParent) {
-        forParent->pn_pos.begin = pn->pn_pos.begin;
-        forParent->pn_pos.end = pn->pn_pos.end;
-    }
-
 #if JS_HAS_BLOCK_SCOPE
     if (blockObj)
         PopStatementPC(context, pc);
 #endif
     PopStatementPC(context, pc);
-    return forParent ? forParent : pn;
+
+    ParseNode *forLoop = handler.newForStatement(begin, forHead, body, iflags);
+    if (!forLoop)
+        return null();
+
+    if (hoistedVar) {
+        ParseNode *pnseq = handler.newList(PNK_SEQ, hoistedVar);
+        if (!pnseq)
+            return null();
+        pnseq->pn_pos = forLoop->pn_pos;
+        pnseq->append(forLoop);
+        return pnseq;
+    }
+    if (forLetImpliedBlock) {
+        forLetImpliedBlock->pn_expr = forLoop;
+        forLetImpliedBlock->pn_pos = forLoop->pn_pos;
+        ParseNode *let = handler.newBinary(PNK_LET, forLetDecl, forLetImpliedBlock);
+        if (!let)
+            return null();
+        let->pn_pos = forLoop->pn_pos;
+        return let;
+    }
+    return forLoop;
 }
 
 template <>
 SyntaxParseHandler::Node
 Parser<SyntaxParseHandler>::forStatement()
 {
     /*
      * 'for' statement parsing is fantastically complicated and requires being
@@ -4114,31 +4119,31 @@ Parser<SyntaxParseHandler>::forStatement
     if (allowsForEachIn() && tokenStream.peekToken() == TOK_NAME) {
         JS_ALWAYS_FALSE(abortIfSyntaxParser());
         return null();
     }
 
     MUST_MATCH_TOKEN(TOK_LP, JSMSG_PAREN_AFTER_FOR);
 
     /* True if we have 'for (var ...)'. */
-    bool forDecl = false;
+    bool isForDecl = false;
     bool simpleForDecl = true;
 
     /* Set to 'x' in 'for (x ;... ;...)' or 'for (x in ...)'. */
     Node lhsNode;
 
     {
         TokenKind tt = tokenStream.peekToken(TSF_OPERAND);
         if (tt == TOK_SEMI) {
             lhsNode = null();
         } else {
             /* Set lhsNode to a var list or an initializing expression. */
             pc->parsingForInit = true;
             if (tt == TOK_VAR) {
-                forDecl = true;
+                isForDecl = true;
                 tokenStream.consumeKnownToken(tt);
                 lhsNode = variables(tt == TOK_VAR ? PNK_VAR : PNK_CONST, &simpleForDecl);
             }
 #if JS_HAS_BLOCK_SCOPE
             else if (tt == TOK_CONST || tt == TOK_LET) {
                 JS_ALWAYS_FALSE(abortIfSyntaxParser());
                 return null();
             }
@@ -4153,37 +4158,37 @@ Parser<SyntaxParseHandler>::forStatement
     }
 
     /*
      * We can be sure that it's a for/in loop if there's still an 'in'
      * keyword here, even if JavaScript recognizes 'in' as an operator,
      * as we've excluded 'in' from being parsed in RelExpr by setting
      * pc->parsingForInit.
      */
-    bool forOf;
-    if (lhsNode && matchInOrOf(&forOf)) {
+    bool isForOf;
+    if (lhsNode && matchInOrOf(&isForOf)) {
         /* Parse the rest of the for/in or for/of head. */
         forStmt.type = STMT_FOR_IN_LOOP;
 
         /* Check that the left side of the 'in' or 'of' is valid. */
-        if (!forDecl &&
+        if (!isForDecl &&
             lhsNode != SyntaxParseHandler::NodeName &&
             lhsNode != SyntaxParseHandler::NodeGetProp &&
             lhsNode != SyntaxParseHandler::NodeLValue)
         {
             JS_ALWAYS_FALSE(abortIfSyntaxParser());
             return null();
         }
 
         if (!simpleForDecl) {
             JS_ALWAYS_FALSE(abortIfSyntaxParser());
             return null();
         }
 
-        if (!forDecl && !setAssignmentLhsOps(lhsNode, JSOP_NOP))
+        if (!isForDecl && !setAssignmentLhsOps(lhsNode, JSOP_NOP))
             return null();
 
         if (!expr())
             return null();
     } else {
         /* Parse the loop condition or null. */
         MUST_MATCH_TOKEN(TOK_SEMI, JSMSG_SEMI_AFTER_FOR_INIT);
         if (tokenStream.peekToken(TSF_OPERAND) != TOK_SEMI) {
@@ -5866,22 +5871,22 @@ Parser<FullParseHandler>::comprehensionT
 
           default:
             report(ParseError, false, null(), JSMSG_NO_VARIABLE_NAME);
 
           case TOK_ERROR:
             return null();
         }
 
-        bool forOf;
-        if (!matchInOrOf(&forOf)) {
+        bool isForOf;
+        if (!matchInOrOf(&isForOf)) {
             report(ParseError, false, null(), JSMSG_IN_AFTER_FOR_NAME);
             return null();
         }
-        if (forOf) {
+        if (isForOf) {
             if (pn2->pn_iflags != JSITER_ENUMERATE) {
                 JS_ASSERT(pn2->pn_iflags == (JSITER_FOREACH | JSITER_ENUMERATE));
                 report(ParseError, false, null(), JSMSG_BAD_FOR_EACH_LOOP);
                 return null();
             }
             pn2->pn_iflags = JSITER_FOR_OF;
         }
 
@@ -5904,17 +5909,17 @@ Parser<FullParseHandler>::comprehensionT
 #if JS_HAS_DESTRUCTURING
           case TOK_LB:
           case TOK_LC:
             if (!checkDestructuring(&data, pn3))
                 return null();
 
             if (versionNumber() == JSVERSION_1_7 &&
                 !(pn2->pn_iflags & JSITER_FOREACH) &&
-                !forOf)
+                !isForOf)
             {
                 /* Destructuring requires [key, value] enumeration in JS1.7. */
                 if (!pn3->isKind(PNK_ARRAY) || pn3->pn_count != 2) {
                     report(ParseError, false, null(), JSMSG_BAD_FOR_LEFTSIDE);
                     return null();
                 }
 
                 JS_ASSERT(pn2->isOp(JSOP_ITER));