[ 
https://issues.apache.org/jira/browse/GROOVY-12363?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18111979#comment-18111979
 ] 

ASF GitHub Bot commented on GROOVY-12363:
-----------------------------------------

codecov-commenter commented on PR #2887:
URL: https://github.com/apache/groovy/pull/2887#issuecomment-5556796564

   ## 
[Codecov](https://app.codecov.io/gh/apache/groovy/pull/2887?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
 Report
   :x: Patch coverage is `87.50000%` with `2 lines` in your changes missing 
coverage. Please review.
   :white_check_mark: Project coverage is 71.1013%. Comparing base 
([`0c4dd93`](https://app.codecov.io/gh/apache/groovy/commit/0c4dd93ee000639493bdaf480edbd05f02633fd8?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache))
 to head 
([`9c85780`](https://app.codecov.io/gh/apache/groovy/commit/9c85780beb40e03e6b66a25a5f3d388a77ffcae2?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
   :warning: Report is 3 commits behind head on master.
   
   | [Files with missing 
lines](https://app.codecov.io/gh/apache/groovy/pull/2887?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
 | Patch % | Lines |
   |---|---|---|
   | 
[.../groovy/transform/sc/StaticCompilationVisitor.java](https://app.codecov.io/gh/apache/groovy/pull/2887?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Ftransform%2Fsc%2FStaticCompilationVisitor.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L3RyYW5zZm9ybS9zYy9TdGF0aWNDb21waWxhdGlvblZpc2l0b3IuamF2YQ==)
 | 88.8889% | [0 Missing and 1 partial :warning: 
](https://app.codecov.io/gh/apache/groovy/pull/2887?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
 |
   | 
[...oovy/transform/sc/StaticCompileTransformation.java](https://app.codecov.io/gh/apache/groovy/pull/2887?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Ftransform%2Fsc%2FStaticCompileTransformation.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L3RyYW5zZm9ybS9zYy9TdGF0aWNDb21waWxlVHJhbnNmb3JtYXRpb24uamF2YQ==)
 | 85.7143% | [0 Missing and 1 partial :warning: 
](https://app.codecov.io/gh/apache/groovy/pull/2887?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
 |
   
   <details><summary>Additional details and impacted files</summary>
   
   
   
   [![Impacted file tree 
graph](https://app.codecov.io/gh/apache/groovy/pull/2887/graphs/tree.svg?width=650&height=150&src=pr&token=1r45138NfQ&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)](https://app.codecov.io/gh/apache/groovy/pull/2887?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
   
   ```diff
   @@                Coverage Diff                 @@
   ##               master      #2887        +/-   ##
   ==================================================
   + Coverage     71.1006%   71.1013%   +0.0007%     
   - Complexity      37348      37350         +2     
   ==================================================
     Files            1579       1579                
     Lines          135477     135487        +10     
     Branches        25104      25106         +2     
   ==================================================
   + Hits            96325      96333         +8     
     Misses          30478      30478                
   - Partials         8674       8676         +2     
   ```
   
   | [Files with missing 
lines](https://app.codecov.io/gh/apache/groovy/pull/2887?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
 | Coverage Δ | |
   |---|---|---|
   | 
[.../groovy/transform/sc/StaticCompilationVisitor.java](https://app.codecov.io/gh/apache/groovy/pull/2887?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Ftransform%2Fsc%2FStaticCompilationVisitor.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L3RyYW5zZm9ybS9zYy9TdGF0aWNDb21waWxhdGlvblZpc2l0b3IuamF2YQ==)
 | `83.9506% <88.8889%> (-0.0830%)` | :arrow_down: |
   | 
[...oovy/transform/sc/StaticCompileTransformation.java](https://app.codecov.io/gh/apache/groovy/pull/2887?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Ftransform%2Fsc%2FStaticCompileTransformation.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L3RyYW5zZm9ybS9zYy9TdGF0aWNDb21waWxlVHJhbnNmb3JtYXRpb24uamF2YQ==)
 | `88.3721% <85.7143%> (+1.5300%)` | :arrow_up: |
   
   ... and [211 files with indirect coverage 
changes](https://app.codecov.io/gh/apache/groovy/pull/2887/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
   </details>
   <details><summary> :rocket: New features to boost your workflow: </summary>
   
   - :snowflake: [Test 
Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, 
report on failures, and find test suite problems.
   - :package: [JS Bundle 
Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save 
yourself from yourself by tracking and limiting bundle sizes in JS merges.
   </details>




> 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)

Reply via email to