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-reproducible output of the compiler for closure shared variables
Date Fri, 07 Apr 2017 09:16:42 GMT
Repository: groovy
Updated Branches:
  refs/heads/master d52d28a46 -> fb1f14a39


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

Branch: refs/heads/master
Commit: fb1f14a39d3ee4fd7dc30907096b0603ce126e21
Parents: d52d28a
Author: Cedric Champeau <cchampeau@apache.org>
Authored: Fri Apr 7 11:14:01 2017 +0200
Committer: Cedric Champeau <cchampeau@apache.org>
Committed: Fri Apr 7 11:16:24 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/fb1f14a3/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/fb1f14a3/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/fb1f14a3/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)")
+            }
+        }
+    }
+}


Mime
View raw message