Parser: Allow more complex expressions as for loop variables.

Also, use LValue in ForStatement.

--
MOS_MIGRATED_REVID=89122760

2 files changed, 28 insertions, 37 deletions

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 76a46eeb43..2faf06c857 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 @@ import java.util.List;
  */
 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 @@ public final class ForStatement extends Statement {
 
     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 @@ public final class ForStatement extends Statement {
     // 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 d736a272e5..187f11bbf0 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 @@ class Parser {
                                  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();
-    }
-
-    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");
+      expect(TokenKind.COMMA);
+      if (EXPR_LIST_TERMINATOR_SET.contains(token.kind)) {
+        break;
+      }
+      tuple.add(parsePrimaryWithSuffix());
     }
-    return multipleVariables ? makeErrorExpression(start, end) : firstIdent;
+    return setLocation(ListLiteral.makeTuple(tuple), start, token.right);
   }
 
   // list_maker ::= '[' ']'
@@ -766,11 +757,11 @@ class Parser {
           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 @@ class Parser {
     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 @@ class Parser {
   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));
   }