Repository: groovy Updated Branches: refs/heads/master 709bf8255 -> 8294b6272
GROOVY-8383: OptimizerVisitor#setConstField not @CS friendly (closes #640) Project: http://git-wip-us.apache.org/repos/asf/groovy/repo Commit: http://git-wip-us.apache.org/repos/asf/groovy/commit/0bdbe559 Tree: http://git-wip-us.apache.org/repos/asf/groovy/tree/0bdbe559 Diff: http://git-wip-us.apache.org/repos/asf/groovy/diff/0bdbe559 Branch: refs/heads/master Commit: 0bdbe559c072276b2558c7207c04434a8c1114d8 Parents: 709bf82 Author: paulk <[email protected]> Authored: Sun Nov 19 14:49:31 2017 +1000 Committer: paulk <[email protected]> Committed: Sun Nov 19 15:01:01 2017 +1000 ---------------------------------------------------------------------- .../groovy/control/OptimizerVisitor.java | 57 +++++++++++--------- src/test/groovy/bugs/Groovy8383Bug.groovy | 51 ++++++++++++++++++ 2 files changed, 84 insertions(+), 24 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/groovy/blob/0bdbe559/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 f32651c..6bb343b 100644 --- a/src/main/org/codehaus/groovy/control/OptimizerVisitor.java +++ b/src/main/org/codehaus/groovy/control/OptimizerVisitor.java @@ -33,18 +33,23 @@ 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 final Map const2Var = new HashMap(); + // 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/0bdbe559/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 + ''' + } +}
