From 5d4a951bb653bb1a7c8570d28246396eaa3e0713 Mon Sep 17 00:00:00 2001 From: Paul King Date: Wed, 21 Nov 2018 14:35:36 +1000 Subject: [PATCH 1/2] GROOVY-8882: CS: Loop over elements of String has different element type (closes #827) --- .../transform/sc/StaticCompilationVisitor.java | 11 ++++++++++- .../stc/StaticTypeCheckingVisitor.java | 10 ++++++++-- .../groovy/transform/stc/LoopsSTCTest.groovy | 18 ++++++++++++++++++ 3 files changed, 36 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/codehaus/groovy/transform/sc/StaticCompilationVisitor.java b/src/main/java/org/codehaus/groovy/transform/sc/StaticCompilationVisitor.java index f66b24a04c9..036dbe54d38 100644 --- a/src/main/java/org/codehaus/groovy/transform/sc/StaticCompilationVisitor.java +++ b/src/main/java/org/codehaus/groovy/transform/sc/StaticCompilationVisitor.java @@ -72,6 +72,8 @@ import java.util.Map; import java.util.Set; +import static org.codehaus.groovy.ast.ClassHelper.Character_TYPE; +import static org.codehaus.groovy.ast.ClassHelper.STRING_TYPE; import static org.codehaus.groovy.ast.tools.GenericsUtils.addMethodGenerics; import static org.codehaus.groovy.ast.tools.GenericsUtils.applyGenericsContextToPlaceHolders; import static org.codehaus.groovy.ast.tools.GenericsUtils.correctToGenericsSpecRecurse; @@ -440,7 +442,14 @@ public void visitForLoop(final ForStatement forLoop) { Expression collectionExpression = forLoop.getCollectionExpression(); if (!(collectionExpression instanceof ClosureListExpression)) { final ClassNode collectionType = getType(forLoop.getCollectionExpression()); - ClassNode componentType = inferLoopElementType(collectionType); + ClassNode forLoopVariableType = forLoop.getVariableType(); + ClassNode componentType; + if (Character_TYPE.equals(ClassHelper.getWrapper(forLoopVariableType)) && STRING_TYPE.equals(collectionType)) { + // we allow auto-coercion here + componentType = forLoopVariableType; + } else { + componentType = inferLoopElementType(collectionType); + } forLoop.getVariable().setType(componentType); forLoop.getVariable().setOriginType(componentType); } 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 de2abff5a64..12a48d3f864 100644 --- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java +++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java @@ -1886,8 +1886,14 @@ public void visitForLoop(final ForStatement forLoop) { } else { collectionExpression.visit(this); final ClassNode collectionType = getType(collectionExpression); - ClassNode componentType = inferLoopElementType(collectionType); ClassNode forLoopVariableType = forLoop.getVariableType(); + ClassNode componentType; + if (Character_TYPE.equals(ClassHelper.getWrapper(forLoopVariableType)) && STRING_TYPE.equals(collectionType)) { + // we allow auto-coercion here + componentType = forLoopVariableType; + } else { + componentType = inferLoopElementType(collectionType); + } if (ClassHelper.getUnwrapper(componentType) == forLoopVariableType) { // prefer primitive type over boxed type componentType = forLoopVariableType; @@ -1931,7 +1937,7 @@ public static ClassNode inferLoopElementType(final ClassNode collectionType) { componentType = MAP_ENTRY_TYPE.getPlainNodeReference(); componentType.setGenericsTypes(genericsTypes); } else if (STRING_TYPE.equals(collectionType)) { - componentType = ClassHelper.Character_TYPE; + componentType = ClassHelper.STRING_TYPE; } else if (ENUMERATION_TYPE.equals(collectionType)) { // GROOVY-6123 ClassNode intf = GenericsUtils.parameterizeType(collectionType, ENUMERATION_TYPE); diff --git a/src/test/groovy/transform/stc/LoopsSTCTest.groovy b/src/test/groovy/transform/stc/LoopsSTCTest.groovy index cdb07abaace..0bf2e1fdc86 100644 --- a/src/test/groovy/transform/stc/LoopsSTCTest.groovy +++ b/src/test/groovy/transform/stc/LoopsSTCTest.groovy @@ -18,6 +18,8 @@ */ package groovy.transform.stc +import groovy.transform.CompileStatic + /** * Unit tests for static type checking : loops. * @@ -45,6 +47,22 @@ class LoopsSTCTest extends StaticTypeCheckingTestCase { ''' } + // GROOVY-8882 + void testStringCollectionLoop() { + for (char c in 'abc') assert c instanceof Character + for (Character c in 'abc') assert c instanceof Character + for (String s in 'abc') assert s instanceof String + for (s in 'abc') assert s instanceof String + } + + // GROOVY-8882 + @CompileStatic + void testStringCollectionLoopCS() { + for (char c in 'abc') assert c instanceof Character + for (Character c in 'abc') assert c instanceof Character + for (String s in 'abc') assert s instanceof String + for (s in 'abc') assert s instanceof String + } void testMethodCallWithEachAndDefAndTwoFooMethods() { shouldFailWithMessages ''' From 95f692dc450b2348655564ef69be51937cb95422 Mon Sep 17 00:00:00 2001 From: Paul King Date: Fri, 23 Nov 2018 16:29:06 +1000 Subject: [PATCH 2/2] GROOVY-7647: Incorrect line information for debug (closes #830) --- .../org/codehaus/groovy/classgen/asm/StatementWriter.java | 7 +++++++ .../groovy/classgen/asm/sc/StaticCompilationTest.groovy | 3 +-- 2 files changed, 8 insertions(+), 2 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 d92f67e08e5..f19b9cc6ddb 100644 --- a/src/main/java/org/codehaus/groovy/classgen/asm/StatementWriter.java +++ b/src/main/java/org/codehaus/groovy/classgen/asm/StatementWriter.java @@ -94,6 +94,13 @@ public void writeBlockStatement(BlockStatement block) { } compileStack.pop(); + // GROOVY-7647 + if (block.getLastLineNumber() > 0) { + MethodVisitor mv = controller.getMethodVisitor(); + Label blockEnd = new Label(); mv.visitLabel(blockEnd); + mv.visitLineNumber(block.getLastLineNumber(), blockEnd); + } + controller.getOperandStack().popDownTo(mark); } diff --git a/src/test/org/codehaus/groovy/classgen/asm/sc/StaticCompilationTest.groovy b/src/test/org/codehaus/groovy/classgen/asm/sc/StaticCompilationTest.groovy index b83430a8a30..318f692b932 100644 --- a/src/test/org/codehaus/groovy/classgen/asm/sc/StaticCompilationTest.groovy +++ b/src/test/org/codehaus/groovy/classgen/asm/sc/StaticCompilationTest.groovy @@ -19,8 +19,6 @@ package org.codehaus.groovy.classgen.asm.sc import org.codehaus.groovy.classgen.asm.AbstractBytecodeTestCase -import org.codehaus.groovy.control.CompilerConfiguration -import org.codehaus.groovy.transform.stc.GroovyTypeCheckingExtensionSupport import static org.codehaus.groovy.control.CompilerConfiguration.DEFAULT as config @@ -33,6 +31,7 @@ class StaticCompilationTest extends AbstractBytecodeTestCase { assert bytecode.hasStrictSequence( ['public m()V', 'L0', + 'LINENUMBER 3 L0', 'RETURN'] ) }