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 2020/02/05 21:25:15 UTC

[groovy] branch GROOVY-9385 created (now 01e7a68)

This is an automated email from the ASF dual-hosted git repository.

emilles pushed a change to branch GROOVY-9385
in repository https://gitbox.apache.org/repos/asf/groovy.git.


      at 01e7a68  GROOVY-9385: add private field accessor for each mutated private field

This branch includes the following new commits:

     new 01e7a68  GROOVY-9385: add private field accessor for each mutated private field

The 1 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.



[groovy] 01/01: GROOVY-9385: add private field accessor for each mutated private field

Posted by em...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

emilles pushed a commit to branch GROOVY-9385
in repository https://gitbox.apache.org/repos/asf/groovy.git

commit 01e7a6807e7d070fe82f9139299113b22b379979
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 645a6de..94f7eb1 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 '''
@@ -868,7 +916,7 @@ class STCAssignmentTest extends StaticTypeCheckingTestCase {
                 a = new B()
                 a.bbb()
             }
-            assert fooParameterAssignment(null) == 42            
+            assert fooParameterAssignment(null) == 42
         '''
     }
 
@@ -877,7 +925,7 @@ class STCAssignmentTest extends StaticTypeCheckingTestCase {
         def m() {
             def a  = 1
             Integer[] b = [a]
-        }            
+        }
         '''
     }
 
@@ -886,7 +934,7 @@ class STCAssignmentTest extends StaticTypeCheckingTestCase {
         def m() {
             def a = new int[5]
             int[][] b = [a]
-        }            
+        }
         '''
     }
 
@@ -896,7 +944,7 @@ class STCAssignmentTest extends StaticTypeCheckingTestCase {
             def row = ["", "", ""]
             def (left, right) = [row[0], row[1]]
             left.toUpperCase()
-        }            
+        }
         '''
     }
 
@@ -962,9 +1010,9 @@ class STCAssignmentTest extends StaticTypeCheckingTestCase {
         assertScript '''
         interface A1{}
         interface A2 extends A1{}
-        
+
         class C1 implements A1{}
-        
+
         def m(A2 a2) {
             C1 c1 = (C1) a2
         }
@@ -975,13 +1023,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