Fix crash when updating collections while iterating over them in comprehensions

This makes For clauses have the same semantics as For loops.

--
MOS_MIGRATED_REVID=128368124

2 files changed, 10 insertions, 1 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/syntax/AbstractComprehension.java b/src/main/java/com/google/devtools/build/lib/syntax/AbstractComprehension.java
index b5680bf576..ea727ea8fa 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/AbstractComprehension.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/AbstractComprehension.java
@@ -121,7 +121,7 @@ public abstract class AbstractComprehension extends Expression {
       Object listValueObject = list.eval(env);
       Location loc = getLocation();
       Iterable<?> listValue = EvalUtils.toIterable(listValueObject, loc);
-      for (Object listElement : listValue) {
+      for (Object listElement : ImmutableList.copyOf(listValue)) {
         variables.assign(env, loc, listElement);
         evalStep(env, collector, step);
       }
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/EvaluationTest.java b/src/test/java/com/google/devtools/build/lib/syntax/EvaluationTest.java
index 4396608f66..78f9962d74 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/EvaluationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/EvaluationTest.java
@@ -575,6 +575,15 @@ public class EvaluationTest extends EvaluationTestCase {
   }
 
   @Test
+  public void testListComprehensionUpdate() throws Exception {
+    new BuildTest()
+        .setUp("xs = [1, 2, 3]")
+        .testStatement("[xs.append(4) for x in xs]",
+            MutableList.of(env, Runtime.NONE, Runtime.NONE, Runtime.NONE))
+        .testLookup("xs", MutableList.of(env, 1, 2, 3));
+  }
+
+  @Test
   public void testInOperator() throws Exception {
     newTest()
         .testStatement("'b' in ['a', 'b']", Boolean.TRUE)