Fix NodeTraversal's handling of control flow graph under ES6

Modify NodeTraversal to only push to the control flow graph stack for
function or global scopes.
-------------
Created by MOE: http://code.google.com/p/moe-java
MOE_MIGRATED_REVID=98782004

src/com/google/javascript/jscomp/ControlFlowAnalysis.java

@@ -146,6 +146,8 @@ ControlFlowGraph<Node> getCfg() {
 
   @Override
   public void process(Node externs, Node root) {
+    Preconditions.checkArgument(
+        NodeUtil.isValidCfgRoot(root), "Unexpected control flow graph root %s", root);
     this.root = root;
     astPositionCounter = 0;
     astPosition = new HashMap<>();
@@ -265,7 +267,7 @@ public boolean shouldTraverse(
         case Token.LABEL:
           return n != parent.getFirstChild();
         case Token.FUNCTION:
-          return n == parent.getFirstChild().getNext().getNext();
+          return n == parent.getLastChild();
         case Token.CONTINUE:
         case Token.BREAK:
         case Token.EXPR_RESULT:
@@ -528,9 +530,10 @@ private void handleStmtList(Node node) {
 
   private void handleFunction(Node node) {
     // A block transfer control to its first child if it is not empty.
-    Preconditions.checkState(node.getChildCount() >= 3);
+    Preconditions.checkState(node.isFunction());
+    Preconditions.checkState(node.getChildCount() == 3);
     createEdge(node, Branch.UNCOND,
-        computeFallThrough(node.getFirstChild().getNext().getNext()));
+        computeFallThrough(node.getLastChild()));
     Preconditions.checkState(exceptionHandler.peek() == node);
     exceptionHandler.pop();
   }

src/com/google/javascript/jscomp/FlowSensitiveInlineVariables.java

@@ -134,7 +134,7 @@ public void enterScope(NodeTraversal t) {
     ControlFlowAnalysis cfa = new ControlFlowAnalysis(compiler, false, true);
     // Process the body of the function.
     Preconditions.checkState(t.getScopeRoot().isFunction());
-    cfa.process(null, t.getScopeRoot().getLastChild());
+    cfa.process(null, t.getScopeRoot());
     cfg = cfa.getCfg();
     reachingDef = new MustBeReachingVariableDef(cfg, t.getScope(), compiler);
     reachingDef.analyze();

src/com/google/javascript/jscomp/InstrumentFunctions.java

@@ -220,12 +220,13 @@ private class InstrumentReturns implements NodeTraversal.Callback {
     }
 
     /**
-     * @param body  body of function with id == this.functionId
+     * @param function function with id == this.functionId
      */
-    void process(Node body) {
+    void process(Node function) {
+      Node body = function.getLastChild();
       NodeTraversal.traverse(compiler, body, this);
 
-      if (!allPathsReturn(body)) {
+      if (!allPathsReturn(function)) {
         Node call = newReportFunctionExitNode();
         Node expr = IR.exprResult(call);
         body.addChildToBack(expr);
@@ -264,11 +265,11 @@ private Node newReportFunctionExitNode() {
     /**
      * @return true if all paths from block must exit with an explicit return.
      */
-    private boolean allPathsReturn(Node block) {
+    private boolean allPathsReturn(Node function) {
       // Computes the control flow graph.
       ControlFlowAnalysis cfa = new ControlFlowAnalysis(
           compiler, false, false);
-      cfa.process(null, block);
+      cfa.process(null, function);
       ControlFlowGraph<Node> cfg = cfa.getCfg();
 
       Node returnPathsParent = cfg.getImplicitReturn().getValue();
@@ -297,7 +298,7 @@ public void visit(NodeTraversal t, Node n, Node parent) {
       }
 
       if (!reportFunctionName.isEmpty()) {
-        Node body = n.getFirstChild().getNext().getNext();
+        Node body = n.getLastChild();
         Node call = IR.call(
             IR.name(reportFunctionName),
             IR.number(id));
@@ -308,8 +309,7 @@ public void visit(NodeTraversal t, Node n, Node parent) {
       }
 
       if (!reportFunctionExitName.isEmpty()) {
-        Node body = n.getFirstChild().getNext().getNext();
-        (new InstrumentReturns(id)).process(body);
+        (new InstrumentReturns(id)).process(n);
       }
 
       if (!definedFunctionName.isEmpty()) {

src/com/google/javascript/jscomp/NodeTraversal.java

@@ -55,6 +55,12 @@ public class NodeTraversal {
    */
   private final Deque<Node> scopeRoots = new ArrayDeque<>();
 
+  /**
+   * A stack of scope roots that are valid cfg roots. All cfg roots that have not been created
+   * are represented in this Deque.
+   */
+  private final Deque<Node> cfgRoots = new ArrayDeque<>();
+
 
   /**
    * Stack of control flow graphs (CFG). There is one CFG per scope. CFGs
@@ -635,23 +641,18 @@ private void traverseBlockScope(Node n) {
 
   /** Examines the functions stack for the last instance of a function node. */
   public Node getEnclosingFunction() {
-    if (scopes.size() + scopeRoots.size() < 2) {
-      return null;
-    } else {
-      if (scopeRoots.isEmpty()) {
-        return scopes.peek().getRootNode();
-      } else {
-        return scopeRoots.peek();
-      }
-    }
+    return getScopeDepth() < 2 ? null : getCfgRoot();
   }
 
   /** Creates a new scope (e.g. when entering a function). */
   private void pushScope(Node node) {
     Preconditions.checkState(curNode != null);
     compiler.setScope(node);
     scopeRoots.push(node);
-    cfgs.push(null);
+    if (NodeUtil.isValidCfgRoot(node)) {
+      cfgRoots.push(node);
+      cfgs.push(null);
+    }
     if (scopeCallback != null) {
       scopeCallback.enterScope(this);
     }
@@ -670,7 +671,9 @@ private void pushScope(Scope s, boolean quietly) {
     Preconditions.checkState(curNode != null);
     compiler.setScope(s.getRootNode());
     scopes.push(s);
-    cfgs.push(null);
+    if (NodeUtil.isValidCfgRoot(s.getRootNode())) {
+      cfgs.push(null);
+    }
     if (!quietly && scopeCallback != null) {
       scopeCallback.enterScope(this);
     }
@@ -688,12 +691,18 @@ private void popScope(boolean quietly) {
     if (!quietly && scopeCallback != null) {
       scopeCallback.exitScope(this);
     }
+    Node scopeRoot;
     if (scopeRoots.isEmpty()) {
-      scopes.pop();
+      scopeRoot = scopes.pop().getRootNode();
     } else {
-      scopeRoots.pop();
+      scopeRoot = scopeRoots.pop();
+    }
+    if (NodeUtil.isValidCfgRoot(scopeRoot)) {
+      cfgs.pop();
+      if (!cfgRoots.isEmpty()) {
+        Preconditions.checkState(cfgRoots.pop() == scopeRoot);
+      }
     }
-    cfgs.pop();
     if (hasScope()) {
       compiler.setScope(getScopeRoot());
     }
@@ -712,6 +721,7 @@ public Scope getScope() {
       scopes.push(scope);
     }
     scopeRoots.clear();
+    cfgRoots.clear();
     // No need to call compiler.setScope; the top scopeRoot is now the top scope
     return scope;
   }
@@ -727,7 +737,7 @@ public TypedScope getTypedScope() {
   public ControlFlowGraph<Node> getControlFlowGraph() {
     if (cfgs.peek() == null) {
       ControlFlowAnalysis cfa = new ControlFlowAnalysis(compiler, false, true);
-      cfa.process(null, getScopeRoot());
+      cfa.process(null, getCfgRoot());
       cfgs.pop();
       cfgs.push(cfa.getCfg());
     }
@@ -743,6 +753,18 @@ public Node getScopeRoot() {
     }
   }
 
+  private Node getCfgRoot() {
+    if (cfgRoots.isEmpty()) {
+      Scope currScope = scopes.peek();
+      while (currScope.isBlockScope()) {
+        currScope = currScope.getParent();
+      }
+      return currScope.getRootNode();
+    } else {
+      return cfgRoots.peek();
+    }
+  }
+
   /**
    * Determines whether the traversal is currently in the global scope.
    */

src/com/google/javascript/jscomp/NodeUtil.java

@@ -2109,14 +2109,30 @@ static boolean createsBlockScope(Node n) {
           return false;
         }
         Node child = n.getFirstChild();
-        return child != null && !child.isScript(); // Optimization for empty blocks
+        return child != null && !child.isScript();
       case Token.FOR:
       case Token.FOR_OF:
         return true;
     }
     return false;
   }
 
+  static boolean isValidCfgRoot(Node n) {
+    switch (n.getType()) {
+      case Token.FUNCTION:
+      case Token.SCRIPT:
+        return true;
+      case Token.BLOCK:
+        // Only valid for top level synthetic block
+        if (n.getParent() == null
+            || n.getFirstChild() != null && n.getFirstChild().isScript()) {
+          return true;
+        }
+      default:
+        return false;
+    }
+  }
+
   /**
    * @return Whether the node is used as a statement.
    */

src/com/google/javascript/jscomp/TypedScope.java

@@ -264,7 +264,8 @@ void setTypeResolver(TypeResolver resolver) {
   }
 
   public boolean isBlockScope() {
-    throw new IllegalStateException("Method isBlockScope cannot be called on typed scopes.");
+    // TypedScope is not ES6 compatible yet, so always return false for now.
+    return false;
   }
 
   public boolean isHoistScope() {

test/com/google/javascript/jscomp/CheckUnreachableCodeTest.java

@@ -257,9 +257,13 @@ public void testInCorrectForOfBreakAndContinues() {
     assertUnreachable("for(x of [1, 2, 3]) {foo(); continue; bar();}");
 
     assertUnreachable("for(x of [1, 2, 3]) {if(x) {break; bar();}}");
-    //TODO(user): ES6 scope creator could not correctly handle its scope yet.
-    //                Enable the test after functional scope creator is in place.
-    //assertUnreachable("for(x of [1, 2, 3]) {if(x) {continue; bar();}}");
+    assertUnreachable("for(x of [1, 2, 3]) {if(x) {continue; bar();}}");
+  }
+
+  public void testForLoopsEs6() {
+    setAcceptedLanguage(LanguageMode.ECMASCRIPT6);
+    assertUnreachable("for(;;) {if(x) {continue; bar();}}");
+    assertUnreachable("for(x in y) {if(x) {continue; bar();}}");
   }
 
   public void testReturnsInShorthandFunctionOfObjLit() {