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/03 19:37:34 UTC
[groovy] 02/04: GROOVY-10457: support `@CompileStatic` class with `@CompileDynamic` ctor
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 bb1789f8f35bc8199615e3238a413fa8cc2750c5
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Fri Jan 21 13:45:48 2022 -0600
GROOVY-10457: support `@CompileStatic` class with `@CompileDynamic` ctor
Conflicts:
src/main/java/org/codehaus/groovy/transform/sc/StaticCompilationVisitor.java
---
.../transform/sc/StaticCompilationVisitor.java | 58 +++++-----
.../classgen/asm/sc/CompileDynamicTest.groovy | 40 +++++--
.../asm/sc/MixedModeStaticCompilationTest.groovy | 14 +++
.../asm/sc/StaticCompileConstructorsTest.groovy | 126 +++++++++------------
4 files changed, 131 insertions(+), 107 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 f78f1c3..2484e1c 100644
--- a/src/main/java/org/codehaus/groovy/transform/sc/StaticCompilationVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/sc/StaticCompilationVisitor.java
@@ -192,38 +192,44 @@ public class StaticCompilationVisitor extends StaticTypeCheckingVisitor {
return false;
}
- /**
- * If we are in a constructor, that is static compiled, but in a class, that
- * is not, it may happen that init code from object initializers, fields
- * or properties is added into the constructor code. The backend assumes
- * a purely static constructor, so it may fail if it encounters dynamic
- * code here. Thus we make this kind of code fail
- */
- private void checkForConstructorWithCSButClassWithout(MethodNode node) {
- if (!(node instanceof ConstructorNode)) return;
- Object meta = node.getNodeMetaData(STATIC_COMPILE_NODE);
- if (!Boolean.TRUE.equals(meta)) return;
- ClassNode clz = typeCheckingContext.getEnclosingClassNode();
- meta = clz.getNodeMetaData(STATIC_COMPILE_NODE);
- if (Boolean.TRUE.equals(meta)) return;
- if ( clz.getObjectInitializerStatements().isEmpty() &&
- clz.getFields().isEmpty() &&
- clz.getProperties().isEmpty())
- {
- return;
+ private void visitConstructorOrMethod(final MethodNode node) {
+ boolean isSkipped = isSkipMode(node); // @CompileDynamic
+ boolean isSC = !isSkipped && isStaticallyCompiled(node);
+ if (isSkipped) {
+ node.putNodeMetaData(STATIC_COMPILE_NODE, Boolean.FALSE);
}
+ if (node instanceof ConstructorNode) {
+ super.visitConstructor((ConstructorNode) node);
+ ClassNode declaringClass = node.getDeclaringClass();
+ if (isSC && !isStaticallyCompiled(declaringClass)) {
+ // In a constructor that is statically compiled within a class that is
+ // not, it may happen that init code from object initializers, fields or
+ // properties is added into the constructor code. The backend assumes a
+ // purely static constructor, so it may fail if it encounters dynamic
+ // code here. Thus we make this kind of code fail.
+ if (!declaringClass.getFields().isEmpty()
+ || !declaringClass.getProperties().isEmpty()
+ || !declaringClass.getObjectInitializerStatements().isEmpty()) {
+ addStaticTypeError("Cannot statically compile constructor implicitly including non-static elements from fields, properties or initializers", node);
+ }
+ }
+ } else {
+ super.visitMethod(node);
+ }
+ if (isSC) {
+ ClassNode declaringClass = node.getDeclaringClass();
+ addDynamicOuterClassAccessorsCallback(declaringClass);
+ }
+ }
- addStaticTypeError("Cannot statically compile constructor implicitly including non static elements from object initializers, properties or fields.",node);
+ @Override
+ public void visitConstructor(final ConstructorNode node) {
+ visitConstructorOrMethod(node);
}
@Override
public void visitMethod(final MethodNode node) {
- if (isSkipMode(node)) {
- node.putNodeMetaData(STATIC_COMPILE_NODE, false);
- }
- super.visitMethod(node);
- checkForConstructorWithCSButClassWithout(node);
- if (isStaticallyCompiled(node)) addDynamicOuterClassAccessorsCallback(node.getDeclaringClass());
+ visitConstructorOrMethod(node);
}
/**
diff --git a/src/test/org/codehaus/groovy/classgen/asm/sc/CompileDynamicTest.groovy b/src/test/org/codehaus/groovy/classgen/asm/sc/CompileDynamicTest.groovy
index c6db3f6..22bdb30 100644
--- a/src/test/org/codehaus/groovy/classgen/asm/sc/CompileDynamicTest.groovy
+++ b/src/test/org/codehaus/groovy/classgen/asm/sc/CompileDynamicTest.groovy
@@ -19,21 +19,47 @@
package org.codehaus.groovy.classgen.asm.sc
import groovy.transform.stc.StaticTypeCheckingTestCase
+import org.codehaus.groovy.control.customizers.ASTTransformationCustomizer
/**
* Test case for {@link groovy.transform.CompileDynamic}.
*/
-class CompileDynamicTest extends StaticTypeCheckingTestCase implements StaticCompilationTestSupport {
+final class CompileDynamicTest extends StaticTypeCheckingTestCase implements StaticCompilationTestSupport {
- void testCompileDynamic() {
- assertScript '''import groovy.transform.CompileDynamic
- class Foo {
+ @Override
+ protected void setUp() {
+ super.setUp()
+ def customizers = config.compilationCustomizers
+ // ASTTransformationCustomizer(CompileStatic) only uses visitMethod
+ customizers.removeAll { it instanceof ASTTransformationCustomizer }
+ customizers[0].addImports('groovy.transform.CompileDynamic', 'groovy.transform.CompileStatic')
+ }
+
+ void testCompileDynamicMethod() {
+ assertScript '''
+ @CompileStatic
+ class C {
+ @CompileDynamic
+ void skip() {
+ int i = 'cannot assign string to int'
+ }
+ }
+ new C()
+ '''
+ }
+
+ // GROOVY-10457
+ void testCompileDynamicConstructor() {
+ assertScript '''
+ @CompileStatic
+ class C {
@CompileDynamic
- void skipped() {
- int i = 'should not pass'
+ C() {
+ String result = new StringReader('works').text
+ assert result == 'works'
}
}
- new Foo()
+ new C()
'''
}
}
diff --git a/src/test/org/codehaus/groovy/classgen/asm/sc/MixedModeStaticCompilationTest.groovy b/src/test/org/codehaus/groovy/classgen/asm/sc/MixedModeStaticCompilationTest.groovy
index 395e682..36b9a9f 100644
--- a/src/test/org/codehaus/groovy/classgen/asm/sc/MixedModeStaticCompilationTest.groovy
+++ b/src/test/org/codehaus/groovy/classgen/asm/sc/MixedModeStaticCompilationTest.groovy
@@ -292,6 +292,20 @@ class MixedModeStaticCompilationTest extends StaticTypeCheckingTestCase implemen
}
+ void testDynamicClassWithStaticConstructorAndInitialization() {
+ shouldFailWithMessages '''
+ class A {
+ }
+ class B {
+ A a = new A() // may require dynamic support...
+ @groovy.transform.CompileStatic
+ B() {
+ }
+ }
+ ''',
+ 'Cannot statically compile constructor implicitly including non-static elements from fields, properties or initializers'
+ }
+
void testSCClosureCanAccessPrivateFieldsOfNonSCEnclosingClass() {
assertScript '''
class Test {
diff --git a/src/test/org/codehaus/groovy/classgen/asm/sc/StaticCompileConstructorsTest.groovy b/src/test/org/codehaus/groovy/classgen/asm/sc/StaticCompileConstructorsTest.groovy
index e44cec6..dce86d3 100644
--- a/src/test/org/codehaus/groovy/classgen/asm/sc/StaticCompileConstructorsTest.groovy
+++ b/src/test/org/codehaus/groovy/classgen/asm/sc/StaticCompileConstructorsTest.groovy
@@ -26,95 +26,73 @@ import groovy.transform.stc.ConstructorsSTCTest
class StaticCompileConstructorsTest extends ConstructorsSTCTest implements StaticCompilationTestSupport {
void testMapConstructorError() {
- assertScript '''import groovy.transform.Canonical
-
- class WTF {
- public static void main(String[] args) {
- new Person(name:"First")
- first(new Person(name:"First"))
+ assertScript '''
+ class C {
+ static void test() {
+ new Person(name:"First")
+ first(new Person(name:"First"))
+ }
+ static Person first(Person p) {
+ p
+ }
}
-
- static Person first(Person p) {
- p
+ @groovy.transform.Canonical
+ class Person {
+ String name
}
- }
-
- @Canonical
- class Person {
- String name
- }
- WTF.main()
+ C.test()
'''
}
-
- void testMixedDynamicStaticConstructor() {
- shouldFailWithMessages("""
- class A{}
- class B {
- A a = new A();
- @groovy.transform.CompileStatic
- B(){}
- }
- """, "Cannot statically compile constructor implicitly including non static elements from object initializers, properties or fields")
- }
void testPrivateConstructorFromClosure() {
- try {
- assertScript '''
- class Foo {
- String s
- private Foo(String s) { this.s = s }
- static Foo makeFoo(String s) {
- def cl = { new Foo(s) }
- cl()
- }
+ assertScript '''
+ class C {
+ String s
+ private C(String s) {
+ this.s = s
+ }
+ static C make(String s) {
+ def cl = { new C(s) }
+ cl()
}
- assert Foo.makeFoo('pls').s == 'pls'
- '''
- } finally {
- //println astTrees
- }
+ }
+ assert C.make('pls').s == 'pls'
+ '''
}
void testPrivateConstructorFromNestedClass() {
- try {
- assertScript '''
- class Foo {
- String s
- private Foo(String s) { this.s = s }
- static class Bar {
- static Foo makeFoo(String s) { new Foo(s) }
- }
-
+ assertScript '''
+ class Foo {
+ String s
+ private Foo(String s) {
+ this.s = s
+ }
+ static class Bar {
+ static Foo makeFoo(String s) { new Foo(s) }
}
- assert Foo.Bar.makeFoo('pls').s == 'pls'
- '''
- } finally {
- //println astTrees
- }
+
+ }
+ assert Foo.Bar.makeFoo('pls').s == 'pls'
+ '''
}
void testPrivateConstructorFromAIC() {
- try {
- assertScript '''
- class Foo {
- String s
- private Foo(String s) { this.s = s }
- static Foo makeFoo(String s) {
- return new Object() {
- Foo makeFoo(String x) {
- new Foo(x)
- }
- }.makeFoo(s)
- }
+ assertScript '''
+ class Foo {
+ String s
+ private Foo(String s) {
+ this.s = s
}
- assert Foo.makeFoo('pls').s == 'pls'
- '''
- } finally {
- //println astTrees
- }
+ static Foo makeFoo(String s) {
+ new Object() {
+ Foo makeFoo(String x) {
+ new Foo(x)
+ }
+ }.makeFoo(s)
+ }
+ }
+ assert Foo.makeFoo('pls').s == 'pls'
+ '''
}
-
}
-