Remove loop counter for foreach loops (#71602)

This changes remove the loop counter for for each loops which is a regression from 7.9.3 documented in #71584. Not loop counting for each loops should be relatively safe as they have a natural ending point. We can consider adding the check back in once we have configurable loop counters.
elastic · Apr 12, 2021 · ac0b757 · ac0b757
1 parent 14f775f
commit ac0b757
Show file tree

Hide file tree

Showing 2 changed files with 22 additions and 12 deletions.
diff --git a/...painless/src/main/java/org/elasticsearch/painless/phase/DefaultIRTreeToASMBytesPhase.java b/...painless/src/main/java/org/elasticsearch/painless/phase/DefaultIRTreeToASMBytesPhase.java
@@ -524,12 +524,6 @@ public void visitForEachSubArrayLoop(ForEachSubArrayNode irForEachSubArrayNode,
         methodWriter.writeCast(irForEachSubArrayNode.getDecorationValue(IRDCast.class));
         methodWriter.visitVarInsn(variable.getAsmType().getOpcode(Opcodes.ISTORE), variable.getSlot());
 
-        Variable loop = writeScope.getInternalVariable("loop");
-
-        if (loop != null) {
-            methodWriter.writeLoopCounter(loop.getSlot(), irForEachSubArrayNode.getLocation());
-        }
-
         visit(irForEachSubArrayNode.getBlockNode(), writeScope.newLoopScope(begin, end));
 
         methodWriter.goTo(begin);
@@ -575,12 +569,6 @@ public void visitForEachSubIterableLoop(ForEachSubIterableNode irForEachSubItera
         methodWriter.writeCast(irForEachSubIterableNode.getDecorationValue(IRDCast.class));
         methodWriter.visitVarInsn(variable.getAsmType().getOpcode(Opcodes.ISTORE), variable.getSlot());
 
-        Variable loop = writeScope.getInternalVariable("loop");
-
-        if (loop != null) {
-            methodWriter.writeLoopCounter(loop.getSlot(), irForEachSubIterableNode.getLocation());
-        }
-
         visit(irForEachSubIterableNode.getBlockNode(), writeScope.newLoopScope(begin, end));
         methodWriter.goTo(begin);
         methodWriter.mark(end);

diff --git a/modules/lang-painless/src/test/java/org/elasticsearch/painless/BasicStatementTests.java b/modules/lang-painless/src/test/java/org/elasticsearch/painless/BasicStatementTests.java
@@ -4,6 +4,7 @@
 import org.elasticsearch.script.ScriptContext;
 
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
@@ -508,4 +509,25 @@ public void testForEachWithBreak() {
             "[i, j];"
         ));
     }
+
+    public void testNoLoopCounterInForEach() {
+        String bytecode = Debugger.toString("def x = []; for (y in x) { int z; }");
+        assertFalse(bytecode.contains("IINC"));
+        bytecode = Debugger.toString("List x = []; for (y in x) { int z; }");
+        assertFalse(bytecode.contains("IINC"));
+        bytecode = Debugger.toString("int[] x = new int[] {}; for (y in x) { int z; }");
+        assertFalse(bytecode.contains("IINC 1 -1"));
+
+        int[] test = new int[10000000];
+        Arrays.fill(test, 2);
+        Map<String, Object> params = new HashMap<>();
+        params.put("values", test);
+        int total = (int)exec("int total = 0; for (int value : params['values']) total += value; return total", params, false);
+        assertEquals(total, 20000000);
+
+        PainlessError pe = expectScriptThrows(PainlessError.class, () ->
+            exec("int total = 0; for (int value = 0; value < params['values'].length; ++value) total += value; return total",
+                    params, false));
+        assertEquals("The maximum number of statements that can be executed in a loop has been reached.", pe.getMessage());
+    }
 }