You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by su...@apache.org on 2020/06/13 09:47:32 UTC

[groovy] branch GROOVY_3_0_X updated (f2e9538 -> 9a5aabc)

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

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


    from f2e9538  GROOVY-9485: add comment to reduce risk of subsequent regression
     new e5f08d8  minor fix-ups
     new 18fb3ff  minor fix-ups
     new 485e2ab  refactor AST building
     new cf019af  remove redundant call
     new 570361e  reduce nesting
     new 9a5aabc  GROOVY-9586: add trait to owner list when getting for trait helper class

The 6 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:
 .../TimedInterruptibleASTTransformation.groovy     |  35 ++-
 .../classgen/asm/BinaryExpressionHelper.java       |  15 +-
 .../classgen/asm/sc/StaticInvocationWriter.java    | 290 ++++++++++-----------
 .../classgen/asm/sc/StaticTypesCallSiteWriter.java |   1 -
 .../transform/stc/StaticTypeCheckingVisitor.java   |  46 ++--
 .../groovy/classgen/asm/sc/bugs/Groovy7242.groovy  | 156 +++++++++++
 .../classgen/asm/sc/bugs/Groovy7242Bug.groovy      | 101 -------
 7 files changed, 347 insertions(+), 297 deletions(-)
 create mode 100644 src/test/org/codehaus/groovy/classgen/asm/sc/bugs/Groovy7242.groovy
 delete mode 100644 src/test/org/codehaus/groovy/classgen/asm/sc/bugs/Groovy7242Bug.groovy


[groovy] 03/06: refactor AST building

Posted by su...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit 485e2ab40fd3ccbe85c583021a5e958fc2a9b063
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Tue Jun 9 09:33:46 2020 -0500

    refactor AST building
    
    (cherry picked from commit d353db2a9cfdc75c574d3cabb7edcb6f0de350ba)
---
 .../classgen/asm/sc/StaticInvocationWriter.java    | 178 ++++++++++-----------
 1 file changed, 81 insertions(+), 97 deletions(-)

diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java
index 9537de9..72fbe89 100644
--- a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java
@@ -40,7 +40,6 @@ import org.codehaus.groovy.ast.expr.MethodCallExpression;
 import org.codehaus.groovy.ast.expr.PropertyExpression;
 import org.codehaus.groovy.ast.expr.TupleExpression;
 import org.codehaus.groovy.ast.expr.VariableExpression;
-import org.codehaus.groovy.ast.stmt.ExpressionStatement;
 import org.codehaus.groovy.ast.stmt.ForStatement;
 import org.codehaus.groovy.classgen.AsmClassGenerator;
 import org.codehaus.groovy.classgen.Verifier;
@@ -66,21 +65,25 @@ import org.codehaus.groovy.transform.stc.StaticTypesMarker;
 import org.objectweb.asm.Label;
 import org.objectweb.asm.MethodVisitor;
 
-import java.util.LinkedList;
+import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
 import java.util.concurrent.atomic.AtomicInteger;
 
 import static org.apache.groovy.ast.tools.ClassNodeUtils.samePackageName;
-import static org.codehaus.groovy.ast.ClassHelper.CLOSURE_TYPE;
-import static org.codehaus.groovy.ast.ClassHelper.OBJECT_TYPE;
-import static org.codehaus.groovy.ast.ClassHelper.getWrapper;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.args;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.callX;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.classX;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.constX;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.ctorX;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.nullX;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.propX;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.stmt;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.varX;
 import static org.codehaus.groovy.classgen.AsmClassGenerator.isNullConstant;
 import static org.codehaus.groovy.classgen.AsmClassGenerator.isSuperExpression;
 import static org.codehaus.groovy.classgen.AsmClassGenerator.isThisExpression;
-import static org.codehaus.groovy.transform.sc.StaticCompilationMetadataKeys.PRIVATE_BRIDGE_METHODS;
-import static org.codehaus.groovy.transform.stc.StaticTypesMarker.PARAMETER_TYPE;
+import static org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport.missesGenericsTypes;
 import static org.objectweb.asm.Opcodes.ACONST_NULL;
 import static org.objectweb.asm.Opcodes.ALOAD;
 import static org.objectweb.asm.Opcodes.CHECKCAST;
