Return-Path: X-Original-To: archive-asf-public-internal@cust-asf2.ponee.io Delivered-To: archive-asf-public-internal@cust-asf2.ponee.io Received: from cust-asf.ponee.io (cust-asf.ponee.io [163.172.22.183]) by cust-asf2.ponee.io (Postfix) with ESMTP id 6E5CD200D3F for ; Sun, 19 Nov 2017 08:27:08 +0100 (CET) Received: by cust-asf.ponee.io (Postfix) id 6D02F160BE3; Sun, 19 Nov 2017 07:27:08 +0000 (UTC) Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by cust-asf.ponee.io (Postfix) with SMTP id 8CDD4160C0B for ; Sun, 19 Nov 2017 08:27:07 +0100 (CET) Received: (qmail 21650 invoked by uid 500); 19 Nov 2017 07:27:06 -0000 Mailing-List: contact commits-help@groovy.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@groovy.apache.org Delivered-To: mailing list commits@groovy.apache.org Received: (qmail 21622 invoked by uid 99); 19 Nov 2017 07:27:06 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Sun, 19 Nov 2017 07:27:06 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id 0C5AADFB7D; Sun, 19 Nov 2017 07:27:04 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: paulk@apache.org To: commits@groovy.apache.org Date: Sun, 19 Nov 2017 07:27:06 -0000 Message-Id: In-Reply-To: <8d9f58d8ad3f45eeb4d7516c20239e8a@git.apache.org> References: <8d9f58d8ad3f45eeb4d7516c20239e8a@git.apache.org> X-Mailer: ASF-Git Admin Mailer Subject: [3/4] groovy git commit: GROOVY-8383: OptimizerVisitor#setConstField not @CS friendly (port to 2_4_X) archived-at: Sun, 19 Nov 2017 07:27:08 -0000 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 Authored: Sun Nov 19 14:49:31 2017 +1000 Committer: paulk 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: *
    - *
  • to replace numbered constants with references to static fields
    • - *
  • remove superfluous references to GroovyObject interface
    • + *
  • to replace numbered constants with references to static fields
    • + *
  • remove superfluous references to GroovyObject interface
    • *
*/ public class OptimizerVisitor extends ClassCodeExpressionTransformer { private ClassNode currentClass; private SourceUnit source; - private Map const2Var = new HashMap(); - private List missingFields = new LinkedList(); + // TODO make @CS lookup smarter so that we don't need both these maps + private final Map const2Objects = new HashMap(); + private final Map const2Prims = new HashMap(); + private int index; + private final List missingFields = new LinkedList(); 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 + ''' + } +}