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 2025/04/24 14:13:55 UTC

(groovy) branch master updated (d77d909df2 -> 8f6b1923b2)

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

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


    from d77d909df2 GROOVY-11614: Enums in switch/case statements that are not fully qualified will cause a groovy compile error for switch expressions even with type checking enabled
     new cb7facb24b GROOVY-9530: use parsed static field value for constant inlining
     new 8f6b1923b2 GROOVY-9530: load static field value under `configureClassNode(...)`

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../apache/groovy/ast/tools/ExpressionUtils.java   | 41 +++++++++++-----------
 .../ast/decompiled/MemberSignatureParser.java      |  2 +-
 .../org/codehaus/groovy/vmplugin/v8/Java8.java     | 23 +++++++++++-
 .../bugs/{Groovy11362.groovy => Groovy9530.groovy} | 27 ++++++++------
 4 files changed, 61 insertions(+), 32 deletions(-)
 copy src/test/groovy/bugs/{Groovy11362.groovy => Groovy9530.groovy} (59%)


(groovy) 02/02: GROOVY-9530: load static field value under `configureClassNode(...)`

Posted by em...@apache.org.
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

commit 8f6b1923b22bc93dc6d7854aae0236efdfc3b197
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Sun Apr 20 12:08:45 2025 -0500

    GROOVY-9530: load static field value under `configureClassNode(...)`
---
 .../apache/groovy/ast/tools/ExpressionUtils.java   |  4 ++--
 .../org/codehaus/groovy/vmplugin/v8/Java8.java     | 23 +++++++++++++++++++++-
 src/test/groovy/bugs/Groovy9530.groovy             | 21 ++++++++++----------
 3 files changed, 35 insertions(+), 13 deletions(-)

