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 2022/08/13 22:56:14 UTC
[groovy] branch GROOVY_4_0_X updated: direct constructor delegation for variadic scenarios
This is an automated email from the ASF dual-hosted git repository.
sunlan pushed a commit to branch GROOVY_4_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
The following commit(s) were added to refs/heads/GROOVY_4_0_X by this push:
new 7be1897750 direct constructor delegation for variadic scenarios
7be1897750 is described below
commit 7be1897750bfefc12f990352679cd18ae58ea03d
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Sat Aug 13 11:53:39 2022 -0500
direct constructor delegation for variadic scenarios
(cherry picked from commit 208310085af6c4ad43cea6f05d12073c433ea0ea)
---
.../groovy/classgen/asm/InvocationWriter.java | 93 +++++++++++-----------
.../invocation/ConstructorDelegationTest.groovy | 69 ++++++++++------
2 files changed, 90 insertions(+), 72 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 ca59b88d2d..8f9d38a04b 100644
--- a/src/main/java/org/codehaus/groovy/classgen/asm/InvocationWriter.java
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/InvocationWriter.java
@@ -60,6 +60,7 @@ import static org.codehaus.groovy.ast.ClassHelper.isObjectType;
import static org.codehaus.groovy.ast.ClassHelper.isPrimitiveType;
import static org.codehaus.groovy.ast.ClassHelper.isPrimitiveVoid;
import static org.codehaus.groovy.ast.ClassHelper.isStringType;
+import static org.codehaus.groovy.ast.tools.ParameterUtils.isVargs;
import static org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport.isClassClassNodeWrappingConcreteType;
import static org.objectweb.asm.Opcodes.AALOAD;
import static org.objectweb.asm.Opcodes.ACC_FINAL;
@@ -514,34 +515,42 @@ public class InvocationWriter {
makeCall(call, receiver, messageName, call.getArguments(), InvocationWriter.invokeStaticMethod, false, false, false);
}
+ //--------------------------------------------------------------------------
+
+ public void writeInvokeConstructor(final ConstructorCallExpression call) {
+ if (writeDirectConstructorCall(call)) return;
+ if (writeAICCall (call)) return;
+ writeNormalConstructorCall(call);
+ }
+
private boolean writeDirectConstructorCall(final ConstructorCallExpression call) {
if (!controller.isFastPath()) return false;
OptimizingStatementWriter.StatementMeta meta = call.getNodeMetaData(OptimizingStatementWriter.StatementMeta.class);
- ConstructorNode cn = null;
- if (meta != null) cn = (ConstructorNode) meta.target;
- if (cn == null) return false;
+ ConstructorNode ctor = meta != null ? (ConstructorNode) meta.target : null;
+ if (ctor == null) return false;
- String ownerDescriptor = prepareConstructorCall(cn);
- TupleExpression args = makeArgumentList(call.getArguments());
- loadArguments(args.getExpressions(), cn.getParameters());
- finnishConstructorCall(cn, ownerDescriptor, args.getExpressions().size());
+ List<Expression> args = makeArgumentList(call.getArguments()).getExpressions();
+
+ loadArguments(args, ctor.getParameters());
+ String ownerDescriptor = prepareConstructorCall(ctor);
+ finnishConstructorCall(ctor, ownerDescriptor, args.size());
return true;
}
protected String prepareConstructorCall(final ConstructorNode cn) {
- String owner = BytecodeHelper.getClassInternalName(cn.getDeclaringClass());
+ String type = BytecodeHelper.getClassInternalName(cn.getDeclaringClass());
MethodVisitor mv = controller.getMethodVisitor();
- mv.visitTypeInsn(NEW, owner);
+ mv.visitTypeInsn(NEW, type);
mv.visitInsn(DUP);
- return owner;
+ return type;
}
- protected void finnishConstructorCall(final ConstructorNode cn, final String ownerDescriptor, final int argsToRemove) {
- String desc = BytecodeHelper.getMethodDescriptor(ClassHelper.VOID_TYPE, cn.getParameters());
+ protected void finnishConstructorCall(final ConstructorNode cn, final String ownerDescriptor, final int argsToRemove) {
+ String signature = BytecodeHelper.getMethodDescriptor(ClassHelper.VOID_TYPE, cn.getParameters());
MethodVisitor mv = controller.getMethodVisitor();
- mv.visitMethodInsn(INVOKESPECIAL, ownerDescriptor, "<init>", desc, false);
+ mv.visitMethodInsn(INVOKESPECIAL, ownerDescriptor, "<init>", signature, false);
controller.getOperandStack().remove(argsToRemove);
controller.getOperandStack().push(cn.getDeclaringClass());
@@ -561,21 +570,14 @@ public class InvocationWriter {
controller.getCallSiteWriter().makeCallSite(receiver, CallSiteWriter.CONSTRUCTOR, arguments, false, false, false, false);
}
- public void writeInvokeConstructor(final ConstructorCallExpression call) {
- if (writeDirectConstructorCall(call)) return;
- if (writeAICCall(call)) return;
- writeNormalConstructorCall(call);
- }
-
protected boolean writeAICCall(final ConstructorCallExpression call) {
if (!call.isUsingAnonymousInnerClass()) return false;
- ConstructorNode cn = call.getType().getDeclaredConstructors().get(0);
- OperandStack os = controller.getOperandStack();
- String ownerDescriptor = prepareConstructorCall(cn);
+ ConstructorNode ctor = call.getType().getDeclaredConstructors().get(0);
+
+ String ownerDescriptor = prepareConstructorCall(ctor);
List<Expression> args = makeArgumentList(call.getArguments()).getExpressions();
- Parameter[] params = cn.getParameters();
// if a this appears as parameter here, then it should be
// not static, unless we are in a static method. But since
// ACG#visitVariableExpression does the opposite for this case, we
@@ -583,19 +585,19 @@ public class InvocationWriter {
// sine visiting a method call or property with implicit this will push
// a new value for this again.
controller.getCompileStack().pushImplicitThis(true);
- for (int i = 0, n = params.length; i < n; i += 1) {
- Parameter p = params[i];
- Expression arg = args.get(i);
+ int i = 0; Parameter[] params = ctor.getParameters();
+ for (Expression arg : args) {
+ Parameter p = params[Math.min(i++, params.length)];
if (arg instanceof VariableExpression) {
VariableExpression var = (VariableExpression) arg;
loadVariableWithReference(var);
} else {
arg.visit(controller.getAcg());
}
- os.doGroovyCast(p.getType());
+ controller.getOperandStack().doGroovyCast(p.getType());
}
controller.getCompileStack().popImplicitThis();
- finnishConstructorCall(cn, ownerDescriptor, args.size());
+ finnishConstructorCall(ctor, ownerDescriptor, args.size());
return true;
}
@@ -607,6 +609,8 @@ public class InvocationWriter {
}
}
+ //--------------------------------------------------------------------------
+
public final void makeSingleArgumentCall(final Expression receiver, final String message, final Expression arguments) {
makeSingleArgumentCall(receiver, message, arguments, false);
}
@@ -657,7 +661,21 @@ public class InvocationWriter {
if (argument instanceof SpreadExpression) return false;
}
- ConstructorNode ctor = getMatchingConstructor(constructors, argumentList);
+ ConstructorNode ctor = null;
+ ConstructorNode varg = null;
+ int nArguments = argumentList.size();
+ for (ConstructorNode constructor : constructors) {
+ int nParameters = constructor.getParameters().length;
+ if (nArguments == nParameters) {
+ if (ctor == null) ctor = constructor;
+ else return false; // ambiguous match
+ } else if (isVargs(constructor.getParameters())
+ && (nArguments == nParameters - 1 || nArguments > nParameters)) {
+ if (varg == null) varg = constructor;
+ else return false; // ambiguous match
+ }
+ }
+ if (ctor == null) ctor = varg;
if (ctor == null) return false;
MethodVisitor mv = controller.getMethodVisitor();
@@ -812,23 +830,6 @@ public class InvocationWriter {
operandStack.remove(2);
}
- // we match only on the number of arguments, not anything else
- private static ConstructorNode getMatchingConstructor(final List<ConstructorNode> constructors, final List<Expression> argumentList) {
- ConstructorNode lastMatch = null;
- for (ConstructorNode cn : constructors) {
- Parameter[] params = cn.getParameters();
- // if number of parameters does not match we have no match
- if (argumentList.size() != params.length) continue;
- if (lastMatch == null) {
- lastMatch = cn;
- } else {
- // we already had a match so we don't make a direct call at all
- return null;
- }
- }
- return lastMatch;
- }
-
/**
* Converts sourceType to a non primitive by using Groovy casting.
* sourceType might be a primitive
diff --git a/src/test/gls/invocation/ConstructorDelegationTest.groovy b/src/test/gls/invocation/ConstructorDelegationTest.groovy
index 0e9a61c5f0..496e222f6a 100644
--- a/src/test/gls/invocation/ConstructorDelegationTest.groovy
+++ b/src/test/gls/invocation/ConstructorDelegationTest.groovy
@@ -18,12 +18,17 @@
*/
package gls.invocation
-import gls.CompilableTestSupport
+import org.codehaus.groovy.control.CompilationFailedException
+import org.junit.Test
-class ConstructorDelegationTest extends CompilableTestSupport {
+import static groovy.test.GroovyAssert.assertScript
+import static groovy.test.GroovyAssert.shouldFail
+final class ConstructorDelegationTest {
+
+ @Test
void testThisCallWithParameter() {
- assertScript """
+ assertScript '''
class A {
def foo
A(String x){foo=x}
@@ -31,11 +36,12 @@ class ConstructorDelegationTest extends CompilableTestSupport {
}
def a = new A()
assert a.foo == "bar"
- """
+ '''
}
+ @Test
void testThisCallWithoutParameter() {
- assertScript """
+ assertScript '''
class A {
def foo
A(String x){this(); foo=x}
@@ -43,11 +49,12 @@ class ConstructorDelegationTest extends CompilableTestSupport {
}
def a = new A("foo")
assert a.foo == "foo"
- """
+ '''
}
+ @Test
void testThisConstructorCallNotOnFirstStmt() {
- shouldNotCompile """
+ shouldFail CompilationFailedException, '''
class ThisConstructorCall {
public ThisConstructorCall() {
println 'dummy first statement'
@@ -58,11 +65,12 @@ class ConstructorDelegationTest extends CompilableTestSupport {
}
}
1
- """
+ '''
}
+ @Test
void testSuperConstructorCallNotOnFirstStmt() {
- shouldNotCompile """
+ shouldFail CompilationFailedException, '''
class SuperConstructorCall {
public SuperConstructorCall() {
println 'dummy first statement'
@@ -70,33 +78,32 @@ class ConstructorDelegationTest extends CompilableTestSupport {
}
}
1
- """
+ '''
}
+ @Test
void testConstructorDelegationWithThisOrSuperInArgs() {
- def scriptStr
// all 4 cases below were compiling earlier but giving VerifyError at runtime
- scriptStr = """
+
+ shouldFail CompilationFailedException, '''
class MyClosure3128V1 extends Closure {
MyClosure3128V1() {
super(this)
}
void run() { println 'running' }
}
- """
- shouldNotCompile(scriptStr)
+ '''
- scriptStr = """
+ shouldFail CompilationFailedException, '''
class MyClosure3128V2 extends Closure {
MyClosure3128V2() {
super(super)
}
void run() { println 'running' }
}
- """
- shouldNotCompile(scriptStr)
+ '''
- scriptStr = """
+ shouldFail CompilationFailedException, '''
class MyClosure3128V3 extends Closure {
MyClosure3128V3() {
this(this)
@@ -104,10 +111,9 @@ class ConstructorDelegationTest extends CompilableTestSupport {
MyClosure3128V3(owner) {}
void run() { println 'running' }
}
- """
- shouldNotCompile(scriptStr)
+ '''
- scriptStr = """
+ shouldFail CompilationFailedException, '''
class MyClosure3128V4 extends Closure {
MyClosure3128V4() {
this(super)
@@ -115,12 +121,11 @@ class ConstructorDelegationTest extends CompilableTestSupport {
MyClosure3128V4(owner) {}
void run() { println 'running' }
}
- """
- shouldNotCompile(scriptStr)
+ '''
}
- // GROOVY-6618
- void testVarsConstructor() {
+ @Test // GROOVY-6618
+ void testVariadicConstructor() {
assertScript '''
class Foo {
public info
@@ -129,6 +134,7 @@ class ConstructorDelegationTest extends CompilableTestSupport {
}
assert new Foo().info == [1]
'''
+
assertScript '''
class Foo {
public info
@@ -137,6 +143,7 @@ class ConstructorDelegationTest extends CompilableTestSupport {
}
assert new Foo().info == null
'''
+
assertScript '''
class Foo {
public info
@@ -145,6 +152,16 @@ class ConstructorDelegationTest extends CompilableTestSupport {
}
assert new Foo().info == [1,2,3]
'''
+
+ assertScript '''
+ class Foo {
+ public info
+ Foo(String s,Integer[] a){info=a}
+ Foo() {this("foo",new Integer[]{1,2,3})}
+ }
+ assert new Foo().info == [1,2,3]
+ '''
+
assertScript '''
class Foo {
public info
@@ -154,4 +171,4 @@ class ConstructorDelegationTest extends CompilableTestSupport {
assert new Foo().info == []
'''
}
-}
\ No newline at end of file
+}