You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by pa...@apache.org on 2020/07/15 07:55:49 UTC

[groovy] branch GROOVY_2_5_X updated: minor refactor: move some helper methods to a more logical spot

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

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


The following commit(s) were added to refs/heads/GROOVY_2_5_X by this push:
     new 28cfd55  minor refactor: move some helper methods to a more logical spot
28cfd55 is described below

commit 28cfd55515211aad9e6e84144a4cca43d9dfa5cb
Author: Paul King <pa...@asert.com.au>
AuthorDate: Wed Jul 15 17:55:40 2020 +1000

    minor refactor: move some helper methods to a more logical spot
---
 .../apache/groovy/ast/tools/ExpressionUtils.java   | 26 ++++++++++++++++++-
 .../groovy/classgen/AsmClassGenerator.java         | 29 +++++++---------------
 .../groovy/classgen/asm/InvocationWriter.java      | 15 ++++++-----
 .../classgen/asm/OptimizingStatementWriter.java    |  3 ++-
 4 files changed, 45 insertions(+), 28 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 e1399ec..4aa67a5 100644
--- a/src/main/java/org/apache/groovy/ast/tools/ExpressionUtils.java
+++ b/src/main/java/org/apache/groovy/ast/tools/ExpressionUtils.java
@@ -47,7 +47,7 @@ import static org.codehaus.groovy.syntax.Types.POWER;
 import static org.codehaus.groovy.syntax.Types.RIGHT_SHIFT;
 import static org.codehaus.groovy.syntax.Types.RIGHT_SHIFT_UNSIGNED;
 
