This is an automated email from the ASF dual-hosted git repository. emilles pushed a commit to branch GROOVY-11817 in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 965272c48d1934ba6be18a2eae9d9c00ceace4a0 Author: Eric Milles <[email protected]> AuthorDate: Mon Dec 15 11:11:42 2025 -0600 GROOVY-11817: SC: dynamic property or method call keeps method static - prevent static invocation writer and dynamic call site writer error --- .../groovy/classgen/AsmClassGenerator.java | 11 +++--- .../groovy/classgen/asm/DelegatingController.java | 6 ++++ .../groovy/classgen/asm/WriterController.java | 5 +++ .../classgen/asm/sc/StaticInvocationWriter.java | 7 ++-- .../classgen/asm/sc/StaticTypesClosureWriter.java | 13 +++---- .../asm/sc/StaticTypesWriterController.java | 15 +++++--- .../stc/AbstractTypeCheckingExtension.java | 11 +----- .../transform/stc/StaticTypeCheckingVisitor.java | 4 +-- .../asm/sc/MixedModeStaticCompilationTest.groovy | 40 ++++++++++++++++++---- 9 files changed, 71 insertions(+), 41 deletions(-) diff --git a/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java b/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java index fb107c8bd1..3869830267 100644 --- a/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java +++ b/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java @@ -96,6 +96,7 @@ import org.codehaus.groovy.ast.stmt.TryCatchStatement; import org.codehaus.groovy.ast.stmt.WhileStatement; import org.codehaus.groovy.classgen.asm.BytecodeHelper; import org.codehaus.groovy.classgen.asm.BytecodeVariable; +import org.codehaus.groovy.classgen.asm.CallSiteWriter; import org.codehaus.groovy.classgen.asm.CompileStack; import org.codehaus.groovy.classgen.asm.MethodCaller; import org.codehaus.groovy.classgen.asm.MethodCallerMultiAdapter; @@ -1179,12 +1180,14 @@ public class AsmClassGenerator extends ClassGenerator { return; } - if (adapter == getGroovyObjectProperty && propertyName != null && !pexp.isSpreadSafe()) { // TODO: spread safe should be handled by each - controller.getCallSiteWriter().makeGroovyObjectGetPropertySite(objectExpression, propertyName, pexp.isSafe(), pexp.isImplicitThis()); + CallSiteWriter callSiteWriter = controller.getCallSiteWriterFor(pexp); // GROOVY-11817 + + if (adapter == getGroovyObjectProperty && propertyName != null && !pexp.isSpreadSafe()) { // TODO: propagate spread safe + callSiteWriter.makeGroovyObjectGetPropertySite(objectExpression, propertyName, pexp.isSafe(), pexp.isImplicitThis()); } else if (adapter == getProperty && propertyName != null && !pexp.isSpreadSafe()) { - controller.getCallSiteWriter().makeGetPropertySite(objectExpression, propertyName, pexp.isSafe(), pexp.isImplicitThis()); + callSiteWriter.makeGetPropertySite(objectExpression, propertyName, pexp.isSafe(), pexp.isImplicitThis()); } else { - controller.getCallSiteWriter().fallbackAttributeOrPropertySite(pexp, objectExpression, propertyName, adapter); + callSiteWriter.fallbackAttributeOrPropertySite(pexp, objectExpression, propertyName, adapter); } } diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/DelegatingController.java b/src/main/java/org/codehaus/groovy/classgen/asm/DelegatingController.java index 3029237535..031fd27246 100644 --- a/src/main/java/org/codehaus/groovy/classgen/asm/DelegatingController.java +++ b/src/main/java/org/codehaus/groovy/classgen/asm/DelegatingController.java @@ -22,6 +22,7 @@ import org.codehaus.groovy.ast.ClassNode; import org.codehaus.groovy.ast.ConstructorNode; import org.codehaus.groovy.ast.InterfaceHelperClassNode; import org.codehaus.groovy.ast.MethodNode; +import org.codehaus.groovy.ast.expr.Expression; import org.codehaus.groovy.classgen.AsmClassGenerator; import org.codehaus.groovy.classgen.GeneratorContext; import org.codehaus.groovy.control.SourceUnit; @@ -64,6 +65,11 @@ public class DelegatingController extends WriterController { return delegationController.getCallSiteWriter(); } + @Override + public CallSiteWriter getCallSiteWriterFor(final Expression expression) { + return delegationController.getCallSiteWriterFor(expression); + } + @Override public StatementWriter getStatementWriter() { return delegationController.getStatementWriter(); diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/WriterController.java b/src/main/java/org/codehaus/groovy/classgen/asm/WriterController.java index 6c26671836..0968583dd8 100644 --- a/src/main/java/org/codehaus/groovy/classgen/asm/WriterController.java +++ b/src/main/java/org/codehaus/groovy/classgen/asm/WriterController.java @@ -23,6 +23,7 @@ import org.codehaus.groovy.ast.ClassNode; import org.codehaus.groovy.ast.ConstructorNode; import org.codehaus.groovy.ast.InterfaceHelperClassNode; import org.codehaus.groovy.ast.MethodNode; +import org.codehaus.groovy.ast.expr.Expression; import org.codehaus.groovy.classgen.AsmClassGenerator; import org.codehaus.groovy.classgen.GeneratorContext; import org.codehaus.groovy.classgen.asm.indy.IndyBinHelper; @@ -210,6 +211,10 @@ public class WriterController { return callSiteWriter; } + public CallSiteWriter getCallSiteWriterFor(final Expression expression) { + return callSiteWriter; + } + public ClosureWriter getClosureWriter() { return closureWriter; } 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 816df9717a..914ec67217 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 @@ -469,11 +469,8 @@ public class StaticInvocationWriter extends InvocationWriter { @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) { if (origin.getNodeMetaData(StaticTypesMarker.DYNAMIC_RESOLUTION) != null) { - StaticTypesWriterController staticController = (StaticTypesWriterController) controller; - if (origin instanceof MethodCallExpression) { - ((MethodCallExpression) origin).setMethodTarget(null); - } - InvocationWriter dynamicInvocationWriter = staticController.getRegularInvocationWriter(); + if (origin instanceof MethodCallExpression) ((MethodCallExpression) origin).setMethodTarget(null); // ensure dynamic resolve + InvocationWriter dynamicInvocationWriter = ((StaticTypesWriterController) controller).getRegularInvocationWriter(); dynamicInvocationWriter.makeCall(origin, receiver, message, arguments, adapter, safe, spreadSafe, implicitThis); return; } diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesClosureWriter.java b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesClosureWriter.java index a1ad0da2d8..b2e9dda3e7 100644 --- a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesClosureWriter.java +++ b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesClosureWriter.java @@ -32,7 +32,6 @@ import org.codehaus.groovy.classgen.asm.ClosureWriter; import org.codehaus.groovy.classgen.asm.WriterController; import org.codehaus.groovy.control.SourceUnit; import org.codehaus.groovy.transform.sc.StaticCompilationMetadataKeys; -import org.codehaus.groovy.transform.stc.StaticTypesMarker; import org.objectweb.asm.Opcodes; import java.util.Collections; @@ -58,19 +57,15 @@ public class StaticTypesClosureWriter extends ClosureWriter { protected ClassNode createClosureClass(final ClosureExpression expression, final int mods) { ClassNode closureClass = super.createClosureClass(expression, mods); List<MethodNode> methods = closureClass.getDeclaredMethods("call"); - List<MethodNode> doCall = closureClass.getMethods("doCall"); - if (doCall.size() != 1) { - throw new GroovyBugError("Expected to find one (1) doCall method on generated closure, but found " + doCall.size()); + List<MethodNode> doCalls = closureClass.getMethods("doCall"); + if (doCalls.size() != 1) { + throw new GroovyBugError("Expected to find one (1) doCall method on generated closure, but found " + doCalls.size()); } - MethodNode doCallMethod = doCall.get(0); + MethodNode doCallMethod = doCalls.get(0); if (methods.isEmpty() && doCallMethod.getParameters().length == 1) { createDirectCallMethod(closureClass, doCallMethod); } MethodTargetCompletionVisitor visitor = new MethodTargetCompletionVisitor(doCallMethod); - Object dynamic = expression.getNodeMetaData(StaticTypesMarker.DYNAMIC_RESOLUTION); - if (dynamic != null) { - doCallMethod.putNodeMetaData(StaticTypesMarker.DYNAMIC_RESOLUTION, dynamic); - } for (MethodNode method : methods) { visitor.visitMethod(method); } diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesWriterController.java b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesWriterController.java index 5a7e216c77..78ceed9196 100644 --- a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesWriterController.java +++ b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesWriterController.java @@ -21,6 +21,7 @@ package org.codehaus.groovy.classgen.asm.sc; import org.codehaus.groovy.ast.ClassNode; import org.codehaus.groovy.ast.ConstructorNode; import org.codehaus.groovy.ast.MethodNode; +import org.codehaus.groovy.ast.expr.Expression; import org.codehaus.groovy.classgen.AsmClassGenerator; import org.codehaus.groovy.classgen.GeneratorContext; import org.codehaus.groovy.classgen.asm.BinaryExpressionHelper; @@ -37,10 +38,10 @@ import org.codehaus.groovy.classgen.asm.UnaryExpressionHelper; import org.codehaus.groovy.classgen.asm.WriterController; import org.codehaus.groovy.classgen.asm.indy.sc.IndyStaticTypesMultiTypeDispatcher; import org.codehaus.groovy.control.CompilerConfiguration; -import org.codehaus.groovy.transform.stc.StaticTypesMarker; import org.objectweb.asm.ClassVisitor; import static org.codehaus.groovy.transform.sc.StaticCompilationVisitor.isStaticallyCompiled; +import static org.codehaus.groovy.transform.stc.StaticTypesMarker.DYNAMIC_RESOLUTION; /** * An alternative {@link org.codehaus.groovy.classgen.asm.WriterController} which handles static types and method @@ -103,14 +104,20 @@ public class StaticTypesWriterController extends DelegatingController { @Override public CallSiteWriter getCallSiteWriter() { - MethodNode methodNode = getMethodNode(); - if (isInStaticallyCheckedMethod && (methodNode == null - || !Boolean.TRUE.equals(methodNode.getNodeMetaData(StaticTypesMarker.DYNAMIC_RESOLUTION)))) { + if (isInStaticallyCheckedMethod) { return callSiteWriter; } return super.getCallSiteWriter(); } + @Override + public CallSiteWriter getCallSiteWriterFor(final Expression expression) { + if (expression.getNodeMetaData(DYNAMIC_RESOLUTION) != null) { + return getRegularCallSiteWriter(); // GROOVY-6263 + } + return getCallSiteWriter(); + } + public CallSiteWriter getRegularCallSiteWriter() { return super.getCallSiteWriter(); } diff --git a/src/main/java/org/codehaus/groovy/transform/stc/AbstractTypeCheckingExtension.java b/src/main/java/org/codehaus/groovy/transform/stc/AbstractTypeCheckingExtension.java index abcfb6ed22..5d9ee2121d 100644 --- a/src/main/java/org/codehaus/groovy/transform/stc/AbstractTypeCheckingExtension.java +++ b/src/main/java/org/codehaus/groovy/transform/stc/AbstractTypeCheckingExtension.java @@ -274,14 +274,7 @@ public class AbstractTypeCheckingExtension extends TypeCheckingExtension { * @return a virtual method node with the same name as the expected call */ public MethodNode makeDynamic(MethodCall call, ClassNode returnType) { - TypeCheckingContext.EnclosingClosure enclosingClosure = context.getEnclosingClosure(); - MethodNode enclosingMethod = context.getEnclosingMethod(); - ((ASTNode)call).putNodeMetaData(StaticTypesMarker.DYNAMIC_RESOLUTION, returnType); - if (enclosingClosure!=null) { - enclosingClosure.getClosureExpression().putNodeMetaData(StaticTypesMarker.DYNAMIC_RESOLUTION, Boolean.TRUE); - } else { - enclosingMethod.putNodeMetaData(StaticTypesMarker.DYNAMIC_RESOLUTION, Boolean.TRUE); - } + ((ASTNode) call).putNodeMetaData(StaticTypesMarker.DYNAMIC_RESOLUTION, returnType); setHandled(true); if (debug) { log("Turning " + call.getText() + " into a dynamic method call returning " + StaticTypeCheckingSupport.prettyPrintType(returnType)); @@ -305,7 +298,6 @@ public class AbstractTypeCheckingExtension extends TypeCheckingExtension { * @param returnType the type of the property */ public void makeDynamic(PropertyExpression pexp, ClassNode returnType) { - context.getEnclosingMethod().putNodeMetaData(StaticTypesMarker.DYNAMIC_RESOLUTION, Boolean.TRUE); pexp.putNodeMetaData(StaticTypesMarker.DYNAMIC_RESOLUTION, returnType); storeType(pexp, returnType); setHandled(true); @@ -330,7 +322,6 @@ public class AbstractTypeCheckingExtension extends TypeCheckingExtension { * @param vexp the dynamic variable */ public void makeDynamic(VariableExpression vexp, ClassNode returnType) { - context.getEnclosingMethod().putNodeMetaData(StaticTypesMarker.DYNAMIC_RESOLUTION, Boolean.TRUE); vexp.putNodeMetaData(StaticTypesMarker.DYNAMIC_RESOLUTION, returnType); storeType(vexp, returnType); setHandled(true); diff --git a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java index bf4451ecae..a1ef0d5107 100644 --- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java +++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java @@ -739,7 +739,7 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport { PropertyExpression pexp = thisPropX(true, dynName); if (existsProperty(pexp, !typeCheckingContext.isTargetOfEnclosingAssignment(vexp))) { vexp.copyNodeMetaData(pexp.getObjectExpression()); - for (Object key : new Object[]{IMPLICIT_RECEIVER, READONLY_PROPERTY, PV_FIELDS_ACCESS, PV_FIELDS_MUTATION, DIRECT_METHOD_CALL_TARGET}) { + for (Object key : new Object[]{DIRECT_METHOD_CALL_TARGET, DYNAMIC_RESOLUTION, IMPLICIT_RECEIVER, PV_FIELDS_ACCESS, PV_FIELDS_MUTATION, READONLY_PROPERTY}) { vexp.putNodeMetaData(key, pexp.getNodeMetaData(key)); } ClassNode type = pexp.getNodeMetaData(INFERRED_TYPE); @@ -748,7 +748,7 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport { } if (type == null) type = OBJECT_TYPE; vexp.putNodeMetaData(INFERRED_TYPE, type); // GROOVY-11007 - String receiver = vexp.getNodeMetaData(IMPLICIT_RECEIVER); + String receiver = pexp.getNodeMetaData(IMPLICIT_RECEIVER); Boolean dynamic = pexp.getNodeMetaData(DYNAMIC_RESOLUTION); // GROOVY-7701, GROOVY-7996: correct false assumption made by VariableScopeVisitor if (((receiver != null && !receiver.endsWith("owner")) || Boolean.TRUE.equals(dynamic)) diff --git a/src/test/groovy/org/codehaus/groovy/classgen/asm/sc/MixedModeStaticCompilationTest.groovy b/src/test/groovy/org/codehaus/groovy/classgen/asm/sc/MixedModeStaticCompilationTest.groovy index db6a514c57..3a9b0e59c8 100644 --- a/src/test/groovy/org/codehaus/groovy/classgen/asm/sc/MixedModeStaticCompilationTest.groovy +++ b/src/test/groovy/org/codehaus/groovy/classgen/asm/sc/MixedModeStaticCompilationTest.groovy @@ -186,7 +186,7 @@ final class MixedModeStaticCompilationTest extends StaticTypeCheckingTestCase im ''' } - void testAllowMetaClass() { + void testMetaClassUsage() { assertScript ''' @CompileStatic(extensions='groovy/transform/sc/MixedMode.groovy') void test() { @@ -204,23 +204,49 @@ final class MixedModeStaticCompilationTest extends StaticTypeCheckingTestCase im ''' } - void testRecognizeStaticMethodCall() { + void testDynamicClassMethod1() { assertScript ''' @CompileStatic(extensions='groovy/transform/sc/MixedMode2.groovy') Map<String, Integer> foo() { String.map() } + try { + String.metaClass.static.map = { -> [a:1,b:2] } + assert foo().values().sort() == [1,2] + } finally { + GroovySystem.getMetaClassRegistry().removeMetaClass(String) + } + ''' + } + + void testDynamicClassMethod2() { + assertScript ''' @CompileStatic(extensions='groovy/transform/sc/MixedMode2.groovy') - List<Integer> bar() { + List<Integer> foo() { Date.list() } try { - String.metaClass.static.map = { [a: 1, b:2 ]} - Date.metaClass.static.list = { [1,2] } - assert foo().values().sort() == bar() + Date.metaClass.static.list = { -> [1,2] } + assert foo() == [1,2] + } finally { + GroovySystem.getMetaClassRegistry().removeMetaClass(Date) + } + ''' + } + + // GROOVY-11817 + void testDynamicClassMethod3() { + assertScript ''' + @CompileStatic(extensions='groovy/transform/sc/MixedMode2.groovy') + def foo() { + def list_of_integer = Date.list() + def integer = list_of_integer[0] + } + try { + Date.metaClass.static.list = { -> [1,2] } + assert foo() == 1 } finally { GroovySystem.getMetaClassRegistry().removeMetaClass(Date) - GroovySystem.getMetaClassRegistry().removeMetaClass(String) } ''' }
