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:44 UTC
[groovy] 02/03: GROOVY-8487: SC: for-in loop over iterator
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 49dde88f7220be29cba0be3af2409738239d9d09
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Sun Jul 31 07:17:15 2022 -0500
GROOVY-8487: SC: for-in loop over iterator
---
.../asm/sc/StaticTypesStatementWriter.java | 130 ++++++++-------------
.../groovy/runtime/DefaultGroovyMethods.java | 3 +-
src/test/groovy/transform/stc/LoopsSTCTest.groovy | 11 ++
3 files changed, 64 insertions(+), 80 deletions(-)
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 30cfbc48b8..d725d65bf2 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
@@ -21,24 +21,23 @@ package org.codehaus.groovy.classgen.asm.sc;
import org.codehaus.groovy.ast.ClassHelper;
import org.codehaus.groovy.ast.ClassNode;
import org.codehaus.groovy.ast.Parameter;
-import org.codehaus.groovy.ast.expr.ArgumentListExpression;
import org.codehaus.groovy.ast.expr.Expression;
import org.codehaus.groovy.ast.expr.MethodCallExpression;
import org.codehaus.groovy.ast.stmt.BlockStatement;
import org.codehaus.groovy.ast.stmt.ForStatement;
+import org.codehaus.groovy.ast.tools.GeneralUtils;
import org.codehaus.groovy.classgen.AsmClassGenerator;
import org.codehaus.groovy.classgen.asm.BytecodeVariable;
import org.codehaus.groovy.classgen.asm.CompileStack;
import org.codehaus.groovy.classgen.asm.MethodCaller;
import org.codehaus.groovy.classgen.asm.OperandStack;
import org.codehaus.groovy.classgen.asm.StatementWriter;
-import org.codehaus.groovy.classgen.asm.TypeChooser;
-import org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport;
import org.objectweb.asm.Label;
import org.objectweb.asm.MethodVisitor;
import java.util.Enumeration;
+import static org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport.implementsInterfaceOrIsSubclassOf;
import static org.objectweb.asm.Opcodes.AALOAD;
import static org.objectweb.asm.Opcodes.ALOAD;
import static org.objectweb.asm.Opcodes.ARRAYLENGTH;
@@ -68,59 +67,56 @@ public class StaticTypesStatementWriter extends StatementWriter {
private static final MethodCaller ENUMERATION_NEXT_METHOD = MethodCaller.newInterface(Enumeration.class, "nextElement");
private static final MethodCaller ENUMERATION_HASMORE_METHOD = MethodCaller.newInterface(Enumeration.class, "hasMoreElements");
- public StaticTypesStatementWriter(StaticTypesWriterController controller) {
+ public StaticTypesStatementWriter(final StaticTypesWriterController controller) {
super(controller);
}
@Override
- public void writeBlockStatement(BlockStatement statement) {
+ public void writeBlockStatement(final BlockStatement statement) {
controller.switchToFastPath();
super.writeBlockStatement(statement);
controller.switchToSlowPath();
}
+ //--------------------------------------------------------------------------
+
@Override
protected void writeForInLoop(final ForStatement loop) {
- controller.getAcg().onLineNumber(loop,"visitForLoop");
+ controller.getAcg().onLineNumber(loop, "visitForLoop");
writeStatementLabel(loop);
CompileStack compileStack = controller.getCompileStack();
- MethodVisitor mv = controller.getMethodVisitor();
OperandStack operandStack = controller.getOperandStack();
compileStack.pushLoop(loop.getVariableScope(), loop.getStatementLabels());
- // Identify type of collection
- TypeChooser typeChooser = controller.getTypeChooser();
+ // identify type of collection
Expression collectionExpression = loop.getCollectionExpression();
- ClassNode collectionType = typeChooser.resolveType(collectionExpression, controller.getClassNode());
+ ClassNode collectionType = controller.getTypeChooser().resolveType(collectionExpression, controller.getClassNode());
+
+ int mark = operandStack.getStackLength();
Parameter loopVariable = loop.getVariable();
- int size = operandStack.getStackLength();
if (collectionType.isArray() && loopVariable.getType().equals(collectionType.getComponentType())) {
- writeOptimizedForEachLoop(compileStack, operandStack, mv, loop, collectionExpression, collectionType, loopVariable);
- } else if (ENUMERATION_CLASSNODE.equals(collectionType)) {
+ writeOptimizedForEachLoop(loop, loopVariable, collectionExpression, collectionType);
+ } else if (GeneralUtils.isOrImplements(collectionType, ENUMERATION_CLASSNODE)) {
writeEnumerationBasedForEachLoop(loop, collectionExpression, collectionType);
} else {
writeIteratorBasedForEachLoop(loop, collectionExpression, collectionType);
}
- operandStack.popDownTo(size);
+ operandStack.popDownTo(mark);
compileStack.pop();
}
- private void writeOptimizedForEachLoop(
- CompileStack compileStack,
- OperandStack operandStack,
- MethodVisitor mv,
- ForStatement loop,
- Expression arrayExpression,
- ClassNode arrayType,
- Parameter loopVariable) {
+ private void writeOptimizedForEachLoop(final ForStatement loop, final Parameter loopVariable, final Expression arrayExpression, final ClassNode arrayType) {
+ CompileStack compileStack = controller.getCompileStack();
+ OperandStack operandStack = controller.getOperandStack();
+ MethodVisitor mv = controller.getMethodVisitor();
+ AsmClassGenerator acg = controller.getAcg();
+
BytecodeVariable variable = compileStack.defineVariable(loopVariable, arrayType.getComponentType(), false);
Label continueLabel = compileStack.getContinueLabel();
Label breakLabel = compileStack.getBreakLabel();
- AsmClassGenerator acg = controller.getAcg();
-
// load array on stack
arrayExpression.visit(acg);
mv.visitInsn(DUP);
@@ -145,9 +141,9 @@ public class StaticTypesStatementWriter extends StatementWriter {
mv.visitJumpInsn(IF_ICMPGE, breakLabel);
// get array element
- loadFromArray(mv, variable, array, loopIdx);
+ loadFromArray(mv, operandStack, variable, array, loopIdx);
- // $idx++
+ // $idx += 1
mv.visitIincInsn(loopIdx, 1);
// loop body
@@ -162,39 +158,24 @@ public class StaticTypesStatementWriter extends StatementWriter {
compileStack.removeVar(array);
}
- private void loadFromArray(MethodVisitor mv, BytecodeVariable variable, int array, int iteratorIdx) {
- OperandStack os = controller.getOperandStack();
+ private static void loadFromArray(final MethodVisitor mv, final OperandStack os, final BytecodeVariable variable, final int array, final int index) {
mv.visitVarInsn(ALOAD, array);
- mv.visitVarInsn(ILOAD, iteratorIdx);
-
+ mv.visitVarInsn(ILOAD, index);
ClassNode varType = variable.getType();
- boolean primitiveType = ClassHelper.isPrimitiveType(varType);
- boolean isByte = ClassHelper.byte_TYPE.equals(varType);
- boolean isShort = ClassHelper.short_TYPE.equals(varType);
- boolean isInt = ClassHelper.int_TYPE.equals(varType);
- boolean isLong = ClassHelper.long_TYPE.equals(varType);
- boolean isFloat = ClassHelper.float_TYPE.equals(varType);
- boolean isDouble = ClassHelper.double_TYPE.equals(varType);
- boolean isChar = ClassHelper.char_TYPE.equals(varType);
- boolean isBoolean = ClassHelper.boolean_TYPE.equals(varType);
-
- if (primitiveType) {
- if (isByte) {
+ if (ClassHelper.isPrimitiveType(varType)) {
+ if (varType.equals(ClassHelper.int_TYPE)) {
+ mv.visitInsn(IALOAD);
+ } else if (varType.equals(ClassHelper.long_TYPE)) {
+ mv.visitInsn(LALOAD);
+ } else if (varType.equals(ClassHelper.byte_TYPE) || varType.equals(ClassHelper.boolean_TYPE)) {
mv.visitInsn(BALOAD);
- }
- if (isShort) {
+ } else if (varType.equals(ClassHelper.char_TYPE)) {
+ mv.visitInsn(CALOAD);
+ } else if (varType.equals(ClassHelper.short_TYPE)) {
mv.visitInsn(SALOAD);
- }
- if (isInt || isChar || isBoolean) {
- mv.visitInsn(isChar ? CALOAD : isBoolean ? BALOAD : IALOAD);
- }
- if (isLong) {
- mv.visitInsn(LALOAD);
- }
- if (isFloat) {
+ } else if (varType.equals(ClassHelper.float_TYPE)) {
mv.visitInsn(FALOAD);
- }
- if (isDouble) {
+ } else if (varType.equals(ClassHelper.double_TYPE)) {
mv.visitInsn(DALOAD);
}
} else {
@@ -204,33 +185,10 @@ public class StaticTypesStatementWriter extends StatementWriter {
os.storeVar(variable);
}
- private void writeIteratorBasedForEachLoop(
- ForStatement loop,
- Expression collectionExpression,
- ClassNode collectionType) {
-
- if (StaticTypeCheckingSupport.implementsInterfaceOrIsSubclassOf(collectionType, ITERABLE_CLASSNODE)) {
- MethodCallExpression iterator = new MethodCallExpression(collectionExpression, "iterator", new ArgumentListExpression());
- iterator.setMethodTarget(collectionType.getMethod("iterator", Parameter.EMPTY_ARRAY));
- iterator.setImplicitThis(false);
- iterator.visit(controller.getAcg());
- } else {
- collectionExpression.visit(controller.getAcg());
- controller.getMethodVisitor().visitMethodInsn(INVOKESTATIC, "org/codehaus/groovy/runtime/DefaultGroovyMethods", "iterator", "(Ljava/lang/Object;)Ljava/util/Iterator;", false);
- controller.getOperandStack().replace(ClassHelper.Iterator_TYPE);
- }
-
- writeForInLoopControlAndBlock(loop);
- }
-
- private void writeEnumerationBasedForEachLoop(
- ForStatement loop,
- Expression collectionExpression,
- ClassNode collectionType) {
-
+ private void writeEnumerationBasedForEachLoop(final ForStatement loop, final Expression collectionExpression, final ClassNode collectionType) {
CompileStack compileStack = controller.getCompileStack();
- MethodVisitor mv = controller.getMethodVisitor();
OperandStack operandStack = controller.getOperandStack();
+ MethodVisitor mv = controller.getMethodVisitor();
// Declare the loop counter.
BytecodeVariable variable = compileStack.defineVariable(loop.getVariable(), false);
@@ -261,4 +219,18 @@ public class StaticTypesStatementWriter extends StatementWriter {
mv.visitJumpInsn(GOTO, continueLabel);
mv.visitLabel(breakLabel);
}
+
+ private void writeIteratorBasedForEachLoop(final ForStatement loop, final Expression collectionExpression, final ClassNode collectionType) {
+ if (implementsInterfaceOrIsSubclassOf(collectionType, ITERABLE_CLASSNODE)) {
+ MethodCallExpression call = GeneralUtils.callX(collectionExpression, "iterator");
+ call.setMethodTarget(collectionType.getMethod("iterator",Parameter.EMPTY_ARRAY));
+ call.setImplicitThis(false);
+ call.visit(controller.getAcg());
+ } else {
+ collectionExpression.visit(controller.getAcg());
+ controller.getMethodVisitor().visitMethodInsn(INVOKESTATIC, "org/codehaus/groovy/runtime/DefaultGroovyMethods", "iterator", "(Ljava/lang/Object;)Ljava/util/Iterator;", false);
+ controller.getOperandStack().replace(ClassHelper.Iterator_TYPE);
+ }
+ writeForInLoopControlAndBlock(loop);
+ }
}
diff --git a/src/main/java/org/codehaus/groovy/runtime/DefaultGroovyMethods.java b/src/main/java/org/codehaus/groovy/runtime/DefaultGroovyMethods.java
index 69e6c3c3e8..3df39e4f46 100644
--- a/src/main/java/org/codehaus/groovy/runtime/DefaultGroovyMethods.java
+++ b/src/main/java/org/codehaus/groovy/runtime/DefaultGroovyMethods.java
@@ -18278,7 +18278,8 @@ public class DefaultGroovyMethods extends DefaultGroovyMethodsSupport {
* @see org.codehaus.groovy.runtime.typehandling.DefaultTypeTransformation#asCollection(java.lang.Object)
* @since 1.0
*/
- public static Iterator iterator(Object o) {
+ public static Iterator iterator(final Object o) {
+ if (o instanceof Iterator) return (Iterator)o;
return DefaultTypeTransformation.asCollection(o).iterator();
}
diff --git a/src/test/groovy/transform/stc/LoopsSTCTest.groovy b/src/test/groovy/transform/stc/LoopsSTCTest.groovy
index e2baa17557..9d5e615e4e 100644
--- a/src/test/groovy/transform/stc/LoopsSTCTest.groovy
+++ b/src/test/groovy/transform/stc/LoopsSTCTest.groovy
@@ -205,6 +205,17 @@ class LoopsSTCTest extends StaticTypeCheckingTestCase {
'''
}
+ // GROOVY-8487, GROOVY-10712
+ void testForInLoopOnIterator() {
+ assertScript '''
+ def list = []
+ for (item in ['a','b','c'].iterator()) {
+ list.add(item.toUpperCase())
+ }
+ assert list == ['A','B','C']
+ '''
+ }
+
// GROOVY-6123
void testForInLoopOnEnumeration() {
assertScript '''