groovy-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From pa...@apache.org
Subject [3/4] groovy git commit: GROOVY-8383: OptimizerVisitor#setConstField not @CS friendly (port to 2_4_X)
Date Sun, 19 Nov 2017 07:27:06 GMT
GROOVY-8383: OptimizerVisitor#setConstField not @CS friendly (port to 2_4_X)


Project: http://git-wip-us.apache.org/repos/asf/groovy/repo
Commit: http://git-wip-us.apache.org/repos/asf/groovy/commit/e4a31f2c
Tree: http://git-wip-us.apache.org/repos/asf/groovy/tree/e4a31f2c
Diff: http://git-wip-us.apache.org/repos/asf/groovy/diff/e4a31f2c

Branch: refs/heads/GROOVY_2_4_X
Commit: e4a31f2c1ec0f8e3cdb90f95514fe6306318a3dd
Parents: bb6764f
Author: paulk <paulk@asert.com.au>
Authored: Sun Nov 19 14:49:31 2017 +1000
Committer: paulk <paulk@asert.com.au>
Committed: Sun Nov 19 17:26:31 2017 +1000

----------------------------------------------------------------------
 .../groovy/control/OptimizerVisitor.java        | 59 +++++++++++---------
 src/test/groovy/bugs/Groovy8383Bug.groovy       | 51 +++++++++++++++++
 2 files changed, 85 insertions(+), 25 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/groovy/blob/e4a31f2c/src/main/org/codehaus/groovy/control/OptimizerVisitor.java
----------------------------------------------------------------------
diff --git a/src/main/org/codehaus/groovy/control/OptimizerVisitor.java b/src/main/org/codehaus/groovy/control/OptimizerVisitor.java
index 6aa0b28..6bb343b 100644
--- a/src/main/org/codehaus/groovy/control/OptimizerVisitor.java
+++ b/src/main/org/codehaus/groovy/control/OptimizerVisitor.java
@@ -33,19 +33,24 @@ import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
 
