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 2021/01/30 16:51:56 UTC
[groovy] branch master updated: GROOVY-9918: produce varargs
binding when one arg short despite arg type
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 <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