groovy-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From cchamp...@apache.org
Subject groovy git commit: Fix non-deterministic selection of interface method in `@CompileStatic`
Date Thu, 06 Apr 2017 16:58:44 GMT
Repository: groovy
Updated Branches:
  refs/heads/GROOVY_2_4_X 22f327f6f -> 61a586dda


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/61a586dd
Tree: http://git-wip-us.apache.org/repos/asf/groovy/tree/61a586dd
Diff: http://git-wip-us.apache.org/repos/asf/groovy/diff/61a586dd

Branch: refs/heads/GROOVY_2_4_X
Commit: 61a586dda42e924495c163b5d59e0496c3215253
Parents: 22f327f
Author: Cedric Champeau <cchampeau@apache.org>
Authored: Thu Apr 6 18:51:30 2017 +0200
Committer: Cedric Champeau <cchampeau@apache.org>
Committed: Thu Apr 6 18:51:30 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/61a586dd/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 07edbc3..4b80103 100644
--- a/src/main/org/codehaus/groovy/ast/ClassNode.java
+++ b/src/main/org/codehaus/groovy/ast/ClassNode.java
@@ -40,7 +40,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;
@@ -421,7 +420,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/61a586dd/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 65e6f28..f6455e5 100644
--- a/src/main/org/codehaus/groovy/ast/tools/GeneralUtils.java
+++ b/src/main/org/codehaus/groovy/ast/tools/GeneralUtils.java
@@ -63,7 +63,7 @@ import org.codehaus.groovy.transform.AbstractASTTransformation;
 
 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;
@@ -392,7 +392,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/61a586dd/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 5312dc2..3154932 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/61a586dd/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"
+        }
+    }
+}


Mime
View raw message