groovy-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From emil...@apache.org
Subject [groovy] 01/01: GROOVY-9918: produce varargs binding when one arg short despite arg type
Date Fri, 29 Jan 2021 18:04:32 GMT
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 <eric.milles@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


Mime
View raw message