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/23 20:40:03 UTC
[groovy] 02/02: GROOVY-9344, GROOVY-9516, GROOVY-9607: SC: track closure shared variable
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 <er...@thomsonreuters.com>
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
-
'''
}
}