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:01 UTC
[groovy] branch GROOVY_2_5_X updated (dd79fdc -> 54aa63e)
This is an automated email from the ASF dual-hosted git repository.
emilles pushed a change to branch GROOVY_2_5_X
in repository https://gitbox.apache.org/repos/asf/groovy.git.
from dd79fdc GROOVY-10289: check for static enclosing type(s) before accessing this$0
new 535a086 GROOVY-10424: SC: prevent infinite recursion for `isExtended(ClassNode)`
new 54aa63e GROOVY-9344, GROOVY-9516, GROOVY-9607: SC: track closure shared variable
The 2 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:
.../classgen/asm/sc/StaticTypesTypeChooser.java | 34 ++++++-----
.../transformers/BooleanExpressionTransformer.java | 10 ++--
.../transform/stc/StaticTypeCheckingVisitor.java | 13 ++--
src/test/groovy/transform/stc/BugsSTCTest.groovy | 36 +++++++++++
.../groovy/transform/stc/ClosuresSTCTest.groovy | 70 +++++++++++++++++++---
.../asm/sc/StaticCompileFlowTypingTest.groovy | 40 ++++++++++++-
6 files changed, 167 insertions(+), 36 deletions(-)
[groovy] 01/02: GROOVY-10424: SC: prevent infinite recursion for `isExtended(ClassNode)`
Posted by em...@apache.org.
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 535a08684bf51379d5c79310dd775e8c21771ed3
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Sat Dec 18 10:56:39 2021 -0600
GROOVY-10424: SC: prevent infinite recursion for `isExtended(ClassNode)`
---
.../transformers/BooleanExpressionTransformer.java | 10 +++---
src/test/groovy/transform/stc/BugsSTCTest.groovy | 36 ++++++++++++++++++++++
2 files changed, 40 insertions(+), 6 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/transform/sc/transformers/BooleanExpressionTransformer.java b/src/main/java/org/codehaus/groovy/transform/sc/transformers/BooleanExpressionTransformer.java
index 146be81..851dd06 100644
--- a/src/main/java/org/codehaus/groovy/transform/sc/transformers/BooleanExpressionTransformer.java
+++ b/src/main/java/org/codehaus/groovy/transform/sc/transformers/BooleanExpressionTransformer.java
@@ -76,14 +76,12 @@ public class BooleanExpressionTransformer {
}
return transformer.superTransform(booleanExpression);
}
-
+
private static boolean isExtended(ClassNode owner, Iterator<InnerClassNode> classes) {
while (classes.hasNext()) {
- InnerClassNode next = classes.next();
+ InnerClassNode next = classes.next();
if (next!=owner && next.isDerivedFrom(owner)) return true;
- }
- if (owner.getInnerClasses().hasNext()) {
- return isExtended(owner, owner.getInnerClasses());
+ if (isExtended(owner,next.getInnerClasses())) return true;
}
return false;
}
@@ -203,4 +201,4 @@ public class BooleanExpressionTransformer {
}
}
}
-}
\ No newline at end of file
+}
diff --git a/src/test/groovy/transform/stc/BugsSTCTest.groovy b/src/test/groovy/transform/stc/BugsSTCTest.groovy
index 6635e0b..bbfa7db 100644
--- a/src/test/groovy/transform/stc/BugsSTCTest.groovy
+++ b/src/test/groovy/transform/stc/BugsSTCTest.groovy
@@ -914,4 +914,40 @@ Printer
}
'''
}
+
+ // GROOVY-10424
+ void testPrivateInnerClassOptimizedBooleanExpr1() {
+ assertScript '''
+ class Outer {
+ private static class Inner {
+ private Inner() {} // triggers creation of Inner$1 in StaticCompilationVisitor$addPrivateBridgeMethods
+ }
+ void test() {
+ def inner = new Inner()
+ if (inner) { // optimized boolean expression; StackOverflowError
+ assert true
+ }
+ }
+ }
+ new Outer().test()
+ '''
+ }
+
+ // GROOVY-10424
+ void testPrivateInnerClassOptimizedBooleanExpr2() {
+ assertScript '''
+ class Outer {
+ private static class Inner {
+ static class Three {}
+ }
+ void test() {
+ def inner = new Inner()
+ if (inner) { // optimized boolean expression; StackOverflowError
+ assert true
+ }
+ }
+ }
+ new Outer().test()
+ '''
+ }
}
[groovy] 02/02: GROOVY-9344, GROOVY-9516, GROOVY-9607: SC: track closure shared 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_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
-
'''
}
}