This is an automated email from the ASF dual-hosted git repository.
sunlan 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 c919f2c GROOVY-9918: produce varargs binding when one arg short
despite arg type
c919f2c is described below
commit c919f2ca2597ad5d8a9ca0305b3d9f922979f6cb
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
