You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by em...@apache.org on 2021/06/06 22:09:50 UTC

[groovy] branch master updated: GROOVY-3421: evaluate spread-map value only once

This is an automated email from the ASF dual-hosted git repository.

emilles pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git


The following commit(s) were added to refs/heads/master by this push:
     new 05a3963  GROOVY-3421: evaluate spread-map value only once
05a3963 is described below

commit 05a39632565bee88949c8db9d56b9f9598321fc2
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Sun Jun 6 17:09:36 2021 -0500

    GROOVY-3421: evaluate spread-map value only once
---
 .../groovy/classgen/AsmClassGenerator.java         | 31 +++++++++-------------
 .../groovy/operator/SpreadMapOperatorTest.groovy   | 26 +++++++++++-------
 2 files changed, 30 insertions(+), 27 deletions(-)

diff --git a/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java b/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
index 711ef84..9a23d8d 100644
--- a/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
+++ b/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
@@ -119,6 +119,7 @@ import java.io.PrintWriter;
 import java.io.Writer;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.LinkedList;
@@ -129,15 +130,6 @@ import java.util.Optional;
 
 import static org.apache.groovy.ast.tools.ClassNodeUtils.getField;
 import static org.apache.groovy.ast.tools.ExpressionUtils.isThisOrSuper;
-import static org.codehaus.groovy.ast.ClassHelper.isPrimitiveType;
-import static org.codehaus.groovy.ast.tools.GeneralUtils.attrX;
-import static org.codehaus.groovy.ast.tools.GeneralUtils.callX;
-import static org.codehaus.groovy.ast.tools.GeneralUtils.classX;
-import static org.codehaus.groovy.ast.tools.GeneralUtils.fieldX;
-import static org.codehaus.groovy.ast.tools.GeneralUtils.getGetterName;
-import static org.codehaus.groovy.ast.tools.GeneralUtils.getSetterName;
-import static org.codehaus.groovy.ast.tools.GeneralUtils.propX;
-import static org.codehaus.groovy.ast.tools.GeneralUtils.thisPropX;
 import static org.codehaus.groovy.ast.ClassHelper.isClassType;
 import static org.codehaus.groovy.ast.ClassHelper.isObjectType;
 import static org.codehaus.groovy.ast.ClassHelper.isPrimitiveBoolean;
@@ -148,7 +140,16 @@ import static org.codehaus.groovy.ast.ClassHelper.isPrimitiveFloat;
 import static org.codehaus.groovy.ast.ClassHelper.isPrimitiveInt;
 import static org.codehaus.groovy.ast.ClassHelper.isPrimitiveLong;
 import static org.codehaus.groovy.ast.ClassHelper.isPrimitiveShort;
+import static org.codehaus.groovy.ast.ClassHelper.isPrimitiveType;
 import static org.codehaus.groovy.ast.ClassHelper.isStringType;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.attrX;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.callX;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.classX;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.fieldX;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.getGetterName;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.getSetterName;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.propX;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.thisPropX;
 import static org.codehaus.groovy.transform.sc.StaticCompilationMetadataKeys.PROPERTY_OWNER;
 import static org.objectweb.asm.Opcodes.AASTORE;
 import static org.objectweb.asm.Opcodes.ACC_ENUM;
@@ -872,15 +873,9 @@ public class AsmClassGenerator extends ClassGenerator {
 
     @Override
     public void visitSpreadMapExpression(final SpreadMapExpression expression) {
-        Expression subExpression = expression.getExpression();
-        // to not record the underlying MapExpression twice,
-        // we disable the assertion tracker
-        // see https://issues.apache.org/jira/browse/GROOVY-3421
-        controller.getAssertionWriter().disableTracker();
-        subExpression.visit(this);
-        controller.getOperandStack().box();
-        spreadMap.call(controller.getMethodVisitor());
-        controller.getAssertionWriter().reenableTracker();
+        // GROOVY-3421: SpreadMapExpression is key expression and contains value
+        callX(ClassHelper.make(Collections.class), "emptyMap").visit(this);
+        spreadMap.call(controller.getMethodVisitor()); // dummy SpreadMap
         controller.getOperandStack().replace(ClassHelper.OBJECT_TYPE);
     }
 
diff --git a/src/test/groovy/operator/SpreadMapOperatorTest.groovy b/src/test/groovy/operator/SpreadMapOperatorTest.groovy
index 855f71d..47f1632 100644
--- a/src/test/groovy/operator/SpreadMapOperatorTest.groovy
+++ b/src/test/groovy/operator/SpreadMapOperatorTest.groovy
@@ -100,18 +100,27 @@ class SpreadMapOperatorTest extends GroovyTestCase {
 
     void testSpecialSpreadMapIndexNotation() {
         assertScript '''
-        @groovy.transform.ToString
-        class Person { String name; int age }
+            @groovy.transform.ToString
+            class Person { String name; int age }
 
-        assert Person[ name:'Dave', age:32 ].toString() == 'Person(Dave, 32)'
+            assert Person[ name:'Dave', age:32 ].toString() == 'Person(Dave, 32)'
 
-        def timMap = [ name:'Tim', age:49 ]
-        assert Person[ *:timMap ].toString() == 'Person(Tim, 49)'
+            def timMap = [ name:'Tim', age:49 ]
+            assert Person[ *:timMap ].toString() == 'Person(Tim, 49)'
 
-        assert Person[ *:[ name:'John', age:29 ] ].toString() == 'Person(John, 29)'
+            assert Person[ *:[ name:'John', age:29 ] ].toString() == 'Person(John, 29)'
 
-        def ppl = [ [ name:'Tim', age:49 ], [ name:'Dave', age:32 ], [ name:'Steve', age:18 ] ]
-        assert ppl.collect { Person [ *:it ] }*.age == [49, 32, 18]
+            def ppl = [ [ name:'Tim', age:49 ], [ name:'Dave', age:32 ], [ name:'Steve', age:18 ] ]
+            assert ppl.collect { Person [ *:it ] }*.age == [49, 32, 18]
+        '''
+    }
+
+    // GROOVY-3421
+    void testSpreadMapSingleEval() {
+        assertScript '''
+            int i = 1
+            Map map = [a:i, *:[b:++i]]
+            assert map == [a: 1, b: 2]
         '''
     }
 
@@ -131,4 +140,3 @@ class SpreadMapOperatorTest extends GroovyTestCase {
         // Call with one usual argument, one named argument, one spread list argument, and one spread map argument
     }
 }
-