Automated rollback of commit e114d8a04fe07dd32fee41b3cd2e5794f10a5a9d.

*** Reason for rollback ***

TAP has detected 10 or more targets failed to build at https://github.com/bazelbuild/bazel/commit/e114d8a04fe07dd32fee41b3cd2e5794f10a5a9d.

TO ROLLFORWARD (without additional approval): Use[]

To see all broken targets visit []
To prevent noise from flakes, TAP double-checked the following target fails to build:
[]
but used to build fine:
[]

Questions? Comments? See the URL:[]

*** Original change description ***

bazel syntax: don't use exceptions for control flow

Previously, break, continue, and return were implemented by
throwing and catching exceptions.

Now, the Eval.exec methods return a token to indicate the
continuation: one of PASS, BREAK, CONTINUE, or RETURN.
In addition, the return value is saved in the Eval,
which is good for only one function invocation.

This is simpler (and slightly more efficient).

Also:
- FlowStatement now includes PASS too; PassStatement is gone.
- ReturnException is g...

***

PiperOrigin-RevId: 255883540
diff --git a/src/main/java/com/google/devtools/build/lib/skylarkdebug/server/SkylarkDebugServer.java b/src/main/java/com/google/devtools/build/lib/skylarkdebug/server/SkylarkDebugServer.java
index b546fa6f..d87bfda 100644
--- a/src/main/java/com/google/devtools/build/lib/skylarkdebug/server/SkylarkDebugServer.java
+++ b/src/main/java/com/google/devtools/build/lib/skylarkdebug/server/SkylarkDebugServer.java
@@ -26,7 +26,6 @@
 import com.google.devtools.build.lib.syntax.Eval;
 import com.google.devtools.build.lib.syntax.EvalException;
 import com.google.devtools.build.lib.syntax.Statement;
-import com.google.devtools.build.lib.syntax.TokenKind;
 import java.io.IOException;
 import java.net.ServerSocket;
 import java.util.List;
@@ -255,9 +254,9 @@
     }
 
     @Override
-    protected TokenKind exec(Statement st) throws EvalException, InterruptedException {
+    public void exec(Statement st) throws EvalException, InterruptedException {
       pauseIfNecessary(env, st.getLocation());
-      return super.exec(st);
+      super.exec(st);
     }
   }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Environment.java b/src/main/java/com/google/devtools/build/lib/syntax/Environment.java
index 8348d60..4a6d014 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/Environment.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/Environment.java
@@ -56,7 +56,9 @@
  * respect to this {@code Environment}.
  *
  * <p>{@link Continuation}-s are explicitly represented, but only partly, with another part being
