groovy-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From emil...@apache.org
Subject [groovy] branch master updated: minor refactor
Date Fri, 04 Oct 2019 19:33:23 GMT
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


The following commit(s) were added to refs/heads/master by this push:
     new 0bc09ca  minor refactor
0bc09ca is described below

commit 0bc09cab89f84b7923af68e006317dec86ba8d3e
Author: Eric Milles <eric.milles@thomsonreuters.com>
AuthorDate: Fri Oct 4 14:21:49 2019 -0500

    minor refactor
---
 .../codehaus/groovy/ast/tools/ParameterUtils.java  | 32 ++++----
 .../codehaus/groovy/classgen/asm/MopWriter.java    | 87 ++++++++--------------
 .../InheritConstructorsASTTransformation.java      |  4 +-
 .../transformers/StaticCompilationTransformer.java | 38 ++++------
 4 files changed, 63 insertions(+), 98 deletions(-)

diff --git a/src/main/java/org/codehaus/groovy/ast/tools/ParameterUtils.java b/src/main/java/org/codehaus/groovy/ast/tools/ParameterUtils.java
index 79135ff..b3b7756 100644
--- a/src/main/java/org/codehaus/groovy/ast/tools/ParameterUtils.java
+++ b/src/main/java/org/codehaus/groovy/ast/tools/ParameterUtils.java
@@ -18,15 +18,14 @@
  */
 package org.codehaus.groovy.ast.tools;
 
-import groovy.lang.Tuple;
-import groovy.lang.Tuple2;
 import org.codehaus.groovy.ast.ClassHelper;
 import org.codehaus.groovy.ast.ClassNode;
 import org.codehaus.groovy.ast.Parameter;
 
