Cleanup in the parser
- Move break/continue check from ValidationEnvironment to the Parser
- Remove some differences between BUILD / Skylark parsing mode
- Fix location off-by-one error in the break/continue tokens
- Remove duplicated error message ('for loops are not allowed on top-level')
--
MOS_MIGRATED_REVID=137259929
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 e498b9a..15f1ee2 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
@@ -58,11 +58,7 @@
}
@Override
- void validate(ValidationEnvironment env) throws EvalException {
- if (!env.isInsideLoop()) {
- throw new EvalException(getLocation(), kind.name + " statement must be inside a for loop");
- }
- }
+ void validate(ValidationEnvironment env) throws EvalException {}
@Override
public String toString() {
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/ForStatement.java b/src/main/java/com/google/devtools/build/lib/syntax/ForStatement.java
index b641be3..97ac506 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/ForStatement.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/ForStatement.java
@@ -30,16 +30,14 @@
import com.google.devtools.build.lib.syntax.compiler.Variable.InternalVariable;
import com.google.devtools.build.lib.syntax.compiler.VariableScope;
import com.google.devtools.build.lib.util.Preconditions;
-
+import java.util.ArrayList;
+import java.util.Iterator;
+import java.util.List;
import net.bytebuddy.description.type.TypeDescription;
import net.bytebuddy.implementation.bytecode.ByteCodeAppender;
import net.bytebuddy.implementation.bytecode.Duplication;
import net.bytebuddy.implementation.bytecode.constant.IntegerConstant;
-import java.util.ArrayList;
-import java.util.Iterator;
-import java.util.List;
-
/**
* Syntax node for a for loop statement.
*/
@@ -111,21 +109,12 @@
@Override
void validate(ValidationEnvironment env) throws EvalException {
- if (env.isTopLevel()) {
- throw new EvalException(getLocation(), "'For' is not allowed as a top level statement");
- }
- env.enterLoop();
+ // TODO(bazel-team): validate variable. Maybe make it temporarily readonly.
+ collection.validate(env);
+ variable.validate(env, getLocation());
- try {
- // TODO(bazel-team): validate variable. Maybe make it temporarily readonly.
- collection.validate(env);
- variable.validate(env, getLocation());
-
- for (Statement stmt : block) {
- stmt.validate(env);
- }
- } finally {
- env.exitLoop(getLocation());
+ for (Statement stmt : block) {
+ stmt.validate(env);
}
}
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 4101062..8b28b3e 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
@@ -135,6 +135,7 @@
private Token token; // current lookahead token
private Token pushedToken = null; // used to implement LL(2)
+ private int loopCount; // break/continue keywords can be used only inside a loop
private static final boolean DEBUGGING = false;
@@ -1171,8 +1172,7 @@
int start = token.left;
if (token.kind == TokenKind.RETURN) {
return parseReturnStatement();
- } else if ((parsingMode == SKYLARK)
- && (token.kind == TokenKind.BREAK || token.kind == TokenKind.CONTINUE)) {
+ } else if (token.kind == TokenKind.BREAK || token.kind == TokenKind.CONTINUE) {
return parseFlowStatement(token.kind);
}
Expression expression = parseExpression();
@@ -1232,9 +1232,14 @@
expect(TokenKind.IN);
Expression collection = parseExpression();
expect(TokenKind.COLON);
- List<Statement> block = parseSuite();
- Statement stmt = new ForStatement(loopVar, collection, block);
- list.add(setLocation(stmt, start, token.right));
+ enterLoop();
+ try {
+ List<Statement> block = parseSuite();
+ Statement stmt = new ForStatement(loopVar, collection, block);
+ list.add(setLocation(stmt, start, token.right));
+ } finally {
+ exitLoop();
+ }
}
// def foo(bar1, bar2):
@@ -1401,10 +1406,16 @@
// flow_stmt ::= break_stmt | continue_stmt
private FlowStatement parseFlowStatement(TokenKind kind) {
int start = token.left;
+ int end = token.right;
expect(kind);
+ if (loopCount == 0) {
+ reportError(
+ lexer.createLocation(start, end),
+ kind.getPrettyName() + " statement must be inside a for loop");
+ }
FlowStatement.Kind flowKind =
kind == TokenKind.BREAK ? FlowStatement.Kind.BREAK : FlowStatement.Kind.CONTINUE;
- return setLocation(new FlowStatement(flowKind), start, token.right);
+ return setLocation(new FlowStatement(flowKind), start, end);
}
// return_stmt ::= RETURN [expr]
@@ -1429,7 +1440,7 @@
int start = token.left;
Token blockToken = token;
syncTo(EnumSet.of(TokenKind.COLON, TokenKind.EOF)); // skip over expression or name
- if (blockToken.kind == TokenKind.ELSE && parsingMode == SKYLARK) {
+ if (blockToken.kind == TokenKind.ELSE) {
reportError(
lexer.createLocation(blockToken.left, blockToken.right),
"syntax error at 'else': not allowed here.");
@@ -1450,4 +1461,13 @@
private void makeComment(Token token) {
comments.add(setLocation(new Comment((String) token.value), token.left, token.right));
}
+
+ private void enterLoop() {
+ loopCount++;
+ }
+
+ private void exitLoop() {
+ Preconditions.checkState(loopCount > 0);
+ loopCount--;
+ }
}
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 095b46f..c1a94c5 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
@@ -46,12 +46,6 @@
private final Stack<Set<String>> futureReadOnlyVariables = new Stack<>();
/**
- * Tracks the number of nested for loops that contain the statement that is currently being
- * validated
- */
- private int loopCount = 0;
-
- /**
* Create a ValidationEnvironment for a given global Environment.
*/
public ValidationEnvironment(Environment env) {
@@ -169,33 +163,4 @@
return false;
}
}
-
- /**
- * Returns whether the current statement is inside a for loop (either in this environment or one
- * of its parents)
- *
- * @return True if the current statement is inside a for loop
- */
- public boolean isInsideLoop() {
- return (loopCount > 0);
- }
-
- /**
- * Signals that the block of a for loop was entered
- */
- public void enterLoop() {
- ++loopCount;
- }
-
- /**
- * Signals that the block of a for loop was left
- *
- * @param location The current location
- * @throws EvalException If there was no corresponding call to
- * {@code ValidationEnvironment#enterLoop}
- */
- public void exitLoop(Location location) throws EvalException {
- Preconditions.checkState(loopCount > 0);
- --loopCount;
- }
}
