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)
+ """
+ }
+}