[
https://issues.apache.org/jira/browse/GROOVY-12363?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18112039#comment-18112039
]
ASF GitHub Bot commented on GROOVY-12363:
-----------------------------------------
Copilot commented on code in PR #2887:
URL: https://github.com/apache/groovy/pull/2887#discussion_r3943607395
##########
src/main/java/org/codehaus/groovy/transform/sc/StaticCompilationVisitor.java:
##########
@@ -268,14 +268,27 @@ public void visitMethodCallExpression(final
MethodCallExpression call) {
*/
@Override
public void visitConstructorCallExpression(final ConstructorCallExpression
call) {
- super.visitConstructorCallExpression(call);
-
- if (call.isUsingAnonymousInnerClass() &&
call.getType().getNodeMetaData(StaticTypeCheckingVisitor.class) != null) {
+ if (call.isUsingAnonymousInnerClass()) {
ClassNode anonType = call.getType();
- anonType.putNodeMetaData(STATIC_COMPILE_NODE,
anonType.getEnclosingMethod().getNodeMetaData(STATIC_COMPILE_NODE));
- anonType.putNodeMetaData(WriterControllerFactory.class,
anonType.getOuterClass().getNodeMetaData(WriterControllerFactory.class));
+ if (anonType.getNodeMetaData(STATIC_COMPILE_NODE) == null) {
+ // GROOVY-6925: an anonymous inner class follows its enclosing
method
+ // (visitClass has already decided for the inner classes of an
annotated
+ // class). GROOVY-12363: decide before the type checker visits
the body
+ // below, otherwise its methods are checked while still
counting as
+ // dynamic (no direct call targets, no
SUPER_MOP_METHOD_REQUIRED) and yet
+ // the class is generated as statically compiled, which omits
the super$
+ // MOP bridges a dynamic super call needs.
+ MethodNode enclosingMethod = anonType.getEnclosingMethod();
+ boolean isSC = enclosingMethod != null
+ ? isStaticallyCompiled(enclosingMethod)
+ : isStaticallyCompiled(getEnclosingDeclaration());
+ anonType.putNodeMetaData(STATIC_COMPILE_NODE, isSC);
+ anonType.putNodeMetaData(WriterControllerFactory.class,
anonType.getOuterClass().getNodeMetaData(WriterControllerFactory.class));
+ }
Review Comment:
This null guard can preserve an earlier `false` class-level decision even
when the anonymous class is inside a more-specific `@CompileStatic` method. For
example, `@CompileDynamic class C { @CompileStatic Runnable m() { new
Runnable() { void run() { ... } } } }` is supported (the most-specific
annotation wins), but the outer-class visit records
`STATIC_COMPILE_NODE=false`; this method-level visit then refuses to change it
before checking the body, recreating the dynamic-body/static-writer mismatch.
Recompute the value from the enclosing method whenever this constructor is
actually visited; skipped methods do not reach this hook.
> Anonymous inner class declared in a @CompileStatic method: super call fails
> with MissingMethodException (no MOP bridge generated)
> ---------------------------------------------------------------------------------------------------------------------------------
>
> Key: GROOVY-12363
> URL: https://issues.apache.org/jira/browse/GROOVY-12363
> Project: Groovy
> Issue Type: Bug
> Reporter: Paul King
> Priority: Major
>
> An anonymous inner class declared inside a method annotated with
> {{@CompileStatic}} (the enclosing *class* is not annotated) that calls
> {{super.someMethod(...)}} fails at run time:
> {noformat}
> groovy.lang.MissingMethodException: No signature of method: publish for
> class: java.util.logging.StreamHandler is applicable for argument types:
> (java.util.logging.LogRecord) values: [java.util.logging.LogRecord@783a467b]
> Possible solutions: publish(java.util.logging.LogRecord),
> publish(java.util.logging.LogRecord)
> at
> org.codehaus.groovy.runtime.ScriptBytecodeAdapter.unwrap(ScriptBytecodeAdapter.java:85)
> at
> org.codehaus.groovy.runtime.ScriptBytecodeAdapter.invokeMethodOnSuperN(ScriptBytecodeAdapter.java:156)
> at Holders$1.publish(Repro.groovy:6)
> {noformat}
> h3. Reproducer
> {code:groovy}
> import java.util.logging.*
> class Holders {
> @groovy.transform.CompileStatic
> static Logger csLogger() {
> def l = Logger.getLogger('cs'); l.useParentHandlers = false
> l.addHandler(new StreamHandler(System.out, new SimpleFormatter()) {
> void publish(LogRecord r) { super.publish(r); flush() } })
> l
> }
> static Logger dynLogger() { // identical, no annotation
> def l = Logger.getLogger('dyn'); l.useParentHandlers = false
> l.addHandler(new StreamHandler(System.out, new SimpleFormatter()) {
> void publish(LogRecord r) { super.publish(r); flush() } })
> l
> }
> }
> Holders.dynLogger().info('works')
> Holders.csLogger().info('fails') // MissingMethodException
> {code}
> Putting {{@CompileStatic}} on the class {{Holders}} instead of the method
> makes it work (the super call then compiles to {{invokespecial}}). The JDK
> superclass is not essential; any {{super}} call from such an anonymous class
> takes the same path.
> h3. Root cause
> The two anonymous classes compile to *identical* bytecode for the super call,
> {{ScriptBytecodeAdapter.invokeMethodOnSuperN(Holders$1.class, this,
> "publish", args)}}, i.e. a dynamic MOP super call. That call only works if
> the class carries the compiler-generated {{super$N$publish}} bridge, which
> the MOP uses to reach the superclass implementation
> ({{MetaMethodIndex.methodsForSuper}}). {{javap}} shows the difference:
> {noformat}
> Holders$1 (in @CompileStatic method): public void publish(LogRecord)
> Holders$2 (in dynamic method): public void publish(LogRecord)
> public void super$3$publish(LogRecord)
> {noformat}
> With no bridge, {{MetaClassImpl.getSuperMethodWithCaching}} finds
> {{methodsForSuper == null}} and the call ends in {{invokeMissingMethod}}.
> The inconsistency comes from the order in which {{StaticCompilationVisitor}}
> marks things:
> # {{visitClass(outer)}} marks every inner class, including the anonymous one,
> with {{STATIC_COMPILE_NODE = false}} because the outer class is not
> statically compiled (the {{isSC}} computation over
> {{node.getInnerClasses()}}).
> # The type checker then visits the anonymous class body from the constructor
> call. At that point {{isStaticallyCompiled(publish)}} resolves through the
> declaring class to {{false}}, so the {{super.publish(r)}} call gets no
> {{DIRECT_METHOD_CALL_TARGET}} and no {{SUPER_MOP_METHOD_REQUIRED}} entry is
> recorded.
> # Only afterwards does {{visitConstructorCallExpression}} flip the anonymous
> class to {{STATIC_COMPILE_NODE = true}} (copied from the enclosing method)
> and give it the static {{WriterControllerFactory}}.
> # At class generation the class is therefore treated as statically compiled:
> {{StaticTypesWriterController}} uses {{StaticCompilationMopWriter}}, which
> emits bridges only for the (empty) {{SUPER_MOP_METHOD_REQUIRED}} list, while
> the method body, lacking a direct target, falls back to the dynamic
> {{invokeMethodOnSuperN}}.
> So the class is compiled as "static" for the purpose of omitting MOP bridges
> and as "dynamic" for the purpose of the call itself.
> h3. Expected
> Either outcome is acceptable as long as both halves agree: the anonymous
> class body should be statically compiled like the enclosing method (matching
> the class-level {{@CompileStatic}} behaviour and giving an
> {{invokespecial}}), or, if it stays dynamic, the ordinary {{MopWriter}} must
> still generate the {{super$}} bridges. Marking the anonymous class from the
> enclosing method's flag *before* its body is type-checked (in
> {{visitConstructorCallExpression}} ahead of
> {{super.visitConstructorCallExpression}}, or when {{visitClass}} marks inner
> classes) would give the first.
> Verified on Groovy 6.0.0-SNAPSHOT master and 5.1.2 on JDK 21, and inside a
> GraalVM 25.2.4 native image where it was first noticed.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
