groovy-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From pa...@apache.org
Subject [20/50] groovy git commit: GROOVY-7584: transient fields in trait are not transient in implementing class (closes #463)
Date Thu, 26 Jan 2017 05:30:54 GMT
GROOVY-7584: transient fields in trait are not transient in implementing class (closes #463)


Project: http://git-wip-us.apache.org/repos/asf/groovy/repo
Commit: http://git-wip-us.apache.org/repos/asf/groovy/commit/c00a75a8
Tree: http://git-wip-us.apache.org/repos/asf/groovy/tree/c00a75a8
Diff: http://git-wip-us.apache.org/repos/asf/groovy/diff/c00a75a8

Branch: refs/heads/GROOVY_2_4_X
Commit: c00a75a89a15e46c2afacf6e72256cd4484bd5f1
Parents: 67c5fa1
Author: paulk <paulk@asert.com.au>
Authored: Thu Nov 24 04:42:29 2016 +1000
Committer: paulk <paulk@asert.com.au>
Committed: Tue Nov 29 05:24:19 2016 +1000

----------------------------------------------------------------------
 .../transform/trait/TraitASTTransformation.java | 27 +++++-----
 .../groovy/transform/trait/TraitComposer.java   | 55 +++++++++++++------
 .../codehaus/groovy/transform/trait/Traits.java |  9 ++++
 src/test/groovy/bugs/Groovy7584Bug.groovy       | 56 ++++++++++++++++++++
 4 files changed, 117 insertions(+), 30 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/groovy/blob/c00a75a8/src/main/org/codehaus/groovy/transform/trait/TraitASTTransformation.java
----------------------------------------------------------------------
diff --git a/src/main/org/codehaus/groovy/transform/trait/TraitASTTransformation.java b/src/main/org/codehaus/groovy/transform/trait/TraitASTTransformation.java
index 6479e07..1ced372 100644
--- a/src/main/org/codehaus/groovy/transform/trait/TraitASTTransformation.java
+++ b/src/main/org/codehaus/groovy/transform/trait/TraitASTTransformation.java
@@ -388,7 +388,7 @@ public class TraitASTTransformation extends AbstractASTTransformation
implements
     private void processField(final FieldNode field, final MethodNode initializer, final
MethodNode staticInitializer, final ClassNode fieldHelper, final ClassNode trait, final Set<String>
knownFields) {
         Expression initialExpression = field.getInitialExpression();
         MethodNode selectedMethod = field.isStatic()?staticInitializer:initializer;
-        if (initialExpression != null) {
+        if (initialExpression != null && !field.isFinal()) {
             VariableExpression thisObject = new VariableExpression(selectedMethod.getParameters()[0]);
             ExpressionStatement initCode = new ExpressionStatement(initialExpression);
             processBody(thisObject, selectedMethod, initCode, trait, fieldHelper, knownFields);
@@ -416,14 +416,16 @@ public class TraitASTTransformation extends AbstractASTTransformation
implements
             code.addStatement(new ExpressionStatement(mce));
         }
         // define setter/getter helper methods
-        fieldHelper.addMethod(
-                Traits.helperSetterName(field),
-                ACC_PUBLIC | ACC_ABSTRACT,
-                field.getOriginType(),
-                new Parameter[]{new Parameter(field.getOriginType(), "val")},
-                ClassNode.EMPTY_ARRAY,
-                null
-        );
+        if (!Modifier.isFinal(field.getModifiers())) {
+            fieldHelper.addMethod(
+                    Traits.helperSetterName(field),
+                    ACC_PUBLIC | ACC_ABSTRACT,
+                    field.getOriginType(),
+                    new Parameter[]{new Parameter(field.getOriginType(), "val")},
+                    ClassNode.EMPTY_ARRAY,
+                    null
+            );
+        }
         fieldHelper.addMethod(
                 Traits.helperGetterName(field),
                 ACC_PUBLIC | ACC_ABSTRACT,
@@ -435,15 +437,14 @@ public class TraitASTTransformation extends AbstractASTTransformation
implements
 
         // dummy fields are only used to carry annotations if instance field
         // and to differentiate from static fields otherwise
-        String dummyFieldName = (field.isStatic() ? Traits.STATIC_FIELD_PREFIX : Traits.FIELD_PREFIX)
+
-                (field.isPublic()? Traits.PUBLIC_FIELD_PREFIX : Traits.PRIVATE_FIELD_PREFIX)+
-                Traits.remappedFieldName(field.getOwner(), field.getName());
+        int mods = field.getModifiers() & Traits.FIELD_PREFIX_MASK;
+        String dummyFieldName = String.format("$0x%04x", mods) + Traits.remappedFieldName(field.getOwner(),
field.getName());
         FieldNode dummyField = new FieldNode(
                 dummyFieldName,
                 ACC_STATIC | ACC_PUBLIC | ACC_FINAL | ACC_SYNTHETIC,
                 field.getOriginType(),
                 fieldHelper,
-                null
+                field.isFinal() ? initialExpression : null
         );
         // copy annotations from field to dummy field
         List<AnnotationNode> copied = new LinkedList<AnnotationNode>();

http://git-wip-us.apache.org/repos/asf/groovy/blob/c00a75a8/src/main/org/codehaus/groovy/transform/trait/TraitComposer.java
----------------------------------------------------------------------
diff --git a/src/main/org/codehaus/groovy/transform/trait/TraitComposer.java b/src/main/org/codehaus/groovy/transform/trait/TraitComposer.java
index 27bbe98..763dc75 100644
--- a/src/main/org/codehaus/groovy/transform/trait/TraitComposer.java
+++ b/src/main/org/codehaus/groovy/transform/trait/TraitComposer.java
@@ -70,6 +70,9 @@ import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
 
+import static org.codehaus.groovy.ast.tools.GeneralUtils.returnS;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.stmt;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.varX;
 import static org.codehaus.groovy.ast.tools.GenericsUtils.correctToGenericsSpecRecurse;
 
 /**
@@ -208,6 +211,7 @@ public abstract class TraitComposer {
                     String operation = methodNode.getName().substring(suffixIdx + 1);
                     boolean getter = "get".equals(operation);
                     ClassNode returnType = correctToGenericsSpecRecurse(genericsSpec, methodNode.getReturnType());
+                    int fieldMods = 0;
                     int isStatic = 0;
                     boolean publicField = true;
                     FieldNode helperField = fieldHelperClassNode.getField(Traits.FIELD_PREFIX
+ Traits.PUBLIC_FIELD_PREFIX + fieldName);
@@ -223,15 +227,30 @@ public abstract class TraitComposer {
                             publicField = false;
                             helperField = fieldHelperClassNode.getField(Traits.STATIC_FIELD_PREFIX+Traits.PRIVATE_FIELD_PREFIX
+fieldName);
                         }
+                        fieldMods = fieldMods | Opcodes.ACC_STATIC;
                         isStatic = Opcodes.ACC_STATIC;
                     }
+                    if (helperField == null) {
+                        fieldMods = 0;
+                        isStatic = 0;
+                        for (Integer mod : Traits.FIELD_PREFIXES) {
+                            helperField = fieldHelperClassNode.getField(String.format("$0x%04x",
mod) + fieldName);
+                            if (helperField != null) {
+                                if ((mod & Opcodes.ACC_STATIC) != 0) isStatic = Opcodes.ACC_STATIC;
+                                fieldMods = fieldMods | mod;
+                                break;
+                            }
+                        }
+                    } else {
+                        fieldMods = fieldMods | (publicField?Opcodes.ACC_PUBLIC:Opcodes.ACC_PRIVATE);
+                    }
                     if (getter) {
                         // add field
                         if (helperField!=null) {
                             List<AnnotationNode> copied = new LinkedList<AnnotationNode>();
                             List<AnnotationNode> notCopied = new LinkedList<AnnotationNode>();
                             GeneralUtils.copyAnnotatedNodeAnnotations(helperField, copied,
notCopied);
-                            FieldNode fieldNode = cNode.addField(fieldName, (publicField?Opcodes.ACC_PUBLIC:Opcodes.ACC_PRIVATE)
| isStatic, returnType, null);
+                            FieldNode fieldNode = cNode.addField(fieldName, fieldMods, returnType,
(fieldMods & Opcodes.ACC_FINAL) == 0 ? null : helperField.getInitialExpression());
                             fieldNode.addAnnotations(copied);
                         }
                     }
@@ -244,28 +263,30 @@ public abstract class TraitComposer {
                         newParams = new Parameter[]{new Parameter(fixedType, "val")};
                     }
 
-                    Expression fieldExpr = new VariableExpression(cNode.getField(fieldName));
+                    Expression fieldExpr = varX(cNode.getField(fieldName));
                     Statement body =
-                            getter ? new ReturnStatement(fieldExpr) :
-                                    new ExpressionStatement(
+                            getter ? returnS(fieldExpr) :
+                                    stmt(
                                             new BinaryExpression(
                                                     fieldExpr,
                                                     Token.newSymbol(Types.EQUAL, 0, 0),
-                                                    new VariableExpression(newParams[0])
+                                                    varX(newParams[0])
                                             )
                                     );
-                    MethodNode impl = new MethodNode(
-                            methodNode.getName(),
-                            Opcodes.ACC_PUBLIC | isStatic,
-                            returnType,
-                            newParams,
-                            ClassNode.EMPTY_ARRAY,
-                            body
-                    );
-                    AnnotationNode an = new AnnotationNode(COMPILESTATIC_CLASSNODE);
-                    impl.addAnnotation(an);
-                    cNode.addTransform(StaticCompileTransformation.class, an);
-                    cNode.addMethod(impl);
+                    if (getter || (fieldMods & Opcodes.ACC_FINAL) == 0) {
+                        MethodNode impl = new MethodNode(
+                                methodNode.getName(),
+                                Opcodes.ACC_PUBLIC | isStatic,
+                                returnType,
+                                newParams,
+                                ClassNode.EMPTY_ARRAY,
+                                body
+                        );
+                        AnnotationNode an = new AnnotationNode(COMPILESTATIC_CLASSNODE);
+                        impl.addAnnotation(an);
+                        cNode.addTransform(StaticCompileTransformation.class, an);
+                        cNode.addMethod(impl);
+                    }
                 }
             }
         }

http://git-wip-us.apache.org/repos/asf/groovy/blob/c00a75a8/src/main/org/codehaus/groovy/transform/trait/Traits.java
----------------------------------------------------------------------
diff --git a/src/main/org/codehaus/groovy/transform/trait/Traits.java b/src/main/org/codehaus/groovy/transform/trait/Traits.java
index 61d602f..4a0ac91 100644
--- a/src/main/org/codehaus/groovy/transform/trait/Traits.java
+++ b/src/main/org/codehaus/groovy/transform/trait/Traits.java
@@ -34,12 +34,14 @@ import org.codehaus.groovy.ast.expr.ListExpression;
 import org.codehaus.groovy.ast.tools.GenericsUtils;
 import org.codehaus.groovy.classgen.asm.BytecodeHelper;
 import org.codehaus.groovy.runtime.DefaultGroovyMethods;
+import org.objectweb.asm.Opcodes;
 
 import java.lang.annotation.ElementType;
 import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
 import java.lang.annotation.Target;
 import java.lang.reflect.Method;
+import java.util.Arrays;
 import java.util.Iterator;
 import java.util.LinkedHashSet;
 import java.util.List;
@@ -71,6 +73,13 @@ public abstract class Traits {
     static final String FIELD_PREFIX = "$ins";
     static final String PUBLIC_FIELD_PREFIX = "$0";
     static final String PRIVATE_FIELD_PREFIX = "$1";
+    // TODO decide if we should support VOLATILE
+//    def hex(s) {new BigInteger(s, 16).intValue()}
+//    def optionals = [[0, 1], [0, 1], [0, 1], [0, 1]].combinations{ a, b, c, d ->
+//            (a ? hex('80') : 0) + (b ? hex('10') : 0) + (c ? hex('8') : 0) + (d ? hex('2')
: hex('1'))
+//    }.sort()
+    static final List<Integer> FIELD_PREFIXES = Arrays.asList(1, 2, 9, 10, 17, 18,
25, 26, 129, 130, 137, 138, 145, 146, 153, 154);
+    static final int FIELD_PREFIX_MASK = Opcodes.ACC_PRIVATE | Opcodes.ACC_PUBLIC | Opcodes.ACC_STATIC
| Opcodes.ACC_FINAL | Opcodes.ACC_TRANSIENT;
     static final String SUPER_TRAIT_METHOD_PREFIX = "trait$super$";
 
     static String fieldHelperClassName(final ClassNode traitNode) {

http://git-wip-us.apache.org/repos/asf/groovy/blob/c00a75a8/src/test/groovy/bugs/Groovy7584Bug.groovy
----------------------------------------------------------------------
diff --git a/src/test/groovy/bugs/Groovy7584Bug.groovy b/src/test/groovy/bugs/Groovy7584Bug.groovy
new file mode 100644
index 0000000..6f4772f
--- /dev/null
+++ b/src/test/groovy/bugs/Groovy7584Bug.groovy
@@ -0,0 +1,56 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing,
+ *  software distributed under the License is distributed on an
+ *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ *  specific language governing permissions and limitations
+ *  under the License.
+ */
+package groovy.bugs
+
+class Groovy7584Bug extends GroovyTestCase {
+    void testTraitFieldModifiersAreRetained() {
+        assertScript """
+            import static java.lang.reflect.Modifier.*
+
+            trait User {
+                final String name = 'Foo'
+                public static final boolean loggedIn = true
+                private transient final int ANSWER = 42
+            }
+
+            @groovy.transform.ToString(includeFields=true, includeNames=true)
+            class Person implements User { }
+
+            def tos = new Person().toString()
+            assert tos.contains('name:Foo')
+            assert tos.contains('User__ANSWER:42')
+            assert tos.contains('User__name:Foo')
+
+            def loggedInField = Person.getDeclaredFields().find {
+                it.name.contains('loggedIn')
+            }
+            assert isPublic(loggedInField.modifiers)
+            assert isFinal(loggedInField.modifiers)
+            assert isStatic(loggedInField.modifiers)
+            assert Person.User__loggedIn
+
+            def answerField = Person.getDeclaredFields().find {
+                it.name.contains('ANSWER')
+            }
+            assert isPrivate(answerField.modifiers)
+            assert isTransient(answerField.modifiers)
+            assert isFinal(answerField.modifiers)
+        """
+    }
+}


Mime
View raw message