diff --git a/src/main/java/org/apache/groovy/ast/tools/ExpressionUtils.java b/src/main/java/org/apache/groovy/ast/tools/ExpressionUtils.java
index 1731dcbd7d..803dc7b842 100644
--- a/src/main/java/org/apache/groovy/ast/tools/ExpressionUtils.java
+++ b/src/main/java/org/apache/groovy/ast/tools/ExpressionUtils.java
@@ -278,7 +278,7 @@ public final class ExpressionUtils {
                     if (e instanceof ConstantExpression) {
                         return e;
                     }
-                } else if (cn.isResolved()) {
+                }/* else if (cn.isResolved()) { // GROOVY-9530
                     try {
                         var field = cn.getTypeClass().getField(pe.getPropertyAsString());
                         if (field != null) {
@@ -287,7 +287,7 @@ public final class ExpressionUtils {
                     } catch (Exception | LinkageError ignore) {
                         // leave property expression and we will report later
                     }
-                }
+                }*/
             }
         } else if (exp instanceof VariableExpression) {
             VariableExpression ve = (VariableExpression) exp;
diff --git a/src/main/java/org/codehaus/groovy/vmplugin/v8/Java8.java b/src/main/java/org/codehaus/groovy/vmplugin/v8/Java8.java
index 5e2901eb03..9a6e8bb0e6 100644
--- a/src/main/java/org/codehaus/groovy/vmplugin/v8/Java8.java
+++ b/src/main/java/org/codehaus/groovy/vmplugin/v8/Java8.java
@@ -62,6 +62,7 @@ import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.MalformedParameterizedTypeException;
 import java.lang.reflect.Member;
 import java.lang.reflect.Method;
+import java.lang.reflect.Modifier;
 import java.lang.reflect.ParameterizedType;
 import java.lang.reflect.ReflectPermission;
 import java.lang.reflect.Type;
@@ -393,7 +394,7 @@ public class Java8 implements VMPlugin {
             Field[] fields = clazz.getDeclaredFields();
             for (Field f : fields) {
                 ClassNode rt = makeClassNode(compileUnit, f.getGenericType(), f.getType());
-                FieldNode fn = new FieldNode(f.getName(), f.getModifiers(), rt, classNode, null);
+                FieldNode fn = new FieldNode(f.getName(), f.getModifiers(), rt, classNode, getValue(f));
                 setAnnotationMetaData(f.getAnnotations(), fn);
                 classNode.addField(fn);
             }
@@ -440,6 +441,26 @@ public class Java8 implements VMPlugin {
         }
     }
 
+    /**
+     * Returns the initial expression for given field.
+     *
+     * @return value expression or null
+     * @since 5.0.0
+     */
+    protected Expression getValue(final Field field) {
+        int modifiers = field.getModifiers();
+        // TODO: read ConstantValue from field attributes
+        if (Modifier.isFinal(modifiers) && Modifier.isStatic(modifiers)
+                && (Modifier.isPublic(modifiers) || Modifier.isProtected(modifiers))
+                && (field.getType().isPrimitive() || field.getType().equals(String.class))) {
+            try {
+                return new ConstantExpression(field.get(null), true);
+            } catch (ReflectiveOperationException | LinkageError e) {
+            }
+        }
+        return null;
+    }
+
     /**
      * Synthetic parameters such as those added for inner class constructors may
      * not be included in the parameter annotations array. This is the case when
diff --git a/src/test/groovy/bugs/Groovy9530.groovy b/src/test/groovy/bugs/Groovy9530.groovy
index 20f985a96c..66ae9328f4 100644
--- a/src/test/groovy/bugs/Groovy9530.groovy
+++ b/src/test/groovy/bugs/Groovy9530.groovy
@@ -18,28 +18,29 @@
  */
 package bugs
 
-import org.junit.jupiter.api.Test
+import org.codehaus.groovy.classgen.asm.AbstractBytecodeTestCase
 
-import static groovy.test.GroovyAssert.assertScript
-
-final class Groovy9530 {
+final class Groovy9530 extends AbstractBytecodeTestCase {
 
     static class StaticClass {
         public static final int STATIC_VALUE = getStaticValue()
 
         private static int getStaticValue() {
-            return 'resource from classpath'.length()
+            getClassLoader().getResources('absent thingy').toList().size()
         }
     }
 
-    @Test
     void testConstantInlining() {
-        assertScript """import bugs.Groovy9530.StaticClass
+        def bytecode = compile '''import bugs.Groovy9530.StaticClass
             class C {
                 public static final int VALUE = StaticClass.STATIC_VALUE
             }
-
-            assert C.VALUE == 23
-        """
+        '''
+        assert bytecode.hasSequence([
+            'public final static I VALUE'
+        ])
+        assert !bytecode.hasSequence([
+            'public final static I VALUE = 0' // no initializer should exist!
+        ])
     }
 }


(groovy) 01/02: GROOVY-9530: use parsed static field value for constant inlining

Posted by em...@apache.org.
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

commit cb7facb24b5d598553ec6d1c0014c2aa28d207ed
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Sat Apr 19 10:53:25 2025 -0500

    GROOVY-9530: use parsed static field value for constant inlining
---
 .../apache/groovy/ast/tools/ExpressionUtils.java   | 39 ++++++++++---------
 .../ast/decompiled/MemberSignatureParser.java      |  2 +-
 src/test/groovy/bugs/Groovy9530.groovy             | 45 ++++++++++++++++++++++
 3 files changed, 66 insertions(+), 20 deletions(-)

diff --git a/src/main/java/org/apache/groovy/ast/tools/ExpressionUtils.java b/src/main/java/org/apache/groovy/ast/tools/ExpressionUtils.java
index 178a96e5b6..1731dcbd7d 100644
--- a/src/main/java/org/apache/groovy/ast/tools/ExpressionUtils.java
+++ b/src/main/java/org/apache/groovy/ast/tools/ExpressionUtils.java
@@ -32,8 +32,6 @@ import org.codehaus.groovy.ast.expr.VariableExpression;
 import org.codehaus.groovy.runtime.DefaultGroovyMethods;
 import org.codehaus.groovy.runtime.typehandling.NumberMath;
 
-import java.lang.reflect.Field;
-import java.lang.reflect.Modifier;
 import java.math.BigDecimal;
 import java.util.List;
 import java.util.ListIterator;
@@ -258,6 +256,7 @@ public final class ExpressionUtils {
      *     <li>Binary expressions - string concatenation and numeric +, -, /, *</li>
      *     <li>List expressions - list of constants</li>
      * </ul>
+     *
      * @param exp the original expression
      * @param attrType the type that the final constant should be
      * @return the transformed type or the original if no transformation was possible
@@ -265,26 +264,28 @@ public final class ExpressionUtils {
     public static Expression transformInlineConstants(final Expression exp, final ClassNode attrType) {
         if (exp instanceof PropertyExpression) {
             PropertyExpression pe = (PropertyExpression) exp;
-            ClassNode type = pe.getObjectExpression().getType();
-            if (pe.getObjectExpression() instanceof ClassExpression && !type.isEnum()) {
-                if (type.isPrimaryClassNode()) {
-                    FieldNode fn = type.getField(pe.getPropertyAsString());
-                    if (fn != null && fn.isStatic() && fn.isFinal()) {
-                        Expression e = transformInlineConstants(fn.getInitialValueExpression(), attrType);
-                        if (e != null) {
-                            return e;
-                        }
+            Expression e = pe.getObjectExpression();
+            ClassNode cn;
+            FieldNode fn;
+            if (e instanceof ClassExpression
+                    && !(cn = e.getType().redirect()).isEnum()
+                    && (cn.isPrimaryClassNode() || cn.isResolved())
+                    && (fn = ClassNodeUtils.getField(cn, pe.getPropertyAsString())) != null
+                    && fn.isStatic()
+                    && fn.isFinal()) {
+                if (fn.hasInitialExpression()) {
+                    e = transformInlineConstants(fn.getInitialValueExpression(), attrType);
+                    if (e instanceof ConstantExpression) {
+                        return e;
                     }
-                } else if (type.isResolved()) {
+                } else if (cn.isResolved()) {
                     try {
-                        Field field = type.redirect().getTypeClass().getField(pe.getPropertyAsString());
-                        if (field != null && Modifier.isStatic(field.getModifiers()) && Modifier.isFinal(field.getModifiers())) {
-                            ConstantExpression ce = new ConstantExpression(field.get(null), true);
-                            configure(exp, ce);
-                            return ce;
+                        var field = cn.getTypeClass().getField(pe.getPropertyAsString());
+                        if (field != null) {
+                            return configure(exp, new ConstantExpression(field.get(null), true));
                         }
-                    } catch (Exception | LinkageError e) {
-                        // ignore, leave property expression in place and we'll report later
+                    } catch (Exception | LinkageError ignore) {
+                        // leave property expression and we will report later
                     }
                 }
             }
diff --git a/src/main/java/org/codehaus/groovy/ast/decompiled/MemberSignatureParser.java b/src/main/java/org/codehaus/groovy/ast/decompiled/MemberSignatureParser.java
index 6e25c3f9ae..48426dedbc 100644
--- a/src/main/java/org/codehaus/groovy/ast/decompiled/MemberSignatureParser.java
+++ b/src/main/java/org/codehaus/groovy/ast/decompiled/MemberSignatureParser.java
@@ -53,7 +53,7 @@ class MemberSignatureParser {
             // ex: java.util.Collections#EMPTY_LIST/EMPTY_MAP/EMPTY_SET
             type[0] = GenericsUtils.nonGeneric(type[0]);
         }
-        return new FieldNode(field.fieldName, field.accessModifiers, type[0], owner, field.value != null ? new ConstantExpression(field.value) : null);
+        return new FieldNode(field.fieldName, field.accessModifiers, type[0], owner, field.value != null ? new ConstantExpression(field.value, true) : null);
     }
 
     static MethodNode createMethodNode(final AsmReferenceResolver resolver, final MethodStub method) {
diff --git a/src/test/groovy/bugs/Groovy9530.groovy b/src/test/groovy/bugs/Groovy9530.groovy
new file mode 100644
index 0000000000..20f985a96c
--- /dev/null
+++ b/src/test/groovy/bugs/Groovy9530.groovy
@@ -0,0 +1,45 @@
+/*
+ *  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 bugs
+
+import org.junit.jupiter.api.Test
+
+import static groovy.test.GroovyAssert.assertScript
+
+final class Groovy9530 {
+
+    static class StaticClass {
+        public static final int STATIC_VALUE = getStaticValue()
+
+        private static int getStaticValue() {
+            return 'resource from classpath'.length()
+        }
+    }
+
+    @Test
+    void testConstantInlining() {
+        assertScript """import bugs.Groovy9530.StaticClass
+            class C {
+                public static final int VALUE = StaticClass.STATIC_VALUE
+            }
+
+            assert C.VALUE == 23
+        """
+    }
+}