This is an automated email from the ASF dual-hosted git repository. emilles pushed a commit to branch GROOVY_2_5_X in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 54aa63e83c50b638ff907fab34e60bf8f66a58d5 Author: Eric Milles <[email protected]> AuthorDate: Tue Jun 2 21:55:11 2020 -0500 GROOVY-9344, GROOVY-9516, GROOVY-9607: SC: track closure shared variable (cherry picked from commit 57d7723079479decb122da3d9686dfced7163c6f) (cherry picked from commit bf614817fdd5d5136c79f89eca3fa8b4fae3fbf8) (cherry picked from commit 99d0b07c18a2609a5d02aa92090837b1be776695) Conflicts: src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesTypeChooser.java src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java --- .../classgen/asm/sc/StaticTypesTypeChooser.java | 34 ++++++----- .../transform/stc/StaticTypeCheckingVisitor.java | 13 ++-- .../groovy/transform/stc/ClosuresSTCTest.groovy | 70 +++++++++++++++++++--- .../asm/sc/StaticCompileFlowTypingTest.groovy | 40 ++++++++++++- 4 files changed, 127 insertions(+), 30 deletions(-) diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesTypeChooser.java b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesTypeChooser.java index fbcaa94..73d4830 100644 --- a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesTypeChooser.java +++ b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesTypeChooser.java @@ -21,27 +21,22 @@ package org.codehaus.groovy.classgen.asm.sc; import org.codehaus.groovy.ast.ASTNode; import org.codehaus.groovy.ast.ClassHelper; import org.codehaus.groovy.ast.ClassNode; -import org.codehaus.groovy.ast.Parameter; import org.codehaus.groovy.ast.expr.Expression; import org.codehaus.groovy.ast.expr.VariableExpression; import org.codehaus.groovy.classgen.asm.StatementMetaTypeChooser; import org.codehaus.groovy.transform.stc.StaticTypesMarker; /** - * A {@link org.codehaus.groovy.classgen.asm.TypeChooser} which reads type information from node metadata - * generated by the {@link groovy.transform.CompileStatic} annotation. + * A {@link org.codehaus.groovy.classgen.asm.TypeChooser TypeChooser} which reads + * type information from node metadata generated by the static type checker. */ public class StaticTypesTypeChooser extends StatementMetaTypeChooser { @Override public ClassNode resolveType(final Expression exp, final ClassNode current) { - ASTNode target = exp instanceof VariableExpression ? getTarget((VariableExpression) exp) : exp; + ASTNode target = getTarget(exp); // GROOVY-9344, GROOVY-9607 ClassNode inferredType = target.getNodeMetaData(StaticTypesMarker.DECLARATION_INFERRED_TYPE); if (inferredType == null) { inferredType = target.getNodeMetaData(StaticTypesMarker.INFERRED_TYPE); - if (inferredType == null && target instanceof VariableExpression && ((VariableExpression) target).getAccessedVariable() instanceof Parameter) { - target = (Parameter) ((VariableExpression) target).getAccessedVariable(); - inferredType = ((Parameter) target).getOriginType(); - } } if (inferredType != null) { if (ClassHelper.VOID_TYPE == inferredType) { @@ -59,15 +54,22 @@ public class StaticTypesTypeChooser extends StatementMetaTypeChooser { } /** - * The inferred type, in case of a variable expression, can be set on the accessed variable, so we take it instead - * of the facade one. + * The inferred type, in case of a variable expression, can be set on the + * accessed variable, so we take it instead of the reference. * - * @param ve the variable expression for which to return the target expression - * @return the target variable expression + * @param exp the expression for which to return the target expression + * @return the target node */ - private static VariableExpression getTarget(VariableExpression ve) { - if (ve.getAccessedVariable() == null || ve.getAccessedVariable() == ve || (!(ve.getAccessedVariable() instanceof VariableExpression))) - return ve; - return getTarget((VariableExpression) ve.getAccessedVariable()); + private static ASTNode getTarget(final Expression exp) { + ASTNode target = exp; + while (target instanceof VariableExpression) { + Object var = ((VariableExpression) target).getAccessedVariable(); + if (var instanceof ASTNode && var != target) { + target = (ASTNode) var; + } else { + break; + } + } + return target; } } 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 e9b25ea..695b69f 100644 --- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java +++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java @@ -2451,6 +2451,9 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport { VariableExpression ve = entry.getKey(); ListHashMap metadata = entry.getValue(); for (StaticTypesMarker marker : StaticTypesMarker.values()) { + // GROOVY-9344, GROOVY-9516 + if (marker == INFERRED_TYPE) continue; + ve.removeNodeMetaData(marker); Object value = metadata.get(marker); if (value != null) ve.setNodeMetaData(marker, value); @@ -2464,6 +2467,12 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport { // GROOVY-6921: We must force a call to getType in order to update closure shared variable whose // types are inferred thanks to closure parameter type inference getType(ve); + + Variable v; // GROOVY-9344 + while ((v = ve.getAccessedVariable()) != ve && v instanceof VariableExpression) { + ve = (VariableExpression) v; + } + ListHashMap<StaticTypesMarker, Object> metadata = new ListHashMap<StaticTypesMarker, Object>(); for (StaticTypesMarker marker : StaticTypesMarker.values()) { Object value = ve.getNodeMetaData(marker); @@ -2472,10 +2481,6 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport { } } typesBeforeVisit.put(ve, metadata); - Variable accessedVariable = ve.getAccessedVariable(); - if (accessedVariable != ve && accessedVariable instanceof VariableExpression) { - saveVariableExpressionMetadata(Collections.singleton((VariableExpression) accessedVariable), typesBeforeVisit); - } } } diff --git a/src/test/groovy/transform/stc/ClosuresSTCTest.groovy b/src/test/groovy/transform/stc/ClosuresSTCTest.groovy index 64a7bcc..d17a6a8 100644 --- a/src/test/groovy/transform/stc/ClosuresSTCTest.groovy +++ b/src/test/groovy/transform/stc/ClosuresSTCTest.groovy @@ -25,20 +25,18 @@ class ClosuresSTCTest extends StaticTypeCheckingTestCase { void testClosureWithoutArguments() { assertScript ''' - def clos = { println "hello!" } + def clos = { println "hello!" } - println "Executing the Closure:" - clos() //prints "hello!" + println "Executing the Closure:" + clos() //prints "hello!" ''' } + // GROOVY-9079: no params to statically type check but shouldn't get NPE void testClosureWithoutArgumentsExplicit() { - // GROOVY-9079: no params to statically type check but shouldn't get NPE assertScript ''' - import groovy.transform.CompileStatic import java.util.concurrent.Callable - @CompileStatic String makeFoo() { Callable<String> call = { -> 'foo' } call() @@ -244,9 +242,63 @@ class ClosuresSTCTest extends StaticTypeCheckingTestCase { void testClosureSharedVariableWithIncompatibleType() { shouldFailWithMessages ''' def x = '123'; - { -> x = 1 } - x.charAt(0) - ''', 'A closure shared variable [x] has been assigned with various types and the method [charAt(int)] does not exist in the lowest upper bound' + { -> x = 123 } + x.charAt(0) // available in String but not available in Integer + ''', + 'Cannot find matching method java.io.Serializable or java.lang.Comparable' + } + + // GROOVY-9516 + void testClosureSharedVariable3() { + shouldFailWithMessages ''' + class A {} + class B extends A { def m() {} } + class C extends A {} + + void test() { + def x = new B(); + { -> x = new C() }(); + def c = x + c.m() + } + ''', + 'Cannot find matching method A#m()' + } + + // GROOVY-9607 + void testClosureSharedVariableInferredType1() { + assertScript ''' + static help(Runnable runner) { + runner.run() + } + void test(item, MetaProperty prop) { + def name = prop.name + help(new Runnable() { + void run() { + assert item[name] == 'bar' // STC throws GBE if 'name' to infers as Object + } + }) + } + test([foo:'bar'], new MetaBeanProperty('foo', null, null, null)) + ''' + } + + // GROOVY-9607 + void testClosureSharedVariableInferredType2() { + assertScript ''' + static help(Runnable runner) { + runner.run() + } + void test(item, name, MetaProperty prop) { + name = prop.name + help(new Runnable() { + void run() { + assert item[name] == 'bar' // STC throws GBE if 'name' to infers as Object + } + }) + } + test([foo:'bar'], null, new MetaBeanProperty('foo', null, null, null)) + ''' } void testClosureCallAsAMethod() { diff --git a/src/test/org/codehaus/groovy/classgen/asm/sc/StaticCompileFlowTypingTest.groovy b/src/test/org/codehaus/groovy/classgen/asm/sc/StaticCompileFlowTypingTest.groovy index 81894f2..2ef21bf 100644 --- a/src/test/org/codehaus/groovy/classgen/asm/sc/StaticCompileFlowTypingTest.groovy +++ b/src/test/org/codehaus/groovy/classgen/asm/sc/StaticCompileFlowTypingTest.groovy @@ -21,6 +21,7 @@ package org.codehaus.groovy.classgen.asm.sc import org.codehaus.groovy.classgen.asm.AbstractBytecodeTestCase class StaticCompileFlowTypingTest extends AbstractBytecodeTestCase { + void testFlowTyping() { assertScript ''' @groovy.transform.CompileStatic @@ -36,6 +37,44 @@ class StaticCompileFlowTypingTest extends AbstractBytecodeTestCase { ''' } + // GROOVY-9344 + void testFlowTyping2() { + assertScript ''' + class A {} + class B {} + + @groovy.transform.CompileStatic + String m() { + def x = new A() + def c = { -> + x = new B() + x.class.simpleName + } + c() + } + assert m() == 'B' + ''' + } + + // GROOVY-9344 + void testFlowTyping3() { + assertScript ''' + class A {} + class B {} + + @groovy.transform.CompileStatic + String m() { + def x = new A() + def c = { -> + x = new B() + } + c() + x.class.simpleName + } + assert m() == 'B' + ''' + } + void testInstanceOf() { assertScript ''' @groovy.transform.CompileStatic @@ -90,7 +129,6 @@ class StaticCompileFlowTypingTest extends AbstractBytecodeTestCase { assert a.foo(arr[0]) == 1 assert a.foo(arr[1]) == 2 assert a.foo(arr[2]) == 3 - ''' } }