-import java.util.function.Function;
+import java.util.function.BiPredicate;
 
 public class ParameterUtils {
+
     public static boolean parametersEqual(Parameter[] a, Parameter[] b) {
         return parametersEqual(a, b, false);
     }
@@ -36,7 +35,7 @@ public class ParameterUtils {
     }
 
     /**
-     * check whether parameters type are compatible
+     * Checks if two parameter arrays are type-compatible.
      *
      * each parameter should match the following condition:
      * {@code targetParameter.getType().getTypeClass().isAssignableFrom(sourceParameter.getType().getTypeClass())}
@@ -47,34 +46,29 @@ public class ParameterUtils {
      * @since 3.0.0
      */
     public static boolean parametersCompatible(Parameter[] source, Parameter[] target) {
-        return parametersMatch(source, target,
-                t -> ClassHelper.getWrapper(t.getV2()).getTypeClass()
-                        .isAssignableFrom(ClassHelper.getWrapper(t.getV1()).getTypeClass())
+        return parametersMatch(source, target, (sourceType, targetType) ->
+            ClassHelper.getWrapper(targetType).getTypeClass().isAssignableFrom(ClassHelper.getWrapper(sourceType).getTypeClass())
         );
     }
 
-    private static boolean parametersEqual(Parameter[] a, Parameter[] b, final boolean wrapType)
{
-        return parametersMatch(a, b, t -> {
-            ClassNode v1 = t.getV1();
-            ClassNode v2 = t.getV2();
-
+    private static boolean parametersEqual(Parameter[] a, Parameter[] b, boolean wrapType)
{
+        return parametersMatch(a, b, (aType, bType) -> {
             if (wrapType) {
-                v1 = ClassHelper.getWrapper(v1);
-                v2 = ClassHelper.getWrapper(v2);
+                aType = ClassHelper.getWrapper(aType);
+                bType = ClassHelper.getWrapper(bType);
             }
-
-            return v1.equals(v2);
+            return aType.equals(bType);
         });
     }
 
-    private static boolean parametersMatch(Parameter[] a, Parameter[] b, Function<Tuple2<ClassNode,
ClassNode>, Boolean> typeChecker) {
+    private static boolean parametersMatch(Parameter[] a, Parameter[] b, BiPredicate<ClassNode,
ClassNode> typeChecker) {
         if (a.length == b.length) {
             boolean answer = true;
-            for (int i = 0; i < a.length; i++) {
+            for (int i = 0, n = a.length; i < n; i += 1) {
                 ClassNode aType = a[i].getType();
                 ClassNode bType = b[i].getType();
 
-                if (!typeChecker.apply(Tuple.tuple(aType, bType))) {
+                if (!typeChecker.test(aType, bType)) {
                     answer = false;
                     break;
                 }
diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/MopWriter.java b/src/main/java/org/codehaus/groovy/classgen/asm/MopWriter.java
index 8d1c429..8ec0140 100644
--- a/src/main/java/org/codehaus/groovy/classgen/asm/MopWriter.java
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/MopWriter.java
@@ -22,16 +22,18 @@ import org.codehaus.groovy.ast.ClassHelper;
 import org.codehaus.groovy.ast.ClassNode;
 import org.codehaus.groovy.ast.MethodNode;
 import org.codehaus.groovy.ast.Parameter;
+import org.codehaus.groovy.ast.tools.ParameterUtils;
 import org.objectweb.asm.MethodVisitor;
 
 import java.lang.reflect.Modifier;
 import java.util.Collections;
 import java.util.HashMap;
-import java.util.HashSet;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Set;
+import java.util.stream.Collectors;
 
 import static org.objectweb.asm.Opcodes.ACC_ABSTRACT;
 import static org.objectweb.asm.Opcodes.ACC_BRIDGE;
@@ -42,16 +44,12 @@ import static org.objectweb.asm.Opcodes.INVOKEINTERFACE;
 import static org.objectweb.asm.Opcodes.INVOKESPECIAL;
 
 public class MopWriter {
+    @FunctionalInterface
     public interface Factory {
         MopWriter create(WriterController controller);
     }
 
-    public static final Factory FACTORY = new Factory() {
-        @Override
-        public MopWriter create(final WriterController controller) {
-            return new MopWriter(controller);
-        }
-    };
+    public static final Factory FACTORY = MopWriter::new;
 
     private static class MopKey {
         final int hash;
@@ -72,52 +70,45 @@ public class MopWriter {
             if (!(obj instanceof MopKey)) {
                 return false;
             }
-
             MopKey other = (MopKey) obj;
-            return other.name.equals(name) && equalParameterTypes(other.params,params);
+            return other.name.equals(name) && ParameterUtils.parametersEqual(other.params,
params);
         }
     }
-    
+
+    //--------------------------------------------------------------------------
+
     private final WriterController controller;
-    
-    public MopWriter(WriterController wc) {
-        controller = wc;
+
+    public MopWriter(WriterController controller) {
+        this.controller = Objects.requireNonNull(controller);
     }
-    
+
     public void createMopMethods() {
         ClassNode classNode = controller.getClassNode();
         if (classNode.declaresAnyInterfaces(ClassHelper.GENERATED_CLOSURE_Type, ClassHelper.GENERATED_LAMBDA_TYPE))
{
             return;
         }
-        Set<MopKey> currentClassSignatures = buildCurrentClassSignatureSet(classNode.getMethods());
-        visitMopMethodList(classNode.getMethods(), true, Collections.EMPTY_SET, Collections.EMPTY_LIST);
+        Set<MopKey> currentClassSignatures = classNode.getMethods().stream()
+                .map(mn -> new MopKey(mn.getName(), mn.getParameters())).collect(Collectors.toSet());
+        visitMopMethodList(classNode.getMethods(), true, Collections.emptySet(), Collections.emptyList());
         visitMopMethodList(classNode.getSuperClass().getAllDeclaredMethods(), false, currentClassSignatures,
controller.getSuperMethodNames());
     }
 
-    private static Set<MopKey> buildCurrentClassSignatureSet(List<MethodNode>
methods) {
-        if (methods.isEmpty()) return Collections.EMPTY_SET;
-        Set<MopKey> result = new HashSet<MopKey>(methods.size());
-        for (MethodNode mn : methods) {
-            MopKey key = new MopKey(mn.getName(), mn.getParameters());
-            result.add(key);
-        }
-        return result;
-    }
-    
     /**
-     * filters a list of method for MOP methods. For all methods that are no
+     * Filters a list of method for MOP methods. For all methods that are no
      * MOP methods a MOP method is created if the method is not public and the
      * call would be a call on "this" (isThis == true). If the call is not on
      * "this", then the call is a call on "super" and all methods are used,
-     * unless they are already a MOP method
+     * unless they are already a MOP method.
      *
      * @param methods unfiltered list of methods for MOP
      * @param isThis  if true, then we are creating a MOP method on "this", "super" else
+     *
      * @see #generateMopCalls(LinkedList, boolean)
      */
     private void visitMopMethodList(List<MethodNode> methods, boolean isThis, Set<MopKey>
useOnlyIfDeclaredHereToo, List<String> orNameMentionedHere) {
-        Map<MopKey, MethodNode> mops = new HashMap<MopKey, MethodNode>();
-        LinkedList<MethodNode> mopCalls = new LinkedList<MethodNode>();
+        Map<MopKey, MethodNode> mops = new HashMap<>();
+        LinkedList<MethodNode> mopCalls = new LinkedList<>();
         for (MethodNode mn : methods) {
             // mop methods are helper for this and super calls and do direct calls
             // to the target methods. Such a method cannot be abstract or a bridge
@@ -135,8 +126,7 @@ public class MopWriter {
             }
             if (methodName.startsWith("<")) continue;
             if (!useOnlyIfDeclaredHereToo.contains(new MopKey(methodName, mn.getParameters()))
&&
-                !orNameMentionedHere.contains(methodName))
-            {
+                    !orNameMentionedHere.contains(methodName)) {
                 continue;
             }
             String name = getMopMethodName(mn, isThis);
@@ -151,7 +141,7 @@ public class MopWriter {
     }
 
     /**
-     * creates a MOP method name from a method
+     * Creates a MOP method name from a method.
      *
      * @param method  the method to be called by the mop method
      * @param useThis if true, then it is a call on "this", "super" else
@@ -161,26 +151,25 @@ public class MopWriter {
         ClassNode declaringNode = method.getDeclaringClass();
         int distance = 0;
         for (; declaringNode != null; declaringNode = declaringNode.getSuperClass()) {
-            distance++;
+            distance += 1;
         }
         return (useThis ? "this" : "super") + "$" + distance + "$" + method.getName();
     }
 
     /**
-     * method to determine if a method is a MOP method. This is done by the
-     * method name. If the name starts with "this$" or "super$" but does not 
-     * contain "$dist$", then it is an MOP method
+     * Determines if a method is a MOP method. This is done by the method name.
+     * If the name starts with "this$" or "super$" but does not contain "$dist$",
+     * then it is an MOP method.
      *
      * @param methodName name of the method to test
      * @return true if the method is a MOP method
      */
     public static boolean isMopMethod(String methodName) {
-        return (methodName.startsWith("this$") ||
-                methodName.startsWith("super$")) && !methodName.contains("$dist$");
+        return (methodName.startsWith("this$") || methodName.startsWith("super$")) &&
!methodName.contains("$dist$");
     }
 
     /**
-     * generates a Meta Object Protocol method, that is used to call a non public
+     * Generates a Meta Object Protocol method, that is used to call a non public
      * method, or to make a call to super.
      *
      * @param mopCalls list of methods a mop call method should be generated for
@@ -199,15 +188,14 @@ public class MopWriter {
             for (Parameter parameter : parameters) {
                 ClassNode type = parameter.getType();
                 operandStack.load(parameter.getType(), newRegister);
-                // increment to next register, double/long are using two places
-                newRegister++;
-                if (type == ClassHelper.double_TYPE || type == ClassHelper.long_TYPE) newRegister++;
+                newRegister += 1; // increment to next register; double/long are using two
places
+                if (type == ClassHelper.double_TYPE || type == ClassHelper.long_TYPE) newRegister
+= 1;
             }
             operandStack.remove(parameters.length);
             ClassNode declaringClass = method.getDeclaringClass();
             // JDK 8 support for default methods in interfaces
-            // this should probably be strenghtened when we support the A.super.foo() syntax
-            int opcode = declaringClass.isInterface()?INVOKEINTERFACE:INVOKESPECIAL;
+            // TODO: this should probably be strenghtened when we support the A.super.foo()
syntax
+            int opcode = declaringClass.isInterface() ? INVOKEINTERFACE : INVOKESPECIAL;
             mv.visitMethodInsn(opcode, BytecodeHelper.getClassInternalName(declaringClass),
method.getName(), methodDescriptor, declaringClass.isInterface());
             BytecodeHelper.doReturn(mv, method.getReturnType());
             mv.visitMaxs(0, 0);
@@ -215,13 +203,4 @@ public class MopWriter {
             controller.getClassNode().addMethod(name, ACC_PUBLIC | ACC_SYNTHETIC, method.getReturnType(),
parameters, null, null);
         }
     }
-
-    public static boolean equalParameterTypes(Parameter[] p1, Parameter[] p2) {
-        if (p1.length != p2.length) return false;
-        for (int i = 0; i < p1.length; i++) {
-            if (!p1[i].getType().equals(p2[i].getType())) return false;
-        }
-        return true;
-    }
-
 }
diff --git a/src/main/java/org/codehaus/groovy/transform/InheritConstructorsASTTransformation.java
b/src/main/java/org/codehaus/groovy/transform/InheritConstructorsASTTransformation.java
index 42f95f9..0532d46 100644
--- a/src/main/java/org/codehaus/groovy/transform/InheritConstructorsASTTransformation.java
+++ b/src/main/java/org/codehaus/groovy/transform/InheritConstructorsASTTransformation.java
@@ -26,7 +26,7 @@ import org.codehaus.groovy.ast.ClassNode;
 import org.codehaus.groovy.ast.ConstructorNode;
 import org.codehaus.groovy.ast.Parameter;
 import org.codehaus.groovy.ast.expr.Expression;
-import org.codehaus.groovy.classgen.asm.MopWriter;
+import org.codehaus.groovy.ast.tools.ParameterUtils;
 import org.codehaus.groovy.control.CompilePhase;
 import org.codehaus.groovy.control.SourceUnit;
 
@@ -126,6 +126,6 @@ public class InheritConstructorsASTTransformation extends AbstractASTTransformat
     }
 
     private static boolean matchingTypes(Parameter[] params, Parameter[] existingParams)
{
-        return MopWriter.equalParameterTypes(params, existingParams);
+        return ParameterUtils.parametersEqual(params, existingParams);
     }
 }
diff --git a/src/main/java/org/codehaus/groovy/transform/sc/transformers/StaticCompilationTransformer.java
b/src/main/java/org/codehaus/groovy/transform/sc/transformers/StaticCompilationTransformer.java
index d4cd291..e2a5a79 100644
--- a/src/main/java/org/codehaus/groovy/transform/sc/transformers/StaticCompilationTransformer.java
+++ b/src/main/java/org/codehaus/groovy/transform/sc/transformers/StaticCompilationTransformer.java
@@ -18,6 +18,7 @@
  */
 package org.codehaus.groovy.transform.sc.transformers;
 
+import org.apache.groovy.util.Maps;
 import org.codehaus.groovy.ast.ClassCodeExpressionTransformer;
 import org.codehaus.groovy.ast.ClassHelper;
 import org.codehaus.groovy.ast.ClassNode;
@@ -41,8 +42,6 @@ import org.codehaus.groovy.runtime.ScriptBytecodeAdapter;
 import org.codehaus.groovy.syntax.Types;
 import org.codehaus.groovy.transform.stc.StaticTypeCheckingVisitor;
 
-import java.util.Collections;
-import java.util.HashMap;
 import java.util.Iterator;
 import java.util.Map;
 
@@ -54,20 +53,15 @@ import java.util.Map;
 public class StaticCompilationTransformer extends ClassCodeExpressionTransformer {
 
     protected static final ClassNode BYTECODE_ADAPTER_CLASS = ClassHelper.make(ScriptBytecodeAdapter.class);
-    protected static final Map<Integer, MethodNode> BYTECODE_BINARY_ADAPTERS = Collections.unmodifiableMap(new
HashMap<Integer, MethodNode>() {
-        private static final long serialVersionUID = -9117028399464862605L;
-
-        {
-        put(Types.COMPARE_EQUAL, BYTECODE_ADAPTER_CLASS.getMethods("compareEqual").get(0));
-        put(Types.COMPARE_GREATER_THAN, BYTECODE_ADAPTER_CLASS.getMethods("compareGreaterThan").get(0));
-        put(Types.COMPARE_GREATER_THAN_EQUAL, BYTECODE_ADAPTER_CLASS.getMethods("compareGreaterThanEqual").get(0));
-        put(Types.COMPARE_LESS_THAN, BYTECODE_ADAPTER_CLASS.getMethods("compareLessThan").get(0));
-        put(Types.COMPARE_LESS_THAN_EQUAL, BYTECODE_ADAPTER_CLASS.getMethods("compareLessThanEqual").get(0));
-        put(Types.COMPARE_NOT_EQUAL, BYTECODE_ADAPTER_CLASS.getMethods("compareNotEqual").get(0));
-        put(Types.COMPARE_TO, BYTECODE_ADAPTER_CLASS.getMethods("compareTo").get(0));
-
-    }});
-
+    protected static final Map<Integer, MethodNode> BYTECODE_BINARY_ADAPTERS = Maps.of(
+        Types.COMPARE_EQUAL, BYTECODE_ADAPTER_CLASS.getMethods("compareEqual").get(0),
+        Types.COMPARE_GREATER_THAN, BYTECODE_ADAPTER_CLASS.getMethods("compareGreaterThan").get(0),
+        Types.COMPARE_GREATER_THAN_EQUAL, BYTECODE_ADAPTER_CLASS.getMethods("compareGreaterThanEqual").get(0),
+        Types.COMPARE_LESS_THAN, BYTECODE_ADAPTER_CLASS.getMethods("compareLessThan").get(0),
+        Types.COMPARE_LESS_THAN_EQUAL, BYTECODE_ADAPTER_CLASS.getMethods("compareLessThanEqual").get(0),
+        Types.COMPARE_NOT_EQUAL, BYTECODE_ADAPTER_CLASS.getMethods("compareNotEqual").get(0),
+        Types.COMPARE_TO, BYTECODE_ADAPTER_CLASS.getMethods("compareTo").get(0)
+    );
 
     private ClassNode classNode;
     private final SourceUnit unit;
@@ -151,18 +145,16 @@ public class StaticCompilationTransformer extends ClassCodeExpressionTransformer
     final Expression superTransform(Expression expr) {
         return super.transform(expr);
     }
-    
+
     @Override
     public void visitClass(final ClassNode node) {
-        ClassNode prec = classNode;
+        ClassNode prev = classNode;
         classNode = node;
         super.visitClass(node);
-        Iterator<InnerClassNode> innerClasses = classNode.getInnerClasses();
-        while (innerClasses.hasNext()) {
-            InnerClassNode innerClassNode = innerClasses.next();
-            visitClass(innerClassNode);
+        for (Iterator<InnerClassNode> innerClasses = classNode.getInnerClasses(); innerClasses.hasNext();)
{
+            visitClass(innerClasses.next());
         }
-        classNode = prec;
+        classNode = prev;
     }
 
     @Override


Mime
View raw message