Split up Tokens for vanilla for from for-in

This makes for-in more consistent with for-of and makes it easier to remove some unnecessary helper methods from NodeUtil. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=141230854
google · Dec 7, 2016 · f442f1e · f442f1e
1 parent 720b9bf
commit f442f1e
Show file tree

Hide file tree

Showing 33 changed files with 168 additions and 105 deletions.
diff --git a/src/com/google/javascript/jscomp/AstValidator.java b/src/com/google/javascript/jscomp/AstValidator.java
@@ -137,6 +137,9 @@ public void validateStatement(Node n, boolean isAmbient) {
       case FOR:
         validateFor(n);
         return;
+      case FOR_IN:
+        validateForIn(n);
+        return;
       case FOR_OF:
         validateForOf(n);
         return;
@@ -942,18 +945,18 @@ private void validateObjectPattern(Token type, Node n) {
 
   private void validateFor(Node n) {
     validateNodeType(Token.FOR, n);
-    if (NodeUtil.isForIn(n)) {
-      // FOR-IN
-      validateChildCount(n, 3);
-      validateVarOrAssignmentTarget(n.getFirstChild());
-      validateExpression(n.getSecondChild());
-    } else {
-      // FOR
-      validateChildCount(n, 4);
-      validateVarOrOptionalExpression(n.getFirstChild());
-      validateOptionalExpression(n.getSecondChild());
-      validateOptionalExpression(n.getChildAtIndex(2));
-    }
+    validateChildCount(n, 4);
+    validateVarOrOptionalExpression(n.getFirstChild());
+    validateOptionalExpression(n.getSecondChild());
+    validateOptionalExpression(n.getChildAtIndex(2));
+    validateBlock(n.getLastChild());
+  }
+
+  private void validateForIn(Node n) {
+    validateNodeType(Token.FOR_IN, n);
+    validateChildCount(n, 3);
+    validateVarOrAssignmentTarget(n.getFirstChild());
+    validateExpression(n.getSecondChild());
     validateBlock(n.getLastChild());
   }
 

diff --git a/src/com/google/javascript/jscomp/CheckSuspiciousCode.java b/src/com/google/javascript/jscomp/CheckSuspiciousCode.java
@@ -82,6 +82,7 @@ private void checkMissingSemicolon(NodeTraversal t, Node n) {
 
       case WHILE:
       case FOR:
+      case FOR_IN:
       case FOR_OF:
         reportIfWasEmpty(t, NodeUtil.getLoopCodeBlock(n));
         break;

diff --git a/src/com/google/javascript/jscomp/CodeGenerator.java b/src/com/google/javascript/jscomp/CodeGenerator.java
@@ -652,6 +652,7 @@ protected void add(Node n, Context context) {
         }
 
       case FOR:
+      case FOR_IN:
         if (childCount == 4) {
           add("for");
           cc.maybeInsertSpace();

diff --git a/src/com/google/javascript/jscomp/ControlFlowAnalysis.java b/src/com/google/javascript/jscomp/ControlFlowAnalysis.java
@@ -250,6 +250,7 @@ public boolean shouldTraverse(
     if (parent != null) {
       switch (parent.getToken()) {
         case FOR:
+        case FOR_IN:
         case FOR_OF:
           // Only traverse the body of the for loop.
           return n == parent.getLastChild();
@@ -311,6 +312,7 @@ public void visit(NodeTraversal t, Node n, Node parent) {
         return;
       case FOR_OF:
       case FOR:
+      case FOR_IN:
         handleFor(n);
         return;
       case SWITCH:
@@ -751,6 +753,7 @@ private static Node computeFollowNode(
       case FOR_OF:
         return parent;
       case FOR:
+      case FOR_IN:
         if (NodeUtil.isForIn(parent)) {
           return parent;
         } else {
@@ -814,6 +817,7 @@ static Node computeFallThrough(Node n) {
       case DO:
         return computeFallThrough(n.getFirstChild());
       case FOR:
+      case FOR_IN:
       case FOR_OF:
         if (n.isForOf() || NodeUtil.isForIn(n)) {
           return n.getSecondChild();
@@ -969,6 +973,7 @@ public static boolean mayThrowException(Node n) {
   static boolean isBreakStructure(Node n, boolean labeled) {
     switch (n.getToken()) {
       case FOR:
+      case FOR_IN:
       case FOR_OF:
       case DO:
       case WHILE:
@@ -989,6 +994,7 @@ static boolean isBreakStructure(Node n, boolean labeled) {
   static boolean isContinueStructure(Node n) {
     switch (n.getToken()) {
       case FOR:
+      case FOR_IN:
       case FOR_OF:
       case DO:
       case WHILE:

diff --git a/src/com/google/javascript/jscomp/ControlFlowGraph.java b/src/com/google/javascript/jscomp/ControlFlowGraph.java
@@ -168,6 +168,7 @@ public static boolean isEnteringNewCfgNode(Node n) {
         return NodeUtil.getConditionExpression(parent) != n;
 
       case FOR:
+      case FOR_IN:
         // The FOR(;;) node differs from other control structures in that
         // it has an initialization and an increment statement. Those
         // two statements have corresponding CFG nodes to represent them.

diff --git a/src/com/google/javascript/jscomp/ConvertToTypedInterface.java b/src/com/google/javascript/jscomp/ConvertToTypedInterface.java
@@ -296,18 +296,20 @@ public void visit(NodeTraversal t, Node n, Node parent) {
         case FOR_OF:
         case DO:
         case WHILE:
-        case FOR: {
-          Node body = NodeUtil.getLoopCodeBlock(n);
-          parent.addChildAfter(body.detach(), n);
-          NodeUtil.removeChild(parent, n);
-          Node initializer = n.isFor() ? n.getFirstChild() : IR.empty();
-          if (initializer.isVar() && initializer.hasOneChild()) {
-            parent.addChildBefore(initializer.detach(), body);
-            processName(initializer.getFirstChild(), initializer);
+        case FOR:
+        case FOR_IN:
+          {
+            Node body = NodeUtil.getLoopCodeBlock(n);
+            parent.addChildAfter(body.detach(), n);
+            NodeUtil.removeChild(parent, n);
+            Node initializer = n.isFor() ? n.getFirstChild() : IR.empty();
+            if (initializer.isVar() && initializer.hasOneChild()) {
+              parent.addChildBefore(initializer.detach(), body);
+              processName(initializer.getFirstChild(), initializer);
+            }
+            compiler.reportCodeChange();
+            break;
           }
-          compiler.reportCodeChange();
-          break;
-        }
         case LABEL:
           if (n.getParent() != null) {
             parent.replaceChild(n, n.getSecondChild().detach());

diff --git a/src/com/google/javascript/jscomp/CrossModuleCodeMotion.java b/src/com/google/javascript/jscomp/CrossModuleCodeMotion.java
@@ -226,6 +226,7 @@ private static boolean hasConditionalAncestor(Node n) {
       switch (ancestor.getToken()) {
         case DO:
         case FOR:
+        case FOR_IN:
         case HOOK:
         case IF:
         case SWITCH:

diff --git a/src/com/google/javascript/jscomp/DeadAssignmentsElimination.java b/src/com/google/javascript/jscomp/DeadAssignmentsElimination.java
@@ -163,9 +163,9 @@ private void tryRemoveDeadAssignments(NodeTraversal t,
           tryRemoveAssignment(t, NodeUtil.getConditionExpression(n), state);
           continue;
         case FOR:
+        case FOR_IN:
           if (!NodeUtil.isForIn(n)) {
-            tryRemoveAssignment(
-                t, NodeUtil.getConditionExpression(n), state);
+            tryRemoveAssignment(t, NodeUtil.getConditionExpression(n), state);
           }
           continue;
         case SWITCH:

diff --git a/src/com/google/javascript/jscomp/DeadPropertyAssignmentElimination.java b/src/com/google/javascript/jscomp/DeadPropertyAssignmentElimination.java
@@ -408,6 +408,7 @@ private boolean visitNode(Node n, Node parent) {
 
         case THROW:
         case FOR:
+        case FOR_IN:
         case SWITCH:
           // TODO(kevinoconnor): Switch/for statements need special consideration since they may
           // execute out of order.

diff --git a/src/com/google/javascript/jscomp/Es6RewriteBlockScopedDeclaration.java b/src/com/google/javascript/jscomp/Es6RewriteBlockScopedDeclaration.java
@@ -50,7 +50,7 @@ public final class Es6RewriteBlockScopedDeclaration extends AbstractPostOrderCal
     implements HotSwapCompilerPass {
 
   private static final Set<Token> LOOP_TOKENS =
-      EnumSet.of(Token.WHILE, Token.FOR, Token.FOR_OF, Token.DO, Token.FUNCTION);
+      EnumSet.of(Token.WHILE, Token.FOR, Token.FOR_IN, Token.FOR_OF, Token.DO, Token.FUNCTION);
 
   private final AbstractCompiler compiler;
   private final Table<Node, String, String> renameTable = HashBasedTable.create();

diff --git a/src/com/google/javascript/jscomp/Es6RewriteGenerators.java b/src/com/google/javascript/jscomp/Es6RewriteGenerators.java
@@ -329,6 +329,7 @@ private boolean translateStatementInOriginalBody() {
         case WHILE:
         case DO:
         case FOR:
+        case FOR_IN:
           if (NodeUtil.isForIn(currentStatement)) {
             visitForIn();
             return false;
@@ -978,6 +979,7 @@ public boolean shouldTraverse(NodeTraversal t, Node n, Node parent) {
           break;
         case DO:
         case FOR:
+        case FOR_IN:
         case WHILE:
           visitLoop(n);
           break;
@@ -1021,6 +1023,7 @@ private void visitLoop(Node n) {
       Node incr = null;
       switch (n.getToken()) {
         case FOR:
+        case FOR_IN:
           guard = n.getSecondChild();
           incr = guard.getNext();
           break;
@@ -1098,6 +1101,7 @@ public boolean shouldTraverse(NodeTraversal nodeTraversal, Node n, Node parent)
         case DO:
         case WHILE:
         case FOR:
+        case FOR_IN:
           continueCatchers++;
           breakCatchers++;
           break;
@@ -1164,6 +1168,7 @@ public void visit(NodeTraversal t, Node n, Node parent) {
         case DO:
         case WHILE:
         case FOR:
+        case FOR_IN:
           continueCatchers--;
           breakCatchers--;
           break;

diff --git a/src/com/google/javascript/jscomp/ExpressionDecomposer.java b/src/com/google/javascript/jscomp/ExpressionDecomposer.java
@@ -672,6 +672,7 @@ static Node findExpressionRoot(Node subExpression) {
           return parent;
         // Any of these indicate an unsupported expression:
         case FOR:
+        case FOR_IN:
           if (!NodeUtil.isForIn(parent) && child == parent.getFirstChild()) {
             return parent;
           }

diff --git a/src/com/google/javascript/jscomp/GlobalNamespace.java b/src/com/google/javascript/jscomp/GlobalNamespace.java
@@ -31,7 +31,6 @@
 import com.google.javascript.rhino.jstype.StaticTypedRef;
 import com.google.javascript.rhino.jstype.StaticTypedScope;
 import com.google.javascript.rhino.jstype.StaticTypedSlot;
-
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
@@ -763,6 +762,7 @@ Ref.Type determineGetTypeForHookOrBooleanExpr(
           case IF:
           case WHILE:
           case FOR:
+          case FOR_IN:
           case TYPEOF:
           case VOID:
           case NOT:

diff --git a/src/com/google/javascript/jscomp/LiveVariablesAnalysis.java b/src/com/google/javascript/jscomp/LiveVariablesAnalysis.java
@@ -21,7 +21,6 @@
 import com.google.javascript.jscomp.graph.DiGraph.DiGraphEdge;
 import com.google.javascript.jscomp.graph.LatticeElement;
 import com.google.javascript.rhino.Node;
-
 import java.util.BitSet;
 import java.util.HashSet;
 import java.util.List;
@@ -196,9 +195,9 @@ private void computeGenKill(Node n, BitSet gen, BitSet kill,
         return;
 
       case FOR:
+      case FOR_IN:
         if (!NodeUtil.isForIn(n)) {
-          computeGenKill(NodeUtil.getConditionExpression(n), gen, kill,
-              conditional);
+          computeGenKill(NodeUtil.getConditionExpression(n), gen, kill, conditional);
         } else {
           // for(x in y) {...}
           Node lhs = n.getFirstChild();

diff --git a/src/com/google/javascript/jscomp/MaybeReachingVariableUse.java b/src/com/google/javascript/jscomp/MaybeReachingVariableUse.java
@@ -24,7 +24,6 @@
 import com.google.javascript.jscomp.graph.GraphNode;
 import com.google.javascript.jscomp.graph.LatticeElement;
 import com.google.javascript.rhino.Node;
-
 import java.util.Collection;
 import java.util.HashSet;
 import java.util.List;
@@ -184,9 +183,9 @@ private void computeMayUse(
         return;
 
       case FOR:
+      case FOR_IN:
         if (!NodeUtil.isForIn(n)) {
-          computeMayUse(
-              NodeUtil.getConditionExpression(n), cfgNode, output, conditional);
+          computeMayUse(NodeUtil.getConditionExpression(n), cfgNode, output, conditional);
         } else {
           // for(x in y) {...}
           Node lhs = n.getFirstChild();

diff --git a/src/com/google/javascript/jscomp/MinimizeExitPoints.java b/src/com/google/javascript/jscomp/MinimizeExitPoints.java
@@ -51,6 +51,7 @@ Node optimizeSubtree(Node n) {
         break;
 
       case FOR:
+      case FOR_IN:
       case WHILE:
         tryMinimizeExits(NodeUtil.getLoopCodeBlock(n), Token.CONTINUE, null);
         break;

diff --git a/src/com/google/javascript/jscomp/MustBeReachingVariableDef.java b/src/com/google/javascript/jscomp/MustBeReachingVariableDef.java
@@ -22,13 +22,11 @@
 import com.google.javascript.jscomp.graph.GraphNode;
 import com.google.javascript.jscomp.graph.LatticeElement;
 import com.google.javascript.rhino.Node;
-
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.Set;
-
 import javax.annotation.Nullable;
 
 /**
@@ -247,9 +245,9 @@ private void computeMustDef(
         return;
 
       case FOR:
+      case FOR_IN:
         if (!NodeUtil.isForIn(n)) {
-          computeMustDef(
-              NodeUtil.getConditionExpression(n), cfgNode, output, conditional);
+          computeMustDef(NodeUtil.getConditionExpression(n), cfgNode, output, conditional);
         } else {
           // for(x in y) {...}
           Node lhs = n.getFirstChild();

diff --git a/src/com/google/javascript/jscomp/NameAnalyzer.java b/src/com/google/javascript/jscomp/NameAnalyzer.java
@@ -1841,6 +1841,7 @@ private void replaceTopLevelExpressionWithRhs(Node parent, Node n) {
       case BLOCK:
       case SCRIPT:
       case FOR:
+      case FOR_IN:
       case LABEL:
         break;
       default:
@@ -1917,6 +1918,7 @@ private static boolean valueConsumedByParent(Node n, Node parent) {
         return parent.getFirstChild() == n;
 
       case FOR:
+      case FOR_IN:
         return parent.getSecondChild() == n;
 
       case DO:

diff --git a/src/com/google/javascript/jscomp/NewTypeInference.java b/src/com/google/javascript/jscomp/NewTypeInference.java
@@ -705,11 +705,11 @@ private void buildWorksetHelper(
       case DO:
       case WHILE:
       case FOR:
+      case FOR_IN:
         // Do the loop body first, then the loop follow.
         // For DO loops, we do BODY-CONDT-CONDF-FOLLOW
         // Since CONDT is currently unused, this could be optimized.
-        List<DiGraphEdge<Node, ControlFlowGraph.Branch>> outEdges =
-            dn.getOutEdges();
+        List<DiGraphEdge<Node, ControlFlowGraph.Branch>> outEdges = dn.getOutEdges();
         seen.add(dn);
         workset.add(dn);
         for (DiGraphEdge<Node, ControlFlowGraph.Branch> outEdge : outEdges) {
@@ -911,6 +911,7 @@ private void analyzeFunctionBwd(
           break;
         case DO:
         case FOR:
+        case FOR_IN:
         case IF:
         case WHILE:
           Node expr = NodeUtil.isForIn(n) ?
@@ -1007,6 +1008,7 @@ private void analyzeFunctionFwd(
         case DO:
         case IF:
         case FOR:
+        case FOR_IN:
         case WHILE:
           if (NodeUtil.isForIn(n)) {
             Node obj = n.getSecondChild();
@@ -4306,6 +4308,7 @@ private JSType pickReqObjType(Node expr) {
         return this.commonTypes.getEmptyObjectLiteral();
       }
       case FOR:
+      case FOR_IN:
         Preconditions.checkState(NodeUtil.isForIn(expr));
         return TOP_OBJECT;
       case GETPROP: