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 2016/11/29 16:04:35 UTC

[6/6] groovy git commit: Merge branch 'master' of https://git-wip-us.apache.org/repos/asf/groovy into parrot # Conflicts: # gradle/wrapper/gradle-wrapper.properties

Merge branch 'master' of https://git-wip-us.apache.org/repos/asf/groovy into parrot  # Conflicts: #	gradle/wrapper/gradle-wrapper.properties


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

Branch: refs/heads/parrot
Commit: b9ed6e9691deed01943409e4f37c5b2ac6c7d65f
Parents: 109ed4b
Author: Daniel Sun <su...@apache.org>
Authored: Wed Nov 30 00:02:40 2016 +0800
Committer: Daniel Sun <su...@apache.org>
Committed: Wed Nov 30 00:02:40 2016 +0800

----------------------------------------------------------------------
 build.gradle                                    |   5 +-
 gradle/wrapper/gradle-wrapper.properties        |   2 +-
 lib/jarjar-1.4.1.jar                            | Bin 127181 -> 0 bytes
 .../org/codehaus/groovy/syntax/TokenUtil.java   |  13 +++-
 .../transform/trait/NAryOperationRewriter.java  |  59 +------------------
 .../transform/trait/TraitASTTransformation.java |  27 +++++----
 .../groovy/transform/trait/TraitComposer.java   |  56 ++++++++++++------
 .../codehaus/groovy/transform/trait/Traits.java |   9 +++
 src/test/groovy/bugs/Groovy7584Bug.groovy       |  56 ++++++++++++++++++
 9 files changed, 136 insertions(+), 91 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/groovy/blob/b9ed6e96/build.gradle
