You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by em...@apache.org on 2021/01/29 18:04:31 UTC

[groovy] branch GROOVY-9918 created (now 44d6252)

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

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


      at 44d6252  GROOVY-9918: produce varargs binding when one arg short despite arg type

This branch includes the following new commits:

     new 44d6252  GROOVY-9918: produce varargs binding when one arg short despite arg type

The 1 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.



[groovy] 01/01: GROOVY-9918: produce varargs binding when one arg short despite arg type

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

emilles pushed a commit to branch GROOVY-9918
in repository https://gitbox.apache.org/repos/asf/groovy.git

commit 44d625282c06badf17e06a940a2d314cd31e11d7
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Fri Jan 29 12:01:04 2021 -0600

    GROOVY-9918: produce varargs binding when one arg short despite arg type
    
    - trust upstream method target selection; is default argument path dead?
    
    - add failsafe compiler error instead of throwing a NullPointerException
---
 .../groovy/classgen/asm/InvocationWriter.java      |  2 +-
 .../classgen/asm/sc/StaticInvocationWriter.java    | 21 ++++++-----
 .../classgen/asm/sc/BugsStaticCompileTest.groovy   | 43 ++++++++++++++++------
 3 files changed, 45 insertions(+), 21 deletions(-)

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 b3c51ee..3402fa2 100644
--- a/src/main/java/org/codehaus/groovy/classgen/asm/InvocationWriter.java
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/InvocationWriter.java
@@ -95,7 +95,7 @@ public class InvocationWriter {
     // constructor calls with this() and super()
     private static final MethodCaller selectConstructorAndTransformArguments = MethodCaller.newStatic(ScriptBytecodeAdapter.class, "selectConstructorAndTransformArguments");
 
-    private final WriterController controller;
+    protected final WriterController controller;
 
     public InvocationWriter(final WriterController controller) {
         this.controller = controller;
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 5dedb3a..62e2bce 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
@@ -65,9 +65,11 @@ import org.objectweb.asm.Label;
 import org.objectweb.asm.MethodVisitor;
 
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.List;
 import java.util.Map;
 import java.util.concurrent.atomic.AtomicInteger;
+import java.util.stream.Collectors;
 
 import static org.apache.groovy.ast.tools.ClassNodeUtils.samePackageName;
 import static org.apache.groovy.ast.tools.ExpressionUtils.isNullConstant;
@@ -111,13 +113,10 @@ public class StaticInvocationWriter extends InvocationWriter {
 
     private final AtomicInteger labelCounter = new AtomicInteger();
 
-    final WriterController controller;
-
     private MethodCallExpression currentCall;
 
     public StaticInvocationWriter(final WriterController wc) {
         super(wc);
-        controller = wc;
     }
 
     @Override
@@ -430,12 +429,12 @@ public class StaticInvocationWriter extends InvocationWriter {
         ClassNode lastArgType = nArgs == 0 ? null : typeChooser.resolveType(argumentList.get(nArgs - 1), classNode);
         ClassNode lastPrmType = parameters[nPrms - 1].getOriginType();
 
-        if (lastPrmType.isArray() && (nArgs > nPrms // too many args
-                || (nArgs == nPrms - 1 && !lastPrmType.equals(lastArgType)) // too few args
-                || (nArgs == nPrms && !lastArgType.isArray() // last fits within array type
+        // target is variadic and args are too many or one short or just enough with array compatibility
+        if (lastPrmType.isArray() && (nArgs > nPrms || nArgs == nPrms - 1
+                || (nArgs == nPrms && !lastArgType.isArray()
                     && (StaticTypeCheckingSupport.implementsInterfaceOrIsSubclassOf(lastArgType, lastPrmType.getComponentType())
                         || ClassHelper.GSTRING_TYPE.equals(lastArgType) && ClassHelper.STRING_TYPE.equals(lastPrmType.getComponentType())))
-        )) { // variadic call
+        )) {
             OperandStack operandStack = controller.getOperandStack();
             int stackLength = operandStack.getStackLength() + nArgs;
             // first arguments/parameters as usual
@@ -460,7 +459,7 @@ public class StaticInvocationWriter extends InvocationWriter {
             for (int i = 0; i < nArgs; i += 1) {
                 visitArgument(argumentList.get(i), parameters[i].getType());
             }
-        } else { // method call with default arguments
+        } else { // call with default arguments
             Expression[] arguments = new Expression[nPrms];
             for (int i = 0, j = 0; i < nPrms; i += 1) {
                 Parameter p = parameters[i];
@@ -471,9 +470,13 @@ public class StaticInvocationWriter extends InvocationWriter {
                 Expression expression = getInitialExpression(p); // default argument
                 if (expression != null && !isCompatibleArgumentType(aType, pType)) {
                     arguments[i] = expression;
-                } else {
+                } else if (a != null) {
                     arguments[i] = a;
                     j += 1;
+                } else {
+                    controller.getSourceUnit().addFatalError("Binding failed" +
+                            " for arguments [" + argumentList.stream().map(arg -> typeChooser.resolveType(arg, classNode).toString(false)).collect(Collectors.joining(", ")) + "]" +
+                            " and parameters [" + Arrays.stream(parameters).map(prm -> prm.getType().toString(false)).collect(Collectors.joining(", ")) + "]", getCurrentCall());
                 }
             }
             for (int i = 0; i < nArgs; i += 1) {
diff --git a/src/test/org/codehaus/groovy/classgen/asm/sc/BugsStaticCompileTest.groovy b/src/test/org/codehaus/groovy/classgen/asm/sc/BugsStaticCompileTest.groovy
index ce5dc88..1bcca69 100644
--- a/src/test/org/codehaus/groovy/classgen/asm/sc/BugsStaticCompileTest.groovy
+++ b/src/test/org/codehaus/groovy/classgen/asm/sc/BugsStaticCompileTest.groovy
@@ -895,17 +895,38 @@ import groovy.transform.TypeCheckingMode
 
     // GROOVY-6113
     void testCallObjectVargsMethodWithPrimitiveIntConstant() {
-        try {
-            assertScript '''
-                int sum(Object... elems) {
-                     (Integer)elems.toList().sum()
-                }
-                int x = sum(Closure.DELEGATE_FIRST)
-                assert x == Closure.DELEGATE_FIRST
-            '''
-        } finally {
-//            println astTrees
-        }
+        assertScript '''
+            int sum(... zeroOrMore) {
+                 (Integer) zeroOrMore.toList().sum()
+            }
+            int x = sum(Closure.DELEGATE_FIRST)
+            assert x == Closure.DELEGATE_FIRST
+        '''
+    }
+
+    // GROOVY-9918
+    void testCallObjectObjectVargsMethodWithObjectArray() {
+        assertScript '''
+            def m(one, ... zeroOrMore) {
+            }
+            Object[] array = ['a', 'b']
+            m(array) // NPE in SC
+        '''
+    }
+
+    void testDefaultArgumentAndVargs() {
+        assertScript '''
+            def m(int x=1, int y, String[] z) {
+                [x as String, y as String] + Arrays.asList(z)
+            }
+            assert m(2,'3') == ['1','2','3']
+        '''
+        assertScript '''
+            def m(int x, int y=2, String[] z) {
+                [x as String, y as String] + Arrays.asList(z)
+            }
+            assert m(1,'3') == ['1','2','3']
+        '''
     }
 
     // GROOVY-6095