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 <[email protected]> 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