- * implicit in a series of try-catch statements, to maintain the direct style.
+ * implicit in a series of try-catch statements, to maintain the direct style. One notable trick is
+ * how a {@link UserDefinedFunction} implements returning values as the function catching a {@link
+ * ReturnStatement.ReturnException} thrown by a {@link ReturnStatement} in the body.
  *
  * <p>Every {@code Environment} has a {@link Mutability} field, and must be used within a function
  * that creates and closes this {@link Mutability} with the try-with-resource pattern. This {@link
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Eval.java b/src/main/java/com/google/devtools/build/lib/syntax/Eval.java
index f8e1b6d..7a4273b 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/Eval.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/Eval.java
@@ -25,7 +25,18 @@
  */
 public class Eval {
   protected final Environment env;
-  private Object result = Runtime.NONE;
+
+  /** An exception that signals changes in the control flow (e.g. break or continue) */
+  private static class FlowException extends EvalException {
+    FlowException(String message) {
+      super(null, message);
+    }
+
+    @Override
+    public boolean canBeAddedToStackTrace() {
+      return false;
+    }
+  }
 
   public static Eval fromEnvironment(Environment env) {
     return evalSupplier.apply(env);
@@ -43,6 +54,10 @@
   // TODO(bazel-team): remove this static state in favor of storing Eval instances in Environment
   private static Function<Environment, Eval> evalSupplier = Eval::new;
 
+  private static final FlowException breakException = new FlowException("FlowException - break");
+  private static final FlowException continueException =
+      new FlowException("FlowException - continue");
+
   /**
    * This constructor should never be called directly. Call {@link #fromEnvironment(Environment)}
    * instead.
@@ -51,11 +66,6 @@
     this.env = env;
   }
 
-  /** getResult returns the value returned by executing a ReturnStatement. */
-  Object getResult() {
-    return this.result;
-  }
-
   void execAssignment(AssignmentStatement node) throws EvalException, InterruptedException {
     Object rvalue = node.getExpression().eval(env);
     node.getLValue().assign(rvalue, env, node.getLocation());
@@ -67,12 +77,12 @@
         .assignAugmented(node.getOperator(), node.getExpression(), env, node.getLocation());
   }
 
-  TokenKind execIfBranch(IfStatement.ConditionalStatements node)
+  void execIfBranch(IfStatement.ConditionalStatements node)
       throws EvalException, InterruptedException {
-    return execStatements(node.getStatements());
+    execStatements(node.getStatements());
   }
 
-  TokenKind execFor(ForStatement node) throws EvalException, InterruptedException {
+  void execFor(ForStatement node) throws EvalException, InterruptedException {
     Object o = node.getCollection().eval(env);
     Iterable<?> col = EvalUtils.toIterable(o, node.getLocation(), env);
     EvalUtils.lock(o, node.getLocation());
@@ -80,25 +90,17 @@
       for (Object it : col) {
         node.getVariable().assign(it, env, node.getLocation());
 
-        switch (execStatements(node.getBlock())) {
-          case PASS:
-          case CONTINUE:
-            // Stay in loop.
-            continue;
-          case BREAK:
-            // Finish loop, execute next statement after loop.
-            return TokenKind.PASS;
-          case RETURN:
-            // Finish loop, return from function.
-            return TokenKind.RETURN;
-          default:
-            throw new IllegalStateException("unreachable");
+        try {
+          execStatements(node.getBlock());
+        } catch (FlowException ex) {
+          if (ex == breakException) {
+            return;
+          }
         }
       }
     } finally {
       EvalUtils.unlock(o, node.getLocation());
     }
-    return TokenKind.PASS;
   }
 
   void execDef(FunctionDefStatement node) throws EvalException, InterruptedException {
@@ -128,16 +130,17 @@
             env.getGlobals()));
   }
 
-  TokenKind execIf(IfStatement node) throws EvalException, InterruptedException {
+  void execIf(IfStatement node) throws EvalException, InterruptedException {
     ImmutableList<IfStatement.ConditionalStatements> thenBlocks = node.getThenBlocks();
     // Avoid iterator overhead - most of the time there will be one or few "if"s.
     for (int i = 0; i < thenBlocks.size(); i++) {
       IfStatement.ConditionalStatements stmt = thenBlocks.get(i);
       if (EvalUtils.toBoolean(stmt.getCondition().eval(env))) {
-        return exec(stmt);
+        exec(stmt);
+        return;
       }
     }
-    return execStatements(node.getElseBlock());
+    execStatements(node.getElseBlock());
   }
 
   void execLoad(LoadStatement node) throws EvalException, InterruptedException {
@@ -160,12 +163,12 @@
     }
   }
 
-  TokenKind execReturn(ReturnStatement node) throws EvalException, InterruptedException {
+  void execReturn(ReturnStatement node) throws EvalException, InterruptedException {
     Expression ret = node.getReturnExpression();
-    if (ret != null) {
-      this.result = ret.eval(env);
+    if (ret == null) {
+      throw new ReturnStatement.ReturnException(node.getLocation(), Runtime.NONE);
     }
-    return TokenKind.RETURN;
+    throw new ReturnStatement.ReturnException(ret.getLocation(), ret.eval(env));
   }
 
   /**
@@ -174,54 +177,57 @@
    * @throws EvalException if execution of the statement could not be completed.
    * @throws InterruptedException may be thrown in a sub class.
    */
-  protected TokenKind exec(Statement st) throws EvalException, InterruptedException {
+  public void exec(Statement st) throws EvalException, InterruptedException {
     try {
-      return execDispatch(st);
+      execDispatch(st);
     } catch (EvalException ex) {
       throw st.maybeTransformException(ex);
     }
   }
 
-  TokenKind execDispatch(Statement st) throws EvalException, InterruptedException {
+  void execDispatch(Statement st) throws EvalException, InterruptedException {
     switch (st.kind()) {
       case ASSIGNMENT:
         execAssignment((AssignmentStatement) st);
-        return TokenKind.PASS;
+        break;
       case AUGMENTED_ASSIGNMENT:
         execAugmentedAssignment((AugmentedAssignmentStatement) st);
-        return TokenKind.PASS;
+        break;
       case CONDITIONAL:
-        return execIfBranch((IfStatement.ConditionalStatements) st);
+        execIfBranch((IfStatement.ConditionalStatements) st);
+        break;
       case EXPRESSION:
         ((ExpressionStatement) st).getExpression().eval(env);
-        return TokenKind.PASS;
+        break;
       case FLOW:
-        return ((FlowStatement) st).getKind();
+        throw ((FlowStatement) st).getKind() == FlowStatement.Kind.BREAK
+            ? breakException
+            : continueException;
       case FOR:
-        return execFor((ForStatement) st);
+        execFor((ForStatement) st);
+        break;
       case FUNCTION_DEF:
         execDef((FunctionDefStatement) st);
-        return TokenKind.PASS;
+        break;
       case IF:
-        return execIf((IfStatement) st);
+        execIf((IfStatement) st);
+        break;
       case LOAD:
         execLoad((LoadStatement) st);
-        return TokenKind.PASS;
+        break;
+      case PASS:
+        break;
       case RETURN:
-        return execReturn((ReturnStatement) st);
+        execReturn((ReturnStatement) st);
+        break;
     }
-    throw new IllegalArgumentException("unexpected statement: " + st.kind());
   }
 
-  public TokenKind execStatements(ImmutableList<Statement> statements)
+  private void execStatements(ImmutableList<Statement> statements)
       throws EvalException, InterruptedException {
     // Hot code path, good chance of short lists which don't justify the iterator overhead.
     for (int i = 0; i < statements.size(); i++) {
-      TokenKind flow = exec(statements.get(i));
-      if (flow != TokenKind.PASS) {
-        return flow;
-      }
+      exec(statements.get(i));
     }
-    return TokenKind.PASS;
   }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/EvalException.java b/src/main/java/com/google/devtools/build/lib/syntax/EvalException.java
index cefb389..d0f94ff 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/EvalException.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/EvalException.java
@@ -67,7 +67,25 @@
    * @param dueToIncompleteAST if the error is caused by a previous error, such as parsing.
    */
   public EvalException(Location location, String message, boolean dueToIncompleteAST) {
-    super(null, null, /*enableSuppression=*/ true, /*writableStackTrace=*/ true);
+    this(location, message, dueToIncompleteAST, true);
+  }
+
+  /**
+   * Create an EvalException with the option to not fill in the java stack trace. An optimization
+   * for ReturnException, and potentially others, which aren't exceptional enough to include a
+   * stack trace.
+   *
+   * @param location the location where evaluation/execution failed.
+   * @param message the error message.
+   * @param dueToIncompleteAST if the error is caused by a previous error, such as parsing.
+   * @param fillInJavaStackTrace whether or not to fill in the java stack trace for this exception
+   */
+  EvalException(
+      Location location,
+      String message,
+      boolean dueToIncompleteAST,
+      boolean fillInJavaStackTrace) {
+    super(null, null, /*enableSuppression=*/ true, fillInJavaStackTrace);
     this.location = location;
     this.message = Preconditions.checkNotNull(message);
     this.dueToIncompleteAST = dueToIncompleteAST;
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/FlowStatement.java b/src/main/java/com/google/devtools/build/lib/syntax/FlowStatement.java
index 4477b91..b060b78 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/FlowStatement.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/FlowStatement.java
@@ -15,29 +15,48 @@
 
 import java.io.IOException;
 
-/** A class for flow statements (e.g. break, continue, and pass) */
+/** A class for flow statements (e.g. break and continue) */
 public final class FlowStatement extends Statement {
+  // TODO(laurentlb): This conflicts with Statement.Kind, maybe remove it?
+  public enum Kind {
+    BREAK("break"),
+    CONTINUE("continue");
 
-  private final TokenKind kind; // BREAK | CONTINUE | PASS
+    private final String name;
 
-  /** @param kind The label of the statement (break, continue, or pass) */
-  public FlowStatement(TokenKind kind) {
+    private Kind(String name) {
+      this.name = name;
+    }
+
+    public String getName() {
+      return name;
+    }
+  }
+
+  private final Kind kind;
+
+  /**
+   *
+   * @param kind The label of the statement (either break or continue)
+   */
+  public FlowStatement(Kind kind) {
     this.kind = kind;
   }
 
-  public TokenKind getKind() {
+  public Kind getKind() {
     return kind;
   }
 
   @Override
   public void prettyPrint(Appendable buffer, int indentLevel) throws IOException {
     printIndent(buffer, indentLevel);
-    buffer.append(kind.getPrettyName()).append('\n');
+    buffer.append(kind.name);
+    buffer.append('\n');
   }
 
   @Override
   public String toString() {
-    return kind.getPrettyName() + "\n";
+    return kind.name + "\n";
   }
 
   @Override
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Parser.java b/src/main/java/com/google/devtools/build/lib/syntax/Parser.java
index 169b731..fad276c 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/Parser.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/Parser.java
@@ -1126,16 +1126,26 @@
     }
   }
 
+  // small_stmt | 'pass'
+  private void parseSmallStatementOrPass(List<Statement> list) {
+    if (token.kind == TokenKind.PASS) {
+      list.add(setLocation(new PassStatement(), token.left, token.right));
+      expect(TokenKind.PASS);
+    } else {
+      list.add(parseSmallStatement());
+    }
+  }
+
   // simple_stmt ::= small_stmt (';' small_stmt)* ';'? NEWLINE
   private void parseSimpleStatement(List<Statement> list) {
-    list.add(parseSmallStatement());
+    parseSmallStatementOrPass(list);
 
     while (token.kind == TokenKind.SEMI) {
       nextToken();
       if (token.kind == TokenKind.NEWLINE) {
         break;
       }
-      list.add(parseSmallStatement());
+      parseSmallStatementOrPass(list);
     }
     expectAndRecover(TokenKind.NEWLINE);
   }
@@ -1143,7 +1153,7 @@
   //     small_stmt ::= assign_stmt
   //                  | expr
   //                  | return_stmt
-  //                  | BREAK | CONTINUE | PASS
+  //                  | flow_stmt
   //     assign_stmt ::= expr ('=' | augassign) expr
   //     augassign ::= ('+=' | '-=' | '*=' | '/=' | '%=' | '//=' )
   // Note that these are in Python, but not implemented here (at least for now):
@@ -1152,13 +1162,8 @@
     int start = token.left;
     if (token.kind == TokenKind.RETURN) {
       return parseReturnStatement();
-    } else if (token.kind == TokenKind.BREAK
-        || token.kind == TokenKind.CONTINUE
-        || token.kind == TokenKind.PASS) {
-      TokenKind kind = token.kind;
-      int end = token.right;
-      expect(kind);
-      return setLocation(new FlowStatement(kind), start, end);
+    } else if (token.kind == TokenKind.BREAK || token.kind == TokenKind.CONTINUE) {
+      return parseFlowStatement(token.kind);
     }
     Expression expression = parseExpression();
     if (token.kind == TokenKind.EQUALS) {
@@ -1328,6 +1333,16 @@
     return list;
   }
 
+  // flow_stmt ::= BREAK | CONTINUE
+  private FlowStatement parseFlowStatement(TokenKind kind) {
+    int start = token.left;
+    int end = token.right;
+    expect(kind);
+    FlowStatement.Kind flowKind =
+        kind == TokenKind.BREAK ? FlowStatement.Kind.BREAK : FlowStatement.Kind.CONTINUE;
+    return setLocation(new FlowStatement(flowKind), start, end);
+  }
+
   // return_stmt ::= RETURN [expr]
   private ReturnStatement parseReturnStatement() {
     int start = token.left;
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/PassStatement.java b/src/main/java/com/google/devtools/build/lib/syntax/PassStatement.java
new file mode 100644
index 0000000..e035100
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/syntax/PassStatement.java
@@ -0,0 +1,37 @@
+// Copyright 2017 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.devtools.build.lib.syntax;
+
+import java.io.IOException;
+
+/** Syntax node for a `pass` statement. */
+public class PassStatement extends Statement {
+
+  @Override
+  public void prettyPrint(Appendable buffer, int indentLevel) throws IOException {
+    printIndent(buffer, indentLevel);
+    buffer.append("pass\n");
+  }
+
+  @Override
+  public void accept(SyntaxTreeVisitor visitor) {
+    visitor.visit(this);
+  }
+
+  @Override
+  public Kind kind() {
+    return Kind.PASS;
+  }
+}
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/ReturnStatement.java b/src/main/java/com/google/devtools/build/lib/syntax/ReturnStatement.java
index 6f249cf..4d9c2ad 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/ReturnStatement.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/ReturnStatement.java
@@ -13,12 +13,38 @@
 // limitations under the License.
 package com.google.devtools.build.lib.syntax;
 
+import com.google.devtools.build.lib.events.Location;
 import java.io.IOException;
 import javax.annotation.Nullable;
 
 /** A wrapper Statement class for return expressions. */
 public final class ReturnStatement extends Statement {
 
+  /**
+   * Exception sent by the return statement, to be caught by the function body.
+   */
+  public static class ReturnException extends EvalException {
+    private final Object value;
+
+    public ReturnException(Location location, Object value) {
+      super(
+          location,
+          "return statements must be inside a function",
+          /*dueToIncompleteAST=*/false,
+          /*fillInJavaStackTrace=*/false);
+      this.value = value;
+    }
+
+    public Object getValue() {
+      return value;
+    }
+
+    @Override
+    public boolean canBeAddedToStackTrace() {
+      return false;
+    }
+  }
+
   @Nullable private final Expression returnExpression;
 
   public ReturnStatement(@Nullable Expression returnExpression) {
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Statement.java b/src/main/java/com/google/devtools/build/lib/syntax/Statement.java
index 1f08bbb..43363fc 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/Statement.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/Statement.java
@@ -33,6 +33,7 @@
     FUNCTION_DEF,
     IF,
     LOAD,
+    PASS,
     RETURN,
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SyntaxTreeVisitor.java b/src/main/java/com/google/devtools/build/lib/syntax/SyntaxTreeVisitor.java
index 523177d..117bbf0 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/SyntaxTreeVisitor.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/SyntaxTreeVisitor.java
@@ -143,13 +143,16 @@
     visitBlock(node.getStatements());
   }
 
+  public void visit(PassStatement node) {}
+
   public void visit(ReturnStatement node) {
     if (node.getReturnExpression() != null) {
       visit(node.getReturnExpression());
     }
   }
 
-  public void visit(FlowStatement node) {}
+  public void visit(FlowStatement node) {
+  }
 
   public void visit(DictionaryLiteral node) {
     visitAll(node.getEntries());
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/UserDefinedFunction.java b/src/main/java/com/google/devtools/build/lib/syntax/UserDefinedFunction.java
index 5bc2da9..460bc35 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/UserDefinedFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/UserDefinedFunction.java
@@ -77,8 +77,24 @@
       }
 
       Eval eval = Eval.fromEnvironment(env);
-      eval.execStatements(statements);
-      return eval.getResult();
+      try {
+        for (Statement stmt : statements) {
+          if (stmt instanceof ReturnStatement) {
+            // Performance optimization.
+            // Executing the statement would throw an exception, which is slow.
+            Expression returnExpr = ((ReturnStatement) stmt).getReturnExpression();
+            if (returnExpr == null) {
+              return Runtime.NONE;
+            }
+            return returnExpr.eval(env);
+          } else {
+            eval.exec(stmt);
+          }
+        }
+      } catch (ReturnStatement.ReturnException e) {
+        return e.getValue();
+      }
+      return Runtime.NONE;
     } finally {
       env.exitScope();
     }
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/ValidationEnvironment.java b/src/main/java/com/google/devtools/build/lib/syntax/ValidationEnvironment.java
index c410aaa..ada8a87 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/ValidationEnvironment.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/ValidationEnvironment.java
@@ -144,6 +144,7 @@
       case CONDITIONAL:
       case EXPRESSION:
       case FLOW:
+      case PASS:
       case RETURN:
         // nothing to declare
     }
@@ -212,10 +213,9 @@
 
   @Override
   public void visit(FlowStatement node) {
-    if (node.getKind() != TokenKind.PASS && loopCount <= 0) {
+    if (loopCount <= 0) {
       throw new ValidationException(
-          node.getLocation(),
-          node.getKind().getPrettyName() + " statement must be inside a for loop");
+          node.getLocation(), node.getKind().getName() + " statement must be inside a for loop");
     }
     super.visit(node);
   }
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/ASTPrettyPrintTest.java b/src/test/java/com/google/devtools/build/lib/syntax/ASTPrettyPrintTest.java
index 91b67da..262e4e4 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/ASTPrettyPrintTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/ASTPrettyPrintTest.java
@@ -291,11 +291,11 @@
   @Test
   public void flowStatement() {
     // The parser would complain if we tried to construct them from source.
-    ASTNode breakNode = new FlowStatement(TokenKind.BREAK);
+    ASTNode breakNode = new FlowStatement(FlowStatement.Kind.BREAK);
     assertIndentedPrettyMatches(breakNode, "  break\n");
     assertTostringMatches(breakNode, "break\n");
 
-    ASTNode continueNode = new FlowStatement(TokenKind.CONTINUE);
+    ASTNode continueNode = new FlowStatement(FlowStatement.Kind.CONTINUE);
     assertIndentedPrettyMatches(continueNode, "  continue\n");
     assertTostringMatches(continueNode, "continue\n");
   }
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/ParserTest.java b/src/test/java/com/google/devtools/build/lib/syntax/ParserTest.java
index 28d7a2a..15bbe94 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/ParserTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/ParserTest.java
@@ -608,33 +608,28 @@
   }
 
   @Test
-  public void testForBreakContinuePass() throws Exception {
-    List<Statement> file =
-        parseFileForSkylark(
-            "def foo():",
-            "  for i in [1, 2]:",
-            "    break",
-            "    continue",
-            "    pass",
-            "    break");
+  public void testForBreakContinue() throws Exception {
+    List<Statement> file = parseFileForSkylark(
+        "def foo():",
+        "  for i in [1, 2]:",
+        "    break",
+        "    continue",
+        "    break");
     assertThat(file).hasSize(1);
     List<Statement> body = ((FunctionDefStatement) file.get(0)).getStatements();
     assertThat(body).hasSize(1);
 
     List<Statement> loop = ((ForStatement) body.get(0)).getBlock();
-    assertThat(loop).hasSize(4);
+    assertThat(loop).hasSize(3);
 
-    assertThat(((FlowStatement) loop.get(0)).getKind()).isEqualTo(TokenKind.BREAK);
+    assertThat(((FlowStatement) loop.get(0)).getKind()).isEqualTo(FlowStatement.Kind.BREAK);
     assertLocation(34, 39, loop.get(0).getLocation());
 
-    assertThat(((FlowStatement) loop.get(1)).getKind()).isEqualTo(TokenKind.CONTINUE);
+    assertThat(((FlowStatement) loop.get(1)).getKind()).isEqualTo(FlowStatement.Kind.CONTINUE);
     assertLocation(44, 52, loop.get(1).getLocation());
 
-    assertThat(((FlowStatement) loop.get(2)).getKind()).isEqualTo(TokenKind.PASS);
-    assertLocation(57, 61, loop.get(2).getLocation());
-
-    assertThat(((FlowStatement) loop.get(3)).getKind()).isEqualTo(TokenKind.BREAK);
-    assertLocation(66, 71, loop.get(3).getLocation());
+    assertThat(((FlowStatement) loop.get(2)).getKind()).isEqualTo(FlowStatement.Kind.BREAK);
+    assertLocation(57, 62, loop.get(2).getLocation());
   }
 
   @Test
@@ -1019,6 +1014,13 @@
   }
 
   @Test
+  public void testPass() throws Exception {
+    List<Statement> statements = parseFileForSkylark("pass\n");
+    assertThat(statements).hasSize(1);
+    assertThat(statements.get(0)).isInstanceOf(PassStatement.class);
+  }
+
+  @Test
   public void testForPass() throws Exception {
     List<Statement> statements = parseFileForSkylark(
         "def foo():",
@@ -1026,7 +1028,7 @@
 
     assertThat(statements).hasSize(1);
     FunctionDefStatement stmt = (FunctionDefStatement) statements.get(0);
-    assertThat(stmt.getStatements().get(0)).isInstanceOf(FlowStatement.class);
+    assertThat(stmt.getStatements().get(0)).isInstanceOf(PassStatement.class);
   }
 
   @Test