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/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;
}
}