+import static org.codehaus.groovy.ast.ClassHelper.isPrimitiveType;
+
 /**
  * Visitor to produce several optimizations:
  * <ul>
- *     <li>to replace numbered constants with references to static fields</li>
- *     <li>remove superfluous references to GroovyObject interface</li>
+ * <li>to replace numbered constants with references to static fields</li>
+ * <li>remove superfluous references to GroovyObject interface</li>
  * </ul>
  */
 public class OptimizerVisitor extends ClassCodeExpressionTransformer {
     private ClassNode currentClass;
     private SourceUnit source;
 
-    private Map const2Var = new HashMap();
-    private List<FieldNode> missingFields = new LinkedList<FieldNode>();
+    // TODO make @CS lookup smarter so that we don't need both these maps
+    private final Map<Object, FieldNode> const2Objects = new HashMap<Object, FieldNode>();
+    private final Map<Object, FieldNode> const2Prims = new HashMap<Object, FieldNode>();
+    private int index;
+    private final List<FieldNode> missingFields = new LinkedList<FieldNode>();
 
     public OptimizerVisitor(CompilationUnit cu) {
     }
@@ -53,8 +58,10 @@ public class OptimizerVisitor extends ClassCodeExpressionTransformer {
     public void visitClass(ClassNode node, SourceUnit source) {
         this.currentClass = node;
         this.source = source;
-        const2Var.clear();
+        const2Objects.clear();
+        const2Prims.clear();
         missingFields.clear();
+        index = 0;
         super.visitClass(node);
         addMissingFields();
         pruneUnneededGroovyObjectInterface(node);
@@ -85,8 +92,7 @@ public class OptimizerVisitor extends ClassCodeExpressionTransformer {
     }
 
     private void addMissingFields() {
-        for (Object missingField : missingFields) {
-            FieldNode f = (FieldNode) missingField;
+        for (FieldNode f : missingFields) {
             currentClass.addField(f);
         }
     }
@@ -95,35 +101,38 @@ public class OptimizerVisitor extends ClassCodeExpressionTransformer
{
         final Object n = constantExpression.getValue();
         if (!(n instanceof Number)) return;
         if (n instanceof Integer || n instanceof Double) return;
-        if (n instanceof Long && (0L== (Long) n || 1L==(Long) n )) return; // LCONST_0,
LCONST_1
+        if (n instanceof Long && (0L == (Long) n || 1L == (Long) n)) return; // LCONST_0,
LCONST_1
 
-        FieldNode field = (FieldNode) const2Var.get(n);
-        if (field!=null) {
+        boolean isPrimitive = isPrimitiveType(constantExpression.getType());
+        FieldNode field = isPrimitive ? const2Prims.get(n) : const2Objects.get(n);
+        if (field != null) {
             constantExpression.setConstantName(field.getName());
             return;
         }
-        final String name = "$const$" + const2Var.size();
-        //TODO: this part here needs a bit of rethinking. If it can happen that the field
is defined already,
-        //      then is this code still valid?
-        field = currentClass.getDeclaredField(name);
-        if (field==null) {
-            field = new FieldNode(name,
-                    Opcodes.ACC_PRIVATE|Opcodes.ACC_STATIC|Opcodes.ACC_SYNTHETIC| Opcodes.ACC_FINAL,
-                    constantExpression.getType(),
-                    currentClass,
-                    constantExpression
-                    );
-            field.setSynthetic(true);
-            missingFields.add(field);
+        String name;
+        while (true) {
+            name = "$const$" + index++;
+            if (currentClass.getDeclaredField(name) == null) break;
         }
+        field = new FieldNode(name,
+                Opcodes.ACC_PRIVATE | Opcodes.ACC_STATIC | Opcodes.ACC_SYNTHETIC | Opcodes.ACC_FINAL,
+                constantExpression.getType(),
+                currentClass,
+                constantExpression);
+        field.setSynthetic(true);
+        missingFields.add(field);
         constantExpression.setConstantName(field.getName());
-        const2Var.put(n, field);
+        if (isPrimitive) {
+            const2Prims.put(n, field);
+        } else {
+            const2Objects.put(n, field);
+        }
     }
 
     public Expression transform(Expression exp) {
         if (exp == null) return null;
         if (!currentClass.isInterface() && exp.getClass() == ConstantExpression.class)
{
-            setConstField((ConstantExpression)exp);
+            setConstField((ConstantExpression) exp);
         }
         return exp.transformExpression(this);
     }

http://git-wip-us.apache.org/repos/asf/groovy/blob/e4a31f2c/src/test/groovy/bugs/Groovy8383Bug.groovy
----------------------------------------------------------------------
diff --git a/src/test/groovy/bugs/Groovy8383Bug.groovy b/src/test/groovy/bugs/Groovy8383Bug.groovy
new file mode 100644
index 0000000..1091260
--- /dev/null
+++ b/src/test/groovy/bugs/Groovy8383Bug.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 groovy.bugs
+
+class Groovy8383Bug extends GroovyTestCase {
+    void testCompileStaticWithOptimizedConstants() {
+        assertScript '''
+            @groovy.transform.CompileStatic
+            class Foo {
+                private static String $const$0 = 'xyzzy'
+
+                def method() {
+                    Long wrapper1 = 8L
+                    long prim1 = 8L
+                    long prim2 = 8L
+                    long prim3 = 9L
+                    Long wrapper2 = 9L
+                    wrapper1 + prim1 + prim2 + prim3 + wrapper2
+                }
+            }
+            assert new Foo().method() == 42
+
+            class Bar {
+                private static long $const$0 = 9
+                private static long $const$2 = 10
+                def method2() {
+                    long prim4 = 11L
+                    long prim5 = 12L
+                    prim4 + prim5 + $const$0 + $const$2
+                }
+            }
+            assert new Bar().method2() == 42
+        '''
+    }
+}


Mime
View raw message