@@ -91,7 +94,6 @@ import static org.objectweb.asm.Opcodes.INVOKESTATIC;
 public class StaticInvocationWriter extends InvocationWriter {
 
     private static final ClassNode INVOKERHELPER_CLASSNODE = ClassHelper.make(InvokerHelper.class);
-    private static final Expression INVOKERHELPER_RECEIVER = new ClassExpression(INVOKERHELPER_CLASSNODE);
     private static final MethodNode INVOKERHELPER_INVOKEMETHOD = INVOKERHELPER_CLASSNODE.getMethod(
             "invokeMethodSafe",
             new Parameter[]{
@@ -100,7 +102,6 @@ public class StaticInvocationWriter extends InvocationWriter {
                     new Parameter(ClassHelper.OBJECT_TYPE, "args")
             }
     );
-
     private static final MethodNode INVOKERHELPER_INVOKESTATICMETHOD = INVOKERHELPER_CLASSNODE.getMethod(
             "invokeStaticMethod",
             new Parameter[]{
@@ -169,7 +170,7 @@ public class StaticInvocationWriter extends InvocationWriter {
                     bridge = bridgeMethods != null ? bridgeMethods.get(cn) : null;
                 }
                 if (bridge instanceof ConstructorNode) {
-                    ArgumentListExpression newArgs = new ArgumentListExpression(new ConstantExpression(null));
+                    ArgumentListExpression newArgs = args(nullX());
                     for (Expression arg: args) {
                         newArgs.addExpression(arg);
                     }
@@ -239,28 +240,28 @@ public class StaticInvocationWriter extends InvocationWriter {
         } else {
             lookupClassNode = target.getDeclaringClass().redirect();
         }
-        Map<MethodNode, MethodNode> bridges = lookupClassNode.getNodeMetaData(PRIVATE_BRIDGE_METHODS);
+        Map<MethodNode, MethodNode> bridges = lookupClassNode.getNodeMetaData(StaticCompilationMetadataKeys.PRIVATE_BRIDGE_METHODS);
         MethodNode bridge = bridges == null ? null : bridges.get(target);
         if (bridge != null) {
             Expression fixedReceiver = receiver;
             if (implicitThis) {
                 if (!controller.isInGeneratedFunction()) {
-                    fixedReceiver = new PropertyExpression(new ClassExpression(lookupClassNode), "this");
+                    fixedReceiver = propX(classX(lookupClassNode), "this");
                 } else if (thisClass != null) {
                     ClassNode current = thisClass.getOuterClass();
-                    fixedReceiver = new VariableExpression("thisObject", current);
+                    fixedReceiver = varX("thisObject", current);
                     // adjust for multiple levels of nesting if needed
-                    while (current instanceof InnerClassNode && !lookupClassNode.equals(current)) {
+                    while (current.getOuterClass() != null && !lookupClassNode.equals(current)) {
                         FieldNode thisField = current.getField("this$0");
                         current = current.getOuterClass();
                         if (thisField != null) {
-                            fixedReceiver = new PropertyExpression(fixedReceiver, "this$0");
+                            fixedReceiver = propX(fixedReceiver, "this$0");
                             fixedReceiver.setType(current);
                         }
                     }
                 }
             }
-            ArgumentListExpression newArgs = new ArgumentListExpression(target.isStatic() ? nullX() : fixedReceiver);
+            ArgumentListExpression newArgs = args(target.isStatic() ? nullX() : fixedReceiver);
             for (Expression expression : args.getExpressions()) {
                 newArgs.addExpression(expression);
             }
@@ -275,57 +276,54 @@ public class StaticInvocationWriter extends InvocationWriter {
 
         if (target instanceof ExtensionMethodNode) {
             ExtensionMethodNode emn = (ExtensionMethodNode) target;
-            MethodNode node = emn.getExtensionMethodNode();
-            String methodName = target.getName();
-
             MethodVisitor mv = controller.getMethodVisitor();
-            int argumentsToRemove = 0;
-            List<Expression> argumentList = new LinkedList<Expression>(args.getExpressions());
+            MethodNode node = emn.getExtensionMethodNode();
+            Parameter[] parameters = node.getParameters();
+            ClassNode returnType = node.getReturnType();
 
+            List<Expression> argumentList = new ArrayList<>();
             if (emn.isStaticExtension()) {
-                // it's a static extension method
-                argumentList.add(0, new ConstantExpression(null));
+                argumentList.add(nullX());
             } else {
-                ClassNode classNode = controller.getClassNode();
                 boolean isThisOrSuper = isThisExpression(receiver) || isSuperExpression(receiver);
+                ClassNode classNode = controller.getClassNode();
                 Expression fixedReceiver = null;
-                if (isThisOrSuper && classNode instanceof InnerClassNode && controller.isInGeneratedFunction()) {
+                if (isThisOrSuper && classNode.getOuterClass() != null && controller.isInGeneratedFunction()) {
                     ClassNode current = classNode.getOuterClass();
-                    fixedReceiver = new VariableExpression("thisObject", current);
+                    fixedReceiver = varX("thisObject", current);
                     // adjust for multiple levels of nesting if needed
-                    while (current instanceof InnerClassNode && !classNode.equals(current)) {
+                    while (current.getOuterClass() != null && !classNode.equals(current)) {
                         FieldNode thisField = current.getField("this$0");
                         current = current.getOuterClass();
                         if (thisField != null) {
-                            fixedReceiver = new PropertyExpression(fixedReceiver, "this$0");
+                            fixedReceiver = propX(fixedReceiver, "this$0");
                             fixedReceiver.setType(current);
                         }
                     }
                 }
-
-                argumentList.add(0, fixedReceiver != null ? fixedReceiver : receiver);
+                argumentList.add(fixedReceiver != null ? fixedReceiver : receiver);
             }
-
-            Parameter[] parameters = node.getParameters();
+            argumentList.addAll(args.getExpressions());
             loadArguments(argumentList, parameters);
 
             String owner = BytecodeHelper.getClassInternalName(node.getDeclaringClass());
-            String desc = BytecodeHelper.getMethodDescriptor(target.getReturnType(), parameters);
-            mv.visitMethodInsn(INVOKESTATIC, owner, methodName, desc, false);
-            ClassNode ret = target.getReturnType().redirect();
-            if (ret == ClassHelper.VOID_TYPE) {
-                ret = ClassHelper.OBJECT_TYPE;
+            String desc = BytecodeHelper.getMethodDescriptor(returnType, parameters);
+            mv.visitMethodInsn(INVOKESTATIC, owner, target.getName(), desc, false);
+            controller.getOperandStack().remove(argumentList.size());
+
+            if (ClassHelper.VOID_TYPE.equals(returnType)) {
+                returnType = ClassHelper.OBJECT_TYPE;
                 mv.visitInsn(ACONST_NULL);
+            } else if (missesGenericsTypes(returnType)) {
+                returnType = returnType.redirect();
             }
-            argumentsToRemove += argumentList.size();
-            controller.getOperandStack().remove(argumentsToRemove);
-            controller.getOperandStack().push(ret);
+            controller.getOperandStack().push(returnType);
             return true;
         } else {
             if (target == StaticTypeCheckingVisitor.CLOSURE_CALL_VARGS) {
                 // wrap arguments into an array
                 ArrayExpression arr = new ArrayExpression(ClassHelper.OBJECT_TYPE, args.getExpressions());
-                return super.writeDirectMethodCall(target, implicitThis, receiver, new ArgumentListExpression(arr));
+                return super.writeDirectMethodCall(target, implicitThis, receiver, args(arr));
             }
             ClassNode classNode = controller.getClassNode();
             if (classNode.isDerivedFrom(ClassHelper.CLOSURE_TYPE)
@@ -335,14 +333,12 @@ public class StaticInvocationWriter extends InvocationWriter {
                 if (!tryBridgeMethod(target, receiver, implicitThis, args, classNode)) {
                     // replace call with an invoker helper call
                     ArrayExpression arr = new ArrayExpression(ClassHelper.OBJECT_TYPE, args.getExpressions());
-                    MethodCallExpression mce = new MethodCallExpression(
-                            INVOKERHELPER_RECEIVER,
+                    MethodCallExpression mce = callX(
+                            classX(INVOKERHELPER_CLASSNODE),
                             target.isStatic() ? "invokeStaticMethod" : "invokeMethodSafe",
-                            new ArgumentListExpression(
-                                    target.isStatic() ?
-                                            new ClassExpression(target.getDeclaringClass()) :
-                                            receiver,
-                                    new ConstantExpression(target.getName()),
+                            args(
+                                    target.isStatic() ? classX(target.getDeclaringClass()) : receiver,
+                                    constX(target.getName()),
                                     arr
                             )
                     );
@@ -358,9 +354,7 @@ public class StaticInvocationWriter extends InvocationWriter {
                 if (tryPrivateMethod(target, implicitThis, receiver, args, classNode)) return true;
             } else if (target.isProtected()) {
                 ClassNode node = receiver == null ? ClassHelper.OBJECT_TYPE : controller.getTypeChooser().resolveType(receiver, controller.getClassNode());
-                boolean isThisOrSuper = isThisExpression(receiver) || isSuperExpression(receiver);
-                if (!implicitThis && !isThisOrSuper
-                        && !samePackageName(node, classNode)
+                if (!implicitThis && !isThisExpression(receiver) && !isSuperExpression(receiver) && !samePackageName(node, classNode)
                         && StaticTypeCheckingSupport.implementsInterfaceOrIsSubclassOf(node,target.getDeclaringClass())) {
                     ASTNode src = receiver == null ? args : receiver;
                     controller.getSourceUnit().addError(
@@ -373,30 +367,24 @@ public class StaticInvocationWriter extends InvocationWriter {
                 if (implicitThis
                         && !classNode.isDerivedFrom(target.getDeclaringClass())
                         && !classNode.implementsInterface(target.getDeclaringClass())
-                        && classNode instanceof InnerClassNode && controller.isInGeneratedFunction()) {
+                        && classNode.getOuterClass() != null && controller.isInGeneratedFunction()) {
                     ClassNode current = classNode.getOuterClass();
-                    fixedReceiver = new VariableExpression("thisObject", current);
+                    fixedReceiver = varX("thisObject", current);
                     // adjust for multiple levels of nesting if needed
-                    while (current instanceof InnerClassNode && !target.getDeclaringClass().equals(current)) {
+                    while (current.getOuterClass() != null && !target.getDeclaringClass().equals(current)) {
                         FieldNode thisField = current.getField("this$0");
                         current = current.getOuterClass();
                         if (thisField != null) {
-                            fixedReceiver = new PropertyExpression(fixedReceiver, "this$0");
+                            fixedReceiver = propX(fixedReceiver, "this$0");
                             fixedReceiver.setType(current);
                             fixedImplicitThis = false;
                         }
                     }
                 }
             }
-            if (receiver != null) {
-                boolean callToSuper = isSuperExpression(receiver);
-                if (!callToSuper) {
-                    fixedReceiver = fixedReceiver == null ? receiver : fixedReceiver;
-                    // in order to avoid calls to castToType, which is the dynamic behaviour, we make sure that we call CHECKCAST instead
-                    // then replace the top operand type
-                    Expression checkCastReceiver = new CheckcastReceiverExpression(fixedReceiver, target);
-                    return super.writeDirectMethodCall(target, fixedImplicitThis, checkCastReceiver, args);
-                }
+            if (receiver != null && !isSuperExpression(receiver)) {
+                // in order to avoid calls to castToType, which is the dynamic behaviour, we make sure that we call CHECKCAST instead then replace the top operand type
+                return super.writeDirectMethodCall(target, fixedImplicitThis, new CheckcastReceiverExpression(fixedReceiver != null ? fixedReceiver : receiver, target), args);
             }
             return super.writeDirectMethodCall(target, implicitThis, receiver, args);
         }
@@ -405,7 +393,7 @@ public class StaticInvocationWriter extends InvocationWriter {
     private boolean tryPrivateMethod(final MethodNode target, final boolean implicitThis, final Expression receiver, final TupleExpression args, final ClassNode classNode) {
         ClassNode declaringClass = target.getDeclaringClass();
         if ((isPrivateBridgeMethodsCallAllowed(declaringClass, classNode) || isPrivateBridgeMethodsCallAllowed(classNode, declaringClass))
-                && declaringClass.getNodeMetaData(PRIVATE_BRIDGE_METHODS) != null
+                && declaringClass.getNodeMetaData(StaticCompilationMetadataKeys.PRIVATE_BRIDGE_METHODS) != null
                 && !declaringClass.equals(classNode)) {
             if (tryBridgeMethod(target, receiver, implicitThis, args, classNode)) {
                 return true;
@@ -457,7 +445,7 @@ public class StaticInvocationWriter extends InvocationWriter {
                 visitArgument(argumentList.get(i), para[i].getType());
             }
             // last parameters wrapped in an array
-            List<Expression> lastParams = new LinkedList<>();
+            List<Expression> lastParams = new ArrayList<>();
             for (int i = para.length - 1; i < argumentListSize; i += 1) {
                 lastParams.add(argumentList.get(i));
             }
@@ -505,7 +493,7 @@ public class StaticInvocationWriter extends InvocationWriter {
     }
 
     private void visitArgument(final Expression argumentExpr, final ClassNode parameterType) {
-        argumentExpr.putNodeMetaData(PARAMETER_TYPE, parameterType);
+        argumentExpr.putNodeMetaData(StaticTypesMarker.PARAMETER_TYPE, parameterType);
         argumentExpr.visit(controller.getAcg());
         if (!isNullConstant(argumentExpr)) {
             controller.getOperandStack().doGroovyCast(parameterType);
@@ -533,7 +521,7 @@ public class StaticInvocationWriter extends InvocationWriter {
             dynamicInvocationWriter.makeCall(origin, receiver, message, arguments, adapter, safe, spreadSafe, implicitThis);
             return;
         }
-        if (tryImplicitReceiver(origin, message, arguments, adapter, safe, spreadSafe, implicitThis)) {
+        if (implicitThis && tryImplicitReceiver(origin, message, arguments, adapter, safe, spreadSafe)) {
             return;
         }
         // if call is spread safe, replace it with a for in loop
@@ -551,7 +539,7 @@ public class StaticInvocationWriter extends InvocationWriter {
             int counter = labelCounter.incrementAndGet();
 
             // use a temporary variable for the arraylist in which the results of the spread call will be stored
-            ConstructorCallExpression cce = new ConstructorCallExpression(StaticCompilationVisitor.ARRAYLIST_CLASSNODE, ArgumentListExpression.EMPTY_ARGUMENTS);
+            ConstructorCallExpression cce = ctorX(StaticCompilationVisitor.ARRAYLIST_CLASSNODE);
             cce.setNodeMetaData(StaticTypesMarker.DIRECT_METHOD_CALL_TARGET, StaticCompilationVisitor.ARRAYLIST_CONSTRUCTOR);
             TemporaryVariableExpression result = new TemporaryVariableExpression(cce);
             result.visit(controller.getAcg());
@@ -565,9 +553,9 @@ public class StaticInvocationWriter extends InvocationWriter {
             mv.visitLabel(nonull);
             ClassNode componentType = StaticTypeCheckingVisitor.inferLoopElementType(typeChooser.resolveType(tmpReceiver, classNode));
             Parameter iterator = new Parameter(componentType, "for$it$" + counter);
-            VariableExpression iteratorAsVar = new VariableExpression(iterator);
+            VariableExpression iteratorAsVar = varX(iterator);
             MethodCallExpression origMCE = (MethodCallExpression) origin;
-            MethodCallExpression newMCE = new MethodCallExpression(
+            MethodCallExpression newMCE = callX(
                     iteratorAsVar,
                     origMCE.getMethodAsString(),
                     origMCE.getArguments()
@@ -575,7 +563,7 @@ public class StaticInvocationWriter extends InvocationWriter {
             newMCE.setImplicitThis(false);
             newMCE.setMethodTarget(origMCE.getMethodTarget());
             newMCE.setSafe(true);
-            MethodCallExpression add = new MethodCallExpression(
+            MethodCallExpression add = callX(
                     result,
                     "add",
                     newMCE
@@ -586,7 +574,7 @@ public class StaticInvocationWriter extends InvocationWriter {
             ForStatement stmt = new ForStatement(
                     iterator,
                     tmpReceiver,
-                    new ExpressionStatement(add)
+                    stmt(add)
             );
             stmt.visit(controller.getAcg());
             // else { empty list }
@@ -617,7 +605,7 @@ public class StaticInvocationWriter extends InvocationWriter {
             Label nonull = compileStack.createLocalLabel("nonull_" + counter);
             mv.visitLabel(nonull);
             MethodCallExpression origMCE = (MethodCallExpression) origin;
-            MethodCallExpression newMCE = new MethodCallExpression(
+            MethodCallExpression newMCE = callX(
                     new VariableSlotLoader(slot.getType(), slot.getIndex(), controller.getOperandStack()),
                     origMCE.getMethodAsString(),
                     origMCE.getArguments()
@@ -654,35 +642,31 @@ public class StaticInvocationWriter extends InvocationWriter {
         }
     }
 
-    boolean tryImplicitReceiver(final Expression origin, final Expression message, final Expression arguments, final MethodCallerMultiAdapter adapter, final boolean safe, final boolean spreadSafe, final boolean implicitThis) {
+    private boolean tryImplicitReceiver(final Expression origin, final Expression message, final Expression arguments, final MethodCallerMultiAdapter adapter, final boolean safe, final boolean spreadSafe) {
         Object implicitReceiver = origin.getNodeMetaData(StaticTypesMarker.IMPLICIT_RECEIVER);
-        if (implicitThis && implicitReceiver == null && origin instanceof MethodCallExpression) {
+        if (implicitReceiver == null && origin instanceof MethodCallExpression) {
             implicitReceiver = ((MethodCallExpression) origin).getObjectExpression().getNodeMetaData(StaticTypesMarker.IMPLICIT_RECEIVER);
         }
-        if (implicitThis && implicitReceiver != null) {
-            String[] propertyPath = ((String) implicitReceiver).split("\\.");
+        if (implicitReceiver != null) {
+            String[] path = ((String) implicitReceiver).split("\\.");
             // GROOVY-6021
-            PropertyExpression pexp = new PropertyExpression(new VariableExpression("this", CLOSURE_TYPE), propertyPath[0]);
+            PropertyExpression pexp = propX(varX("this", ClassHelper.CLOSURE_TYPE), path[0]);
             pexp.setImplicitThis(true);
-            for (int i = 1, n = propertyPath.length; i < n; i += 1) {
-                pexp.putNodeMetaData(StaticTypesMarker.INFERRED_TYPE, CLOSURE_TYPE);
-                pexp = new PropertyExpression(pexp, propertyPath[i]);
+            for (int i = 1, n = path.length; i < n; i += 1) {
+                pexp.putNodeMetaData(StaticTypesMarker.INFERRED_TYPE, ClassHelper.CLOSURE_TYPE);
+                pexp = propX(pexp, path[i]);
             }
             pexp.putNodeMetaData(StaticTypesMarker.IMPLICIT_RECEIVER, implicitReceiver);
             origin.removeNodeMetaData(StaticTypesMarker.IMPLICIT_RECEIVER);
             if (origin instanceof PropertyExpression) {
-                PropertyExpression rewritten = new PropertyExpression(
-                        pexp,
-                        ((PropertyExpression) origin).getProperty(),
-                        ((PropertyExpression) origin).isSafe()
-                );
+                PropertyExpression rewritten = propX(pexp, ((PropertyExpression) origin).getProperty(), ((PropertyExpression) origin).isSafe());
                 rewritten.setSpreadSafe(((PropertyExpression) origin).isSpreadSafe());
-                rewritten.setImplicitThis(false);
                 rewritten.visit(controller.getAcg());
+
                 rewritten.putNodeMetaData(StaticTypesMarker.INFERRED_TYPE, origin.getNodeMetaData(StaticTypesMarker.INFERRED_TYPE));
-                return true;
+            } else {
+                makeCall(origin, pexp, message, arguments, adapter, safe, spreadSafe, false);
             }
-            makeCall(origin, pexp, message, arguments, adapter, safe, spreadSafe, false);
             return true;
         }
         return false;
@@ -721,9 +705,9 @@ public class StaticInvocationWriter extends InvocationWriter {
                     controller.getOperandStack().doGroovyCast(type);
                 }
                 if (StaticTypeCheckingSupport.implementsInterfaceOrIsSubclassOf(topOperand, type)) return;
-                controller.getMethodVisitor().visitTypeInsn(CHECKCAST, type.isArray() ?
-                        BytecodeHelper.getTypeDescription(type) :
-                        BytecodeHelper.getClassInternalName(type.getName()));
+                controller.getMethodVisitor().visitTypeInsn(CHECKCAST, type.isArray()
+                        ? BytecodeHelper.getTypeDescription(type)
+                        : BytecodeHelper.getClassInternalName(type.getName()));
                 controller.getOperandStack().replace(type);
             }
         }
@@ -737,22 +721,22 @@ public class StaticInvocationWriter extends InvocationWriter {
             if (target instanceof ExtensionMethodNode) {
                 type = ((ExtensionMethodNode) target).getExtensionMethodNode().getDeclaringClass();
             } else {
-                type = getWrapper(controller.getTypeChooser().resolveType(receiver, controller.getClassNode()));
+                type = ClassHelper.getWrapper(controller.getTypeChooser().resolveType(receiver, controller.getClassNode()));
                 ClassNode declaringClass = target.getDeclaringClass();
                 if (type.getClass() != ClassNode.class
                         && type.getClass() != InnerClassNode.class
                         && type.getClass() != DecompiledClassNode.class) {
                     type = declaringClass; // ex: LUB type
                 }
-                if (OBJECT_TYPE.equals(type) && !OBJECT_TYPE.equals(declaringClass)) {
+                if (ClassHelper.OBJECT_TYPE.equals(type) && !ClassHelper.OBJECT_TYPE.equals(declaringClass)) {
                     // can happen for compiler rewritten code, where type information is missing
                     type = declaringClass;
                 }
-                if (OBJECT_TYPE.equals(declaringClass)) {
+                if (ClassHelper.OBJECT_TYPE.equals(declaringClass)) {
                     // check cast not necessary because Object never evolves
                     // and it prevents a potential ClassCastException if the delegate of a closure
                     // is changed in a statically compiled closure
-                    type = OBJECT_TYPE;
+                    type = ClassHelper.OBJECT_TYPE;
                 }
             }
             resolvedType = type;


[groovy] 06/06: GROOVY-9586: add trait to owner list when getting for trait helper class

Posted by su...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit 9a5aabc016f5ecb4b0d5691da6280c6efea3349b
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Thu Jun 11 14:06:26 2020 -0500

    GROOVY-9586: add trait to owner list when getting for trait helper class
    
    (cherry picked from commit 67d86419e0f6c3a9c8d18fe94b1326e999199586)
---
 .../transform/stc/StaticTypeCheckingVisitor.java   |  46 +++---
 .../groovy/classgen/asm/sc/bugs/Groovy7242.groovy  | 156 +++++++++++++++++++++
 .../classgen/asm/sc/bugs/Groovy7242Bug.groovy      | 101 -------------
 3 files changed, 184 insertions(+), 119 deletions(-)

diff --git a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
index a0b10eb..7945eaf 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -1449,7 +1449,7 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
         boolean foundGetterOrSetter = false;
         String capName = capitalize(propertyName);
         Set<ClassNode> handledNodes = new HashSet<>();
-        List<Receiver<String>> receivers = new LinkedList<>();
+        List<Receiver<String>> receivers = new ArrayList<>();
         addReceivers(receivers, makeOwnerList(objectExpression), pexp.isImplicitThis());
 
         for (Receiver<String> receiver : receivers) {
@@ -3204,9 +3204,8 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
     }
 
     private static void addDelegateReceiver(final List<Receiver<String>> receivers, final ClassNode delegate, final String path) {
-        receivers.add(new Receiver<>(delegate, path));
-        if (Traits.isTrait(delegate.getOuterClass())) {
-            receivers.add(new Receiver<>(delegate.getOuterClass(), path));
+        if (receivers.stream().map(Receiver::getType).noneMatch(delegate::equals)) {
+            receivers.add(new Receiver<>(delegate, path));
         }
     }
 
@@ -3335,9 +3334,9 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
                 //   - the actual receiver as found in the method call expression
                 //   - any of the potential receivers found in the instanceof temporary table
                 // in that order
-                List<Receiver<String>> receivers = new LinkedList<>();
-                List<Receiver<String>> owners = makeOwnerList(objectExpression);
-                addReceivers(receivers, owners, call.isImplicitThis());
+                List<Receiver<String>> receivers = new ArrayList<>();
+                addReceivers(receivers, makeOwnerList(objectExpression), call.isImplicitThis());
+
                 List<MethodNode> mn = null;
                 Receiver<String> chosenReceiver = null;
                 for (Receiver<String> currentReceiver : receivers) {
@@ -3644,19 +3643,23 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
      */
     protected List<Receiver<String>> makeOwnerList(final Expression objectExpression) {
         ClassNode receiver = getType(objectExpression);
-        List<Receiver<String>> owners = new LinkedList<>();
-        owners.add(Receiver.make(receiver));
+        List<Receiver<String>> owners = new ArrayList<>();
         if (isClassClassNodeWrappingConcreteType(receiver)) {
-            GenericsType clazzGT = receiver.getGenericsTypes()[0];
-            owners.add(0, Receiver.make(clazzGT.getType()));
-        }
-        if (receiver.isInterface()) {
-            owners.add(Receiver.make(OBJECT_TYPE));
+            ClassNode staticType = receiver.getGenericsTypes()[0].getType();
+            owners.add(Receiver.make(staticType)); // Type from Class<Type>
+            addTraitType(staticType, owners); // T in Class<T$Trait$Helper>
+            owners.add(Receiver.make(receiver)); // Class<Type>
+        } else {
+            owners.add(Receiver.make(receiver));
+            if (receiver.isInterface()) {
+                owners.add(Receiver.make(OBJECT_TYPE));
+            }
+            addSelfTypes(receiver, owners);
+            addTraitType(receiver, owners);
         }
-        addSelfTypes(receiver, owners);
         if (!typeCheckingContext.temporaryIfBranchTypeInformation.isEmpty()) {
             List<ClassNode> potentialReceiverType = getTemporaryTypesForExpression(objectExpression);
-            if (potentialReceiverType != null) {
+            if (potentialReceiverType != null && !potentialReceiverType.isEmpty()) {
                 for (ClassNode node : potentialReceiverType) {
                     owners.add(Receiver.make(node));
                 }
@@ -3680,12 +3683,19 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
     }
 
     private static void addSelfTypes(final ClassNode receiver, final List<Receiver<String>> owners) {
-        LinkedHashSet<ClassNode> selfTypes = new LinkedHashSet<>();
-        for (ClassNode selfType : Traits.collectSelfTypes(receiver, selfTypes)) {
+        for (ClassNode selfType : Traits.collectSelfTypes(receiver, new LinkedHashSet<>())) {
             owners.add(Receiver.make(selfType));
         }
     }
 
+    private static void addTraitType(final ClassNode receiver, final List<Receiver<String>> owners) {
+        if (Traits.isTrait(receiver.getOuterClass()) && receiver.getName().endsWith("$Helper")) {
+            ClassNode traitType = receiver.getOuterClass();
+            owners.add(Receiver.make(traitType));
+            addSelfTypes(traitType, owners);
+        }
+    }
+
     protected void checkForbiddenSpreadArgument(final ArgumentListExpression argumentList) {
         for (Expression arg : argumentList.getExpressions()) {
             if (arg instanceof SpreadExpression) {
diff --git a/src/test/org/codehaus/groovy/classgen/asm/sc/bugs/Groovy7242.groovy b/src/test/org/codehaus/groovy/classgen/asm/sc/bugs/Groovy7242.groovy
new file mode 100644
index 0000000..6884ea6
--- /dev/null
+++ b/src/test/org/codehaus/groovy/classgen/asm/sc/bugs/Groovy7242.groovy
@@ -0,0 +1,156 @@
+/*
+ *  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 org.codehaus.groovy.classgen.asm.sc.bugs
+
+import groovy.transform.stc.StaticTypeCheckingTestCase
+import org.codehaus.groovy.classgen.asm.sc.StaticCompilationTestSupport
+
+final class Groovy7242 extends StaticTypeCheckingTestCase implements StaticCompilationTestSupport {
+
+    void testWriteTraitPropertyFromTraitClosure() {
+        assertScript '''
+            trait T {
+                void p() {
+                    [1].each { x = it }
+                }
+                int x
+            }
+
+            class C implements T {
+            }
+
+            def c = new C()
+            c.p()
+            assert c.x == 1
+        '''
+    }
+
+    void testCallTraitMethodFromTraitClosure() {
+        assertScript '''
+            trait T {
+                def f() {
+                    ['a'].collect { String s -> g(s) }
+                }
+
+                String g(String s) {
+                    s.toUpperCase()
+                }
+            }
+
+            class C implements T {
+            }
+
+            def c = new C()
+            assert c.f() == ['A']
+        '''
+    }
+
+    void testCallTraitMethodFromTraitClosure_ImplicitParameter() {
+        assertScript '''
+            trait T {
+                def f() {
+                    ['a'].collect { g(it) }
+                }
+
+                String g(String s) {
+                    s.toUpperCase()
+                }
+            }
+
+            class C implements T {
+            }
+
+            def c = new C()
+            assert c.f() == ['A']
+        '''
+    }
+
+    // GROOVY-7456
+    void testCallPrivateTraitMethodFromTraitClosure() {
+        assertScript '''
+            trait T {
+                def f() {
+                    ['a'].collect { String s -> g(s) }
+                }
+
+                private String g(String s) {
+                    s.toUpperCase()
+                }
+            }
+
+            class C implements T {
+            }
+
+            def c = new C()
+            assert c.f() == ['A']
+        '''
+    }
+
+    // GROOVY-9586
+    void testDelegateVsOwnerMethodFromTraitClosure1() {
+        assertScript '''
+            class C {
+                def m(@DelegatesTo(strategy=Closure.DELEGATE_ONLY, value=C) Closure<?> block) {
+                    block.setResolveStrategy(Closure.OWNER_ONLY)
+                    block.setDelegate(this)
+                    return block.call()
+                }
+                def x() { 'C' }
+            }
+
+            trait T {
+                def test() {
+                    new C().m { -> x() } // "x" must come from delegate
+                }
+                def x() { 'T' }
+            }
+
+            class U implements T {
+            }
+
+            assert new U().test() == 'C'
+        '''
+    }
+
+    // GROOVY-9586
+    void testDelegateVsOwnerMethodFromTraitClosure2() {
+        assertScript '''
+            class C {
+                def m(@DelegatesTo(strategy=Closure.OWNER_ONLY, type='Void') Closure<?> block) {
+                    block.setResolveStrategy(Closure.OWNER_ONLY)
+                    block.setDelegate(null)
+                    return block.call()
+                }
+                def x() { 'C' }
+            }
+
+            trait T {
+                def test() {
+                    new C().m { -> x() } // "x" must come from owner
+                }
+                def x() { 'T' }
+            }
+
+            class U implements T {
+            }
+
+            assert new U().test() == 'T'
+        '''
+    }
+}
diff --git a/src/test/org/codehaus/groovy/classgen/asm/sc/bugs/Groovy7242Bug.groovy b/src/test/org/codehaus/groovy/classgen/asm/sc/bugs/Groovy7242Bug.groovy
deleted file mode 100644
index 17e6d62..0000000
--- a/src/test/org/codehaus/groovy/classgen/asm/sc/bugs/Groovy7242Bug.groovy
+++ /dev/null
@@ -1,101 +0,0 @@
-/*
- *  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 org.codehaus.groovy.classgen.asm.sc.bugs
-
-import groovy.transform.stc.StaticTypeCheckingTestCase
-import org.codehaus.groovy.classgen.asm.sc.StaticCompilationTestSupport
-
-class Groovy7242Bug extends StaticTypeCheckingTestCase implements StaticCompilationTestSupport {
-    void testCallMethodOfTraitInsideClosure() {
-        assertScript '''
-            trait MyTrait {
-                def f() {
-                    ['a'].collect {String it -> f2(it)}
-                }
-
-                def f2(String s) {
-                    s.toUpperCase()
-                }
-            }
-
-            class A implements MyTrait {}
-            def a = new A()
-            assert a.f() == ['A']
-        '''
-    }
-
-    void testCallMethodOfTraitInsideClosureAndClosureParamTypeInference() {
-        assertScript '''
-            trait MyTrait {
-                def f() {
-                    ['a'].collect {f2(it)}
-                }
-
-                def f2(String s) {
-                    s.toUpperCase()
-                }
-            }
-
-            class A implements MyTrait {}
-            def a = new A()
-            assert a.f() == ['A']
-        '''
-    }
-
-    void testAccessTraitPropertyFromClosureInTrait() {
-        assertScript '''
-            trait MyTrait {
-                int x
-                def f() {
-                    [1].each { x = it }
-                }
-            }
-            class A implements MyTrait {}
-            def a = new A()
-            a.f()
-            assert a.x == 1
-        '''
-    }
-
-    void testCallPrivateMethodOfTraitInsideClosure() {
-        assertScript '''
-            trait MyTrait {
-                def f() {
-                    ['a'].collect {String it -> f2(it)}
-                }
-
-                private f2(String s) {
-                    s.toUpperCase()
-                }
-            }
-
-            class A implements MyTrait {}
-            def a = new A()
-            assert a.f() == ['A']
-        '''
-    }
-}


[groovy] 01/06: minor fix-ups

Posted by su...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit e5f08d8abecfb41096b8bc02a967bcca72718b44
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Tue Jun 2 12:16:59 2020 -0500

    minor fix-ups
    
    (cherry picked from commit 49ee146850d866513aa84bc49bf22e06687484d5)
---
 .../TimedInterruptibleASTTransformation.groovy     | 35 +++++++++++-----------
 1 file changed, 17 insertions(+), 18 deletions(-)

diff --git a/src/main/groovy/org/codehaus/groovy/transform/TimedInterruptibleASTTransformation.groovy b/src/main/groovy/org/codehaus/groovy/transform/TimedInterruptibleASTTransformation.groovy
index 02b7328..8b64587 100644
--- a/src/main/groovy/org/codehaus/groovy/transform/TimedInterruptibleASTTransformation.groovy
+++ b/src/main/groovy/org/codehaus/groovy/transform/TimedInterruptibleASTTransformation.groovy
@@ -18,6 +18,7 @@
  */
 package org.codehaus.groovy.transform
 
+import groovy.transform.AutoFinal
 import groovy.transform.TimedInterrupt
 import org.codehaus.groovy.ast.ASTNode
 import org.codehaus.groovy.ast.AnnotatedNode
@@ -35,6 +36,7 @@ import org.codehaus.groovy.ast.expr.Expression
 import org.codehaus.groovy.ast.stmt.BlockStatement
 import org.codehaus.groovy.ast.stmt.DoWhileStatement
 import org.codehaus.groovy.ast.stmt.ForStatement
+import org.codehaus.groovy.ast.stmt.Statement
 import org.codehaus.groovy.ast.stmt.WhileStatement
 import org.codehaus.groovy.control.CompilePhase
 import org.codehaus.groovy.control.SourceUnit
@@ -44,6 +46,7 @@ import java.util.concurrent.TimeoutException
 
 import static org.codehaus.groovy.ast.ClassHelper.make
 import static org.codehaus.groovy.ast.tools.GeneralUtils.args
+import static org.codehaus.groovy.ast.tools.GeneralUtils.block
 import static org.codehaus.groovy.ast.tools.GeneralUtils.callX
 import static org.codehaus.groovy.ast.tools.GeneralUtils.classX
 import static org.codehaus.groovy.ast.tools.GeneralUtils.constX
@@ -63,7 +66,7 @@ import static org.codehaus.groovy.ast.tools.GeneralUtils.varX
  * @see groovy.transform.ThreadInterrupt
  * @since 1.8.0
  */
-@GroovyASTTransformation(phase = CompilePhase.CANONICALIZATION)
+@AutoFinal @GroovyASTTransformation(phase = CompilePhase.CANONICALIZATION)
 class TimedInterruptibleASTTransformation extends AbstractASTTransformation {
 
     private static final ClassNode MY_TYPE = make(TimedInterrupt)
@@ -150,7 +153,7 @@ class TimedInterruptibleASTTransformation extends AbstractASTTransformation {
     }
 
     private static class TimedInterruptionVisitor extends ClassCodeVisitorSupport {
-        final private SourceUnit source
+        final SourceUnit sourceUnit
         final private boolean checkOnMethodStart
         final private boolean applyToAllClasses
         final private boolean applyToAllMembers
@@ -162,8 +165,8 @@ class TimedInterruptibleASTTransformation extends AbstractASTTransformation {
         private final String basename
 
         @SuppressWarnings('ParameterCount')
-        TimedInterruptionVisitor(source, checkOnMethodStart, applyToAllClasses, applyToAllMembers, maximum, unit, thrown, hash) {
-            this.source = source
+        TimedInterruptionVisitor(SourceUnit source, checkOnMethodStart, applyToAllClasses, applyToAllMembers, maximum, unit, thrown, hash) {
+            this.sourceUnit = source
             this.checkOnMethodStart = checkOnMethodStart
             this.applyToAllClasses = applyToAllClasses
             this.applyToAllMembers = applyToAllMembers
@@ -176,9 +179,8 @@ class TimedInterruptibleASTTransformation extends AbstractASTTransformation {
         /**
          * @return Returns the interruption check statement.
          */
-        final createInterruptStatement() {
+        private Statement createInterruptStatement() {
             ifS(
-
                     ltX(
                             propX(varX('this'), basename + '$expireTime'),
                             callX(make(System), 'nanoTime')
@@ -210,10 +212,10 @@ class TimedInterruptibleASTTransformation extends AbstractASTTransformation {
          * second one the statement to be wrapped.
          */
         private wrapBlock(statement) {
-            def stmt = new BlockStatement()
-            stmt.addStatement(createInterruptStatement())
-            stmt.addStatement(statement)
-            stmt
+            block().tap {
+                addStatement(createInterruptStatement())
+                addStatement(statement)
+            }
         }
 
         @Override
@@ -234,10 +236,11 @@ class TimedInterruptibleASTTransformation extends AbstractASTTransformation {
                     )
             )
             expireTimeField.synthetic = true
+            ClassNode dateClass = make(Date)
             startTimeField = node.addField(basename + '$startTime',
                     ACC_FINAL | ACC_PRIVATE,
-                    make(Date),
-                    ctorX(make(Date))
+                    dateClass,
+                    ctorX(dateClass)
             )
             startTimeField.synthetic = true
 
@@ -293,13 +296,13 @@ class TimedInterruptibleASTTransformation extends AbstractASTTransformation {
         }
 
         @Override
-        void visitDoWhileLoop(final DoWhileStatement doWhileStatement) {
+        void visitDoWhileLoop(DoWhileStatement doWhileStatement) {
             visitLoop(doWhileStatement)
             super.visitDoWhileLoop(doWhileStatement)
         }
 
         @Override
-        void visitWhileLoop(final WhileStatement whileStatement) {
+        void visitWhileLoop(WhileStatement whileStatement) {
             visitLoop(whileStatement)
             super.visitWhileLoop(whileStatement)
         }
@@ -314,9 +317,5 @@ class TimedInterruptibleASTTransformation extends AbstractASTTransformation {
                 super.visitMethod(node)
             }
         }
-
-        protected SourceUnit getSourceUnit() {
-            source
-        }
     }
 }


[groovy] 04/06: remove redundant call

Posted by su...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit cf019af5c11bcf81a2e6408ac52220dbc86d0913
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Tue Jun 9 10:06:57 2020 -0500

    remove redundant call
    
    (cherry picked from commit d9694f927e9aacebbdef6adb7b289e0350368641)
---
 .../org/codehaus/groovy/classgen/asm/sc/StaticTypesCallSiteWriter.java   | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesCallSiteWriter.java b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesCallSiteWriter.java
index 2eebbea..9c4b041 100644
--- a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesCallSiteWriter.java
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesCallSiteWriter.java
@@ -715,7 +715,6 @@ public class StaticTypesCallSiteWriter extends CallSiteWriter implements Opcodes
                     rType = rType.getPlainNodeReference();
                     nodes = findDGMMethodsByNameAndArguments(controller.getSourceUnit().getClassLoader(), rType, message, args);
                 }
-                nodes = chooseBestMethod(rType, nodes, args);
                 if (nodes.size() == 1 || (nodes.size() > 1 && acceptAnyMethod)) {
                     MethodCallExpression call = callX(receiver, message, arguments);
                     call.setImplicitThis(false);


[groovy] 02/06: minor fix-ups

Posted by su...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit 18fb3ff1eb31dfe1ba4fe885b1587a2f41ba0164
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Fri Jun 5 10:04:21 2020 -0500

    minor fix-ups
    
    (cherry picked from commit 57a4887028304dc088de1063fb59d32c75c4f7cc)
---
 .../groovy/classgen/asm/BinaryExpressionHelper.java       | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/BinaryExpressionHelper.java b/src/main/java/org/codehaus/groovy/classgen/asm/BinaryExpressionHelper.java
index 60967e2..261fc84 100644
--- a/src/main/java/org/codehaus/groovy/classgen/asm/BinaryExpressionHelper.java
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/BinaryExpressionHelper.java
@@ -21,6 +21,7 @@ package org.codehaus.groovy.classgen.asm;
 import org.codehaus.groovy.GroovyBugError;
 import org.codehaus.groovy.ast.ClassHelper;
 import org.codehaus.groovy.ast.ClassNode;
+import org.codehaus.groovy.ast.Variable;
 import org.codehaus.groovy.ast.expr.ArgumentListExpression;
 import org.codehaus.groovy.ast.expr.ArrayExpression;
 import org.codehaus.groovy.ast.expr.BinaryExpression;
@@ -446,18 +447,14 @@ public class BinaryExpressionHelper {
             TupleExpression tuple = (TupleExpression) leftExpression;
             int i = 0;
             for (Expression e : tuple.getExpressions()) {
-                VariableExpression var = (VariableExpression) e;
-                MethodCallExpression call = new MethodCallExpression(
-                        rhsValueLoader, "getAt",
-                        new ArgumentListExpression(new ConstantExpression(i)));
-                call.visit(acg);
-                i += 1;
+                callX(rhsValueLoader, "getAt", args(constX(i++))).visit(acg);
                 if (defineVariable) {
-                    operandStack.doGroovyCast(var);
-                    compileStack.defineVariable(var, true);
+                    Variable v = (Variable) e;
+                    operandStack.doGroovyCast(v);
+                    compileStack.defineVariable(v, true);
                     operandStack.remove(1);
                 } else {
-                    acg.visitVariableExpression(var);
+                    e.visit(acg);
                 }
             }
         } else if (defineVariable) {


[groovy] 05/06: reduce nesting

Posted by su...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit 570361e99ff8f8e59b11638ad9cf9958060be1b5
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Tue Jun 9 11:11:13 2020 -0500

    reduce nesting
    
    (cherry picked from commit 20bc8ad74bb7aeaf6c26f1315eda3a2c48ef6473)
---
 .../classgen/asm/sc/StaticInvocationWriter.java    | 142 +++++++++++----------
 1 file changed, 74 insertions(+), 68 deletions(-)

diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java
index 72fbe89..bb6e934 100644
--- a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java
@@ -18,7 +18,6 @@
  */
 package org.codehaus.groovy.classgen.asm.sc;
 
-import org.codehaus.groovy.ast.ASTNode;
 import org.codehaus.groovy.ast.ClassHelper;
 import org.codehaus.groovy.ast.ClassNode;
 import org.codehaus.groovy.ast.ConstructorNode;
@@ -177,8 +176,10 @@ public class StaticInvocationWriter extends InvocationWriter {
                     cn = (ConstructorNode) bridge;
                     args = newArgs;
                 } else {
-                    controller.getSourceUnit().addError(new SyntaxException("Cannot call private constructor for " + declaringClass.toString(false) +
-                            " from class " + classNode.toString(false), call.getLineNumber(), call.getColumnNumber(), mn.getLastLineNumber(), call.getLastColumnNumber()));
+                    controller.getSourceUnit().addError(new SyntaxException(
+                            "Cannot call private constructor for " + declaringClass.toString(false) + " from class " + classNode.toString(false),
+                            call
+                    ));
                 }
             }
         }
@@ -274,6 +275,8 @@ public class StaticInvocationWriter extends InvocationWriter {
     protected boolean writeDirectMethodCall(final MethodNode target, final boolean implicitThis, final Expression receiver, final TupleExpression args) {
         if (target == null) return false;
 
+        ClassNode classNode = controller.getClassNode();
+
         if (target instanceof ExtensionMethodNode) {
             ExtensionMethodNode emn = (ExtensionMethodNode) target;
             MethodVisitor mv = controller.getMethodVisitor();
@@ -286,7 +289,6 @@ public class StaticInvocationWriter extends InvocationWriter {
                 argumentList.add(nullX());
             } else {
                 boolean isThisOrSuper = isThisExpression(receiver) || isSuperExpression(receiver);
-                ClassNode classNode = controller.getClassNode();
                 Expression fixedReceiver = null;
                 if (isThisOrSuper && classNode.getOuterClass() != null && controller.isInGeneratedFunction()) {
                     ClassNode current = classNode.getOuterClass();
@@ -319,75 +321,76 @@ public class StaticInvocationWriter extends InvocationWriter {
             }
             controller.getOperandStack().push(returnType);
             return true;
-        } else {
-            if (target == StaticTypeCheckingVisitor.CLOSURE_CALL_VARGS) {
-                // wrap arguments into an array
-                ArrayExpression arr = new ArrayExpression(ClassHelper.OBJECT_TYPE, args.getExpressions());
-                return super.writeDirectMethodCall(target, implicitThis, receiver, args(arr));
+        }
+
+        if (target == StaticTypeCheckingVisitor.CLOSURE_CALL_VARGS) {
+            // wrap arguments into an array
+            Expression arr = new ArrayExpression(ClassHelper.OBJECT_TYPE, args.getExpressions());
+            return super.writeDirectMethodCall(target, implicitThis, receiver, args(arr));
+        }
+
+        if (!target.isPublic()
+                && controller.isInGeneratedFunction()
+                && target.getDeclaringClass() != classNode) {
+            if (!tryBridgeMethod(target, receiver, implicitThis, args, classNode)) {
+                // replace call with an invoker helper call
+                MethodNode methodNode = target.isStatic() ? INVOKERHELPER_INVOKESTATICMETHOD : INVOKERHELPER_INVOKEMETHOD;
+                MethodCallExpression mce = callX(
+                        classX(INVOKERHELPER_CLASSNODE),
+                        methodNode.getName(),
+                        args(
+                                target.isStatic() ? classX(target.getDeclaringClass()) : receiver,
+                                constX(target.getName()),
+                                new ArrayExpression(ClassHelper.OBJECT_TYPE, args.getExpressions())
+                        )
+                );
+                mce.setMethodTarget(methodNode);
+                mce.visit(controller.getAcg());
             }
-            ClassNode classNode = controller.getClassNode();
-            if (classNode.isDerivedFrom(ClassHelper.CLOSURE_TYPE)
-                    && controller.isInGeneratedFunction()
-                    && !target.isPublic()
-                    && target.getDeclaringClass() != classNode) {
-                if (!tryBridgeMethod(target, receiver, implicitThis, args, classNode)) {
-                    // replace call with an invoker helper call
-                    ArrayExpression arr = new ArrayExpression(ClassHelper.OBJECT_TYPE, args.getExpressions());
-                    MethodCallExpression mce = callX(
-                            classX(INVOKERHELPER_CLASSNODE),
-                            target.isStatic() ? "invokeStaticMethod" : "invokeMethodSafe",
-                            args(
-                                    target.isStatic() ? classX(target.getDeclaringClass()) : receiver,
-                                    constX(target.getName()),
-                                    arr
-                            )
-                    );
-                    mce.setMethodTarget(target.isStatic() ? INVOKERHELPER_INVOKESTATICMETHOD : INVOKERHELPER_INVOKEMETHOD);
-                    mce.visit(controller.getAcg());
-                    return true;
-                }
+            return true;
+        }
+
+        if (target.isPrivate() && tryPrivateMethod(target, implicitThis, receiver, args, classNode)) {
+            return true;
+        }
+
+        Expression fixedReceiver = null;
+        boolean fixedImplicitThis = implicitThis;
+        if (target.isProtected()) {
+            ClassNode node = receiver == null ? ClassHelper.OBJECT_TYPE : controller.getTypeChooser().resolveType(receiver, controller.getClassNode());
+            if (!implicitThis && !isThisExpression(receiver) && !isSuperExpression(receiver) && !samePackageName(node, classNode)
+                    && StaticTypeCheckingSupport.implementsInterfaceOrIsSubclassOf(node,target.getDeclaringClass())) {
+                controller.getSourceUnit().addError(new SyntaxException(
+                        "Method " + target.getName() + " is protected in " + target.getDeclaringClass().toString(false),
+                        receiver != null ? receiver : args
+                ));
+            } else if (!node.isDerivedFrom(target.getDeclaringClass()) && tryBridgeMethod(target, receiver, implicitThis, args, classNode)) {
                 return true;
             }
-            Expression fixedReceiver = null;
-            boolean fixedImplicitThis = implicitThis;
-            if (target.isPrivate()) {
-                if (tryPrivateMethod(target, implicitThis, receiver, args, classNode)) return true;
-            } else if (target.isProtected()) {
-                ClassNode node = receiver == null ? ClassHelper.OBJECT_TYPE : controller.getTypeChooser().resolveType(receiver, controller.getClassNode());
-                if (!implicitThis && !isThisExpression(receiver) && !isSuperExpression(receiver) && !samePackageName(node, classNode)
-                        && StaticTypeCheckingSupport.implementsInterfaceOrIsSubclassOf(node,target.getDeclaringClass())) {
-                    ASTNode src = receiver == null ? args : receiver;
-                    controller.getSourceUnit().addError(
-                            new SyntaxException("Method " + target.getName() + " is protected in " + target.getDeclaringClass().toString(false),
-                                    src.getLineNumber(), src.getColumnNumber(), src.getLastLineNumber(), src.getLastColumnNumber()));
-                } else if (!node.isDerivedFrom(target.getDeclaringClass()) && tryBridgeMethod(target, receiver, implicitThis, args, classNode)) {
-                    return true;
-                }
-            } else if (target.isPublic() && receiver != null) {
-                if (implicitThis
-                        && !classNode.isDerivedFrom(target.getDeclaringClass())
-                        && !classNode.implementsInterface(target.getDeclaringClass())
-                        && classNode.getOuterClass() != null && controller.isInGeneratedFunction()) {
-                    ClassNode current = classNode.getOuterClass();
-                    fixedReceiver = varX("thisObject", current);
-                    // adjust for multiple levels of nesting if needed
-                    while (current.getOuterClass() != null && !target.getDeclaringClass().equals(current)) {
-                        FieldNode thisField = current.getField("this$0");
-                        current = current.getOuterClass();
-                        if (thisField != null) {
-                            fixedReceiver = propX(fixedReceiver, "this$0");
-                            fixedReceiver.setType(current);
-                            fixedImplicitThis = false;
-                        }
+        } else if (target.isPublic() && receiver != null) {
+            if (implicitThis
+                    && !classNode.isDerivedFrom(target.getDeclaringClass())
+                    && !classNode.implementsInterface(target.getDeclaringClass())
+                    && classNode.getOuterClass() != null && controller.isInGeneratedFunction()) {
+                ClassNode current = classNode.getOuterClass();
+                fixedReceiver = varX("thisObject", current);
+                // adjust for multiple levels of nesting if needed
+                while (current.getOuterClass() != null && !target.getDeclaringClass().equals(current)) {
+                    FieldNode thisField = current.getField("this$0");
+                    current = current.getOuterClass();
+                    if (thisField != null) {
+                        fixedReceiver = propX(fixedReceiver, "this$0");
+                        fixedReceiver.setType(current);
+                        fixedImplicitThis = false;
                     }
                 }
             }
-            if (receiver != null && !isSuperExpression(receiver)) {
-                // in order to avoid calls to castToType, which is the dynamic behaviour, we make sure that we call CHECKCAST instead then replace the top operand type
-                return super.writeDirectMethodCall(target, fixedImplicitThis, new CheckcastReceiverExpression(fixedReceiver != null ? fixedReceiver : receiver, target), args);
-            }
-            return super.writeDirectMethodCall(target, implicitThis, receiver, args);
         }
+        if (receiver != null && !isSuperExpression(receiver)) {
+            // in order to avoid calls to castToType, which is the dynamic behaviour, we make sure that we call CHECKCAST instead then replace the top operand type
+            return super.writeDirectMethodCall(target, fixedImplicitThis, new CheckcastReceiverExpression(fixedReceiver != null ? fixedReceiver : receiver, target), args);
+        }
+        return super.writeDirectMethodCall(target, implicitThis, receiver, args);
     }
 
     private boolean tryPrivateMethod(final MethodNode target, final boolean implicitThis, final Expression receiver, final TupleExpression args, final ClassNode classNode) {
@@ -407,8 +410,10 @@ public class StaticInvocationWriter extends InvocationWriter {
 
     private void checkAndAddCannotCallPrivateMethodError(final MethodNode target, final Expression receiver, final ClassNode classNode, final ClassNode declaringClass) {
         if (declaringClass != classNode) {
-            controller.getSourceUnit().addError(new SyntaxException("Cannot call private method " + (target.isStatic() ? "static " : "") +
-                    declaringClass.toString(false) + "#" + target.getName() + " from class " + classNode.toString(false), receiver.getLineNumber(), receiver.getColumnNumber(), receiver.getLastLineNumber(), receiver.getLastColumnNumber()));
+            controller.getSourceUnit().addError(new SyntaxException(
+                    "Cannot call private method " + (target.isStatic() ? "static " : "") + declaringClass.toString(false) + "#" + target.getName() + " from class " + classNode.toString(false),
+                    receiver
+            ));
         }
     }
 
@@ -420,6 +425,7 @@ public class StaticInvocationWriter extends InvocationWriter {
         return false;
     }
 
+    @Override
     protected void loadArguments(final List<Expression> argumentList, final Parameter[] para) {
         if (para.length == 0) return;
         ClassNode lastParaType = para[para.length - 1].getOriginType();