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/03/03 03:45:57 UTC
[groovy] branch GROOVY_3_0_X 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.
emilles pushed a commit to branch GROOVY_3_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
The following commit(s) were added to refs/heads/GROOVY_3_0_X by this push:
new 668658e GROOVY-9918: produce varargs binding when one arg short despite arg type
668658e is described below
commit 668658ee707bcff9bac1dd04625a7b4964f2fa19
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
Conflicts:
src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java
---
.../groovy/classgen/asm/InvocationWriter.java | 2 +-
.../classgen/asm/sc/StaticInvocationWriter.java | 141 +++++++++++----------
.../classgen/asm/sc/BugsStaticCompileTest.groovy | 43 +++++--
3 files changed, 106 insertions(+), 80 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 ec55cf5..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
@@ -422,78 +421,93 @@ public class StaticInvocationWriter extends InvocationWriter {
}
@Override
- protected void loadArguments(final List<Expression> argumentList, final Parameter[] para) {
- if (para.length == 0) return;
- ClassNode lastParaType = para[para.length - 1].getOriginType();
- AsmClassGenerator acg = controller.getAcg();
+ protected void loadArguments(final List<Expression> argumentList, final Parameter[] parameters) {
+ final int nArgs = argumentList.size(), nPrms = parameters.length; if (nPrms == 0) return;
+
+ ClassNode classNode = controller.getClassNode();
TypeChooser typeChooser = controller.getTypeChooser();
- OperandStack operandStack = controller.getOperandStack();
- int argumentListSize = argumentList.size();
- ClassNode lastArgType = argumentListSize > 0 ?
- typeChooser.resolveType(argumentList.get(argumentListSize -1), controller.getClassNode()) : null;
- if (lastParaType.isArray()
- && ((argumentListSize > para.length)
- || ((argumentListSize == (para.length - 1)) && !lastParaType.equals(lastArgType))
- || ((argumentListSize == para.length && lastArgType!=null && !lastArgType.isArray())
- && (StaticTypeCheckingSupport.implementsInterfaceOrIsSubclassOf(lastArgType,lastParaType.getComponentType())))
- || ClassHelper.GSTRING_TYPE.equals(lastArgType) && ClassHelper.STRING_TYPE.equals(lastParaType.getComponentType()))
- ) {
- int stackLen = operandStack.getStackLength() + argumentListSize;
- MethodVisitor mv = controller.getMethodVisitor();
- controller.setMethodVisitor(mv);
- // varg call
- // first parameters as usual
- for (int i = 0; i < para.length - 1; i += 1) {
- visitArgument(argumentList.get(i), para[i].getType());
+ ClassNode lastArgType = nArgs == 0 ? null : typeChooser.resolveType(argumentList.get(nArgs - 1), classNode);
+ ClassNode lastPrmType = parameters[nPrms - 1].getOriginType();
+
+ // 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())))
+ )) {
+ OperandStack operandStack = controller.getOperandStack();
+ int stackLength = operandStack.getStackLength() + nArgs;
+ // first arguments/parameters as usual
+ for (int i = 0; i < nPrms - 1; i += 1) {
+ visitArgument(argumentList.get(i), parameters[i].getType());
}
- // last parameters wrapped in an array
- List<Expression> lastParams = new ArrayList<>();
- for (int i = para.length - 1; i < argumentListSize; i += 1) {
- lastParams.add(argumentList.get(i));
+ // wrap remaining arguments in an array for last parameter
+ List<Expression> lastArgs = new ArrayList<>();
+ for (int i = nPrms - 1; i < nArgs; i += 1) {
+ lastArgs.add(argumentList.get(i));
}
- ArrayExpression array = new ArrayExpression(lastParaType.getComponentType(), lastParams);
- array.visit(acg);
+ ArrayExpression array = new ArrayExpression(lastPrmType.getComponentType(), lastArgs);
+ array.visit(controller.getAcg());
// adjust stack length
- while (operandStack.getStackLength() < stackLen) {
+ while (operandStack.getStackLength() < stackLength) {
operandStack.push(ClassHelper.OBJECT_TYPE);
}
- if (argumentListSize == para.length - 1) {
+ if (nArgs == nPrms - 1) {
operandStack.remove(1);
}
- } else if (argumentListSize == para.length) {
- for (int i = 0; i < argumentListSize; i++) {
- visitArgument(argumentList.get(i), para[i].getType());
+ } else if (nArgs == nPrms) {
+ for (int i = 0; i < nArgs; i += 1) {
+ visitArgument(argumentList.get(i), parameters[i].getType());
}
- } else {
- // method call with default arguments
- ClassNode classNode = controller.getClassNode();
- Expression[] arguments = new Expression[para.length];
- for (int i = 0, j = 0, n = para.length; i < n; i += 1) {
- Parameter curParam = para[i];
- ClassNode curParamType = curParam.getType();
- Expression curArg = j < argumentListSize ? argumentList.get(j) : null;
- Expression initialExpression = curParam.getNodeMetaData(StaticTypesMarker.INITIAL_EXPRESSION);
- if (initialExpression == null && curParam.hasInitialExpression())
- initialExpression = curParam.getInitialExpression();
- if (initialExpression == null && curParam.getNodeMetaData(Verifier.INITIAL_EXPRESSION) != null) {
- initialExpression = curParam.getNodeMetaData(Verifier.INITIAL_EXPRESSION);
- }
- ClassNode curArgType = curArg == null ? null : typeChooser.resolveType(curArg, classNode);
-
- if (initialExpression != null && !compatibleArgumentType(curArgType, curParamType)) {
- // use default expression
- arguments[i] = initialExpression;
- } else {
- arguments[i] = curArg;
+ } else { // call with default arguments
+ Expression[] arguments = new Expression[nPrms];
+ for (int i = 0, j = 0; i < nPrms; i += 1) {
+ Parameter p = parameters[i];
+ ClassNode pType = p.getType();
+ Expression a = (j < nArgs ? argumentList.get(j) : null);
+ ClassNode aType = (a == null ? null : typeChooser.resolveType(a, classNode));
+
+ Expression expression = getInitialExpression(p); // default argument
+ if (expression != null && !isCompatibleArgumentType(aType, pType)) {
+ arguments[i] = expression;
+ } 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, n = arguments.length; i < n; i += 1) {
- visitArgument(arguments[i], para[i].getType());
+ for (int i = 0; i < nArgs; i += 1) {
+ visitArgument(arguments[i], parameters[i].getType());
}
}
}
+ private static Expression getInitialExpression(final Parameter parameter) {
+ Expression initialExpression = parameter.getNodeMetaData(StaticTypesMarker.INITIAL_EXPRESSION);
+ if (initialExpression == null && parameter.hasInitialExpression()) {
+ initialExpression = parameter.getInitialExpression();
+ }
+ if (initialExpression == null && parameter.getNodeMetaData(Verifier.INITIAL_EXPRESSION) != null) {
+ initialExpression = parameter.getNodeMetaData(Verifier.INITIAL_EXPRESSION);
+ }
+ return initialExpression;
+ }
+
+ private static boolean isCompatibleArgumentType(final ClassNode argumentType, final ClassNode parameterType) {
+ if (argumentType == null)
+ return false;
+ if (ClassHelper.getWrapper(argumentType).equals(ClassHelper.getWrapper(parameterType)))
+ return true;
+ if (parameterType.isInterface())
+ return argumentType.implementsInterface(parameterType);
+ if (parameterType.isArray() && argumentType.isArray())
+ return isCompatibleArgumentType(argumentType.getComponentType(), parameterType.getComponentType());
+ return ClassHelper.getWrapper(argumentType).isDerivedFrom(ClassHelper.getWrapper(parameterType));
+ }
+
private void visitArgument(final Expression argumentExpr, final ClassNode parameterType) {
argumentExpr.putNodeMetaData(StaticTypesMarker.PARAMETER_TYPE, parameterType);
argumentExpr.visit(controller.getAcg());
@@ -502,15 +516,6 @@ public class StaticInvocationWriter extends InvocationWriter {
}
}
- private boolean compatibleArgumentType(final ClassNode argumentType, final ClassNode paramType) {
- if (argumentType == null) return false;
- if (ClassHelper.getWrapper(argumentType).equals(ClassHelper.getWrapper(paramType))) return true;
- if (paramType.isInterface()) return argumentType.implementsInterface(paramType);
- if (paramType.isArray() && argumentType.isArray())
- return compatibleArgumentType(argumentType.getComponentType(), paramType.getComponentType());
- return ClassHelper.getWrapper(argumentType).isDerivedFrom(ClassHelper.getWrapper(paramType));
- }
-
@Override
public void makeCall(final Expression origin, final Expression receiver, final Expression message, final Expression arguments, final MethodCallerMultiAdapter adapter, final boolean safe, final boolean spreadSafe, final boolean implicitThis) {
ClassNode dynamicCallReturnType = origin.getNodeMetaData(StaticTypesMarker.DYNAMIC_RESOLUTION);
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