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/02/09 20:10:33 UTC

[groovy] branch GROOVY_4_0_X updated (181cddd -> e3441dc)

This is an automated email from the ASF dual-hosted git repository.

emilles pushed a change to branch GROOVY_4_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git.


    from 181cddd  GROOVY-8448: "this.name" from anon. inner class
     new 5748ecb  GROOVY-8133: STC: support spread-dot for any iterable type, incl. Stream
     new 4b24737  GROOVY-10476: Stream provides iterator() but does not implement Iterable
     new e3441dc  GROOVY-10477: SC: optimize `for (item in array) { }` -- dynamic variable

The 3 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../java/org/codehaus/groovy/ast/ClassHelper.java  |   2 +
 .../asm/sc/StaticTypesStatementWriter.java         | 142 +++++++++------------
 .../transform/stc/StaticTypeCheckingVisitor.java   |  87 +++++++------
 src/test/groovy/transform/stc/LoopsSTCTest.groovy  |  32 ++---
 .../groovy/transform/stc/MethodCallsSTCTest.groovy |  40 +++++-
 .../stc/PrecompiledExtensionNotExtendingDSL.groovy |  31 +++--
 .../stc/TypeCheckingExtensionsTest.groovy          |   8 +-
 .../classgen/asm/sc/LoopsStaticCompileTest.groovy  |  10 +-
 8 files changed, 192 insertions(+), 160 deletions(-)

[groovy] 01/03: GROOVY-8133: STC: support spread-dot for any iterable type, incl. Stream

Posted by em...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

emilles pushed a commit to branch GROOVY_4_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git

commit 5748ecb8fcf9ba158e9872e3b1982e621fcc1d45
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Tue Feb 8 19:01:53 2022 -0600

    GROOVY-8133: STC: support spread-dot for any iterable type, incl. Stream
---
 .../transform/stc/StaticTypeCheckingVisitor.java   | 50 +++++++++++++---------
 .../groovy/transform/stc/MethodCallsSTCTest.groovy | 29 ++++++++++++-
 .../stc/PrecompiledExtensionNotExtendingDSL.groovy | 31 +++++++-------
 .../stc/TypeCheckingExtensionsTest.groovy          |  8 ++--
 4 files changed, 77 insertions(+), 41 deletions(-)

