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/24 19:37:29 UTC
[groovy] 01/06: GROOVY-9385: add private field accessor for each mutated private field
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 99c71343affedc03a7f21f1cbea81faec2881679
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
(cherry picked from commit 4999bbcb5f41dbd604aad529130a8b3d16637fd4)
Conflicts:
src/main/java/org/codehaus/groovy/transform/sc/StaticCompilationVisitor.java
src/test/org/codehaus/groovy/classgen/asm/sc/FieldsAndPropertiesStaticCompileTest.groovy
---
.../transform/sc/StaticCompilationVisitor.java | 41 ++++++++----
.../groovy/transform/stc/STCAssignmentTest.groovy | 73 ++++++++++++++++++----
.../sc/FieldsAndPropertiesStaticCompileTest.groovy | 14 +++--
3 files changed, 99 insertions(+), 29 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 bca426e..18a4801 100644
--- a/src/main/java/org/codehaus/groovy/transform/sc/StaticCompilationVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/sc/StaticCompilationVisitor.java
@@ -60,10 +60,10 @@ import org.codehaus.groovy.control.CompilationUnit;
import org.codehaus.groovy.control.SourceUnit;
import org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport;
import org.codehaus.groovy.transform.stc.StaticTypeCheckingVisitor;
-import org.codehaus.groovy.transform.stc.StaticTypesMarker;
import java.util.ArrayList;
import java.util.HashMap;
+import java.util.HashSet;
import java.util.Iterator;
import java.util.LinkedList;
import java.util.List;
@@ -79,11 +79,19 @@ import static org.codehaus.groovy.ast.tools.GenericsUtils.createGenericsSpec;
import static org.codehaus.groovy.ast.tools.GenericsUtils.extractSuperClassGenerics;
import static org.codehaus.groovy.transform.sc.StaticCompilationMetadataKeys.BINARY_EXP_TARGET;
import static org.codehaus.groovy.transform.sc.StaticCompilationMetadataKeys.COMPONENT_TYPE;
+import static org.codehaus.groovy.transform.sc.StaticCompilationMetadataKeys.DYNAMIC_OUTER_NODE_CALLBACK;
import static org.codehaus.groovy.transform.sc.StaticCompilationMetadataKeys.PRIVATE_BRIDGE_METHODS;
import static org.codehaus.groovy.transform.sc.StaticCompilationMetadataKeys.PRIVATE_FIELDS_ACCESSORS;
import static org.codehaus.groovy.transform.sc.StaticCompilationMetadataKeys.PRIVATE_FIELDS_MUTATORS;
+import static org.codehaus.groovy.transform.sc.StaticCompilationMetadataKeys.PROPERTY_OWNER;
+import static org.codehaus.groovy.transform.sc.StaticCompilationMetadataKeys.RECEIVER_OF_DYNAMIC_PROPERTY;
import static org.codehaus.groovy.transform.sc.StaticCompilationMetadataKeys.STATIC_COMPILE_NODE;
import static org.codehaus.groovy.transform.stc.StaticTypesMarker.DIRECT_METHOD_CALL_TARGET;
+import static org.codehaus.groovy.transform.stc.StaticTypesMarker.DYNAMIC_RESOLUTION;
+import static org.codehaus.groovy.transform.stc.StaticTypesMarker.INITIAL_EXPRESSION;
+import static org.codehaus.groovy.transform.stc.StaticTypesMarker.PV_FIELDS_ACCESS;
+import static org.codehaus.groovy.transform.stc.StaticTypesMarker.PV_FIELDS_MUTATION;
+import static org.codehaus.groovy.transform.stc.StaticTypesMarker.PV_METHODS_ACCESS;
import static org.objectweb.asm.Opcodes.ACC_PUBLIC;
import static org.objectweb.asm.Opcodes.ACC_STATIC;
import static org.objectweb.asm.Opcodes.ACC_SYNTHETIC;
@@ -148,8 +156,8 @@ public class StaticCompilationVisitor extends StaticTypeCheckingVisitor {
private void addDynamicOuterClassAccessorsCallback(final ClassNode outer) {
if (outer != null && !isStaticallyCompiled(outer)
- && outer.getNodeMetaData(StaticCompilationMetadataKeys.DYNAMIC_OUTER_NODE_CALLBACK) == null) {
- outer.putNodeMetaData(StaticCompilationMetadataKeys.DYNAMIC_OUTER_NODE_CALLBACK, new CompilationUnit.PrimaryClassNodeOperation() {
+ && outer.getNodeMetaData(DYNAMIC_OUTER_NODE_CALLBACK) == null) {
+ outer.putNodeMetaData(DYNAMIC_OUTER_NODE_CALLBACK, new CompilationUnit.PrimaryClassNodeOperation() {
@Override
public void call(SourceUnit source, GeneratorContext context, ClassNode classNode) throws CompilationFailedException {
if (classNode == outer) {
@@ -236,15 +244,24 @@ public class StaticCompilationVisitor extends StaticTypeCheckingVisitor {
* Adds special accessors and mutators for private fields so that inner classes can get/set them
*/
private static void addPrivateFieldsAccessors(ClassNode node) {
- Set<ASTNode> accessedFields = (Set<ASTNode>) node.getNodeMetaData(StaticTypesMarker.PV_FIELDS_ACCESS);
- Set<ASTNode> mutatedFields = (Set<ASTNode>) node.getNodeMetaData(StaticTypesMarker.PV_FIELDS_MUTATION);
- if (accessedFields == null && mutatedFields == null) return;
Map<String, MethodNode> privateFieldAccessors = (Map<String, MethodNode>) node.getNodeMetaData(PRIVATE_FIELDS_ACCESSORS);
Map<String, MethodNode> privateFieldMutators = (Map<String, MethodNode>) node.getNodeMetaData(PRIVATE_FIELDS_MUTATORS);
if (privateFieldAccessors != null || privateFieldMutators != null) {
// already added
return;
}
+ 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) {
+ if (accessedFields != null) {
+ accessedFields = new HashSet<>(accessedFields); accessedFields.addAll(mutatedFields);
+ } else {
+ accessedFields = mutatedFields;
+ }
+ }
+
int acc = -1;
privateFieldAccessors = accessedFields != null ? new HashMap<String, MethodNode>() : null;
privateFieldMutators = mutatedFields != null ? new HashMap<String, MethodNode>() : null;
@@ -292,7 +309,7 @@ public class StaticCompilationVisitor extends StaticTypeCheckingVisitor {
* @param node an inner/outer class node for which to generate bridge methods
*/
private static void addPrivateBridgeMethods(final ClassNode node) {
- Set<ASTNode> accessedMethods = (Set<ASTNode>) node.getNodeMetaData(StaticTypesMarker.PV_METHODS_ACCESS);
+ Set<ASTNode> accessedMethods = (Set<ASTNode>) node.getNodeMetaData(PV_METHODS_ACCESS);
if (accessedMethods==null) return;
List<MethodNode> methods = new ArrayList<MethodNode>(node.getAllDeclaredMethods());
methods.addAll(node.getDeclaredConstructors());
@@ -389,7 +406,7 @@ public class StaticCompilationVisitor extends StaticTypeCheckingVisitor {
// add node metadata for default parameters because they are erased by the Verifier
if (node.getParameters()!=null) {
for (Parameter parameter : node.getParameters()) {
- parameter.putNodeMetaData(StaticTypesMarker.INITIAL_EXPRESSION, parameter.getInitialExpression());
+ parameter.putNodeMetaData(INITIAL_EXPRESSION, parameter.getInitialExpression());
}
}
}
@@ -534,8 +551,8 @@ public class StaticCompilationVisitor extends StaticTypeCheckingVisitor {
boolean exists = super.existsProperty(pexp, checkForReadOnly, receiverMemoizer);
if (exists) {
objectExpressionType = rType.get();
- if (objectExpression.getNodeMetaData(StaticCompilationMetadataKeys.PROPERTY_OWNER) == null) {
- objectExpression.putNodeMetaData(StaticCompilationMetadataKeys.PROPERTY_OWNER, objectExpressionType);
+ if (objectExpression.getNodeMetaData(PROPERTY_OWNER) == null) {
+ objectExpression.putNodeMetaData(PROPERTY_OWNER, objectExpressionType);
}
if (StaticTypeCheckingSupport.implementsInterfaceOrIsSubclassOf(objectExpressionType, ClassHelper.LIST_TYPE)) {
objectExpression.putNodeMetaData(COMPONENT_TYPE, inferComponentType(objectExpressionType, ClassHelper.int_TYPE));
@@ -547,9 +564,9 @@ public class StaticCompilationVisitor extends StaticTypeCheckingVisitor {
@Override
public void visitPropertyExpression(final PropertyExpression pexp) {
super.visitPropertyExpression(pexp);
- Object dynamic = pexp.getNodeMetaData(StaticTypesMarker.DYNAMIC_RESOLUTION);
+ Object dynamic = pexp.getNodeMetaData(DYNAMIC_RESOLUTION);
if (dynamic != null) {
- pexp.getObjectExpression().putNodeMetaData(StaticCompilationMetadataKeys.RECEIVER_OF_DYNAMIC_PROPERTY, dynamic);
+ pexp.getObjectExpression().putNodeMetaData(RECEIVER_OF_DYNAMIC_PROPERTY, dynamic);
}
}
}
diff --git a/src/test/groovy/transform/stc/STCAssignmentTest.groovy b/src/test/groovy/transform/stc/STCAssignmentTest.groovy
index 7d63cb6..3649342 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
@@ -664,7 +712,7 @@ class STCAssignmentTest extends StaticTypeCheckingTestCase {
assert m().getClass() == BigDecimal
'''
}
-
+
void testBigIntegerAssignment() {
assertScript '''
BigInteger bigInt = 6666666666666666666666666666666666666
@@ -728,7 +776,7 @@ class STCAssignmentTest extends StaticTypeCheckingTestCase {
assert c.getClass() == BigDecimal
'''
}
-
+
//GROOVY-6435
void testBigDecAndBigIntSubclass() {
assertScript '''
@@ -902,7 +950,7 @@ class STCAssignmentTest extends StaticTypeCheckingTestCase {
a = new B()
a.bbb()
}
- assert fooParameterAssignment(null) == 42
+ assert fooParameterAssignment(null) == 42
'''
}
@@ -911,7 +959,7 @@ class STCAssignmentTest extends StaticTypeCheckingTestCase {
def m() {
def a = 1
Integer[] b = [a]
- }
+ }
'''
}
@@ -920,7 +968,7 @@ class STCAssignmentTest extends StaticTypeCheckingTestCase {
def m() {
def a = new int[5]
int[][] b = [a]
- }
+ }
'''
}
@@ -930,7 +978,7 @@ class STCAssignmentTest extends StaticTypeCheckingTestCase {
def row = ["", "", ""]
def (left, right) = [row[0], row[1]]
left.toUpperCase()
- }
+ }
'''
}
@@ -996,9 +1044,9 @@ class STCAssignmentTest extends StaticTypeCheckingTestCase {
assertScript '''
interface A1{}
interface A2 extends A1{}
-
+
class C1 implements A1{}
-
+
def m(A2 a2) {
C1 c1 = (C1) a2
}
@@ -1009,13 +1057,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 247d7db..5e562e7 100644
--- a/src/test/org/codehaus/groovy/classgen/asm/sc/FieldsAndPropertiesStaticCompileTest.groovy
+++ b/src/test/org/codehaus/groovy/classgen/asm/sc/FieldsAndPropertiesStaticCompileTest.groovy
@@ -815,10 +815,16 @@ final class FieldsAndPropertiesStaticCompileTest extends FieldsAndPropertiesSTCT
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;')
}
}