You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by em...@apache.org on 2022/07/31 12:24:45 UTC
[groovy] 03/03: GROOVY-8643: SC: null-safe for-in loop
This is an automated email from the ASF dual-hosted git repository.
emilles pushed a commit to branch GROOVY_3_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 78b10c2c873965c2429bfc84f3266cebd8ac4976
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Sun Jul 31 07:22:49 2022 -0500
GROOVY-8643: SC: null-safe for-in loop
---
.../groovy/classgen/asm/StatementWriter.java | 22 ++++++++--------
.../asm/sc/StaticTypesStatementWriter.java | 23 ++++++++---------
src/test/groovy/transform/stc/LoopsSTCTest.groovy | 29 ++++++++++++++++++++--
.../classgen/asm/sc/LoopsStaticCompileTest.groovy | 4 +--
4 files changed, 52 insertions(+), 26 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/StatementWriter.java b/src/main/java/org/codehaus/groovy/classgen/asm/StatementWriter.java
index 7714d0a109..6ed2199d91 100644
--- a/src/main/java/org/codehaus/groovy/classgen/asm/StatementWriter.java
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/StatementWriter.java
@@ -60,6 +60,7 @@ import static org.objectweb.asm.Opcodes.ATHROW;
import static org.objectweb.asm.Opcodes.CHECKCAST;
import static org.objectweb.asm.Opcodes.GOTO;
import static org.objectweb.asm.Opcodes.IFEQ;
+import static org.objectweb.asm.Opcodes.IFNULL;
import static org.objectweb.asm.Opcodes.MONITORENTER;
import static org.objectweb.asm.Opcodes.MONITOREXIT;
import static org.objectweb.asm.Opcodes.NOP;
@@ -149,28 +150,29 @@ public class StatementWriter {
BytecodeVariable variable = compileStack.defineVariable(statement.getVariable(), false);
// get the iterator and generate the loop control
- int iteratorIndex = compileStack.defineTemporaryVariable("iterator", ClassHelper.Iterator_TYPE, true);
- Label continueLabel = compileStack.getContinueLabel();
- Label breakLabel = compileStack.getBreakLabel();
+ int iterator = compileStack.defineTemporaryVariable("iterator", ClassHelper.Iterator_TYPE, true);
+ Label breakLabel = compileStack.getBreakLabel(), continueLabel = compileStack.getContinueLabel();
+
+ mv.visitVarInsn(ALOAD, iterator);
+ mv.visitJumpInsn(IFNULL, breakLabel);
mv.visitLabel(continueLabel);
- mv.visitVarInsn(ALOAD, iteratorIndex);
+
+ mv.visitVarInsn(ALOAD, iterator);
writeIteratorHasNext(mv);
- // note: ifeq tests for ==0, a boolean is 0 if it is false
- mv.visitJumpInsn(IFEQ, breakLabel);
+ mv.visitJumpInsn(IFEQ, breakLabel); // jump if zero (aka false)
- mv.visitVarInsn(ALOAD, iteratorIndex);
+ mv.visitVarInsn(ALOAD, iterator);
writeIteratorNext(mv);
operandStack.push(ClassHelper.OBJECT_TYPE);
operandStack.storeVar(variable);
// generate the loop body
statement.getLoopBlock().visit(controller.getAcg());
-
mv.visitJumpInsn(GOTO, continueLabel);
- mv.visitLabel(breakLabel);
- compileStack.removeVar(iteratorIndex);
+ mv.visitLabel(breakLabel);
+ compileStack.removeVar(iterator);
}
protected void writeIteratorHasNext(final MethodVisitor mv) {
diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesStatementWriter.java b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesStatementWriter.java
index d725d65bf2..f820f4b20a 100644
--- a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesStatementWriter.java
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesStatementWriter.java
@@ -190,33 +190,31 @@ public class StaticTypesStatementWriter extends StatementWriter {
OperandStack operandStack = controller.getOperandStack();
MethodVisitor mv = controller.getMethodVisitor();
- // Declare the loop counter.
BytecodeVariable variable = compileStack.defineVariable(loop.getVariable(), false);
+ Label continueLabel = compileStack.getContinueLabel();
+ Label breakLabel = compileStack.getBreakLabel();
collectionExpression.visit(controller.getAcg());
- // Then get the iterator and generate the loop control
-
- int enumIdx = compileStack.defineTemporaryVariable("$enum", ENUMERATION_CLASSNODE, true);
+ int enumeration = compileStack.defineTemporaryVariable("$enum", ENUMERATION_CLASSNODE, true);
- Label continueLabel = compileStack.getContinueLabel();
- Label breakLabel = compileStack.getBreakLabel();
+ mv.visitVarInsn(ALOAD, enumeration);
+ mv.visitJumpInsn(IFNULL, breakLabel);
mv.visitLabel(continueLabel);
- mv.visitVarInsn(ALOAD, enumIdx);
+
+ mv.visitVarInsn(ALOAD, enumeration);
ENUMERATION_HASMORE_METHOD.call(mv);
- // note: ifeq tests for ==0, a boolean is 0 if it is false
- mv.visitJumpInsn(IFEQ, breakLabel);
+ mv.visitJumpInsn(IFEQ, breakLabel); // jump if zero (aka false)
- mv.visitVarInsn(ALOAD, enumIdx);
+ mv.visitVarInsn(ALOAD, enumeration);
ENUMERATION_NEXT_METHOD.call(mv);
operandStack.push(ClassHelper.OBJECT_TYPE);
operandStack.storeVar(variable);
- // Generate the loop body
loop.getLoopBlock().visit(controller.getAcg());
-
mv.visitJumpInsn(GOTO, continueLabel);
+
mv.visitLabel(breakLabel);
}
@@ -225,6 +223,7 @@ public class StaticTypesStatementWriter extends StatementWriter {
MethodCallExpression call = GeneralUtils.callX(collectionExpression, "iterator");
call.setMethodTarget(collectionType.getMethod("iterator",Parameter.EMPTY_ARRAY));
call.setImplicitThis(false);
+ call.setSafe(true);//GROOVY-8643
call.visit(controller.getAcg());
} else {
collectionExpression.visit(controller.getAcg());
diff --git a/src/test/groovy/transform/stc/LoopsSTCTest.groovy b/src/test/groovy/transform/stc/LoopsSTCTest.groovy
index 9d5e615e4e..4dd97a7226 100644
--- a/src/test/groovy/transform/stc/LoopsSTCTest.groovy
+++ b/src/test/groovy/transform/stc/LoopsSTCTest.groovy
@@ -172,7 +172,27 @@ class LoopsSTCTest extends StaticTypeCheckingTestCase {
'''
}
+ // GROOVY-8643
+ void testForInLoopOnList() {
+ assertScript '''
+ List<String> list = null
+ for (item in list) {
+ item.toUpperCase()
+ }
+ '''
+ }
+
+ // GROOVY-8643
void testForInLoopOnArray() {
+ assertScript '''
+ String[] strings = null
+ for (string in strings) {
+ string.toUpperCase()
+ }
+ '''
+ }
+
+ void testForInLoopOnArray2() {
assertScript '''
String[] strings = ['a','b','c']
for (string in strings) {
@@ -182,7 +202,7 @@ class LoopsSTCTest extends StaticTypeCheckingTestCase {
}
// GROOVY-10579
- void testForInLoopOnArray2() {
+ void testForInLoopOnArray3() {
assertScript '''
int[] numbers = [1,2,3,4,5]
int sum = 0
@@ -221,7 +241,7 @@ class LoopsSTCTest extends StaticTypeCheckingTestCase {
assertScript '''
Vector<String> v = new Vector<>()
v.add('ooo')
- def en = v.elements()
+ Enumeration<String> en = v.elements()
for (e in en) {
assert e.toUpperCase() == 'OOO'
}
@@ -237,6 +257,11 @@ class LoopsSTCTest extends StaticTypeCheckingTestCase {
assert e.toUpperCase() in ['OOO','GROOVY']
if (e=='ooo') continue
}
+
+ en = null
+ for (e in en) {
+ e.toUpperCase()
+ }
'''
}
diff --git a/src/test/org/codehaus/groovy/classgen/asm/sc/LoopsStaticCompileTest.groovy b/src/test/org/codehaus/groovy/classgen/asm/sc/LoopsStaticCompileTest.groovy
index 4d08b38d0c..bb9005e5fa 100644
--- a/src/test/org/codehaus/groovy/classgen/asm/sc/LoopsStaticCompileTest.groovy
+++ b/src/test/org/codehaus/groovy/classgen/asm/sc/LoopsStaticCompileTest.groovy
@@ -26,8 +26,8 @@ import groovy.transform.stc.LoopsSTCTest
final class LoopsStaticCompileTest extends LoopsSTCTest implements StaticCompilationTestSupport {
// GROOVY-10477
- void testForInLoopOnArray() {
- super.testForInLoopOnArray()
+ void testForInLoopOnArray2() {
+ super.testForInLoopOnArray2()
def bytecode = astTrees.values()[0][1]
assert !bytecode.contains('INVOKESTATIC org/codehaus/groovy/runtime/DefaultGroovyMethods.iterator')
}