From f442f1e056a2d56aa1962138a625597c6ae85f7f Mon Sep 17 00:00:00 2001 From: blickly Date: Tue, 6 Dec 2016 14:55:04 -0800 Subject: [PATCH] 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 --- .../javascript/jscomp/AstValidator.java | 27 +++--- .../jscomp/CheckSuspiciousCode.java | 1 + .../javascript/jscomp/CodeGenerator.java | 1 + .../jscomp/ControlFlowAnalysis.java | 6 ++ .../javascript/jscomp/ControlFlowGraph.java | 1 + .../jscomp/ConvertToTypedInterface.java | 24 ++--- .../jscomp/CrossModuleCodeMotion.java | 1 + .../jscomp/DeadAssignmentsElimination.java | 4 +- .../DeadPropertyAssignmentElimination.java | 1 + .../Es6RewriteBlockScopedDeclaration.java | 2 +- .../jscomp/Es6RewriteGenerators.java | 5 + .../jscomp/ExpressionDecomposer.java | 1 + .../javascript/jscomp/GlobalNamespace.java | 2 +- .../jscomp/LiveVariablesAnalysis.java | 5 +- .../jscomp/MaybeReachingVariableUse.java | 5 +- .../javascript/jscomp/MinimizeExitPoints.java | 1 + .../jscomp/MustBeReachingVariableDef.java | 6 +- .../javascript/jscomp/NameAnalyzer.java | 2 + .../javascript/jscomp/NewTypeInference.java | 7 +- .../google/javascript/jscomp/NodeUtil.java | 22 +++-- .../google/javascript/jscomp/Normalize.java | 2 + .../jscomp/PeepholeMinimizeConditions.java | 2 + .../jscomp/PeepholeRemoveDeadCode.java | 14 +-- .../javascript/jscomp/ProcessDefines.java | 1 + .../jscomp/ReferenceCollectingCallback.java | 9 +- .../jscomp/SideEffectsAnalysis.java | 2 +- .../javascript/jscomp/StatementFusion.java | 2 + .../google/javascript/jscomp/TypeCheck.java | 1 + .../javascript/jscomp/parsing/IRFactory.java | 7 +- src/com/google/javascript/rhino/IR.java | 2 +- src/com/google/javascript/rhino/Node.java | 8 ++ src/com/google/javascript/rhino/Token.java | 3 +- .../jscomp/ControlFlowAnalysisTest.java | 96 ++++++++++--------- 33 files changed, 168 insertions(+), 105 deletions(-) diff --git a/src/com/google/javascript/jscomp/AstValidator.java b/src/com/google/javascript/jscomp/AstValidator.java index 9147705e795..605f5225b36 100644 --- 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 index cde54ca7ced..7d145685d6f 100644 --- 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 index b681625d0dd..b87a59ed517 100644 --- 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 index bbd5c259ece..0b639969636 100644 --- 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 index 5867db6f8b6..7e2f8d1dc73 100644 --- 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 index f393e1ca665..9b81816158c 100644 --- 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 index 4f6aa8216b8..dfae1980461 100644 --- 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 index 59c9db143f5..7b1d03a553d 100644 --- 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 index 51f87732011..4bfbc837a99 100644 --- 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 index 91f1093d978..5e7f6c2ec38 100644 --- 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 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 renameTable = HashBasedTable.create(); diff --git a/src/com/google/javascript/jscomp/Es6RewriteGenerators.java b/src/com/google/javascript/jscomp/Es6RewriteGenerators.java index 262280f86ef..84c14b9c75b 100644 --- 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 index 32ed7205fe6..6bad2c5f7ef 100644 --- 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 index 46ef7dbc84e..98f65511266 100644 --- 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 index 06ea2bfb54a..29af12e72bb 100644 --- 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 index a79a04f9d49..c360aa37cbf 100644 --- 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 index a05d86aa695..80e5a6f9396 100644 --- 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 index 836174c8545..d2b7342e259 100644 --- 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 index a6863dc8c5d..78ceba4594c 100644 --- 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 index 37046444244..147d943a8db 100644 --- 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> outEdges = - dn.getOutEdges(); + List> outEdges = dn.getOutEdges(); seen.add(dn); workset.add(dn); for (DiGraphEdge 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: diff --git a/src/com/google/javascript/jscomp/NodeUtil.java b/src/com/google/javascript/jscomp/NodeUtil.java index abd2bc67240..db79d04f17a 100644 --- a/src/com/google/javascript/jscomp/NodeUtil.java +++ b/src/com/google/javascript/jscomp/NodeUtil.java @@ -2362,19 +2362,20 @@ static boolean isVanillaFunction(Node n) { return n.isFunction() && !n.isArrowFunction(); } + /** Use Node.isVanillaFor instead. */ + @Deprecated static boolean isVanillaFor(Node n) { - return n.isFor() && n.getChildCount() == 4; + return n.isVanillaFor(); } static boolean isEnhancedFor(Node n) { - return n.isForOf() || isForIn(n); + return n.isForOf() || n.isForIn(); } - /** - * @return Whether the node represents a FOR-IN loop. - */ + /** Use Node.isForIn instead. */ + @Deprecated public static boolean isForIn(Node n) { - return n.isFor() && n.getChildCount() == 3; + return n.isForIn(); } /** @@ -2383,6 +2384,7 @@ public static boolean isForIn(Node n) { static boolean isLoopStructure(Node n) { switch (n.getToken()) { case FOR: + case FOR_IN: case FOR_OF: case DO: case WHILE: @@ -2400,6 +2402,7 @@ static boolean isLoopStructure(Node n) { static Node getLoopCodeBlock(Node n) { switch (n.getToken()) { case FOR: + case FOR_IN: case FOR_OF: case WHILE: return n.getLastChild(); @@ -2433,6 +2436,7 @@ static boolean isWithinLoop(Node n) { public static boolean isControlStructure(Node n) { switch (n.getToken()) { case FOR: + case FOR_IN: case FOR_OF: case DO: case WHILE: @@ -2461,6 +2465,7 @@ static boolean isControlStructureCodeBlock(Node parent, Node n) { case TRY: return parent.getFirstChild() == n || parent.getLastChild() == n; case FOR: + case FOR_IN: case FOR_OF: case WHILE: case LABEL: @@ -2492,6 +2497,7 @@ static Node getConditionExpression(Node n) { case DO: return n.getLastChild(); case FOR: + case FOR_IN: return NodeUtil.isForIn(n) ? null : n.getSecondChild(); case FOR_OF: case CASE: @@ -2530,6 +2536,7 @@ static boolean createsBlockScope(Node n) { } return true; case FOR: + case FOR_IN: case FOR_OF: case SWITCH: case CLASS: @@ -3696,6 +3703,7 @@ static boolean isPropertyTest(AbstractCompiler compiler, Node propAccess) { case WHILE: case DO: case FOR: + case FOR_IN: return NodeUtil.getConditionExpression(parent) == propAccess; case INSTANCEOF: @@ -4540,6 +4548,7 @@ static boolean isExpressionResultUsed(Node expr) { return (expr == parent.getFirstChild()) ? false : isExpressionResultUsed(parent); case FOR: + case FOR_IN: if (!NodeUtil.isForIn(parent)) { // Only an expression whose result is in the condition part of the // expression is used. @@ -4571,6 +4580,7 @@ static boolean isExecutedExactlyOnce(Node n) { // other ancestors may be conditional continue inspect; case FOR: + case FOR_IN: if (NodeUtil.isForIn(parent)) { if (parent.getSecondChild() != n) { return false; diff --git a/src/com/google/javascript/jscomp/Normalize.java b/src/com/google/javascript/jscomp/Normalize.java index 278aaed999e..c9afcf03311 100644 --- a/src/com/google/javascript/jscomp/Normalize.java +++ b/src/com/google/javascript/jscomp/Normalize.java @@ -549,6 +549,7 @@ private void normalizeLabels(Node n) { case LABEL: case BLOCK: case FOR: + case FOR_IN: case WHILE: case DO: return; @@ -585,6 +586,7 @@ private void extractForInitializer( extractForInitializer(c, insertBefore, insertBeforeParent); break; case FOR: + case FOR_IN: if (NodeUtil.isForIn(c)) { Node first = c.getFirstChild(); if (first.isVar()) { diff --git a/src/com/google/javascript/jscomp/PeepholeMinimizeConditions.java b/src/com/google/javascript/jscomp/PeepholeMinimizeConditions.java index 4b944a41018..42dcb269bbf 100644 --- a/src/com/google/javascript/jscomp/PeepholeMinimizeConditions.java +++ b/src/com/google/javascript/jscomp/PeepholeMinimizeConditions.java @@ -97,6 +97,7 @@ public Node optimizeSubtree(Node node) { return node; case FOR: + case FOR_IN: if (!NodeUtil.isForIn(node)) { tryJoinForCondition(node); tryMinimizeCondition(NodeUtil.getConditionExpression(node)); @@ -947,6 +948,7 @@ private static boolean consumesDanglingElse(Node n) { case WITH: case WHILE: case FOR: + case FOR_IN: n = n.getLastChild(); continue; default: diff --git a/src/com/google/javascript/jscomp/PeepholeRemoveDeadCode.java b/src/com/google/javascript/jscomp/PeepholeRemoveDeadCode.java index dc2bd63a109..6a6e70f88a1 100644 --- a/src/com/google/javascript/jscomp/PeepholeRemoveDeadCode.java +++ b/src/com/google/javascript/jscomp/PeepholeRemoveDeadCode.java @@ -58,13 +58,15 @@ Node optimizeSubtree(Node subtree) { return tryFoldIf(subtree); case WHILE: return tryFoldWhile(subtree); - case FOR: { - Node condition = NodeUtil.getConditionExpression(subtree); - if (condition != null) { - tryFoldForCondition(condition); + case FOR: + case FOR_IN: + { + Node condition = NodeUtil.getConditionExpression(subtree); + if (condition != null) { + tryFoldForCondition(condition); + } + return tryFoldFor(subtree); } - return tryFoldFor(subtree); - } case DO: Node foldedDo = tryFoldDoAway(subtree); if (foldedDo.isDo()) { diff --git a/src/com/google/javascript/jscomp/ProcessDefines.java b/src/com/google/javascript/jscomp/ProcessDefines.java index 234d85df042..6b61f1edb5f 100644 --- a/src/com/google/javascript/jscomp/ProcessDefines.java +++ b/src/com/google/javascript/jscomp/ProcessDefines.java @@ -381,6 +381,7 @@ private void updateAssignAllowedStack(Node n, boolean entering) { switch (n.getToken()) { case CASE: case FOR: + case FOR_IN: case FUNCTION: case HOOK: case IF: diff --git a/src/com/google/javascript/jscomp/ReferenceCollectingCallback.java b/src/com/google/javascript/jscomp/ReferenceCollectingCallback.java index 642b24ae53b..2eb81983884 100644 --- a/src/com/google/javascript/jscomp/ReferenceCollectingCallback.java +++ b/src/com/google/javascript/jscomp/ReferenceCollectingCallback.java @@ -299,6 +299,7 @@ private static boolean isBlockBoundary(Node n, Node parent) { switch (parent.getToken()) { case DO: case FOR: + case FOR_IN: case FOR_OF: case TRY: case WHILE: @@ -808,9 +809,11 @@ static final class BasicBlock { if (root.getParent() != null) { Token pType = root.getParent().getToken(); - this.isLoop = pType == Token.DO || - pType == Token.WHILE || - pType == Token.FOR; + this.isLoop = + pType == Token.DO + || pType == Token.WHILE + || pType == Token.FOR + || pType == Token.FOR_IN; } else { this.isLoop = false; } diff --git a/src/com/google/javascript/jscomp/SideEffectsAnalysis.java b/src/com/google/javascript/jscomp/SideEffectsAnalysis.java index 5a54b39d8df..8ee22333b0f 100644 --- a/src/com/google/javascript/jscomp/SideEffectsAnalysis.java +++ b/src/com/google/javascript/jscomp/SideEffectsAnalysis.java @@ -25,7 +25,6 @@ import com.google.javascript.jscomp.VariableVisibilityAnalysis.VariableVisibility; import com.google.javascript.rhino.Node; import com.google.javascript.rhino.Token; - import java.util.ArrayList; import java.util.HashMap; import java.util.HashSet; @@ -341,6 +340,7 @@ private static boolean isControlDependentChild(Node child) { case HOOK: return (indexOfChildInParent == 1 || indexOfChildInParent == 2); case FOR: + case FOR_IN: // Only initializer is not control dependent return indexOfChildInParent != 0; case SWITCH: diff --git a/src/com/google/javascript/jscomp/StatementFusion.java b/src/com/google/javascript/jscomp/StatementFusion.java index 1d00be16d90..93222ebd2ef 100644 --- a/src/com/google/javascript/jscomp/StatementFusion.java +++ b/src/com/google/javascript/jscomp/StatementFusion.java @@ -135,6 +135,7 @@ private boolean isFusableControlStatement(Node n) { // We don't want to add a new return value. return n.hasChildren(); case FOR: + case FOR_IN: if (NodeUtil.isForIn(n)) { // Avoid cases where we have for(var x = foo() in a) { .... return !mayHaveSideEffects(n.getFirstChild()); @@ -202,6 +203,7 @@ private static void fuseExpressionIntoControlFlowStatement( fuseExpressionIntoFirstChild(before.removeFirstChild(), control); return; case FOR: + case FOR_IN: before.getParent().removeChild(before); if (NodeUtil.isForIn(control)) { fuseExpressionIntoSecondChild(before.removeFirstChild(), control); diff --git a/src/com/google/javascript/jscomp/TypeCheck.java b/src/com/google/javascript/jscomp/TypeCheck.java index 2afcf4a1183..7c608376800 100644 --- a/src/com/google/javascript/jscomp/TypeCheck.java +++ b/src/com/google/javascript/jscomp/TypeCheck.java @@ -815,6 +815,7 @@ public void visit(NodeTraversal t, Node n, Node parent) { break; case FOR: + case FOR_IN: if (NodeUtil.isForIn(n)) { Node obj = n.getSecondChild(); if (getJSType(obj).isStruct()) { diff --git a/src/com/google/javascript/jscomp/parsing/IRFactory.java b/src/com/google/javascript/jscomp/parsing/IRFactory.java index c88bdf63dc9..8f2e9e7916a 100644 --- a/src/com/google/javascript/jscomp/parsing/IRFactory.java +++ b/src/com/google/javascript/jscomp/parsing/IRFactory.java @@ -504,6 +504,7 @@ private void validateBreakContinue(Node n) { private static boolean isBreakTarget(Node n) { switch (n.getToken()) { case FOR: + case FOR_IN: case FOR_OF: case WHILE: case DO: @@ -517,6 +518,7 @@ private static boolean isBreakTarget(Node n) { private static boolean isContinueTarget(Node n) { switch (n.getToken()) { case FOR: + case FOR_IN: case FOR_OF: case WHILE: case DO: @@ -1166,10 +1168,7 @@ Node processForInLoop(ForInStatementTree loopNode) { lineno(loopNode.initializer), charno(loopNode.initializer)); } return newNode( - Token.FOR, - initializer, - transform(loopNode.collection), - transformBlock(loopNode.body)); + Token.FOR_IN, initializer, transform(loopNode.collection), transformBlock(loopNode.body)); } Node processForOf(ForOfStatementTree loopNode) { diff --git a/src/com/google/javascript/rhino/IR.java b/src/com/google/javascript/rhino/IR.java index 848392710d6..e9a256234b8 100644 --- a/src/com/google/javascript/rhino/IR.java +++ b/src/com/google/javascript/rhino/IR.java @@ -246,7 +246,7 @@ public static Node forIn(Node target, Node cond, Node body) { Preconditions.checkState(target.isVar() || mayBeExpression(target)); Preconditions.checkState(mayBeExpression(cond)); Preconditions.checkState(body.isBlock()); - return new Node(Token.FOR, target, cond, body); + return new Node(Token.FOR_IN, target, cond, body); } public static Node forNode(Node init, Node cond, Node incr, Node body) { diff --git a/src/com/google/javascript/rhino/Node.java b/src/com/google/javascript/rhino/Node.java index b3bdc3de738..63a8757f749 100644 --- a/src/com/google/javascript/rhino/Node.java +++ b/src/com/google/javascript/rhino/Node.java @@ -2951,9 +2951,17 @@ public boolean isFalse() { } public boolean isFor() { + return this.isVanillaFor() || this.isForIn(); + } + + public boolean isVanillaFor() { return this.token == Token.FOR; } + public boolean isForIn() { + return this.token == Token.FOR_IN; + } + public boolean isForOf() { return this.token == Token.FOR_OF; } diff --git a/src/com/google/javascript/rhino/Token.java b/src/com/google/javascript/rhino/Token.java index a72a3fa2e0d..4aaa753c6e9 100644 --- a/src/com/google/javascript/rhino/Token.java +++ b/src/com/google/javascript/rhino/Token.java @@ -124,7 +124,8 @@ public enum Token { DEFAULT_CASE, // default keyword WHILE, // while keyword DO, // do keyword - FOR, // for keyword + FOR, // for(;;) statemet + FOR_IN, // for-in BREAK, // break keyword CONTINUE, // continue keyword VAR, // var keyword diff --git a/test/com/google/javascript/jscomp/ControlFlowAnalysisTest.java b/test/com/google/javascript/jscomp/ControlFlowAnalysisTest.java index 4a327fcd0b9..5272b908609 100644 --- a/test/com/google/javascript/jscomp/ControlFlowAnalysisTest.java +++ b/test/com/google/javascript/jscomp/ControlFlowAnalysisTest.java @@ -25,12 +25,10 @@ import com.google.javascript.jscomp.graph.DiGraph.DiGraphNode; import com.google.javascript.rhino.Node; import com.google.javascript.rhino.Token; - -import junit.framework.TestCase; - import java.util.ArrayList; import java.util.Iterator; import java.util.List; +import junit.framework.TestCase; /** * Tests {@link ControlFlowAnalysis}. @@ -716,40 +714,41 @@ public void testNestedDoWithBreak() { public void testForIn() { String src = "var a,b;for(a in b){a()};"; - String expected = "digraph AST {\n" + - " node [color=lightblue2, style=filled];\n" + - " node0 [label=\"SCRIPT\"];\n" + - " node1 [label=\"VAR\"];\n" + - " node0 -> node1 [weight=1];\n" + - " node2 [label=\"NAME\"];\n" + - " node1 -> node2 [weight=1];\n" + - " node3 [label=\"NAME\"];\n" + - " node1 -> node3 [weight=1];\n" + - " node4 [label=\"NAME\"];\n" + - " node1 -> node4 [label=\"UNCOND\", fontcolor=\"red\", weight=0.01, color=\"red\"];\n" + - " node5 [label=\"FOR\"];\n" + - " node0 -> node5 [weight=1];\n" + - " node6 [label=\"NAME\"];\n" + - " node5 -> node6 [weight=1];\n" + - " node5 -> node4 [weight=1];\n" + - " node4 -> node5 [label=\"UNCOND\", fontcolor=\"red\", weight=0.01, color=\"red\"];\n" + - " node7 [label=\"BLOCK\"];\n" + - " node5 -> node7 [weight=1];\n" + - " node8 [label=\"EXPR_RESULT\"];\n" + - " node7 -> node8 [weight=1];\n" + - " node9 [label=\"CALL\"];\n" + - " node8 -> node9 [weight=1];\n" + - " node10 [label=\"NAME\"];\n" + - " node9 -> node10 [weight=1];\n" + - " node8 -> node5 [label=\"UNCOND\", fontcolor=\"red\", weight=0.01, color=\"red\"];\n" + - " node7 -> node8 [label=\"UNCOND\", fontcolor=\"red\", weight=0.01, color=\"red\"];\n" + - " node11 [label=\"EMPTY\"];\n" + - " node5 -> node11 [label=\"ON_FALSE\", fontcolor=\"red\", weight=0.01, color=\"red\"];\n" + - " node5 -> node7 [label=\"ON_TRUE\", fontcolor=\"red\", weight=0.01, color=\"red\"];\n" + - " node0 -> node11 [weight=1];\n" + - " node11 -> RETURN [label=\"UNCOND\", fontcolor=\"red\", weight=0.01, color=\"red\"];\n" + - " node0 -> node1 [label=\"UNCOND\", fontcolor=\"red\", weight=0.01, color=\"red\"];\n" + - "}\n"; + String expected = + "digraph AST {\n" + + " node [color=lightblue2, style=filled];\n" + + " node0 [label=\"SCRIPT\"];\n" + + " node1 [label=\"VAR\"];\n" + + " node0 -> node1 [weight=1];\n" + + " node2 [label=\"NAME\"];\n" + + " node1 -> node2 [weight=1];\n" + + " node3 [label=\"NAME\"];\n" + + " node1 -> node3 [weight=1];\n" + + " node4 [label=\"NAME\"];\n" + + " node1 -> node4 [label=\"UNCOND\", fontcolor=\"red\", weight=0.01, color=\"red\"];\n" + + " node5 [label=\"FOR_IN\"];\n" + + " node0 -> node5 [weight=1];\n" + + " node6 [label=\"NAME\"];\n" + + " node5 -> node6 [weight=1];\n" + + " node5 -> node4 [weight=1];\n" + + " node4 -> node5 [label=\"UNCOND\", fontcolor=\"red\", weight=0.01, color=\"red\"];\n" + + " node7 [label=\"BLOCK\"];\n" + + " node5 -> node7 [weight=1];\n" + + " node8 [label=\"EXPR_RESULT\"];\n" + + " node7 -> node8 [weight=1];\n" + + " node9 [label=\"CALL\"];\n" + + " node8 -> node9 [weight=1];\n" + + " node10 [label=\"NAME\"];\n" + + " node9 -> node10 [weight=1];\n" + + " node8 -> node5 [label=\"UNCOND\", fontcolor=\"red\", weight=0.01, color=\"red\"];\n" + + " node7 -> node8 [label=\"UNCOND\", fontcolor=\"red\", weight=0.01, color=\"red\"];\n" + + " node11 [label=\"EMPTY\"];\n" + + " node5 -> node11 [label=\"ON_FALSE\", fontcolor=\"red\", weight=0.01, color=\"red\"];\n" + + " node5 -> node7 [label=\"ON_TRUE\", fontcolor=\"red\", weight=0.01, color=\"red\"];\n" + + " node0 -> node11 [weight=1];\n" + + " node11 -> RETURN [label=\"UNCOND\", fontcolor=\"red\", weight=0.01, color=\"red\"];\n" + + " node0 -> node1 [label=\"UNCOND\", fontcolor=\"red\", weight=0.01, color=\"red\"];\n" + + "}\n"; testCfg(src, expected); } @@ -1305,14 +1304,23 @@ public void testForLoopOrder() { public void testLabelledForInLoopOrder() { assertNodeOrder( - createCfg("var i = 0; var y = {}; " + - "label: for (var x in y) { " + - " if (x) { break label; } else { i++ } x(); }"), + createCfg( + "var i = 0; var y = {}; " + + "label: for (var x in y) { " + + " if (x) { break label; } else { i++ } x(); }"), ImmutableList.of( - Token.SCRIPT, Token.VAR, Token.VAR, Token.NAME, - Token.FOR, Token.BLOCK, - Token.IF, Token.BLOCK, Token.BREAK, - Token.BLOCK, Token.EXPR_RESULT, Token.EXPR_RESULT)); + Token.SCRIPT, + Token.VAR, + Token.VAR, + Token.NAME, + Token.FOR_IN, + Token.BLOCK, + Token.IF, + Token.BLOCK, + Token.BREAK, + Token.BLOCK, + Token.EXPR_RESULT, + Token.EXPR_RESULT)); } public void testLocalFunctionOrder() {