You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by cc...@apache.org on 2017/04/08 08:41:07 UTC
[1/3] groovy git commit: Fix non-deterministic selection of interface
method in `@CompileStatic`
Repository: groovy
Updated Branches:
refs/heads/GROOVY_2_5_X 2a3ce7b3f -> ec080fe2c
Fix non-deterministic selection of interface method in `@CompileStatic`
This commit makes sure that the list of methods we choose from when multiple methods
are found in interfaces is always in the same order. Without this, it results in non
reproducible builds, where the bytecode would once choose to call the method from
one interface, and another time from the other interface. While the bytecode is valid,
it kills tools like Gradle which rely on the ABI to detect changes.
Project: http://git-wip-us.apache.org/repos/asf/groovy/repo
Commit: http://git-wip-us.apache.org/repos/asf/groovy/commit/c07118ab
Tree: http://git-wip-us.apache.org/repos/asf/groovy/tree/c07118ab
Diff: http://git-wip-us.apache.org/repos/asf/groovy/diff/c07118ab
Branch: refs/heads/GROOVY_2_5_X
Commit: c07118abcae80bf3dddf071be81292e135165f58
Parents: 2a3ce7b
Author: Cedric Champeau <cc...@apache.org>
Authored: Thu Apr 6 18:51:30 2017 +0200
Committer: Cedric Champeau <cc...@apache.org>
Committed: Sat Apr 8 10:39:52 2017 +0200
----------------------------------------------------------------------
src/main/org/codehaus/groovy/ast/ClassNode.java | 3 +-
.../codehaus/groovy/ast/tools/GeneralUtils.java | 4 +-
.../stc/StaticTypeCheckingSupport.java | 2 +-
.../classgen/asm/sc/bugs/Groovy8142Bug.groovy | 51 ++++++++++++++++++++
4 files changed, 55 insertions(+), 5 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/groovy/blob/c07118ab/src/main/org/codehaus/groovy/ast/ClassNode.java
----------------------------------------------------------------------
diff --git a/src/main/org/codehaus/groovy/ast/ClassNode.java b/src/main/org/codehaus/groovy/ast/ClassNode.java
index 9b885fd..f8858b2 100644
--- a/src/main/org/codehaus/groovy/ast/ClassNode.java
+++ b/src/main/org/codehaus/groovy/ast/ClassNode.java
@@ -36,7 +36,6 @@ import java.util.ArrayList;
import java.util.Collections;
import java.util.EnumMap;
import java.util.HashMap;
-import java.util.HashSet;
import java.util.Iterator;
import java.util.LinkedHashSet;
import java.util.LinkedList;
@@ -418,7 +417,7 @@ public class ClassNode extends AnnotatedNode implements Opcodes {
}
public Set<ClassNode> getAllInterfaces () {
- Set<ClassNode> res = new HashSet<ClassNode>();
+ Set<ClassNode> res = new LinkedHashSet<ClassNode>();
getAllInterfaces(res);
return res;
}
http://git-wip-us.apache.org/repos/asf/groovy/blob/c07118ab/src/main/org/codehaus/groovy/ast/tools/GeneralUtils.java
----------------------------------------------------------------------
diff --git a/src/main/org/codehaus/groovy/ast/tools/GeneralUtils.java b/src/main/org/codehaus/groovy/ast/tools/GeneralUtils.java
index bcdf627..413292a 100644
--- a/src/main/org/codehaus/groovy/ast/tools/GeneralUtils.java
+++ b/src/main/org/codehaus/groovy/ast/tools/GeneralUtils.java
@@ -67,7 +67,7 @@ import org.codehaus.groovy.transform.AbstractASTTransformation;
import java.lang.reflect.Modifier;
import java.util.ArrayList;
import java.util.Collections;
-import java.util.HashSet;
+import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
@@ -414,7 +414,7 @@ public class GeneralUtils {
}
public static Set<ClassNode> getInterfacesAndSuperInterfaces(ClassNode type) {
- Set<ClassNode> res = new HashSet<ClassNode>();
+ Set<ClassNode> res = new LinkedHashSet<ClassNode>();
if (type.isInterface()) {
res.add(type);
return res;
http://git-wip-us.apache.org/repos/asf/groovy/blob/c07118ab/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java
----------------------------------------------------------------------
diff --git a/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java b/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java
index 622084b..bcc86a8 100644
--- a/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java
+++ b/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java
@@ -1110,7 +1110,7 @@ public abstract class StaticTypeCheckingSupport {
private static Collection<MethodNode> removeCovariantsAndInterfaceEquivalents(Collection<MethodNode> collection) {
if (collection.size() <= 1) return collection;
List<MethodNode> toBeRemoved = new LinkedList<MethodNode>();
- List<MethodNode> list = new LinkedList<MethodNode>(new HashSet<MethodNode>(collection));
+ List<MethodNode> list = new LinkedList<MethodNode>(new LinkedHashSet<MethodNode>(collection));
for (int i = 0; i < list.size() - 1; i++) {
MethodNode one = list.get(i);
if (toBeRemoved.contains(one)) continue;
http://git-wip-us.apache.org/repos/asf/groovy/blob/c07118ab/src/test/org/codehaus/groovy/classgen/asm/sc/bugs/Groovy8142Bug.groovy
----------------------------------------------------------------------
diff --git a/src/test/org/codehaus/groovy/classgen/asm/sc/bugs/Groovy8142Bug.groovy b/src/test/org/codehaus/groovy/classgen/asm/sc/bugs/Groovy8142Bug.groovy
new file mode 100644
index 0000000..9a95a0e
--- /dev/null
+++ b/src/test/org/codehaus/groovy/classgen/asm/sc/bugs/Groovy8142Bug.groovy
@@ -0,0 +1,51 @@
+/*
+ * 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 org.codehaus.groovy.classgen.asm.sc.bugs
+
+import groovy.transform.stc.StaticTypeCheckingTestCase
+import org.codehaus.groovy.classgen.asm.sc.StaticCompilationTestSupport
+
+class Groovy8142Bug extends StaticTypeCheckingTestCase implements StaticCompilationTestSupport {
+ void testShouldNotProduceDeterministicBytecode() {
+ 100.times {
+ assertScript '''
+ interface Project {
+ File file()
+ }
+ interface FileOperations {
+ File file()
+ }
+ interface ProjectInternal extends Project, FileOperations {
+ }
+
+ class Check {
+ void test(ProjectInternal p) {
+ def f = p.file()
+ }
+ }
+
+ def c = new Check()
+ '''
+
+ assert astTrees['Check'][1].contains('INVOKEINTERFACE FileOperations.file ()Ljava/io/File;') : "Incorrect bytecode found in iteration $it"
+ }
+ }
+}
[3/3] groovy git commit: Reduce memory footprint of the compiler
Posted by cc...@apache.org.
Reduce memory footprint of the compiler
The compiler creates a lot of maps and lists where it could avoid it. This optimizes
creation by doing it lazily, and improves memory pressure in real world context.
Signed-off-by: Cedric Champeau <cc...@apache.org>
Project: http://git-wip-us.apache.org/repos/asf/groovy/repo
Commit: http://git-wip-us.apache.org/repos/asf/groovy/commit/ec080fe2
Tree: http://git-wip-us.apache.org/repos/asf/groovy/tree/ec080fe2
Diff: http://git-wip-us.apache.org/repos/asf/groovy/diff/ec080fe2
Branch: refs/heads/GROOVY_2_5_X
Commit: ec080fe2c7c28020a64c91684007b5b5ad46c469
Parents: 1f2a5b5
Author: Cedric Champeau <cc...@apache.org>
Authored: Fri Apr 7 22:29:49 2017 +0200
Committer: Cedric Champeau <cc...@apache.org>
Committed: Sat Apr 8 10:40:49 2017 +0200
----------------------------------------------------------------------
.../org/codehaus/groovy/ast/AnnotationNode.java | 25 ++-
src/main/org/codehaus/groovy/ast/ClassNode.java | 10 +-
.../groovy/reflection/ParameterTypes.java | 156 +++++++++----------
.../transform/AnnotationCollectorTransform.java | 45 +++++-
4 files changed, 143 insertions(+), 93 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/groovy/blob/ec080fe2/src/main/org/codehaus/groovy/ast/AnnotationNode.java
----------------------------------------------------------------------
diff --git a/src/main/org/codehaus/groovy/ast/AnnotationNode.java b/src/main/org/codehaus/groovy/ast/AnnotationNode.java
index 50eddd0..e00f977 100644
--- a/src/main/org/codehaus/groovy/ast/AnnotationNode.java
+++ b/src/main/org/codehaus/groovy/ast/AnnotationNode.java
@@ -18,11 +18,12 @@
*/
package org.codehaus.groovy.ast;
-import java.util.HashMap;
-import java.util.Map;
-
-import org.codehaus.groovy.ast.expr.Expression;
import org.codehaus.groovy.GroovyBugError;
+import org.codehaus.groovy.ast.expr.Expression;
+
+import java.util.Collections;
+import java.util.LinkedHashMap;
+import java.util.Map;
/**
@@ -44,7 +45,7 @@ public class AnnotationNode extends ASTNode {
| FIELD_TARGET | PARAMETER_TARGET | LOCAL_VARIABLE_TARGET | ANNOTATION_TARGET | PACKAGE_TARGET;
private final ClassNode classNode;
- private final Map<String, Expression> members = new HashMap<String, Expression>();
+ private Map<String, Expression> members;
private boolean runtimeRetention= false, sourceRetention= false, classRetention = false;
private int allowedTargets = ALL_TARGETS;
@@ -57,14 +58,27 @@ public class AnnotationNode extends ASTNode {
}
public Map<String, Expression> getMembers() {
+ if (members == null) {
+ return Collections.emptyMap();
+ }
return members;
}
public Expression getMember(String name) {
+ if (members == null) {
+ return null;
+ }
return members.get(name);
}
+ private void assertMembers() {
+ if (members == null) {
+ members = new LinkedHashMap<String, Expression>();
+ }
+ }
+
public void addMember(String name, Expression value) {
+ assertMembers();
Expression oldValue = members.get(name);
if (oldValue == null) {
members.put(name, value);
@@ -75,6 +89,7 @@ public class AnnotationNode extends ASTNode {
}
public void setMember(String name, Expression value) {
+ assertMembers();
members.put(name, value);
}
http://git-wip-us.apache.org/repos/asf/groovy/blob/ec080fe2/src/main/org/codehaus/groovy/ast/ClassNode.java
----------------------------------------------------------------------
diff --git a/src/main/org/codehaus/groovy/ast/ClassNode.java b/src/main/org/codehaus/groovy/ast/ClassNode.java
index f8858b2..702a0a8 100644
--- a/src/main/org/codehaus/groovy/ast/ClassNode.java
+++ b/src/main/org/codehaus/groovy/ast/ClassNode.java
@@ -102,16 +102,21 @@ import java.util.Set;
*/
public class ClassNode extends AnnotatedNode implements Opcodes {
private static class MapOfLists {
- private final Map<Object, List<MethodNode>> map = new HashMap<Object, List<MethodNode>>();
+ private Map<Object, List<MethodNode>> map;
public List<MethodNode> get(Object key) {
- return map.get(key);
+ return map == null ? null : map.get(key);
}
+
public List<MethodNode> getNotNull(Object key) {
List<MethodNode> ret = get(key);
if (ret==null) ret = Collections.emptyList();
return ret;
}
+
public void put(Object key, MethodNode value) {
+ if (map == null) {
+ map = new HashMap<Object, List<MethodNode>>();
+ }
if (map.containsKey(key)) {
get(key).add(value);
} else {
@@ -120,6 +125,7 @@ public class ClassNode extends AnnotatedNode implements Opcodes {
map.put(key, list);
}
}
+
public void remove(Object key, MethodNode value) {
get(key).remove(value);
}
http://git-wip-us.apache.org/repos/asf/groovy/blob/ec080fe2/src/main/org/codehaus/groovy/reflection/ParameterTypes.java
----------------------------------------------------------------------
diff --git a/src/main/org/codehaus/groovy/reflection/ParameterTypes.java b/src/main/org/codehaus/groovy/reflection/ParameterTypes.java
index 9e683be..4c5d5fa 100644
--- a/src/main/org/codehaus/groovy/reflection/ParameterTypes.java
+++ b/src/main/org/codehaus/groovy/reflection/ParameterTypes.java
@@ -25,17 +25,18 @@ import org.codehaus.groovy.runtime.wrappers.Wrapper;
import java.lang.reflect.Array;
-public class ParameterTypes
-{
- protected volatile Class [] nativeParamTypes;
- protected volatile CachedClass [] parameterTypes;
+public class ParameterTypes {
+ private final static Class[] NO_PARAMETERS = new Class[0];
- protected boolean isVargsMethod;
+ protected volatile Class[] nativeParamTypes;
+ protected volatile CachedClass[] parameterTypes;
- public ParameterTypes () {
+ protected boolean isVargsMethod;
+
+ public ParameterTypes() {
}
- public ParameterTypes(Class pt []) {
+ public ParameterTypes(Class pt[]) {
nativeParamTypes = pt;
}
@@ -43,9 +44,8 @@ public class ParameterTypes
nativeParamTypes = new Class[pt.length];
for (int i = 0; i != pt.length; ++i) {
try {
- nativeParamTypes[i] = Class.forName(pt[i]);
- }
- catch (ClassNotFoundException e){
+ nativeParamTypes[i] = Class.forName(pt[i]);
+ } catch (ClassNotFoundException e) {
NoClassDefFoundError err = new NoClassDefFoundError();
err.initCause(e);
throw err;
@@ -59,29 +59,34 @@ public class ParameterTypes
protected final void setParametersTypes(CachedClass[] pt) {
this.parameterTypes = pt;
- isVargsMethod = pt.length > 0 && pt [pt.length-1].isArray;
+ isVargsMethod = pt.length > 0 && pt[pt.length - 1].isArray;
}
public CachedClass[] getParameterTypes() {
- if (parameterTypes == null) {
- getParametersTypes0();
- }
+ if (parameterTypes == null) {
+ getParametersTypes0();
+ }
- return parameterTypes;
- }
+ return parameterTypes;
+ }
private synchronized void getParametersTypes0() {
- if (parameterTypes != null)
- return;
+ if (parameterTypes != null)
+ return;
- Class [] npt = nativeParamTypes == null ? getPT() : nativeParamTypes;
+ Class[] npt = nativeParamTypes == null ? getPT() : nativeParamTypes;
+ if (npt.length == 0) {
+ nativeParamTypes = NO_PARAMETERS;
+ setParametersTypes(CachedClass.EMPTY_ARRAY);
+ } else {
- CachedClass[] pt = new CachedClass [npt.length];
- for (int i = 0; i != npt.length; ++i)
- pt[i] = ReflectionCache.getCachedClass(npt[i]);
+ CachedClass[] pt = new CachedClass[npt.length];
+ for (int i = 0; i != npt.length; ++i)
+ pt[i] = ReflectionCache.getCachedClass(npt[i]);
- nativeParamTypes = npt;
- setParametersTypes(pt);
+ nativeParamTypes = npt;
+ setParametersTypes(pt);
+ }
}
public Class[] getNativeParameterTypes() {
@@ -92,32 +97,33 @@ public class ParameterTypes
}
private synchronized void getNativeParameterTypes0() {
- if (nativeParamTypes != null)
- return;
-
- Class [] npt;
- if (parameterTypes != null) {
- npt = new Class [parameterTypes.length];
- for (int i = 0; i != parameterTypes.length; ++i) {
- npt[i] = parameterTypes[i].getTheClass();
- }
- }
- else
- npt = getPT ();
- nativeParamTypes = npt;
+ if (nativeParamTypes != null)
+ return;
+
+ Class[] npt;
+ if (parameterTypes != null) {
+ npt = new Class[parameterTypes.length];
+ for (int i = 0; i != parameterTypes.length; ++i) {
+ npt[i] = parameterTypes[i].getTheClass();
+ }
+ } else
+ npt = getPT();
+ nativeParamTypes = npt;
}
- protected Class[] getPT() { throw new UnsupportedOperationException(getClass().getName()); }
+ protected Class[] getPT() {
+ throw new UnsupportedOperationException(getClass().getName());
+ }
public boolean isVargsMethod() {
return isVargsMethod;
}
-
+
public boolean isVargsMethod(Object[] arguments) {
// Uncomment if at some point this method can be called before parameterTypes initialized
// getParameterTypes();
- if(!isVargsMethod)
- return false;
+ if (!isVargsMethod)
+ return false;
final int lenMinus1 = parameterTypes.length - 1;
// -1 because the varg part is optional
@@ -176,7 +182,7 @@ public class ParameterTypes
* arguments to make the method callable
*
* @param argumentArrayOrig the arguments used to call the method
- * @param paramTypes the types of the parameters the method takes
+ * @param paramTypes the types of the parameters the method takes
*/
private static Object[] fitToVargs(Object[] argumentArrayOrig, CachedClass[] paramTypes) {
Class vargsClassOrig = paramTypes[paramTypes.length - 1].getTheClass().getComponentType();
@@ -221,32 +227,30 @@ public class ParameterTypes
throw new GroovyBugError("trying to call a vargs method without enough arguments");
}
}
-
+
private static Object makeCommonArray(Object[] arguments, int offset, Class baseClass) {
Object[] result = (Object[]) Array.newInstance(baseClass, arguments.length - offset);
- for (int i=offset; i<arguments.length; i++) {
+ for (int i = offset; i < arguments.length; i++) {
Object v = arguments[i];
- v = DefaultTypeTransformation.castToType(v,baseClass);
- result[i-offset] = v;
+ v = DefaultTypeTransformation.castToType(v, baseClass);
+ result[i - offset] = v;
}
return result;
}
-
+
public boolean isValidMethod(Class[] arguments) {
if (arguments == null) return true;
final int size = arguments.length;
CachedClass[] pt = getParameterTypes();
- final int paramMinus1 = pt.length-1;
+ final int paramMinus1 = pt.length - 1;
if (isVargsMethod && size >= paramMinus1)
return isValidVarargsMethod(arguments, size, pt, paramMinus1);
- else
- if (pt.length == size)
- return isValidExactMethod(arguments, pt);
- else
- if (pt.length == 1 && size == 0 && !pt[0].isPrimitive)
- return true;
+ else if (pt.length == size)
+ return isValidExactMethod(arguments, pt);
+ else if (pt.length == 1 && size == 0 && !pt[0].isPrimitive)
+ return true;
return false;
}
@@ -261,13 +265,13 @@ public class ParameterTypes
return true;
}
- public boolean isValidExactMethod(Object [] args) {
+ public boolean isValidExactMethod(Object[] args) {
// lets check the parameter types match
getParametersTypes0();
int size = args.length;
if (size != parameterTypes.length)
- return false;
-
+ return false;
+
for (int i = 0; i < size; i++) {
if (args[i] != null && !parameterTypes[i].isAssignableFrom(args[i].getClass())) {
return false;
@@ -276,12 +280,12 @@ public class ParameterTypes
return true;
}
- public boolean isValidExactMethod(Class [] args) {
+ public boolean isValidExactMethod(Class[] args) {
// lets check the parameter types match
getParametersTypes0();
int size = args.length;
if (size != parameterTypes.length)
- return false;
+ return false;
for (int i = 0; i < size; i++) {
if (args[i] != null && !parameterTypes[i].isAssignableFrom(args[i])) {
@@ -293,7 +297,7 @@ public class ParameterTypes
private static boolean testComponentAssignable(Class toTestAgainst, Class toTest) {
Class component = toTest.getComponentType();
- if (component==null) return false;
+ if (component == null) return false;
return MetaClassHelper.isAssignableFrom(toTestAgainst, component);
}
@@ -307,10 +311,9 @@ public class ParameterTypes
// check direct match
CachedClass varg = pt[paramMinus1];
Class clazz = varg.getTheClass().getComponentType();
- if ( size==pt.length &&
- (varg.isAssignableFrom(arguments[paramMinus1]) ||
- testComponentAssignable(clazz, arguments[paramMinus1])))
- {
+ if (size == pt.length &&
+ (varg.isAssignableFrom(arguments[paramMinus1]) ||
+ testComponentAssignable(clazz, arguments[paramMinus1]))) {
return true;
}
@@ -327,28 +330,26 @@ public class ParameterTypes
final int size = arguments.length;
CachedClass[] paramTypes = getParameterTypes();
- final int paramMinus1 = paramTypes.length-1;
+ final int paramMinus1 = paramTypes.length - 1;
- if ( size >= paramMinus1 && paramTypes.length > 0 &&
- paramTypes[(paramMinus1)].isArray)
- {
+ if (size >= paramMinus1 && paramTypes.length > 0 &&
+ paramTypes[(paramMinus1)].isArray) {
// first check normal number of parameters
for (int i = 0; i < paramMinus1; i++) {
if (paramTypes[i].isAssignableFrom(getArgClass(arguments[i]))) continue;
return false;
}
-
-
+
+
// check direct match
CachedClass varg = paramTypes[paramMinus1];
Class clazz = varg.getTheClass().getComponentType();
- if ( size==paramTypes.length &&
- (varg.isAssignableFrom(getArgClass(arguments[paramMinus1])) ||
- testComponentAssignable(clazz, getArgClass(arguments[paramMinus1]))))
- {
+ if (size == paramTypes.length &&
+ (varg.isAssignableFrom(getArgClass(arguments[paramMinus1])) ||
+ testComponentAssignable(clazz, getArgClass(arguments[paramMinus1])))) {
return true;
}
-
+
// check varged
for (int i = paramMinus1; i < size; i++) {
@@ -375,9 +376,8 @@ public class ParameterTypes
cls = null;
} else {
if (arg instanceof Wrapper) {
- cls = ((Wrapper)arg).getType();
- }
- else
+ cls = ((Wrapper) arg).getType();
+ } else
cls = arg.getClass();
}
return cls;
http://git-wip-us.apache.org/repos/asf/groovy/blob/ec080fe2/src/main/org/codehaus/groovy/transform/AnnotationCollectorTransform.java
----------------------------------------------------------------------
diff --git a/src/main/org/codehaus/groovy/transform/AnnotationCollectorTransform.java b/src/main/org/codehaus/groovy/transform/AnnotationCollectorTransform.java
index 62ae932..80188d9 100644
--- a/src/main/org/codehaus/groovy/transform/AnnotationCollectorTransform.java
+++ b/src/main/org/codehaus/groovy/transform/AnnotationCollectorTransform.java
@@ -19,19 +19,36 @@
package org.codehaus.groovy.transform;
import groovy.transform.AnnotationCollector;
-
-import java.lang.reflect.Method;
-import java.util.*;
-
import org.codehaus.groovy.GroovyBugError;
-import org.codehaus.groovy.ast.*;
-import org.codehaus.groovy.ast.expr.*;
+import org.codehaus.groovy.ast.ASTNode;
+import org.codehaus.groovy.ast.AnnotatedNode;
+import org.codehaus.groovy.ast.AnnotationNode;
+import org.codehaus.groovy.ast.ClassHelper;
+import org.codehaus.groovy.ast.ClassNode;
+import org.codehaus.groovy.ast.Parameter;
+import org.codehaus.groovy.ast.expr.AnnotationConstantExpression;
+import org.codehaus.groovy.ast.expr.ArrayExpression;
+import org.codehaus.groovy.ast.expr.ClassExpression;
+import org.codehaus.groovy.ast.expr.ConstantExpression;
+import org.codehaus.groovy.ast.expr.Expression;
+import org.codehaus.groovy.ast.expr.ListExpression;
+import org.codehaus.groovy.ast.expr.MapExpression;
import org.codehaus.groovy.ast.stmt.ReturnStatement;
import org.codehaus.groovy.ast.stmt.Statement;
import org.codehaus.groovy.control.SourceUnit;
import org.codehaus.groovy.control.messages.SyntaxErrorMessage;
import org.codehaus.groovy.syntax.SyntaxException;
+import java.lang.reflect.Method;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.ListIterator;
+import java.util.Map;
+import java.util.Set;
+
import static org.objectweb.asm.Opcodes.*;
/**
@@ -188,7 +205,7 @@ public class AnnotationCollectorTransform {
List<AnnotationNode> ret = new ArrayList<AnnotationNode>(orig.size());
for (AnnotationNode an : orig) {
AnnotationNode newAn = new AnnotationNode(an.getClassNode());
- newAn.getMembers().putAll(an.getMembers());
+ copyMembers(an, newAn);
newAn.setSourcePosition(aliasAnnotationUsage);
ret.add(newAn);
}
@@ -204,12 +221,23 @@ public class AnnotationCollectorTransform {
ClassNode type = an.getClassNode();
if (type.getName().equals(AnnotationCollector.class.getName())) continue;
AnnotationNode toAdd = new AnnotationNode(type);
- toAdd.getMembers().putAll(an.getMembers());
+ copyMembers(an, toAdd);
ret.add(toAdd);
}
return ret;
}
+ private static void copyMembers(final AnnotationNode from, final AnnotationNode to) {
+ Map<String, Expression> members = from.getMembers();
+ copyMembers(members, to);
+ }
+
+ private static void copyMembers(final Map<String, Expression> members, final AnnotationNode to) {
+ for (Map.Entry<String, Expression> entry : members.entrySet()) {
+ to.addMember(entry.getKey(), entry.getValue());
+ }
+ }
+
private static List<AnnotationNode> getTargetListFromClass(ClassNode alias) {
Class<?> c = alias.getTypeClass();
Object[][] data;
@@ -239,6 +267,7 @@ public class AnnotationCollectorTransform {
Object val = member.get(name);
generated.put(name, makeExpression(val));
}
+ copyMembers(generated, toAdd);
toAdd.getMembers().putAll(generated);
}
return ret;
[2/3] groovy git commit: Fix non-reproducible output of the compiler
for closure shared variables
Posted by cc...@apache.org.
Fix non-reproducible output of the compiler for closure shared variables
This commit fixes the variable scope collector, to make sure that the collected variables
are always collected _in the same order_. Without this, the compiler may generate different
bytecode for the same sources. The problem is that if the bytecode is used as a cache key,
like in Gradle, the same sources producing different bytecode becomes an issue.
Project: http://git-wip-us.apache.org/repos/asf/groovy/repo
Commit: http://git-wip-us.apache.org/repos/asf/groovy/commit/1f2a5b56
Tree: http://git-wip-us.apache.org/repos/asf/groovy/tree/1f2a5b56
Diff: http://git-wip-us.apache.org/repos/asf/groovy/diff/1f2a5b56
Branch: refs/heads/GROOVY_2_5_X
Commit: 1f2a5b567b0cb00796474eb756f75df8c6ae33c6
Parents: c07118a
Author: Cedric Champeau <cc...@apache.org>
Authored: Fri Apr 7 11:14:01 2017 +0200
Committer: Cedric Champeau <cc...@apache.org>
Committed: Sat Apr 8 10:40:01 2017 +0200
----------------------------------------------------------------------
.../org/codehaus/groovy/ast/VariableScope.java | 17 ++--
.../classgen/asm/sc/bugs/Groovy8142Bug.groovy | 51 -----------
.../asm/sc/bugs/ReproducibleBytecodeBugs.groovy | 92 ++++++++++++++++++++
3 files changed, 99 insertions(+), 61 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/groovy/blob/1f2a5b56/src/main/org/codehaus/groovy/ast/VariableScope.java
----------------------------------------------------------------------
diff --git a/src/main/org/codehaus/groovy/ast/VariableScope.java b/src/main/org/codehaus/groovy/ast/VariableScope.java
index 077c7f1..521b301 100644
--- a/src/main/org/codehaus/groovy/ast/VariableScope.java
+++ b/src/main/org/codehaus/groovy/ast/VariableScope.java
@@ -19,8 +19,8 @@
package org.codehaus.groovy.ast;
import java.util.Collections;
-import java.util.HashMap;
import java.util.Iterator;
+import java.util.LinkedHashMap;
import java.util.Map;
/**
@@ -99,18 +99,15 @@ public class VariableScope {
VariableScope copy = new VariableScope();
copy.clazzScope = clazzScope;
if (!declaredVariables.isEmpty()) {
- copy.declaredVariables = new HashMap<String, Variable>();
- copy.declaredVariables.putAll(declaredVariables);
+ copy.declaredVariables = new LinkedHashMap<String, Variable>(declaredVariables);
}
copy.inStaticContext = inStaticContext;
copy.parent = parent;
if (!referencedClassVariables.isEmpty()) {
- copy.referencedClassVariables = new HashMap<String, Variable>();
- copy.referencedClassVariables.putAll(referencedClassVariables);
+ copy.referencedClassVariables = new LinkedHashMap<String, Variable>(referencedClassVariables);
}
if (!referencedLocalVariables.isEmpty()) {
- copy.referencedLocalVariables = new HashMap<String, Variable>();
- copy.referencedLocalVariables.putAll(referencedLocalVariables);
+ copy.referencedLocalVariables = new LinkedHashMap<String, Variable>(referencedLocalVariables);
}
copy.resolvesDynamic = resolvesDynamic;
return copy;
@@ -118,7 +115,7 @@ public class VariableScope {
public void putDeclaredVariable(Variable var) {
if (declaredVariables == Collections.EMPTY_MAP)
- declaredVariables = new HashMap<String, Variable>();
+ declaredVariables = new LinkedHashMap<String, Variable>();
declaredVariables.put(var.getName(), var);
}
@@ -136,13 +133,13 @@ public class VariableScope {
public void putReferencedLocalVariable(Variable var) {
if (referencedLocalVariables == Collections.EMPTY_MAP)
- referencedLocalVariables = new HashMap<String, Variable>();
+ referencedLocalVariables = new LinkedHashMap<String, Variable>();
referencedLocalVariables.put(var.getName(), var);
}
public void putReferencedClassVariable(Variable var) {
if (referencedClassVariables == Collections.EMPTY_MAP)
- referencedClassVariables = new HashMap<String, Variable>();
+ referencedClassVariables = new LinkedHashMap<String, Variable>();
referencedClassVariables.put(var.getName(), var);
}
http://git-wip-us.apache.org/repos/asf/groovy/blob/1f2a5b56/src/test/org/codehaus/groovy/classgen/asm/sc/bugs/Groovy8142Bug.groovy
----------------------------------------------------------------------
diff --git a/src/test/org/codehaus/groovy/classgen/asm/sc/bugs/Groovy8142Bug.groovy b/src/test/org/codehaus/groovy/classgen/asm/sc/bugs/Groovy8142Bug.groovy
deleted file mode 100644
index 9a95a0e..0000000
--- a/src/test/org/codehaus/groovy/classgen/asm/sc/bugs/Groovy8142Bug.groovy
+++ /dev/null
@@ -1,51 +0,0 @@
-/*
- * 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 org.codehaus.groovy.classgen.asm.sc.bugs
-
-import groovy.transform.stc.StaticTypeCheckingTestCase
-import org.codehaus.groovy.classgen.asm.sc.StaticCompilationTestSupport
-
-class Groovy8142Bug extends StaticTypeCheckingTestCase implements StaticCompilationTestSupport {
- void testShouldNotProduceDeterministicBytecode() {
- 100.times {
- assertScript '''
- interface Project {
- File file()
- }
- interface FileOperations {
- File file()
- }
- interface ProjectInternal extends Project, FileOperations {
- }
-
- class Check {
- void test(ProjectInternal p) {
- def f = p.file()
- }
- }
-
- def c = new Check()
- '''
-
- assert astTrees['Check'][1].contains('INVOKEINTERFACE FileOperations.file ()Ljava/io/File;') : "Incorrect bytecode found in iteration $it"
- }
- }
-}
http://git-wip-us.apache.org/repos/asf/groovy/blob/1f2a5b56/src/test/org/codehaus/groovy/classgen/asm/sc/bugs/ReproducibleBytecodeBugs.groovy
----------------------------------------------------------------------
diff --git a/src/test/org/codehaus/groovy/classgen/asm/sc/bugs/ReproducibleBytecodeBugs.groovy b/src/test/org/codehaus/groovy/classgen/asm/sc/bugs/ReproducibleBytecodeBugs.groovy
new file mode 100644
index 0000000..d7e8f8a
--- /dev/null
+++ b/src/test/org/codehaus/groovy/classgen/asm/sc/bugs/ReproducibleBytecodeBugs.groovy
@@ -0,0 +1,92 @@
+/*
+ * 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 org.codehaus.groovy.classgen.asm.sc.bugs
+
+import groovy.transform.stc.StaticTypeCheckingTestCase
+import org.codehaus.groovy.classgen.asm.sc.StaticCompilationTestSupport
+
+class ReproducibleBytecodeBugs extends StaticTypeCheckingTestCase implements StaticCompilationTestSupport {
+ // GROOVY-8142
+ void testShouldNotProduceDeterministicBytecode() {
+ 100.times {
+ assertScript '''
+ interface Project {
+ File file()
+ }
+ interface FileOperations {
+ File file()
+ }
+ interface ProjectInternal extends Project, FileOperations {
+ }
+
+ class Check {
+ void test(ProjectInternal p) {
+ def f = p.file()
+ }
+ }
+
+ def c = new Check()
+ '''
+
+ assert astTrees['Check'][1].contains('INVOKEINTERFACE FileOperations.file ()Ljava/io/File;') : "Incorrect bytecode found in iteration $it"
+ }
+ }
+
+ // GROOVY-8148
+ void testShouldAlwaysAddClosureSharedVariablesInSameOrder() {
+ 100.times {
+ assertScript '''
+ class Check {
+ void test() {
+ def xx = true
+ def moot = "bar"
+ def kr = [:]
+ def zorg = []
+ def cl = {
+ def (x,y,z,t) = [xx, moot, kr , zorg]
+ }
+ }
+ }
+
+ def c = new Check()
+ '''
+
+ def bytecode = astTrees['Check$_test_closure1'][1]
+ assertOrdered it, bytecode,
+ 'PUTFIELD Check$_test_closure1.xx',
+ 'PUTFIELD Check$_test_closure1.moot',
+ 'PUTFIELD Check$_test_closure1.kr',
+ 'PUTFIELD Check$_test_closure1.zorg'
+
+ }
+ }
+
+
+ private static void assertOrdered(int iteration, String bytecode, String... elements) {
+ int start = 0
+ elements.eachWithIndex { it, i ->
+ start = bytecode.indexOf(it, start)
+ if (start == -1) {
+ throw new AssertionError("Iteration $iteration - Element [$it] not found in order (expected to find it at index $i)")
+ }
+ }
+ }
+}