----------------------------------------------------------------------
diff --git a/build.gradle b/build.gradle
index 7dad8c8..b21ae4f 100644
--- a/build.gradle
+++ b/build.gradle
@@ -131,6 +131,7 @@ subprojects {
 repositories {
     // todo Some repos are needed only for some configs. Declare them just for the configuration once Gradle allows this.
     maven { url 'http://repository.jboss.org/nexus/content/groups/m2-release-proxy' } // examples, tools
+    maven { url 'http://repo.bodar.com' } // tools - jarjar
 }
 
 // todo do we need compile and runtime scope for examples?
@@ -208,8 +209,8 @@ dependencies {
     testCompile "org.slf4j:jcl-over-slf4j:$slf4jVersion"
     testCompile "com.thoughtworks.qdox:qdox:$qdoxVersion"
 
-//    tools "com.googlecode.jarjar:jarjar:$jarjarVersion"
-    tools files("lib/jarjar-${jarjarVersion}.jar")
+    tools "com.googlecode.jarjar:jarjar:$jarjarVersion"
+//    tools files("lib/jarjar-${jarjarVersion}.jar")
     tools "org.jboss.bridger:bridger:$bridgerVersion"
 
     tools("net.sourceforge.cobertura:cobertura:$coberturaVersion") {

http://git-wip-us.apache.org/repos/asf/groovy/blob/b9ed6e96/gradle/wrapper/gradle-wrapper.properties
----------------------------------------------------------------------
diff --git a/gradle/wrapper/gradle-wrapper.properties b/gradle/wrapper/gradle-wrapper.properties
index 143c4f7..5633292 100644
--- a/gradle/wrapper/gradle-wrapper.properties
+++ b/gradle/wrapper/gradle-wrapper.properties
@@ -1,4 +1,4 @@
-#Thu Nov 10 22:20:14 CST 2016
+#Tue Nov 29 01:04:03 AEST 2016
 distributionBase=GRADLE_USER_HOME
 distributionPath=wrapper/dists
 zipStoreBase=GRADLE_USER_HOME

http://git-wip-us.apache.org/repos/asf/groovy/blob/b9ed6e96/lib/jarjar-1.4.1.jar
----------------------------------------------------------------------
diff --git a/lib/jarjar-1.4.1.jar b/lib/jarjar-1.4.1.jar
deleted file mode 100644
index b85742a..0000000
Binary files a/lib/jarjar-1.4.1.jar and /dev/null differ

http://git-wip-us.apache.org/repos/asf/groovy/blob/b9ed6e96/src/main/org/codehaus/groovy/syntax/TokenUtil.java
----------------------------------------------------------------------
diff --git a/src/main/org/codehaus/groovy/syntax/TokenUtil.java b/src/main/org/codehaus/groovy/syntax/TokenUtil.java
index bfad787..603be11 100644
--- a/src/main/org/codehaus/groovy/syntax/TokenUtil.java
+++ b/src/main/org/codehaus/groovy/syntax/TokenUtil.java
@@ -16,15 +16,26 @@
  *  specific language governing permissions and limitations
  *  under the License.
  */
-
 package org.codehaus.groovy.syntax;
 
 import static org.codehaus.groovy.syntax.Types.*;
 
+/**
+ * Utility methods for working with Tokens.
+ *
+ */
 public class TokenUtil {
     private TokenUtil() {
     }
 
+    /**
+     * Removes the assignment portion of a given token.  If the given token
+     * is not an operator with assignment, the given token is returned.
+     *
+     * @param op token for which to remove assignment
+     * @return token without assignment, or the original token
+     *          if it was not an assignment operator
+     */
     public static int removeAssignment(int op) {
         switch (op) {
             case PLUS_EQUAL: return PLUS;

http://git-wip-us.apache.org/repos/asf/groovy/blob/b9ed6e96/src/main/org/codehaus/groovy/transform/trait/NAryOperationRewriter.java
----------------------------------------------------------------------
diff --git a/src/main/org/codehaus/groovy/transform/trait/NAryOperationRewriter.java b/src/main/org/codehaus/groovy/transform/trait/NAryOperationRewriter.java
index 53d5488..a713cb3 100644
--- a/src/main/org/codehaus/groovy/transform/trait/NAryOperationRewriter.java
+++ b/src/main/org/codehaus/groovy/transform/trait/NAryOperationRewriter.java
@@ -30,6 +30,7 @@ import org.codehaus.groovy.ast.expr.VariableExpression;
 import org.codehaus.groovy.control.SourceUnit;
 import org.codehaus.groovy.syntax.SyntaxException;
 import org.codehaus.groovy.syntax.Token;
+import org.codehaus.groovy.syntax.TokenUtil;
 
 import java.util.Collection;
 
@@ -107,8 +108,8 @@ class NAryOperationRewriter extends ClassCodeExpressionTransformer {
 
     private Expression transformBinaryExpression(final BinaryExpression exp) {
         final int op = exp.getOperation().getType();
-        int token = getOperationWithoutEqual(op);
-        if (token < 0) {
+        int token = TokenUtil.removeAssignment(op);
+        if (token == op) {
             // no transform needed
             return super.transform(exp);
         }
@@ -126,58 +127,4 @@ class NAryOperationRewriter extends ClassCodeExpressionTransformer {
         result.setSourcePosition(exp);
         return result;
     }
-
-    private static int getOperationWithoutEqual(final int op) {
-        int token = -1;
-        switch (op) {
-            case LOGICAL_OR_EQUAL:
-                token = LOGICAL_OR;
-                break;
-            case LOGICAL_AND_EQUAL:
-                token = LOGICAL_AND;
-                break;
-            case PLUS_EQUAL:
-                token = PLUS;
-                break;
-            case MINUS_EQUAL:
-                token = MINUS;
-                break;
-            case MULTIPLY_EQUAL:
-                token = MULTIPLY;
-                break;
-            case DIVIDE_EQUAL:
-                token = DIVIDE;
-                break;
-            case INTDIV_EQUAL:
-                token = INTDIV;
-                break;
-            case MOD_EQUAL:
-                token = MOD;
-                break;
-            case POWER_EQUAL:
-                token = POWER;
-                break;
-            case LEFT_SHIFT_EQUAL:
-                token = LEFT_SHIFT;
-                break;
-            case RIGHT_SHIFT_EQUAL:
-                token = RIGHT_SHIFT;
-                break;
-            case RIGHT_SHIFT_UNSIGNED_EQUAL:
-                token = RIGHT_SHIFT_UNSIGNED;
-                break;
-            case BITWISE_OR_EQUAL:
-                token = BITWISE_OR;
-                break;
-            case BITWISE_AND_EQUAL:
-                token = BITWISE_AND;
-                break;
-            case BITWISE_XOR_EQUAL:
-                token = BITWISE_XOR;
-                break;
-            default:
-                token = -1;
-        }
-        return token;
-    }
 }

http://git-wip-us.apache.org/repos/asf/groovy/blob/b9ed6e96/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 29d0ae6..6e852ce 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/b9ed6e96/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 4b05908..fc4e0e5 100644
--- a/src/main/org/codehaus/groovy/transform/trait/TraitComposer.java
+++ b/src/main/org/codehaus/groovy/transform/trait/TraitComposer.java
@@ -70,8 +70,10 @@ 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;
-import static org.codehaus.groovy.ast.tools.GenericsUtils.newClass;
 
 /**
  * This class contains a static utility method {@link #doExtendTraits(org.codehaus.groovy.ast.ClassNode, org.codehaus.groovy.control.SourceUnit, org.codehaus.groovy.control.CompilationUnit)}
@@ -209,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);
@@ -224,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);
                         }
                     }
@@ -245,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/b9ed6e96/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/b9ed6e96/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)
+        """
+    }
+}