-public class ExpressionUtils {
+public final class ExpressionUtils {
     private static ArrayList<Integer> handledTypes = new ArrayList<Integer>();
 
     private ExpressionUtils() {
@@ -347,4 +347,28 @@ public class ExpressionUtils {
         return null;
     }
 
+    public static boolean isThisExpression(Expression expression) {
+        if (expression instanceof VariableExpression) {
+            VariableExpression varExp = (VariableExpression) expression;
+            return varExp.getName().equals("this");
+        }
+        return false;
+    }
+
+    public static boolean isSuperExpression(Expression expression) {
+        if (expression instanceof VariableExpression) {
+            VariableExpression varExp = (VariableExpression) expression;
+            return varExp.getName().equals("super");
+        }
+        return false;
+    }
+
+    public static boolean isThisOrSuper(Expression expression) {
+        return isThisExpression(expression) || isSuperExpression(expression);
+    }
+
+    public static boolean isNullConstant(Expression expr) {
+        return expr instanceof ConstantExpression && ((ConstantExpression) expr).isNullExpression();
+    }
+
 }
diff --git a/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java b/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
index 4887026..c015e83 100644
--- a/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
+++ b/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
@@ -19,6 +19,7 @@
 package org.codehaus.groovy.classgen;
 
 import groovy.lang.GroovyRuntimeException;
+import org.apache.groovy.ast.tools.ExpressionUtils;
 import org.apache.groovy.io.StringBuilderWriter;
 import org.codehaus.groovy.GroovyBugError;
 import org.codehaus.groovy.ast.ASTNode;
@@ -124,6 +125,8 @@ import java.util.List;
 import java.util.Map;
 import java.util.Objects;
 
+import static org.apache.groovy.ast.tools.ExpressionUtils.isSuperExpression;
+import static org.apache.groovy.ast.tools.ExpressionUtils.isThisOrSuper;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.attrX;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.callX;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.classX;
@@ -805,7 +808,7 @@ public class AsmClassGenerator extends ClassGenerator {
         if (castExpression.isCoerce()) {
             controller.getOperandStack().doAsType(type);
         } else {
-            if (isNullConstant(subExpression) && !ClassHelper.isPrimitiveType(type)) {
+            if (ExpressionUtils.isNullConstant(subExpression) && !ClassHelper.isPrimitiveType(type)) {
                 controller.getOperandStack().replace(type);
             } else {
                 ClassNode subExprType = controller.getTypeChooser().resolveType(subExpression, controller.getClassNode());
@@ -880,8 +883,9 @@ public class AsmClassGenerator extends ClassGenerator {
         controller.getAssertionWriter().record(call);
     }
 
+    @Deprecated
     public static boolean isNullConstant(Expression expr) {
-        return expr instanceof ConstantExpression && ((ConstantExpression) expr).getValue()==null;
+        return ExpressionUtils.isNullConstant(expr);
     }
 
     public void visitConstructorCallExpression(ConstructorCallExpression call) {
@@ -1246,7 +1250,7 @@ public class AsmClassGenerator extends ClassGenerator {
     }
 
     private static boolean isGroovyObject(Expression objectExpression) {
-        return isThisExpression(objectExpression) || objectExpression.getType().isDerivedFromGroovyObject() && !(objectExpression instanceof ClassExpression);
+        return ExpressionUtils.isThisExpression(objectExpression) || objectExpression.getType().isDerivedFromGroovyObject() && !(objectExpression instanceof ClassExpression);
     }
 
     public void visitFieldExpression(FieldExpression expression) {
@@ -2200,24 +2204,9 @@ public class AsmClassGenerator extends ClassGenerator {
         controller.getOperandStack().push(cle.getType());
     }
 
+    @Deprecated
     public static boolean isThisExpression(Expression expression) {
-        if (expression instanceof VariableExpression) {
-            VariableExpression varExp = (VariableExpression) expression;
-            return varExp.getName().equals("this");
-        }
-        return false;
-    }
-
-    public static boolean isSuperExpression(Expression expression) {
-        if (expression instanceof VariableExpression) {
-            VariableExpression varExp = (VariableExpression) expression;
-            return varExp.getName().equals("super");
-        }
-        return false;
-    }
-
-    private static boolean isThisOrSuper(Expression expression) {
-        return isThisExpression(expression) || isSuperExpression(expression);
+        return ExpressionUtils.isThisExpression(expression);
     }
 
     public void onLineNumber(ASTNode statement, String message) {
diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/InvocationWriter.java b/src/main/java/org/codehaus/groovy/classgen/asm/InvocationWriter.java
index 1e7741e..b676bf1 100644
--- a/src/main/java/org/codehaus/groovy/classgen/asm/InvocationWriter.java
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/InvocationWriter.java
@@ -57,6 +57,9 @@ import java.util.LinkedList;
 import java.util.List;
 import java.util.TreeMap;
 
+import static org.apache.groovy.ast.tools.ExpressionUtils.isNullConstant;
+import static org.apache.groovy.ast.tools.ExpressionUtils.isSuperExpression;
+import static org.apache.groovy.ast.tools.ExpressionUtils.isThisExpression;
 import static org.objectweb.asm.Opcodes.AALOAD;
 import static org.objectweb.asm.Opcodes.ACONST_NULL;
 import static org.objectweb.asm.Opcodes.ALOAD;
@@ -133,11 +136,11 @@ public class InvocationWriter {
             boolean safe, boolean spreadSafe, boolean implicitThis
     ) {
         ClassNode sender = controller.getClassNode();
-        if (AsmClassGenerator.isSuperExpression(receiver) || (AsmClassGenerator.isThisExpression(receiver) && !implicitThis)) {
+        if (isSuperExpression(receiver) || (isThisExpression(receiver) && !implicitThis)) {
             while (sender.getOuterClass() != null && sender.getSuperClass() == ClassHelper.CLOSURE_TYPE) {
                 sender = sender.getOuterClass();
             }
-            if (AsmClassGenerator.isSuperExpression(receiver)) {
+            if (isSuperExpression(receiver)) {
                 sender = sender.getSuperClass(); // GROOVY-4035
                 implicitThis = false; // prevent recursion
                 safe = false; // GROOVY-6045
@@ -500,7 +503,7 @@ public class InvocationWriter {
                 // but keeping the flag would trigger a VerifyError (see GROOVY-6045)
                 call.setSafe(false);
             }
-            if (AsmClassGenerator.isThisExpression(call.getObjectExpression())) adapter = invokeMethodOnCurrent;
+            if (isThisExpression(call.getObjectExpression())) adapter = invokeMethodOnCurrent;
             if (isSuperMethodCall) adapter = invokeMethodOnSuper;
             if (isStaticInvocation(call)) adapter = invokeStaticMethod;
             makeInvokeMethodCall(call, isSuperMethodCall, adapter);
@@ -516,7 +519,7 @@ public class InvocationWriter {
         String methodName = call.getMethodAsString();
         if (methodName==null) return false;
         if (!call.isImplicitThis()) return false;
-        if (!AsmClassGenerator.isThisExpression(call.getObjectExpression())) return false;
+        if (!isThisExpression(call.getObjectExpression())) return false;
         FieldNode field = classNode.getDeclaredField(methodName);
         if (field == null) return false;
         if (isStaticInvocation(call) && !field.isStatic()) return false;
@@ -538,7 +541,7 @@ public class InvocationWriter {
     }
 
     private boolean isStaticInvocation(MethodCallExpression call) {
-        if (!AsmClassGenerator.isThisExpression(call.getObjectExpression())) return false;
+        if (!isThisExpression(call.getObjectExpression())) return false;
         if (controller.isStaticMethod()) return true;
         return controller.isStaticContext() && !call.isImplicitThis();
     }
@@ -720,7 +723,7 @@ public class InvocationWriter {
         for (int i=0; i<params.length; i++) {
             Expression expression = argumentList.get(i);
             expression.visit(controller.getAcg());
-            if (!AsmClassGenerator.isNullConstant(expression)) {
+            if (!isNullConstant(expression)) {
                 operandStack.doGroovyCast(params[i].getType());
             }
             operandStack.remove(1);
diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/OptimizingStatementWriter.java b/src/main/java/org/codehaus/groovy/classgen/asm/OptimizingStatementWriter.java
index fb07f94..c3f62a5 100644
--- a/src/main/java/org/codehaus/groovy/classgen/asm/OptimizingStatementWriter.java
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/OptimizingStatementWriter.java
@@ -63,6 +63,7 @@ import org.objectweb.asm.MethodVisitor;
 import java.util.LinkedList;
 import java.util.List;
 
+import static org.apache.groovy.ast.tools.ExpressionUtils.isThisExpression;
 import static org.codehaus.groovy.ast.ClassHelper.BigDecimal_TYPE;
 import static org.codehaus.groovy.ast.ClassHelper.GROOVY_INTERCEPTABLE_TYPE;
 import static org.codehaus.groovy.ast.ClassHelper.OBJECT_TYPE;
@@ -847,7 +848,7 @@ public class OptimizingStatementWriter extends StatementWriter {
             super.visitMethodCallExpression(expression);
 
             Expression object = expression.getObjectExpression();
-            boolean setTarget = AsmClassGenerator.isThisExpression(object);
+            boolean setTarget = isThisExpression(object);
             if (!setTarget) {
                 if (!(object instanceof ClassExpression)) return;
                 setTarget = object.equals(node);