Skip to content

Commit

Permalink
GROOVY-5881, GROOVY-6324: defer to metaclass for closure call
Browse files Browse the repository at this point in the history 
GROOVY-8466: use type chooser
  • Loading branch information
@eric-milles
eric-milles committed Sep 13, 2024
1 parent f50ce0c commit dcfe41d
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 103 deletions.
90 changes: 25 additions & 65 deletions src/main/java/org/codehaus/groovy/classgen/asm/InvocationWriter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
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;
Expand Down Expand Up @@ -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")});
Expand Down Expand Up @@ -457,78 +457,38 @@ protected String getMethodName(final Expression message) {
}

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) {
Expand Down
36 changes: 12 additions & 24 deletions src/test/groovy/bugs/Groovy5267Bug.groovy → src/test/groovy/bugs/Groovy5267.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;'])
}
}
24 changes: 10 additions & 14 deletions src/test/org/codehaus/groovy/transform/DelegateTransformTest.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand All @@ -1114,20 +1115,16 @@ class Foo4244 {
@Delegate Bar4244 bar = new Bar4244()
}

@groovy.transform.TupleConstructor(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 {
Expand All @@ -1137,8 +1134,7 @@ interface SomeInterface4619 {
interface SomeOtherInterface4619 extends SomeInterface4619 {}

class SomeClass4619 {
@Delegate
SomeOtherInterface4619 delegate
@Delegate SomeOtherInterface4619 delegate
}

interface BarInt {
Expand Down

0 comments on commit dcfe41d

Please # to comment.