Parser: Allow more complex expressions as for loop variables.
Also, use LValue in ForStatement.
--
MOS_MIGRATED_REVID=89122760
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 76a46ee..2faf06c 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
@@ -23,20 +23,20 @@
*/
public final class ForStatement extends Statement {
- private final Ident variable;
+ private final LValue variable;
private final Expression collection;
private final ImmutableList<Statement> block;
/**
* Constructs a for loop statement.
*/
- ForStatement(Ident variable, Expression collection, List<Statement> block) {
- this.variable = Preconditions.checkNotNull(variable);
+ ForStatement(Expression variable, Expression collection, List<Statement> block) {
+ this.variable = new LValue(Preconditions.checkNotNull(variable));
this.collection = Preconditions.checkNotNull(collection);
this.block = ImmutableList.copyOf(block);
}
- public Ident getVariable() {
+ public LValue getVariable() {
return variable;
}
@@ -62,7 +62,7 @@
int i = 0;
for (Object it : ImmutableList.copyOf(col)) {
- env.update(variable.getName(), it);
+ variable.assign(env, getLocation(), it);
for (Statement stmt : block) {
stmt.exec(env);
}
@@ -89,7 +89,7 @@
// TODO(bazel-team): validate variable. Maybe make it temporarily readonly.
SkylarkType type = collection.validate(env);
env.checkIterable(type, getLocation());
- env.update(variable.getName(), SkylarkType.UNKNOWN, getLocation());
+ variable.validate(env, getLocation(), SkylarkType.UNKNOWN);
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 d736a27..187f11b 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
@@ -705,37 +705,28 @@
start, token.right);
}
- // loop_variables ::= '(' variables ')'
- // | variables
- // variables ::= ident (',' ident)*
- private Ident parseForLoopVariables() {
+ // Equivalent to 'exprlist' rule in Python grammar.
+ // loop_variables ::= primary_with_suffix ( ',' primary_with_suffix )* ','?
+ private Expression parseForLoopVariables() {
+ // We cannot reuse parseExpression because it would parse the 'in' operator.
+ // e.g. "for i in e: pass" -> we want to parse only "i" here.
int start = token.left;
- boolean hasParen = false;
- if (token.kind == TokenKind.LPAREN) {
- hasParen = true;
- nextToken();
+ Expression e1 = parsePrimaryWithSuffix();
+ if (token.kind != TokenKind.COMMA) {
+ return e1;
}
- // TODO(bazel-team): allow multiple variables in the core Blaze language too.
- Ident firstIdent = parseIdent();
- boolean multipleVariables = false;
-
+ // It's a tuple
+ List<Expression> tuple = new ArrayList<>();
+ tuple.add(e1);
while (token.kind == TokenKind.COMMA) {
- multipleVariables = true;
- nextToken();
- parseIdent();
+ expect(TokenKind.COMMA);
+ if (EXPR_LIST_TERMINATOR_SET.contains(token.kind)) {
+ break;
+ }
+ tuple.add(parsePrimaryWithSuffix());
}
-
- if (hasParen) {
- expect(TokenKind.RPAREN);
- }
-
- int end = token.right;
- if (multipleVariables && !parsePython) {
- reportError(lexer.createLocation(start, end),
- "For loops with multiple variables are not yet supported");
- }
- return multipleVariables ? makeErrorExpression(start, end) : firstIdent;
+ return setLocation(ListLiteral.makeTuple(tuple), start, token.right);
}
// list_maker ::= '[' ']'
@@ -766,11 +757,11 @@
new ListComprehension(expression);
do {
nextToken();
- Ident ident = parseForLoopVariables();
+ Expression loopVar = parseForLoopVariables();
if (token.kind == TokenKind.IN) {
nextToken();
Expression listExpression = parseExpression();
- listComprehension.add(ident, listExpression);
+ listComprehension.add(loopVar, listExpression);
} else {
break;
}
@@ -824,7 +815,7 @@
if (token.kind == TokenKind.FOR) {
// Dict comprehension
nextToken();
- Ident loopVar = parseForLoopVariables();
+ Expression loopVar = parseForLoopVariables();
expect(TokenKind.IN);
Expression listExpression = parseExpression();
expect(TokenKind.RBRACE);
@@ -1144,12 +1135,12 @@
private void parseForStatement(List<Statement> list) {
int start = token.left;
expect(TokenKind.FOR);
- Ident ident = parseIdent();
+ Expression loopVar = parseForLoopVariables();
expect(TokenKind.IN);
Expression collection = parseExpression();
expect(TokenKind.COLON);
List<Statement> block = parseSuite();
- Statement stmt = new ForStatement(ident, collection, block);
+ Statement stmt = new ForStatement(loopVar, collection, block);
list.add(setLocation(stmt, start, token.right));
}
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 9698474..07501da 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
@@ -844,50 +844,28 @@
}
@Test
- public void testForLoopMultipleVariablesFail() throws Exception {
- // For loops with multiple variables are not allowed, when parsePython is not set
- syntaxEvents.setFailFast(false);
- List<Statement> stmts = parseFile(
- "[ i for i, j, k in [(1, 2, 3)] ]\n",
- false /* no parsePython */);
- assertThat(stmts).hasSize(1);
- syntaxEvents.assertContainsEvent("For loops with multiple variables are not yet supported");
- }
-
- @Test
public void testForLoopMultipleVariables() throws Exception {
- // For loops with multiple variables is ok, when parsePython is set
- List<Statement> stmts1 = parseFile(
- "[ i for i, j, k in [(1, 2, 3)] ]\n",
- true /* parsePython */);
+ List<Statement> stmts1 = parseFile("[ i for i, j, k in [(1, 2, 3)] ]\n");
assertThat(stmts1).hasSize(1);
- List<Statement> stmts2 = parseFile(
- "[ i for i, j in [(1, 2, 3)] ]\n",
- true /* parsePython */);
+ List<Statement> stmts2 = parseFile("[ i for i, j in [(1, 2, 3)] ]\n");
assertThat(stmts2).hasSize(1);
- List<Statement> stmts3 = parseFile(
- "[ i for (i, j, k) in [(1, 2, 3)] ]\n",
- true /* parsePython */);
+ List<Statement> stmts3 = parseFile("[ i for (i, j, k) in [(1, 2, 3)] ]\n");
assertThat(stmts3).hasSize(1);
}
@Test
public void testForLoopBadSyntax() throws Exception {
syntaxEvents.setFailFast(false);
- parseFile(
- "[1 for (a, b, c in var]\n",
- false /* no parsePython */);
+ parseFile("[1 for (a, b, c in var]\n");
syntaxEvents.assertContainsEvent("syntax error");
}
@Test
public void testForLoopBadSyntax2() throws Exception {
syntaxEvents.setFailFast(false);
- parseFile(
- "[1 for () in var]\n",
- false /* no parsePython */);
+ parseFile("[1 for in var]\n");
syntaxEvents.assertContainsEvent("syntax error");
}
