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 2017/04/07 13:31:32 UTC
[49/50] [abbrv] groovy git commit: Fix non-reproducible output of the
compiler for closure shared variables
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/parrot
Commit: fb1f14a39d3ee4fd7dc30907096b0603ce126e21
Parents: d52d28a
Author: Cedric Champeau <cc...@apache.org>
Authored: Fri Apr 7 11:14:01 2017 +0200
Committer: Cedric Champeau <cc...@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)")
+ }
+ }
+ }
+}