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

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

commit 3a498f81ecb64b2a737689f5db93deb01f8ec01a
Author: Paul King <[email protected]>
AuthorDate: Wed Jan 24 11:55:16 2024 +1000

    GROOVY-11295: Optimise generated next() and previous() in enums
---
 .../codehaus/groovy/ast/tools/GeneralUtils.java    |  20 ++
 .../org/codehaus/groovy/classgen/EnumVisitor.java  | 235 ++++-----------------
 2 files changed, 66 insertions(+), 189 deletions(-)

diff --git a/src/main/java/org/codehaus/groovy/ast/tools/GeneralUtils.java 
b/src/main/java/org/codehaus/groovy/ast/tools/GeneralUtils.java
index dfef6ea087..59a33acd94 100644
--- a/src/main/java/org/codehaus/groovy/ast/tools/GeneralUtils.java
+++ b/src/main/java/org/codehaus/groovy/ast/tools/GeneralUtils.java
@@ -98,11 +98,15 @@ public class GeneralUtils {
     public static final Token EQ = Token.newSymbol(Types.COMPARE_EQUAL, -1, 
-1);
     public static final Token NE = Token.newSymbol(Types.COMPARE_NOT_EQUAL, 
-1, -1);
     public static final Token NOT_IDENTICAL = 
Token.newSymbol(Types.COMPARE_NOT_IDENTICAL, -1, -1);
+    public static final Token GE = 
Token.newSymbol(Types.COMPARE_GREATER_THAN_EQUAL, -1, -1);
+    public static final Token GT = Token.newSymbol(Types.COMPARE_GREATER_THAN, 
-1, -1);
+    public static final Token LE = 
Token.newSymbol(Types.COMPARE_LESS_THAN_EQUAL, -1, -1);
     public static final Token LT = Token.newSymbol(Types.COMPARE_LESS_THAN, 
-1, -1);
     public static final Token AND = Token.newSymbol(Types.LOGICAL_AND, -1, -1);
     public static final Token OR = Token.newSymbol(Types.LOGICAL_OR, -1, -1);
     public static final Token CMP = Token.newSymbol(Types.COMPARE_TO, -1, -1);
     public static final Token INSTANCEOF = 
Token.newSymbol(Types.KEYWORD_INSTANCEOF, -1, -1);
+    public static final Token MINUS = Token.newSymbol(Types.MINUS, -1, -1);
     public static final Token PLUS = Token.newSymbol(Types.PLUS, -1, -1);
     private static final Token INDEX = Token.newSymbol("[", -1, -1);
 
@@ -603,6 +607,14 @@ public class GeneralUtils {
         return propX(receiver, pNode.getName());
     }
 
+    public static BinaryExpression geX(final Expression lhv, final Expression 
rhv) {
+        return binX(lhv, GE, rhv);
+    }
+
+    public static BinaryExpression gtX(final Expression lhv, final Expression 
rhv) {
+        return binX(lhv, GT, rhv);
+    }
+
     public static BinaryExpression hasClassX(final Expression instance, final 
ClassNode cNode) {
         return eqX(classX(cNode), callX(instance, "getClass"));
     }
@@ -727,6 +739,10 @@ public class GeneralUtils {
         return result;
     }
 
+    public static BinaryExpression leX(final Expression lhv, final Expression 
rhv) {
+        return binX(lhv, LE, rhv);
+    }
+
     public static BinaryExpression ltX(final Expression lhv, final Expression 
rhv) {
         return binX(lhv, LT, rhv);
     }
@@ -743,6 +759,10 @@ public class GeneralUtils {
         return new MapExpression(expressions);
     }
 
+    public static BinaryExpression minusX(final Expression lhv, final 
Expression rhv) {
+        return binX(lhv, MINUS, rhv);
+    }
+
     public static BinaryExpression neX(final Expression lhv, final Expression 
rhv) {
         return binX(lhv, NE, rhv);
     }
diff --git a/src/main/java/org/codehaus/groovy/classgen/EnumVisitor.java 
b/src/main/java/org/codehaus/groovy/classgen/EnumVisitor.java
index 2940fa37da..7d2a8b542a 100644
--- a/src/main/java/org/codehaus/groovy/classgen/EnumVisitor.java
+++ b/src/main/java/org/codehaus/groovy/classgen/EnumVisitor.java
@@ -30,39 +30,26 @@ import org.codehaus.groovy.ast.MethodNode;
 import org.codehaus.groovy.ast.Parameter;
 import org.codehaus.groovy.ast.expr.ArgumentListExpression;
 import org.codehaus.groovy.ast.expr.ArrayExpression;
-import org.codehaus.groovy.ast.expr.BinaryExpression;
-import org.codehaus.groovy.ast.expr.BooleanExpression;
 import org.codehaus.groovy.ast.expr.ClassExpression;
-import org.codehaus.groovy.ast.expr.ConstantExpression;
 import org.codehaus.groovy.ast.expr.ConstructorCallExpression;
-import org.codehaus.groovy.ast.expr.DeclarationExpression;
 import org.codehaus.groovy.ast.expr.Expression;
-import org.codehaus.groovy.ast.expr.FieldExpression;
 import org.codehaus.groovy.ast.expr.ListExpression;
 import org.codehaus.groovy.ast.expr.MapEntryExpression;
-import org.codehaus.groovy.ast.expr.MapExpression;
 import org.codehaus.groovy.ast.expr.MethodCallExpression;
 import org.codehaus.groovy.ast.expr.SpreadExpression;
-import org.codehaus.groovy.ast.expr.StaticMethodCallExpression;
 import org.codehaus.groovy.ast.expr.VariableExpression;
-import org.codehaus.groovy.ast.stmt.BlockStatement;
-import org.codehaus.groovy.ast.stmt.EmptyStatement;
-import org.codehaus.groovy.ast.stmt.ExpressionStatement;
-import org.codehaus.groovy.ast.stmt.IfStatement;
-import org.codehaus.groovy.ast.stmt.ReturnStatement;
 import org.codehaus.groovy.ast.stmt.Statement;
 import org.codehaus.groovy.control.CompilationUnit;
 import org.codehaus.groovy.control.SourceUnit;
 import org.codehaus.groovy.control.messages.SyntaxErrorMessage;
 import org.codehaus.groovy.syntax.SyntaxException;
-import org.codehaus.groovy.syntax.Token;
-import org.codehaus.groovy.syntax.Types;
 
 import java.util.ArrayList;
 import java.util.List;
 
 import static org.apache.groovy.ast.tools.ClassNodeUtils.addGeneratedMethod;
-import static org.codehaus.groovy.ast.tools.GeneralUtils.localVarX;
+import static org.codehaus.groovy.ast.ClassHelper.int_TYPE;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.*;
 import static org.objectweb.asm.Opcodes.ACC_ABSTRACT;
 import static org.objectweb.asm.Opcodes.ACC_FINAL;
 import static org.objectweb.asm.Opcodes.ACC_PRIVATE;
@@ -109,7 +96,7 @@ public class EnumVisitor extends ClassCodeVisitorSupport {
                 }
             }
 
