This is an automated email from the ASF dual-hosted git repository. emilles pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/groovy.git
commit dcfe41d8a57d672a7cd1c96e8715e09830b990aa Author: Eric Milles <[email protected]> AuthorDate: Fri Sep 13 12:14:42 2024 -0500 GROOVY-5881, GROOVY-6324: defer to metaclass for closure call GROOVY-8466: use type chooser --- .../groovy/classgen/asm/InvocationWriter.java | 90 ++++++---------------- .../{Groovy5267Bug.groovy => Groovy5267.groovy} | 36 +++------ .../groovy/transform/DelegateTransformTest.groovy | 24 +++--- 3 files changed, 47 insertions(+), 103 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 21b69fda94..89eac79c99 100644 --- a/src/main/java/org/codehaus/groovy/classgen/asm/InvocationWriter.java +++ b/src/main/java/org/codehaus/groovy/classgen/asm/InvocationWriter.java @@ -21,7 +21,6 @@ package org.codehaus.groovy.classgen.asm; import org.codehaus.groovy.ast.ClassHelper; import org.codehaus.groovy.ast.ClassNode; import org.codehaus.groovy.ast.ConstructorNode; -import org.codehaus.groovy.ast.FieldNode; import org.codehaus.groovy.ast.MethodNode; import org.codehaus.groovy.ast.Parameter; import org.codehaus.groovy.ast.expr.ArgumentListExpression; @@ -94,6 +93,7 @@ public class InvocationWriter { public static final MethodCallerMultiAdapter invokeMethodOnSuper = MethodCallerMultiAdapter.newStatic(ScriptBytecodeAdapter.class, "invokeMethodOnSuper", true, false); public static final MethodCallerMultiAdapter invokeMethod = MethodCallerMultiAdapter.newStatic(ScriptBytecodeAdapter.class, "invokeMethod", true, false); public static final MethodCallerMultiAdapter invokeStaticMethod = MethodCallerMultiAdapter.newStatic(ScriptBytecodeAdapter.class, "invokeStaticMethod", true, true); + @Deprecated(since = "5.0.0") public static final MethodCaller invokeClosureMethod = MethodCaller.newStatic(ScriptBytecodeAdapter.class, "invokeClosure"); public static final MethodCaller castToVargsArray = MethodCaller.newStatic(DefaultTypeTransformation.class, "castToVargsArray"); private static final MethodNode CLASS_FOR_NAME_STRING = ClassHelper.CLASS_Type.getDeclaredMethod("forName", new Parameter[]{new Parameter(ClassHelper.STRING_TYPE, "name")}); @@ -457,78 +457,38 @@ public class InvocationWriter { } public void writeInvokeMethod(MethodCallExpression call) { - if (isClosureCall(call)) { - invokeClosure(call.getArguments(), call.getMethodAsString()); - } else { - if (isFunctionInterfaceCall(call)) { - call = transformToRealMethodCall(call); - } - Expression receiver = call.getObjectExpression(); - MethodCallerMultiAdapter adapter = invokeMethod; - if (isSuperExpression(receiver)) { - adapter = invokeMethodOnSuper; - } else if (isThisExpression(receiver)) { - adapter = invokeMethodOnCurrent; - } - if (isStaticInvocation(call)) { - adapter = invokeStaticMethod; - } - Expression messageName = new CastExpression(ClassHelper.STRING_TYPE, call.getMethod()); - makeCall(call, receiver, messageName, call.getArguments(), adapter, call.isSafe(), call.isSpreadSafe(), call.isImplicitThis()); + Expression receiver = call.getObjectExpression(); + // GROOVY-8466: replace "rcvr.call(args)" with "rcvr.abstractMethod(args)" + if (!isThisExpression(receiver) && "call".equals(call.getMethodAsString())) { + ClassNode type = controller.getTypeChooser().resolveType(receiver, controller.getClassNode()); + if (isFunctionalInterface(type)) call = transformToRealMethodCall(call, type); } - } - - private static boolean isFunctionInterfaceCall(final MethodCallExpression call) { - if ("call".equals(call.getMethodAsString())) { - Expression objectExpression = call.getObjectExpression(); - if (!isThisExpression(objectExpression)) { - return isFunctionalInterface(objectExpression.getType()); - } + MethodCallerMultiAdapter adapter = invokeMethod; + if (isSuperExpression(receiver)) { + adapter = invokeMethodOnSuper; + } else if (isThisExpression(receiver)) { + adapter = invokeMethodOnCurrent; } - return false; + if (isStaticInvocation(call)) { + adapter = invokeStaticMethod; + } + Expression messageName = new CastExpression(ClassHelper.STRING_TYPE, call.getMethod()); + makeCall(call, receiver, messageName, call.getArguments(), adapter, call.isSafe(), call.isSpreadSafe(), call.isImplicitThis()); } - private static MethodCallExpression transformToRealMethodCall(MethodCallExpression call) { - ClassNode type = call.getObjectExpression().getType(); + private static MethodCallExpression transformToRealMethodCall(final MethodCallExpression call, final ClassNode type) { MethodNode methodNode = ClassHelper.findSAM(type); - call = (MethodCallExpression) call.transformExpression(expression -> { - if (!(expression instanceof ConstantExpression)) { - return expression; + var rewrite = (MethodCallExpression) call.transformExpression(expression -> { + if (expression == call.getMethod()) { + var methodName = new ConstantExpression(methodNode.getName()); + methodName.setSourcePosition(call.getMethod()); + return methodName; } - return new ConstantExpression(methodNode.getName()); + return expression; }); - call.setMethodTarget(methodNode); - return call; - } - - private boolean isClosureCall(final MethodCallExpression call) { - // are we a local variable? - // it should not be an explicitly "this" qualified method call - // and the current class should have a possible method - ClassNode classNode = controller.getClassNode(); - String methodName = call.getMethodAsString(); - if (methodName == null) return false; - if (!call.isImplicitThis()) return false; - if (!isThisExpression(call.getObjectExpression())) return false; - FieldNode field = classNode.getDeclaredField(methodName); - if (field == null) return false; - if (isStaticInvocation(call) && !field.isStatic()) return false; - Expression arguments = call.getArguments(); - return !classNode.hasPossibleMethod(methodName, arguments); - } - - private void invokeClosure(final Expression arguments, final String methodName) { - AsmClassGenerator acg = controller.getAcg(); - acg.visitVariableExpression(new VariableExpression(methodName)); - controller.getOperandStack().box(); - if (arguments instanceof TupleExpression) { - arguments.visit(acg); - } else { - new TupleExpression(arguments).visit(acg); - } - invokeClosureMethod.call(controller.getMethodVisitor()); - controller.getOperandStack().replace(ClassHelper.OBJECT_TYPE); + rewrite.setMethodTarget(methodNode); + return rewrite; } private boolean isStaticInvocation(final MethodCallExpression call) { diff --git a/src/test/groovy/bugs/Groovy5267Bug.groovy b/src/test/groovy/bugs/Groovy5267.groovy similarity index 56% rename from src/test/groovy/bugs/Groovy5267Bug.groovy rename to src/test/groovy/bugs/Groovy5267.groovy index 14d782a045..85a4e94752 100644 --- a/src/test/groovy/bugs/Groovy5267Bug.groovy +++ b/src/test/groovy/bugs/Groovy5267.groovy @@ -18,33 +18,21 @@ */ package groovy.bugs -import org.codehaus.groovy.classgen.asm.AbstractBytecodeTestCase +import org.junit.Test -class Groovy5267Bug extends AbstractBytecodeTestCase { +import static groovy.test.GroovyAssert.shouldFail + +final class Groovy5267 { + @Test void testShouldNotThrowVerifyError() { - shouldFail(MissingMethodException) { - assertScript ''' - class Bar { - int defaultValue = 40 - void m() { - defaultValue() - } + shouldFail MissingMethodException, ''' + class C { + int defaultValue = 40 + void m() { + defaultValue() + } } - new Bar().m() + new C().m() ''' - } - } - - void testClosureCallBytecode() { - def bytecode= compile(method:'m', ''' - class BarScript { - int defaultValue = 40 - void m() { - defaultValue() - } - } - ''') - - assert bytecode.hasSequence(['INVOKESTATIC java/lang/Integer.valueOf (I)Ljava/lang/Integer;']) } } diff --git a/src/test/org/codehaus/groovy/transform/DelegateTransformTest.groovy b/src/test/org/codehaus/groovy/transform/DelegateTransformTest.groovy index 984b9b499a..f481e2a431 100644 --- a/src/test/org/codehaus/groovy/transform/DelegateTransformTest.groovy +++ b/src/test/org/codehaus/groovy/transform/DelegateTransformTest.groovy @@ -23,6 +23,7 @@ import org.codehaus.groovy.control.MultipleCompilationErrorsException import org.codehaus.groovy.tools.javac.JavaAwareCompilationUnit import org.junit.Test +import java.util.concurrent.Callable import java.util.concurrent.locks.Lock import static groovy.test.GroovyAssert.assertScript @@ -1090,11 +1091,11 @@ interface DelegateBar extends DelegateFoo { } class DelegateBarImpl implements DelegateBar { - @Delegate DelegateFoo foo; + @Delegate DelegateFoo foo - DelegateBarImpl(DelegateFoo f) { this.foo = f} + DelegateBarImpl(DelegateFoo foo) { this.foo = foo } - def bar() { 'bar impl'} + @Override bar() { 'bar impl' } } class BazWithDeprecatedFoo { @@ -1114,20 +1115,16 @@ class Foo4244 { @Delegate Bar4244 bar = new Bar4244() } [email protected](includeFields=true) class FooToMethod { - private final Closure<DelegateFoo> strategy + private final Callable<DelegateFoo> supplier - FooToMethod(Closure<DelegateFoo> strategy) { - this.strategy = strategy - } - - @Delegate - DelegateFoo getStrategy() { strategy() } + @Delegate DelegateFoo getStrategy() { supplier() } } class Bar4244 { - String nonFinalBaz = "Initial value - nonFinalBaz" - final String finalBaz = "Initial value - finalBaz" + String nonFinalBaz = 'Initial value - nonFinalBaz' + final String finalBaz = 'Initial value - finalBaz' } interface SomeInterface4619 { @@ -1137,8 +1134,7 @@ interface SomeInterface4619 { interface SomeOtherInterface4619 extends SomeInterface4619 {} class SomeClass4619 { - @Delegate - SomeOtherInterface4619 delegate + @Delegate SomeOtherInterface4619 delegate } interface BarInt {
