You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by su...@apache.org on 2020/02/12 15:53:21 UTC
[groovy] branch master updated: GROOVY-9385: add private field
accessor for each mutated private field
This is an automated email from the ASF dual-hosted git repository.
sunlan pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git
The following commit(s) were added to refs/heads/master by this push:
new 4999bbc GROOVY-9385: add private field accessor for each mutated private field
4999bbc is described below
commit 4999bbcb5f41dbd604aad529130a8b3d16637fd4
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Wed Feb 5 15:24:57 2020 -0600
GROOVY-9385: add private field accessor for each mutated private field
---
.../transform/sc/StaticCompilationVisitor.java | 8 +++
.../groovy/transform/stc/STCAssignmentTest.groovy | 73 ++++++++++++++++++----
.../sc/FieldsAndPropertiesStaticCompileTest.groovy | 48 +++++++-------
3 files changed, 95 insertions(+), 34 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 73b10a4..79fe33a 100644
--- a/src/main/java/org/codehaus/groovy/transform/sc/StaticCompilationVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/sc/StaticCompilationVisitor.java
@@ -58,10 +58,13 @@ import org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport;
import org.codehaus.groovy.transform.stc.StaticTypeCheckingVisitor;
import java.util.ArrayList;
+import java.util.Collections;
import java.util.HashMap;
+import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
+import java.util.Optional;
import java.util.Set;
import static org.codehaus.groovy.ast.ClassHelper.Character_TYPE;
@@ -250,6 +253,11 @@ public class StaticCompilationVisitor extends StaticTypeCheckingVisitor {
Set<ASTNode> accessedFields = node.getNodeMetaData(PV_FIELDS_ACCESS);
Set<ASTNode> mutatedFields = node.getNodeMetaData(PV_FIELDS_MUTATION);
if (accessedFields == null && mutatedFields == null) return;
+ // GROOVY-9385: mutation includes access in case of compound assignment or pre/post-increment/decrement
+ if (mutatedFields != null) {
+ accessedFields = new HashSet<>(Optional.ofNullable(accessedFields).orElseGet(Collections::emptySet));
+ accessedFields.addAll(mutatedFields);
+ }
int acc = -1;
privateFieldAccessors = (accessedFields != null ? new HashMap<>() : null);
diff --git a/src/test/groovy/transform/stc/STCAssignmentTest.groovy b/src/test/groovy/transform/stc/STCAssignmentTest.groovy
index f9a7a06..d691ad2 100644
--- a/src/test/groovy/transform/stc/STCAssignmentTest.groovy
+++ b/src/test/groovy/transform/stc/STCAssignmentTest.groovy
@@ -138,6 +138,54 @@ class STCAssignmentTest extends StaticTypeCheckingTestCase {
"""
}
+ // GROOVY-9385
+ void testPlusEqualsOnPrivateField() {
+ assertScript '''
+ class C {
+ private int i
+
+ int test() {
+ { ->
+ i += 1
+ }.call()
+ }
+ }
+ assert new C().test() == 1
+ '''
+ }
+
+ // GROOVY-9385
+ void testPrefixPlusPlusOnPrivateField() {
+ assertScript '''
+ class C {
+ private int i
+
+ int test() {
+ { ->
+ ++i
+ }.call()
+ }
+ }
+ assert new C().test() == 1
+ '''
+ }
+
+ // GROOVY-9385
+ void testPostfixPlusPlusOnPrivateField() {
+ assertScript '''
+ class C {
+ private int i
+
+ int test() {
+ { ->
+ i++
+ }.call()
+ }
+ }
+ assert new C().test() == 0
+ '''
+ }
+
void testPossibleLooseOfPrecision() {
shouldFailWithMessages '''
long a = Long.MAX_VALUE
@@ -210,7 +258,7 @@ class STCAssignmentTest extends StaticTypeCheckingTestCase {
float f = (float) 1
'''
}
-
+
void testCompatibleTypeCast() {
assertScript '''
String s = 'Hello'
@@ -420,7 +468,7 @@ class STCAssignmentTest extends StaticTypeCheckingTestCase {
def c = (Character) null
'''
}
-
+
void testCastObjectToSubclass() {
assertScript '''
Object o = null
@@ -630,7 +678,7 @@ class STCAssignmentTest extends StaticTypeCheckingTestCase {
assert m().getClass() == BigDecimal
'''
}
-
+
void testBigIntegerAssignment() {
assertScript '''
BigInteger bigInt = 6666666666666666666666666666666666666
@@ -694,7 +742,7 @@ class STCAssignmentTest extends StaticTypeCheckingTestCase {
assert c.getClass() == BigDecimal
'''
}
-
+
//GROOVY-6435
void testBigDecAndBigIntSubclass() {
assertScript '''
@@ -888,7 +936,7 @@ class STCAssignmentTest extends StaticTypeCheckingTestCase {
a = new B()
a.bbb()
}
- assert fooParameterAssignment(null) == 42
+ assert fooParameterAssignment(null) == 42
'''
}
@@ -897,7 +945,7 @@ class STCAssignmentTest extends StaticTypeCheckingTestCase {
def m() {
def a = 1
Integer[] b = [a]
- }
+ }
'''
}
@@ -906,7 +954,7 @@ class STCAssignmentTest extends StaticTypeCheckingTestCase {
def m() {
def a = new int[5]
int[][] b = [a]
- }
+ }
'''
}
@@ -916,7 +964,7 @@ class STCAssignmentTest extends StaticTypeCheckingTestCase {
def row = ["", "", ""]
def (left, right) = [row[0], row[1]]
left.toUpperCase()
- }
+ }
'''
}
@@ -982,9 +1030,9 @@ class STCAssignmentTest extends StaticTypeCheckingTestCase {
assertScript '''
interface A1{}
interface A2 extends A1{}
-
+
class C1 implements A1{}
-
+
def m(A2 a2) {
C1 c1 = (C1) a2
}
@@ -995,13 +1043,12 @@ class STCAssignmentTest extends StaticTypeCheckingTestCase {
shouldFailWithMessages '''
interface A1{}
interface A2 extends A1{}
-
+
final class C1 implements A1{}
-
+
def m(A2 a2) {
C1 c1 = (C1) a2
}
''', "Inconvertible types: cannot cast A2 to C1"
}
}
-
diff --git a/src/test/org/codehaus/groovy/classgen/asm/sc/FieldsAndPropertiesStaticCompileTest.groovy b/src/test/org/codehaus/groovy/classgen/asm/sc/FieldsAndPropertiesStaticCompileTest.groovy
index 330fbb3..32f2eb0 100644
--- a/src/test/org/codehaus/groovy/classgen/asm/sc/FieldsAndPropertiesStaticCompileTest.groovy
+++ b/src/test/org/codehaus/groovy/classgen/asm/sc/FieldsAndPropertiesStaticCompileTest.groovy
@@ -20,7 +20,7 @@ package org.codehaus.groovy.classgen.asm.sc
import groovy.transform.stc.FieldsAndPropertiesSTCTest
-class FieldsAndPropertiesStaticCompileTest extends FieldsAndPropertiesSTCTest implements StaticCompilationTestSupport{
+final class FieldsAndPropertiesStaticCompileTest extends FieldsAndPropertiesSTCTest implements StaticCompilationTestSupport {
void testMapGetAt() {
assertScript '''
@@ -217,7 +217,7 @@ class FieldsAndPropertiesStaticCompileTest extends FieldsAndPropertiesSTCTest im
class B extends A {
// B.x visible in B A.x in A, but reflection depending on the runtime type
// would see B.x in A#sameAs and not A.x
- private int x
+ private int x
public B(int x) {
super(x)
this.@x = x + 50
@@ -241,7 +241,7 @@ class FieldsAndPropertiesStaticCompileTest extends FieldsAndPropertiesSTCTest im
class B extends A {
// B.x visible in B A.x in A, but reflection depending on the runtime type
// would see B.x in A#sameAs and not A.x
- private int x
+ private int x
public B(int x) {
super(x)
this.x = x + 50
@@ -348,6 +348,7 @@ class FieldsAndPropertiesStaticCompileTest extends FieldsAndPropertiesSTCTest im
assert a.isSetterCalled() == false
'''
}
+
void testUseAttributeExternalSafe() {
assertScript '''
class A {
@@ -364,6 +365,7 @@ class FieldsAndPropertiesStaticCompileTest extends FieldsAndPropertiesSTCTest im
assert a.isSetterCalled() == false
'''
}
+
void testUseAttributeExternalSafeWithNull() {
assertScript '''
class A {
@@ -378,6 +380,7 @@ class FieldsAndPropertiesStaticCompileTest extends FieldsAndPropertiesSTCTest im
a?.@x = 100
'''
}
+
void testUseGetterExternal() {
assertScript '''
class A {
@@ -483,6 +486,7 @@ class FieldsAndPropertiesStaticCompileTest extends FieldsAndPropertiesSTCTest im
assert o.helpOption
'''
}
+
void testShouldNotThrowStackOverflow() {
new GroovyShell().evaluate '''class HaveOption {
@@ -588,7 +592,7 @@ import org.codehaus.groovy.transform.sc.ListOfExpressionsExpression
}
}
- //GROOVY-7698
+ // GROOVY-7698
void testSafePropertyStyleSetterCalls() {
assertScript '''
class Foo {
@@ -759,30 +763,32 @@ import org.codehaus.groovy.transform.sc.ListOfExpressionsExpression
new A().test()
'''
} finally {
- assert !astTrees['A'][1].contains('pfaccess$00') // no mutator bridge method for 'accessed'
- assert !astTrees['A'][1].contains('pfaccess$1') // no accessor bridge method for 'mutated'
- assert astTrees['A$_closure1'][1].contains('INVOKESTATIC A.pfaccess$2 (LA;)Ljava/lang/String;')
- assert astTrees['A$_closure1'][1].contains('INVOKESTATIC A.pfaccess$02 (LA;Ljava/lang/String;)Ljava/lang/String;')
+ def dump = astTrees['A'][1]
+ assert dump.contains('pfaccess$0') // accessor bridge method for 'accessed'
+ assert !dump.contains('pfaccess$00') // no mutator bridge method for 'accessed'
+ assert dump.contains('pfaccess$01') // mutator bridge method for 'mutated'
+ assert dump.contains('pfaccess$1') // accessor bridge method for 'mutated' -- GROOVY-9385
+ assert dump.contains('pfaccess$2') // accessor bridge method for 'accessedAndMutated'
+ assert dump.contains('pfaccess$02') // mutator bridge method for 'accessedAndMutated'
+ dump = astTrees['A$_closure1'][1]
+ assert dump.contains('INVOKESTATIC A.pfaccess$2 (LA;)Ljava/lang/String;')
+ assert dump.contains('INVOKESTATIC A.pfaccess$02 (LA;Ljava/lang/String;)Ljava/lang/String;')
}
}
- //GROOVY-8369
+ // GROOVY-8369
void testPropertyAccessOnEnumClass() {
- try {
- assertScript '''
- enum Foo {}
+ assertScript '''
+ enum Foo {}
- def test() {
- assert Foo.getModifiers() == Foo.modifiers
- }
- test()
- '''
- } finally {
- //println astTrees
- }
+ def test() {
+ assert Foo.getModifiers() == Foo.modifiers
+ }
+ test()
+ '''
}
- //GROOVY-8753
+ // GROOVY-8753
void testPrivateFieldWithPublicGetter() {
assertScript '''
@groovy.transform.CompileStatic