-            addMethods(enumClass, values);
+            addMethods(enumClass, values, minValue, maxValue);
             checkForAbstractMethods(enumClass);
         }
 
@@ -126,16 +113,22 @@ public class EnumVisitor extends ClassCodeVisitorSupport {
         }
     }
 
-    private static void addMethods(final ClassNode enumClass, final FieldNode 
values) {
-        List<MethodNode> methods = enumClass.getMethods();
+    private static void addMethods(final ClassNode enumClass, final FieldNode 
values, FieldNode minValue, FieldNode maxValue) {
 
         boolean hasNext = false;
         boolean hasPrevious = false;
-        for (MethodNode m : methods) {
+        for (MethodNode m : enumClass.getMethods()) {
             if (m.getName().equals("next") && m.getParameters().length == 0) 
hasNext = true;
             if (m.getName().equals("previous") && m.getParameters().length == 
0) hasPrevious = true;
             if (hasNext && hasPrevious) break;
         }
+        boolean empty = true;
+        for (FieldNode f : enumClass.getFields()) {
+            if (f.isEnum()) {
+                empty = false;
+                break;
+            }
+        }
 
         ClassNode enumRef = enumClass.getPlainNodeReference();
 
@@ -143,153 +136,55 @@ public class EnumVisitor extends ClassCodeVisitorSupport 
{
             // create values() method
             MethodNode valuesMethod = new MethodNode("values", ACC_FINAL | 
ACC_PUBLIC | ACC_STATIC, enumRef.makeArray(), Parameter.EMPTY_ARRAY, 
ClassNode.EMPTY_ARRAY, null);
             valuesMethod.setSynthetic(true);
-            BlockStatement code = new BlockStatement();
-            MethodCallExpression cloneCall = new MethodCallExpression(new 
FieldExpression(values), "clone", MethodCallExpression.NO_ARGUMENTS);
+            MethodCallExpression cloneCall = callX(fieldX(values), "clone");
             cloneCall.setMethodTarget(values.getType().getMethod("clone", 
Parameter.EMPTY_ARRAY));
-            code.addStatement(new ReturnStatement(cloneCall));
-            valuesMethod.setCode(code);
+            valuesMethod.setCode(block(returnS(cloneCall)));
             addGeneratedMethod(enumClass, valuesMethod);
         }
 
         if (!hasNext) {
             // create next() method, code:
-            //     Day next() {
-            //        int ordinal = ordinal().next()
-            //        if (ordinal >= values().size()) ordinal = 0
+            //     Enum next() {
+            //        int ordinal = ordinal() + 1
+            //        if (ordinal >= values().size()) return MIN_VALUE
             //        return values()[ordinal]
             //     }
-            Token assign = Token.newSymbol(Types.ASSIGN, -1, -1);
-            Token ge = Token.newSymbol(Types.COMPARE_GREATER_THAN_EQUAL, -1, 
-1);
             MethodNode nextMethod = new MethodNode("next", ACC_PUBLIC, 
enumRef, Parameter.EMPTY_ARRAY, ClassNode.EMPTY_ARRAY, null);
             nextMethod.setSynthetic(true);
-            BlockStatement code = new BlockStatement();
-            BlockStatement ifStatement = new BlockStatement();
-            ifStatement.addStatement(
-                    new ExpressionStatement(
-                            new BinaryExpression(new 
VariableExpression("ordinal"), assign, new ConstantExpression(0))
-                    )
-            );
-
-            code.addStatement(
-                    new ExpressionStatement(
-                            new DeclarationExpression(
-                                    localVarX("ordinal"),
-                                    assign,
-                                    new MethodCallExpression(
-                                            new MethodCallExpression(
-                                                    
VariableExpression.THIS_EXPRESSION,
-                                                    "ordinal",
-                                                    
MethodCallExpression.NO_ARGUMENTS),
-                                            "next",
-                                            MethodCallExpression.NO_ARGUMENTS
-                                    )
-                            )
-                    )
-            );
-            code.addStatement(
-                    new IfStatement(
-                            new BooleanExpression(new BinaryExpression(
-                                    new VariableExpression("ordinal"),
-                                    ge,
-                                    new MethodCallExpression(
-                                            new FieldExpression(values),
-                                            "size",
-                                            MethodCallExpression.NO_ARGUMENTS
-                                    )
-                            )),
-                            ifStatement,
-                            EmptyStatement.INSTANCE
-                    )
-            );
-            code.addStatement(
-                    new ReturnStatement(
-                            new MethodCallExpression(new 
FieldExpression(values), "getAt", new VariableExpression("ordinal"))
-                    )
-            );
-            nextMethod.setCode(code);
+            VariableExpression ordinal = localVarX("ordinal", int_TYPE);
+            nextMethod.setCode(empty ? block(returnS(nullX())) : block(
+                declS(ordinal, plusX(callThisX("ordinal"), constX(1, true))),
+                ifS(geX(ordinal, propX(fieldX(values), "length")), 
returnS(varX(minValue))),
+                returnS(indexX(fieldX(values), ordinal))
+            ));
             addGeneratedMethod(enumClass, nextMethod);
         }
 
         if (!hasPrevious) {
             // create previous() method, code:
-            //    Day previous() {
-            //        int ordinal = ordinal().previous()
-            //        if (ordinal < 0) ordinal = values().size() - 1
+            //    Enum previous() {
+            //        int ordinal = ordinal() - 1
+            //        if (ordinal < 0) return MAX_VALUE
             //        return values()[ordinal]
             //    }
-            Token assign = Token.newSymbol(Types.ASSIGN, -1, -1);
-            Token lt = Token.newSymbol(Types.COMPARE_LESS_THAN, -1, -1);
             MethodNode prevMethod = new MethodNode("previous", ACC_PUBLIC, 
enumRef, Parameter.EMPTY_ARRAY, ClassNode.EMPTY_ARRAY, null);
             prevMethod.setSynthetic(true);
-            BlockStatement code = new BlockStatement();
-            BlockStatement ifStatement = new BlockStatement();
-            ifStatement.addStatement(
-                    new ExpressionStatement(
-                            new BinaryExpression(new 
VariableExpression("ordinal"), assign,
-                                    new MethodCallExpression(
-                                            new MethodCallExpression(
-                                                    new 
FieldExpression(values),
-                                                    "size",
-                                                    
MethodCallExpression.NO_ARGUMENTS
-                                            ),
-                                            "minus",
-                                            new ConstantExpression(1)
-                                    )
-                            )
-                    )
-            );
-
-            code.addStatement(
-                    new ExpressionStatement(
-                            new DeclarationExpression(
-                                    localVarX("ordinal"),
-                                    assign,
-                                    new MethodCallExpression(
-                                            new MethodCallExpression(
-                                                    
VariableExpression.THIS_EXPRESSION,
-                                                    "ordinal",
-                                                    
MethodCallExpression.NO_ARGUMENTS),
-                                            "previous",
-                                            MethodCallExpression.NO_ARGUMENTS
-                                    )
-                            )
-                    )
-            );
-            code.addStatement(
-                    new IfStatement(
-                            new BooleanExpression(new BinaryExpression(
-                                    new VariableExpression("ordinal"),
-                                    lt,
-                                    new ConstantExpression(0)
-                            )),
-                            ifStatement,
-                            EmptyStatement.INSTANCE
-                    )
-            );
-            code.addStatement(
-                    new ReturnStatement(
-                            new MethodCallExpression(new 
FieldExpression(values), "getAt", new VariableExpression("ordinal"))
-                    )
-            );
-            prevMethod.setCode(code);
+            VariableExpression ordinal = localVarX("ordinal", int_TYPE);
+            prevMethod.setCode(empty ? block(returnS(nullX())) : block(
+                declS(ordinal, minusX(callThisX("ordinal"), constX(1, true))),
+                ifS(ltX(ordinal, constX(0, true)), returnS(varX(maxValue))),
+                returnS(indexX(fieldX(values), ordinal))
+            ));
             addGeneratedMethod(enumClass, prevMethod);
         }
 
         {
             // create valueOf
-            Parameter stringParameter = new Parameter(ClassHelper.STRING_TYPE, 
"name");
+            Parameter stringParameter = param(ClassHelper.STRING_TYPE, "name");
             MethodNode valueOfMethod = new MethodNode("valueOf", ACC_PUBLIC | 
ACC_STATIC, enumRef, new Parameter[]{stringParameter}, ClassNode.EMPTY_ARRAY, 
null);
-            ArgumentListExpression callArguments = new 
ArgumentListExpression();
-            callArguments.addExpression(new ClassExpression(enumClass));
-            callArguments.addExpression(new VariableExpression("name"));
-
-            BlockStatement code = new BlockStatement();
-            code.addStatement(
-                    new ReturnStatement(
-                            new MethodCallExpression(new 
ClassExpression(ClassHelper.Enum_Type), "valueOf", callArguments)
-                    )
-            );
-            valueOfMethod.setCode(code);
+            valueOfMethod.setCode(block(returnS(
+                callX(ClassHelper.Enum_Type, "valueOf", 
args(classX(enumClass), varX("name")))
+            )));
             valueOfMethod.setSynthetic(true);
             addGeneratedMethod(enumClass, valueOfMethod);
         }
@@ -308,25 +203,17 @@ public class EnumVisitor extends ClassCodeVisitorSupport {
         //  return this(*para)
         // }
         ClassNode enumRef = enumClass.getPlainNodeReference();
-        Parameter[] parameter = new Parameter[]{new 
Parameter(ClassHelper.OBJECT_TYPE.makeArray(), "para")};
+        Parameter[] parameter = 
params(param(ClassHelper.OBJECT_TYPE.makeArray(), "para"));
         MethodNode initMethod = new MethodNode("$INIT", ACC_FINAL | ACC_PUBLIC 
| ACC_STATIC | ACC_SYNTHETIC, enumRef, parameter, ClassNode.EMPTY_ARRAY, null);
         initMethod.setSynthetic(true);
-        ConstructorCallExpression cce = new ConstructorCallExpression(
-                ClassNode.THIS,
-                new ArgumentListExpression(
-                        new SpreadExpression(new VariableExpression("para"))
-                )
-        );
-        BlockStatement code = new BlockStatement();
-        code.addStatement(new ReturnStatement(cce));
-        initMethod.setCode(code);
+        ConstructorCallExpression cce = ctorThisX(args(new 
SpreadExpression(varX("para"))));
+        initMethod.setCode(block(returnS(cce)));
         addGeneratedMethod(enumClass, initMethod);
 
         // static init
         List<FieldNode> fields = enumClass.getFields();
         List<Expression> arrayInit = new ArrayList<>();
         int value = -1;
-        Token assign = Token.newSymbol(Types.ASSIGN, -1, -1);
         List<Statement> block = new ArrayList<>();
         FieldNode tempMin = null;
         FieldNode tempMax = null;
@@ -337,9 +224,7 @@ public class EnumVisitor extends ClassCodeVisitorSupport {
             tempMax = field;
 
             ClassNode enumBase = enumClass;
-            ArgumentListExpression args = new ArgumentListExpression();
-            args.addExpression(new ConstantExpression(field.getName()));
-            args.addExpression(new ConstantExpression(value));
+            ArgumentListExpression args = args(constX(field.getName()), 
constX(value));
             if (field.getInitialExpression() == null) {
                 if (enumClass.isAbstract()) {
                     addError(field, "The enum constant " + field.getName() + " 
must override abstract methods from " + enumBase.getName() + ".");
@@ -385,51 +270,23 @@ public class EnumVisitor extends ClassCodeVisitorSupport {
                     args.addExpression(exp);
                 }
                 if (!savedMapEntries.isEmpty()) {
-                    args.getExpressions().add(2, new 
MapExpression(savedMapEntries));
+                    args.getExpressions().add(2, mapX(savedMapEntries));
                 }
             }
             field.setInitialValueExpression(null);
-            block.add(
-                    new ExpressionStatement(
-                            new BinaryExpression(
-                                    new FieldExpression(field),
-                                    assign,
-                                    new StaticMethodCallExpression(enumBase, 
"$INIT", args)
-                            )
-                    )
-            );
-            arrayInit.add(new FieldExpression(field));
+            block.add(assignS(fieldX(field), callX(enumBase, "$INIT", args)));
+            arrayInit.add(fieldX(field));
         }
 
         if (!isAIC) {
             if (tempMin != null) {
-                block.add(
-                        new ExpressionStatement(
-                                new BinaryExpression(
-                                        new FieldExpression(minValue),
-                                        assign,
-                                        new FieldExpression(tempMin)
-                                )
-                        )
-                );
-                block.add(
-                        new ExpressionStatement(
-                                new BinaryExpression(
-                                        new FieldExpression(maxValue),
-                                        assign,
-                                        new FieldExpression(tempMax)
-                                )
-                        )
-                );
+                block.add(assignS(fieldX(minValue), fieldX(tempMin)));
+                block.add(assignS(fieldX(maxValue), fieldX(tempMax)));
                 enumClass.addField(minValue);
                 enumClass.addField(maxValue);
             }
 
-            block.add(
-                    new ExpressionStatement(
-                            new BinaryExpression(new FieldExpression(values), 
assign, new ArrayExpression(enumClass, arrayInit))
-                    )
-            );
+            block.add(assignS(fieldX(values), new ArrayExpression(enumClass, 
arrayInit)));
             enumClass.addField(values);
         }
         enumClass.addStaticInitializerStatements(block, true);

Reply via email to