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 2024/09/13 17:25:59 UTC
(groovy) branch master updated (8aa0b919b8 -> dcfe41d8a5)
This is an automated email from the ASF dual-hosted git repository.
emilles pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git
from 8aa0b919b8 GROOVY-11459: configurable hashing algorithm (additional refactoring)
new f50ce0c1de GROOVY-8600, GROOVY-11366: SC: implicit-`this` closure field call
new dcfe41d8a5 GROOVY-5881, GROOVY-6324: defer to metaclass for closure call
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
.../groovy/classgen/asm/InvocationWriter.java | 90 ++++++----------------
.../MethodCallExpressionTransformer.java | 8 +-
.../transformers/StaticCompilationTransformer.java | 12 +--
.../bugs/{Groovy4365.groovy => Groovy5267.groovy} | 14 ++--
src/test/groovy/bugs/Groovy5267Bug.groovy | 50 ------------
.../{Groovy8600Bug.groovy => Groovy8600.groovy} | 16 ++--
.../groovy/transform/DelegateTransformTest.groovy | 24 +++---
7 files changed, 56 insertions(+), 158 deletions(-)
copy src/test/groovy/bugs/{Groovy4365.groovy => Groovy5267.groovy} (78%)
delete mode 100644 src/test/groovy/bugs/Groovy5267Bug.groovy
rename src/test/groovy/bugs/{Groovy8600Bug.groovy => Groovy8600.groovy} (78%)
(groovy) 01/02: GROOVY-8600, GROOVY-11366: SC: implicit-`this` closure field call
Posted by em...@apache.org.
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 f50ce0c1dee7683d3e9ab15098aefdbda89f4251
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Fri Sep 13 11:52:51 2024 -0500
GROOVY-8600, GROOVY-11366: SC: implicit-`this` closure field call
---
.../sc/transformers/MethodCallExpressionTransformer.java | 8 +++-----
.../sc/transformers/StaticCompilationTransformer.java | 12 ++----------
.../bugs/{Groovy8600Bug.groovy => Groovy8600.groovy} | 16 ++++++++--------
3 files changed, 13 insertions(+), 23 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/transform/sc/transformers/MethodCallExpressionTransformer.java b/src/main/java/org/codehaus/groovy/transform/sc/transformers/MethodCallExpressionTransformer.java
index 095055b191..d24c32571a 100644
--- a/src/main/java/org/codehaus/groovy/transform/sc/transformers/MethodCallExpressionTransformer.java
+++ b/src/main/java/org/codehaus/groovy/transform/sc/transformers/MethodCallExpressionTransformer.java
@@ -31,8 +31,6 @@ import org.codehaus.groovy.transform.stc.ExtensionMethodNode;
import org.codehaus.groovy.transform.stc.StaticTypeCheckingVisitor;
import org.codehaus.groovy.transform.stc.StaticTypesMarker;
-import java.util.Optional;
-
import static org.apache.groovy.ast.tools.ClassNodeUtils.getField;
import static org.codehaus.groovy.classgen.AsmClassGenerator.argumentSize;
import static org.objectweb.asm.Opcodes.ACC_PUBLIC;
@@ -64,10 +62,10 @@ class MethodCallExpressionTransformer {
}
if (isCallOnClosure(mce)) {
- var field = Optional.ofNullable(scTransformer.getClassNode()).map(cn -> getField(cn, mce.getMethodAsString()));
- if (field.isPresent()) {
+ var field = getField(scTransformer.getClassNode(), mce.getMethodAsString());
+ if (field != null) {
var closureFieldCall = new MethodCallExpression(
- new VariableExpression(field.get()),
+ new VariableExpression(field),
"call",
scTransformer.transform(arguments));
// implicit-this "field(args)" expression has no place for safe, spread-safe, or type arguments
diff --git a/src/main/java/org/codehaus/groovy/transform/sc/transformers/StaticCompilationTransformer.java b/src/main/java/org/codehaus/groovy/transform/sc/transformers/StaticCompilationTransformer.java
index 1b8e6fcc0d..577645f9bc 100644
--- a/src/main/java/org/codehaus/groovy/transform/sc/transformers/StaticCompilationTransformer.java
+++ b/src/main/java/org/codehaus/groovy/transform/sc/transformers/StaticCompilationTransformer.java
@@ -22,7 +22,6 @@ import org.apache.groovy.util.Maps;
import org.codehaus.groovy.ast.ClassCodeExpressionTransformer;
import org.codehaus.groovy.ast.ClassHelper;
import org.codehaus.groovy.ast.ClassNode;
-import org.codehaus.groovy.ast.InnerClassNode;
import org.codehaus.groovy.ast.MethodNode;
import org.codehaus.groovy.ast.expr.BinaryExpression;
import org.codehaus.groovy.ast.expr.BooleanExpression;
@@ -43,7 +42,6 @@ import org.codehaus.groovy.runtime.ScriptBytecodeAdapter;
import org.codehaus.groovy.syntax.Types;
import org.codehaus.groovy.transform.stc.StaticTypeCheckingVisitor;
-import java.util.Iterator;
import java.util.Map;
/**
@@ -64,7 +62,6 @@ public class StaticCompilationTransformer extends ClassCodeExpressionTransformer
Types.COMPARE_TO, BYTECODE_ADAPTER_CLASS.getMethods("compareTo").get(0)
);
- private ClassNode classNode;
private final SourceUnit unit;
private final StaticTypesTypeChooser typeChooser = new StaticTypesTypeChooser();
@@ -98,7 +95,7 @@ public class StaticCompilationTransformer extends ClassCodeExpressionTransformer
}
public ClassNode getClassNode() {
- return classNode;
+ return staticCompilationVisitor.getTypeCheckingContext().getEnclosingClassNode();
}
@Override
@@ -153,13 +150,8 @@ public class StaticCompilationTransformer extends ClassCodeExpressionTransformer
@Override
public void visitClass(final ClassNode node) {
- ClassNode prev = classNode;
- classNode = node;
super.visitClass(node);
- for (Iterator<InnerClassNode> innerClasses = classNode.getInnerClasses(); innerClasses.hasNext();) {
- visitClass(innerClasses.next());
- }
- classNode = prev;
+ node.getInnerClasses().forEachRemaining(this::visitClass);
}
@Override
diff --git a/src/test/groovy/bugs/Groovy8600Bug.groovy b/src/test/groovy/bugs/Groovy8600.groovy
similarity index 78%
rename from src/test/groovy/bugs/Groovy8600Bug.groovy
rename to src/test/groovy/bugs/Groovy8600.groovy
index 17fccfcfc2..a71490149f 100644
--- a/src/test/groovy/bugs/Groovy8600Bug.groovy
+++ b/src/test/groovy/bugs/Groovy8600.groovy
@@ -18,22 +18,22 @@
*/
package groovy.bugs
-import groovy.test.GroovyTestCase
+import org.junit.Test
-class Groovy8600Bug extends GroovyTestCase {
+import static groovy.test.GroovyAssert.assertScript
+
+final class Groovy8600 {
+ @Test
void testClosureFieldUseWithinMethod() {
assertScript '''
- import groovy.transform.CompileStatic
-
- class TestCompileStatic {
+ class C {
private Closure<String> func = { it.toUpperCase() }
- @CompileStatic
- def testCallFunc () {
+ @groovy.transform.CompileStatic m() {
func('foo') + func.call('bar')
}
}
- assert new TestCompileStatic().testCallFunc() == 'FOOBAR'
+ assert new C().m() == 'FOOBAR'
'''
}
}
(groovy) 02/02: GROOVY-5881, GROOVY-6324: defer to metaclass for closure call
Posted by em...@apache.org.
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 <er...@thomsonreuters.com>
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()
}
+@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 {
@@ -1137,8 +1134,7 @@ interface SomeInterface4619 {
interface SomeOtherInterface4619 extends SomeInterface4619 {}
class SomeClass4619 {
- @Delegate
- SomeOtherInterface4619 delegate
+ @Delegate SomeOtherInterface4619 delegate
}
interface BarInt {