diff --git a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
index e42bf45..b121c4b 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -218,7 +218,6 @@ import static org.codehaus.groovy.ast.tools.GeneralUtils.elvisX;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.getGetterName;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.getSetterName;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.isOrImplements;
-import static org.codehaus.groovy.ast.tools.GeneralUtils.localVarX;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.propX;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.thisPropX;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.varX;
@@ -2481,9 +2480,7 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
     }
 
     private static ClassNode wrapClosureType(final ClassNode returnType) {
-        ClassNode inferredType = CLOSURE_TYPE.getPlainNodeReference();
-        inferredType.setGenericsTypes(new GenericsType[]{new GenericsType(wrapTypeIfNecessary(returnType))});
-        return inferredType;
+        return makeClassSafe0(CLOSURE_TYPE, wrapTypeIfNecessary(returnType).asGenericsType());
     }
 
     protected DelegationMetadata getDelegationMetadata(final ClosureExpression expression) {
@@ -3307,16 +3304,16 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
         if (objectExpression instanceof ConstructorCallExpression) { // GROOVY-10228
             inferDiamondType((ConstructorCallExpression) objectExpression, receiver.getPlainNodeReference());
         }
-        if (call.isSpreadSafe()) { // make sure receiver is array or collection then check element type
-            if (!receiver.isArray() && !implementsInterfaceOrIsSubclassOf(receiver, Collection_TYPE)) {
-                addStaticTypeError("Spread operator can only be used on collection types", objectExpression);
+        if (call.isSpreadSafe()) {
+            ClassNode componentType = inferComponentType(receiver, null);
+            if (componentType == null) {
+                addStaticTypeError("Spread-dot operator can only be used on iterable types", objectExpression);
             } else {
-                ClassNode componentType = inferComponentType(receiver, int_TYPE);
-                MethodCallExpression subcall = callX(castX(componentType, EmptyExpression.INSTANCE), name, call.getArguments());
+                MethodCallExpression subcall = callX(varX("item", componentType), name, call.getArguments());
                 subcall.setLineNumber(call.getLineNumber()); subcall.setColumnNumber(call.getColumnNumber());
                 subcall.setImplicitThis(call.isImplicitThis());
                 visitMethodCallExpression(subcall);
-                // the inferred type here should be a list of what the subcall returns
+                // inferred type should be a list of what sub-call returns
                 storeType(call, extension.buildListType(getType(subcall)));
                 storeTargetMethod(call, subcall.getNodeMetaData(DIRECT_METHOD_CALL_TARGET));
             }
@@ -4546,23 +4543,36 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
         return Number_TYPE;
     }
 
-    protected ClassNode inferComponentType(final ClassNode containerType, final ClassNode indexType) {
-        ClassNode componentType = containerType.getComponentType();
+    protected ClassNode inferComponentType(final ClassNode receiverType, final ClassNode subscriptType) {
+        ClassNode componentType = receiverType.getComponentType();
         if (componentType == null) {
-            // GROOVY-5521
-            // try to identify a getAt method
+            MethodCallExpression mce;
+            if (subscriptType != null) { // GROOVY-5521: check for a suitable "getAt(T)" method
+                mce = callX(varX("#", receiverType), "getAt", varX("selector", subscriptType));
+            } else { // GROOVY-8133: check for an "iterator()" method
+                mce = callX(varX("#", receiverType), "iterator");
+            }
+            mce.setImplicitThis(false); // GROOVY-8943
+
             typeCheckingContext.pushErrorCollector();
-            MethodCallExpression vcall = callX(localVarX("_hash_", containerType), "getAt", varX("_index_", indexType));
-            vcall.setImplicitThis(false); // GROOVY-8943
             try {
-                visitMethodCallExpression(vcall);
+                visitMethodCallExpression(mce);
             } finally {
                 typeCheckingContext.popErrorCollector();
             }
-            return getType(vcall);
-        } else {
-            return componentType;
+
+            if (subscriptType != null) {
+                componentType = getType(mce);
+            } else {
+                ClassNode iteratorType = getType(mce);
+                if (isOrImplements(iteratorType, Iterator_TYPE) && (iteratorType.getGenericsTypes() != null
+                        // ignore the iterator(Object) extension method, since it makes *everything* appear iterable
+                        || !mce.<MethodNode>getNodeMetaData(DIRECT_METHOD_CALL_TARGET).getDeclaringClass().equals(OBJECT_TYPE))) {
+                    componentType = Optional.ofNullable(iteratorType.getGenericsTypes()).map(gt -> getCombinedBoundType(gt[0])).orElse(OBJECT_TYPE);
+                }
+            }
         }
+        return componentType;
     }
 
     protected MethodNode findMethodOrFail(final Expression expr, final ClassNode receiver, final String name, final ClassNode... args) {
diff --git a/src/test/groovy/transform/stc/MethodCallsSTCTest.groovy b/src/test/groovy/transform/stc/MethodCallsSTCTest.groovy
index 481c496..dc0fb19 100644
--- a/src/test/groovy/transform/stc/MethodCallsSTCTest.groovy
+++ b/src/test/groovy/transform/stc/MethodCallsSTCTest.groovy
@@ -19,6 +19,7 @@
 package groovy.transform.stc
 
 import org.codehaus.groovy.control.MultipleCompilationErrorsException
+import org.codehaus.groovy.control.customizers.ASTTransformationCustomizer
 import org.codehaus.groovy.control.customizers.ImportCustomizer
 
 /**
@@ -1159,6 +1160,32 @@ class MethodCallsSTCTest extends StaticTypeCheckingTestCase {
         'Cannot call closure that accepts [java.lang.String, java.lang.String, java.lang.String] with [java.lang.Object]'
     }
 
+    // GROOVY-8133
+    void testSpreadDotOperator() {
+        assertScript '''
+            def list = ['a','b','c'].stream()*.toUpperCase()
+            assert list == ['A', 'B', 'C']
+        '''
+        assertScript '''
+            def list = 'a,b,c'.split(',')*.toUpperCase()
+            assert list == ['A', 'B', 'C']
+        '''
+
+        shouldFailWithMessages '''
+            def list = 'abc'*.toUpperCase()
+            assert list == ['A', 'B', 'C']
+        ''',
+        'Spread-dot operator can only be used on iterable types'
+
+        config.compilationCustomizers
+              .find { it instanceof ASTTransformationCustomizer }
+              .annotationParameters = [extensions: PrecompiledExtensionNotExtendingDSL.name]
+        assertScript '''
+            def list = 'abc'*.toUpperCase()
+            assert list == ['A', 'B', 'C']
+        '''
+    }
+
     void testBoxingShouldCostMore() {
         assertScript '''
             int foo(int x) { 1 }
@@ -1411,7 +1438,7 @@ class MethodCallsSTCTest extends StaticTypeCheckingTestCase {
     void testStaticContextScoping() {
         assertScript '''
             class A {
-                static List foo = 'a,b,c'.split(/,/).toList()*.trim()
+                static List foo = 'a,b,c'.split(/,/)*.trim()
             }
             assert A.foo == ['a','b','c']
         '''
diff --git a/src/test/groovy/transform/stc/PrecompiledExtensionNotExtendingDSL.groovy b/src/test/groovy/transform/stc/PrecompiledExtensionNotExtendingDSL.groovy
index 020304b..6598a4b 100644
--- a/src/test/groovy/transform/stc/PrecompiledExtensionNotExtendingDSL.groovy
+++ b/src/test/groovy/transform/stc/PrecompiledExtensionNotExtendingDSL.groovy
@@ -18,28 +18,27 @@
  */
 package groovy.transform.stc
 
+import groovy.transform.AutoFinal
+import groovy.transform.InheritConstructors
+import org.codehaus.groovy.ast.ClassHelper
+import org.codehaus.groovy.ast.ClassNode
 import org.codehaus.groovy.ast.MethodNode
 import org.codehaus.groovy.ast.expr.Expression
 import org.codehaus.groovy.transform.stc.AbstractTypeCheckingExtension
-import org.codehaus.groovy.transform.stc.StaticTypeCheckingVisitor
 
-class PrecompiledExtensionNotExtendingDSL extends AbstractTypeCheckingExtension {
-
-
-    PrecompiledExtensionNotExtendingDSL(
-            final StaticTypeCheckingVisitor typeCheckingVisitor) {
-        super(typeCheckingVisitor)
-    }
-
-    @Override
-    void setup() {
-        addStaticTypeError('Error thrown from extension in setup', context.enclosingClassNode)
-    }
+import static org.codehaus.groovy.ast.tools.GenericsUtils.makeClassSafeWithGenerics
 
+@AutoFinal @InheritConstructors
+class PrecompiledExtensionNotExtendingDSL extends AbstractTypeCheckingExtension {
     @Override
-    void onMethodSelection(final Expression expression, final MethodNode target) {
-        if (target.name=='println') {
+    void onMethodSelection(Expression expression, MethodNode target) {
+        switch (target.name) {
+          case 'println':
             addStaticTypeError('Error thrown from extension in onMethodSelection', expression.arguments[0])
+            break
+          case 'iterator':
+            ClassNode iteratorType = makeClassSafeWithGenerics(ClassHelper.Iterator_TYPE, ClassHelper.STRING_TYPE.asGenericsType())
+            storeType(expression, iteratorType) // indicate "string.iterator()" returns Iterator<String>
         }
     }
-}
\ No newline at end of file
+}
diff --git a/src/test/groovy/transform/stc/TypeCheckingExtensionsTest.groovy b/src/test/groovy/transform/stc/TypeCheckingExtensionsTest.groovy
index 3558c23..af5ddce 100644
--- a/src/test/groovy/transform/stc/TypeCheckingExtensionsTest.groovy
+++ b/src/test/groovy/transform/stc/TypeCheckingExtensionsTest.groovy
@@ -483,8 +483,8 @@ class TypeCheckingExtensionsTest extends StaticTypeCheckingTestCase {
         extension = 'groovy.transform.stc.PrecompiledExtension'
         shouldFailWithMessages '''
             println 'Everything is ok'
-        ''', 'Error thrown from extension'
-
+        ''',
+        'Error thrown from extension'
     }
 
     void testPrecompiledExtensionNotExtendingTypeCheckingDSL() {
@@ -495,7 +495,7 @@ class TypeCheckingExtensionsTest extends StaticTypeCheckingTestCase {
         extension = 'groovy.transform.stc.PrecompiledExtensionNotExtendingDSL'
         shouldFailWithMessages '''
             println 'Everything is ok'
-        ''', 'Error thrown from extension in setup', 'Error thrown from extension in onMethodSelection'
-
+        ''',
+        'Error thrown from extension in onMethodSelection'
     }
 }

[groovy] 02/03: GROOVY-10476: Stream provides iterator() but does not implement Iterable

Posted by em...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

emilles pushed a commit to branch GROOVY_4_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git

commit 4b247376a4b1f856b2116abbf8ce27ce7abe5633
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Wed Feb 9 12:05:11 2022 -0600

    GROOVY-10476: Stream provides iterator() but does not implement Iterable
---
 .../java/org/codehaus/groovy/ast/ClassHelper.java  |   2 +
 .../asm/sc/StaticTypesStatementWriter.java         | 140 +++++++++------------
 .../transform/stc/StaticTypeCheckingVisitor.java   |  37 +++---
 .../groovy/transform/stc/MethodCallsSTCTest.groovy |  13 +-
 4 files changed, 91 insertions(+), 101 deletions(-)

diff --git a/src/main/java/org/codehaus/groovy/ast/ClassHelper.java b/src/main/java/org/codehaus/groovy/ast/ClassHelper.java
index c41dfbf..95acea2 100644
--- a/src/main/java/org/codehaus/groovy/ast/ClassHelper.java
+++ b/src/main/java/org/codehaus/groovy/ast/ClassHelper.java
@@ -71,6 +71,7 @@ import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.regex.Pattern;
+import java.util.stream.Stream;
 
 /**
  * Helper for {@link ClassNode} and classes handling them.  Contains a set of
@@ -151,6 +152,7 @@ public class ClassHelper {
             Enum_Type = makeWithoutCaching(Enum.class),
             CLASS_Type = makeWithoutCaching(Class.class),
             TUPLE_TYPE = makeWithoutCaching(Tuple.class),
+            STREAM_TYPE = makeWithoutCaching(Stream.class),
             ITERABLE_TYPE = makeWithoutCaching(Iterable.class),
             REFERENCE_TYPE = makeWithoutCaching(Reference.class),
             COLLECTION_TYPE = makeWithoutCaching(Collection.class),
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 cc02e8f..69c09c3 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
@@ -20,34 +20,25 @@ package org.codehaus.groovy.classgen.asm.sc;
 
 import org.codehaus.groovy.ast.ClassHelper;
 import org.codehaus.groovy.ast.ClassNode;
+import org.codehaus.groovy.ast.MethodNode;
 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 java.util.Objects;
 
-import static org.codehaus.groovy.ast.ClassHelper.isPrimitiveType;
-import static org.codehaus.groovy.ast.ClassHelper.isPrimitiveBoolean;
-import static org.codehaus.groovy.ast.ClassHelper.isPrimitiveByte;
-import static org.codehaus.groovy.ast.ClassHelper.isPrimitiveChar;
-import static org.codehaus.groovy.ast.ClassHelper.isPrimitiveDouble;
-import static org.codehaus.groovy.ast.ClassHelper.isPrimitiveFloat;
-import static org.codehaus.groovy.ast.ClassHelper.isPrimitiveInt;
-import static org.codehaus.groovy.ast.ClassHelper.isPrimitiveLong;
-import static org.codehaus.groovy.ast.ClassHelper.isPrimitiveShort;
 import static org.objectweb.asm.Opcodes.AALOAD;
 import static org.objectweb.asm.Opcodes.ALOAD;
 import static org.objectweb.asm.Opcodes.ARRAYLENGTH;
@@ -72,65 +63,60 @@ import static org.objectweb.asm.Opcodes.SALOAD;
  */
 public class StaticTypesStatementWriter extends StatementWriter {
 
-    private static final ClassNode ITERABLE_CLASSNODE = ClassHelper.make(Iterable.class);
     private static final ClassNode ENUMERATION_CLASSNODE = ClassHelper.make(Enumeration.class);
     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.getOriginType().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 collectionExpression,
-            ClassNode collectionType,
-            Parameter loopVariable) {
-        BytecodeVariable variable = compileStack.defineVariable(loopVariable, false);
+    private void writeOptimizedForEachLoop(final ForStatement loop, final Parameter loopVariable, final Expression collectionExpression, final ClassNode collectionType) {
+        CompileStack compileStack = controller.getCompileStack();
+        OperandStack operandStack = controller.getOperandStack();
+        MethodVisitor mv = controller.getMethodVisitor();
+        AsmClassGenerator acg = controller.getAcg();
 
+        BytecodeVariable variable = compileStack.defineVariable(loopVariable, false);
         Label continueLabel = compileStack.getContinueLabel();
         Label breakLabel = compileStack.getBreakLabel();
 
-        AsmClassGenerator acg = controller.getAcg();
-
         // load array on stack
         collectionExpression.visit(acg);
         mv.visitInsn(DUP);
@@ -155,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
@@ -172,27 +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();
-
-        if (isPrimitiveType(varType)) {
-            if (isPrimitiveInt(varType)) {
+        if (ClassHelper.isPrimitiveType(varType)) {
+            if (ClassHelper.isPrimitiveInt(varType)) {
                 mv.visitInsn(IALOAD);
-            } else if (isPrimitiveLong(varType)) {
+            } else if (ClassHelper.isPrimitiveLong(varType)) {
                 mv.visitInsn(LALOAD);
-            } else if (isPrimitiveByte(varType) || isPrimitiveBoolean(varType)) {
+            } else if (ClassHelper.isPrimitiveByte(varType) || ClassHelper.isPrimitiveBoolean(varType)) {
                 mv.visitInsn(BALOAD);
-            } else if (isPrimitiveChar(varType)) {
+            } else if (ClassHelper.isPrimitiveChar(varType)) {
                 mv.visitInsn(CALOAD);
-            } else if (isPrimitiveShort(varType)) {
+            } else if (ClassHelper.isPrimitiveShort(varType)) {
                 mv.visitInsn(SALOAD);
-            } else if (isPrimitiveFloat(varType)) {
+            } else if (ClassHelper.isPrimitiveFloat(varType)) {
                 mv.visitInsn(FALOAD);
-            } else if (isPrimitiveDouble(varType)) {
+            } else if (ClassHelper.isPrimitiveDouble(varType)) {
                 mv.visitInsn(DALOAD);
             }
         } else {
@@ -202,46 +185,19 @@ 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);
+        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);
 
-        Label continueLabel = compileStack.getContinueLabel();
-        Label breakLabel = compileStack.getBreakLabel();
-
         mv.visitLabel(continueLabel);
         mv.visitVarInsn(ALOAD, enumIdx);
         ENUMERATION_HASMORE_METHOD.call(mv);
@@ -253,12 +209,30 @@ public class StaticTypesStatementWriter extends StatementWriter {
         operandStack.push(ClassHelper.OBJECT_TYPE);
         operandStack.storeVar(variable);
 
-        // Generate the loop body
         loop.getLoopBlock().visit(controller.getAcg());
 
         mv.visitJumpInsn(GOTO, continueLabel);
         mv.visitLabel(breakLabel);
-
     }
 
+    private void writeIteratorBasedForEachLoop(final ForStatement loop, final Expression collectionExpression, final ClassNode collectionType) {
+        // GROOVY-10476: BaseStream provides an iterator() but does not implement Iterable
+        MethodNode iterator = collectionType.getMethod("iterator", Parameter.EMPTY_ARRAY);
+        if (iterator == null) {
+            iterator = GeneralUtils.getInterfacesAndSuperInterfaces(collectionType).stream()
+                    .map(in -> in.getMethod("iterator", Parameter.EMPTY_ARRAY))
+                    .filter(Objects::nonNull).findFirst().orElse(null);
+        }
+        if (iterator != null && iterator.getReturnType().equals(ClassHelper.Iterator_TYPE)) {
+            MethodCallExpression call = GeneralUtils.callX(collectionExpression, "iterator");
+            call.setImplicitThis(false);
+            call.setMethodTarget(iterator);
+            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/transform/stc/StaticTypeCheckingVisitor.java b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
index b121c4b..3dabf27 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -163,6 +163,7 @@ import static org.codehaus.groovy.ast.ClassHelper.OBJECT_TYPE;
 import static org.codehaus.groovy.ast.ClassHelper.PATTERN_TYPE;
 import static org.codehaus.groovy.ast.ClassHelper.RANGE_TYPE;
 import static org.codehaus.groovy.ast.ClassHelper.SET_TYPE;
+import static org.codehaus.groovy.ast.ClassHelper.STREAM_TYPE;
 import static org.codehaus.groovy.ast.ClassHelper.STRING_TYPE;
 import static org.codehaus.groovy.ast.ClassHelper.Short_TYPE;
 import static org.codehaus.groovy.ast.ClassHelper.VOID_TYPE;
@@ -1985,32 +1986,34 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
     }
 
     /**
-     * Given a loop collection type, returns the inferred type of the loop element. Used, for
-     * example, to infer the element type of a (for e in list) loop.
+     * Returns the inferred loop element type given a loop collection type. Used,
+     * for example, to infer the element type of a {@code for (e in list)} loop.
      *
      * @param collectionType the type of the collection
      * @return the inferred component type
+     * @see #inferComponentType
      */
     public static ClassNode inferLoopElementType(final ClassNode collectionType) {
         ClassNode componentType = collectionType.getComponentType();
         if (componentType == null) {
-            if (implementsInterfaceOrIsSubclassOf(collectionType, ITERABLE_TYPE)) {
-                ClassNode intf = GenericsUtils.parameterizeType(collectionType, ITERABLE_TYPE);
-                GenericsType[] genericsTypes = intf.getGenericsTypes();
-                componentType = genericsTypes[0].getType();
-            } else if (implementsInterfaceOrIsSubclassOf(collectionType, MAP_TYPE)) {
-                // GROOVY-6240
-                ClassNode intf = GenericsUtils.parameterizeType(collectionType, MAP_TYPE);
-                GenericsType[] genericsTypes = intf.getGenericsTypes();
-                componentType = MAP_ENTRY_TYPE.getPlainNodeReference();
-                componentType.setGenericsTypes(genericsTypes);
+            if (isOrImplements(collectionType, ITERABLE_TYPE)) {
+                ClassNode col = GenericsUtils.parameterizeType(collectionType, ITERABLE_TYPE);
+                componentType = getCombinedBoundType(col.getGenericsTypes()[0]);
+
+            } else if (isOrImplements(collectionType, MAP_TYPE)) { // GROOVY-6240
+                ClassNode col = GenericsUtils.parameterizeType(collectionType, MAP_TYPE);
+                componentType = makeClassSafe0(MAP_ENTRY_TYPE, col.getGenericsTypes());
+
+            } else if (isOrImplements(collectionType, STREAM_TYPE)) { // GROOVY-10476
+                ClassNode col = GenericsUtils.parameterizeType(collectionType, STREAM_TYPE);
+                componentType = getCombinedBoundType(col.getGenericsTypes()[0]);
+
+            } else if (isOrImplements(collectionType, ENUMERATION_TYPE)) { // GROOVY-6123
+                ClassNode col = GenericsUtils.parameterizeType(collectionType, ENUMERATION_TYPE);
+                componentType = getCombinedBoundType(col.getGenericsTypes()[0]);
+
             } else if (isStringType(collectionType)) {
                 componentType = STRING_TYPE;
-            } else if (ENUMERATION_TYPE.equals(collectionType)) {
-                // GROOVY-6123
-                ClassNode intf = GenericsUtils.parameterizeType(collectionType, ENUMERATION_TYPE);
-                GenericsType[] genericsTypes = intf.getGenericsTypes();
-                componentType = genericsTypes[0].getType();
             } else {
                 componentType = OBJECT_TYPE;
             }
diff --git a/src/test/groovy/transform/stc/MethodCallsSTCTest.groovy b/src/test/groovy/transform/stc/MethodCallsSTCTest.groovy
index dc0fb19..4d29461 100644
--- a/src/test/groovy/transform/stc/MethodCallsSTCTest.groovy
+++ b/src/test/groovy/transform/stc/MethodCallsSTCTest.groovy
@@ -1161,7 +1161,7 @@ class MethodCallsSTCTest extends StaticTypeCheckingTestCase {
     }
 
     // GROOVY-8133
-    void testSpreadDotOperator() {
+    void testSpreadDot() {
         assertScript '''
             def list = ['a','b','c'].stream()*.toUpperCase()
             assert list == ['A', 'B', 'C']
@@ -1186,6 +1186,17 @@ class MethodCallsSTCTest extends StaticTypeCheckingTestCase {
         '''
     }
 
+    // GROOVY-10476
+    void testForInLoop() {
+        assertScript '''
+            def list = []
+            for (item in ['a','b','c'].stream()) {
+                list.add(item.toUpperCase())
+            }
+            assert list == ['A', 'B', 'C']
+        '''
+    }
+
     void testBoxingShouldCostMore() {
         assertScript '''
             int foo(int x) { 1 }

[groovy] 03/03: GROOVY-10477: SC: optimize `for (item in array) { }` -- dynamic variable

Posted by em...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

emilles pushed a commit to branch GROOVY_4_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git

commit e3441dc12e93c31886376562a1165ef2aa4cbd3d
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Wed Feb 9 13:21:07 2022 -0600

    GROOVY-10477: SC: optimize `for (item in array) { }` -- dynamic variable
---
 .../asm/sc/StaticTypesStatementWriter.java         |  2 +-
 src/test/groovy/transform/stc/LoopsSTCTest.groovy  | 32 +++++++++++-----------
 .../classgen/asm/sc/LoopsStaticCompileTest.groovy  | 10 +++++--
 3 files changed, 25 insertions(+), 19 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 69c09c3..ee5829b 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
@@ -96,7 +96,7 @@ public class StaticTypesStatementWriter extends StatementWriter {
 
         int mark = operandStack.getStackLength();
         Parameter loopVariable = loop.getVariable();
-        if (collectionType.isArray() && loopVariable.getOriginType().equals(collectionType.getComponentType())) {
+        if (collectionType.isArray() && loopVariable.getType().equals(collectionType.getComponentType())) {
             writeOptimizedForEachLoop(loop, loopVariable, collectionExpression, collectionType);
         } else if (GeneralUtils.isOrImplements(collectionType, ENUMERATION_CLASSNODE)) {
             writeEnumerationBasedForEachLoop(loop, collectionExpression, collectionType);
diff --git a/src/test/groovy/transform/stc/LoopsSTCTest.groovy b/src/test/groovy/transform/stc/LoopsSTCTest.groovy
index 1d306f0..e91ec80 100644
--- a/src/test/groovy/transform/stc/LoopsSTCTest.groovy
+++ b/src/test/groovy/transform/stc/LoopsSTCTest.groovy
@@ -18,8 +18,6 @@
  */
 package groovy.transform.stc
 
-import groovy.transform.CompileStatic
-
 /**
  * Unit tests for static type checking : loops.
  */
@@ -37,23 +35,25 @@ class LoopsSTCTest extends StaticTypeCheckingTestCase {
         }
     }
 
-    // GROOVY-8882
-    void testStringCollectionLoop() {
-        for (s in 'abc') assert s instanceof String
-        for (String s in 'abc') assert s instanceof String
-
-        for (char c in 'abc') assert c instanceof Character
-        for (Character c in 'abc') assert c instanceof Character
+    void testForInLoopOnArray() {
+        assertScript '''
+            String[] strings = ['a','b','c']
+            for (string in strings) {
+                string.toUpperCase()
+            }
+        '''
     }
 
     // GROOVY-8882
-    @CompileStatic
-    void testStringCollectionLoopCS() {
-        for (s in 'abc') assert s instanceof String
-        for (String s in 'abc') assert s instanceof String
-
-        for (char c in 'abc') assert c instanceof Character
-        for (Character c in 'abc') assert c instanceof Character
+    void testForInLoopOnString() {
+        assertScript '''
+            for (s in 'abc') assert s instanceof String
+            for (String s in 'abc') assert s instanceof String
+        '''
+        assertScript '''
+            for (char c in 'abc') assert c instanceof Character
+            for (Character c in 'abc') assert c instanceof Character
+        '''
     }
 
     void testMethodCallWithEachAndDefAndTwoFooMethods() {
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 d9669f7..614b833 100644
--- a/src/test/org/codehaus/groovy/classgen/asm/sc/LoopsStaticCompileTest.groovy
+++ b/src/test/org/codehaus/groovy/classgen/asm/sc/LoopsStaticCompileTest.groovy
@@ -23,6 +23,12 @@ import groovy.transform.stc.LoopsSTCTest
 /**
  * Unit tests for static type checking : miscellaneous tests.
  */
-class LoopsStaticCompileTest extends LoopsSTCTest implements StaticCompilationTestSupport {
-}
+final class LoopsStaticCompileTest extends LoopsSTCTest implements StaticCompilationTestSupport {
 
+    // GROOVY-10477
+    void testForInLoopOnArray() {
+        super.testForInLoopOnArray()
+        def bytecode = astTrees.values()[0][1]
+        assert !bytecode.contains('INVOKESTATIC org/codehaus/groovy/runtime/DefaultGroovyMethods.iterator')
+    }
+}