[
https://issues.apache.org/jira/browse/GROOVY-12544?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18134119#comment-18134119
]
ASF GitHub Bot commented on GROOVY-12544:
-----------------------------------------
codecov-commenter commented on PR #3077:
URL: https://github.com/apache/groovy/pull/3077#issuecomment-6493393538
##
[Codecov](https://app.codecov.io/gh/apache/groovy/pull/3077?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 73.6063%. Comparing base
([`d82dc98`](https://app.codecov.io/gh/apache/groovy/commit/d82dc98590867204f7ffbf8fb68b473b68cd6d72?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache))
to head
([`9c51de6`](https://app.codecov.io/gh/apache/groovy/commit/9c51de63674918edde19f2e37390b1cb38ebf8c8?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 6 commits behind head on master.
<details><summary>Additional details and impacted files</summary>
[](https://app.codecov.io/gh/apache/groovy/pull/3077?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
```diff
@@ Coverage Diff @@
## master #3077 +/- ##
==================================================
+ Coverage 73.5991% 73.6063% +0.0073%
- Complexity 42419 42431 +12
==================================================
Files 1677 1677
Lines 146688 146717 +29
Branches 27723 27732 +9
==================================================
+ Hits 107961 107993 +32
+ Misses 28613 28611 -2
+ Partials 10114 10113 -1
```
| [Files with missing
lines](https://app.codecov.io/gh/apache/groovy/pull/3077?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
| Coverage Δ | |
|---|---|---|
|
[.../codehaus/groovy/ast/decompiled/AsmDecompiler.java](https://app.codecov.io/gh/apache/groovy/pull/3077?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Fast%2Fdecompiled%2FAsmDecompiler.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L2FzdC9kZWNvbXBpbGVkL0FzbURlY29tcGlsZXIuamF2YQ==)
| `96.5116% <100.0000%> (+0.5657%)` | :arrow_up: |
|
[.../org/codehaus/groovy/ast/decompiled/ClassStub.java](https://app.codecov.io/gh/apache/groovy/pull/3077?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Fast%2Fdecompiled%2FClassStub.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L2FzdC9kZWNvbXBpbGVkL0NsYXNzU3R1Yi5qYXZh)
| `95.0000% <100.0000%> (+0.0847%)` | :arrow_up: |
|
[...aus/groovy/ast/decompiled/DecompiledClassNode.java](https://app.codecov.io/gh/apache/groovy/pull/3077?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Fast%2Fdecompiled%2FDecompiledClassNode.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L2FzdC9kZWNvbXBpbGVkL0RlY29tcGlsZWRDbGFzc05vZGUuamF2YQ==)
| `89.0000% <100.0000%> (+0.5790%)` | :arrow_up: |
|
[...rg/codehaus/groovy/classgen/AsmClassGenerator.java](https://app.codecov.io/gh/apache/groovy/pull/3077?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Fclassgen%2FAsmClassGenerator.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L2NsYXNzZ2VuL0FzbUNsYXNzR2VuZXJhdG9yLmphdmE=)
| `85.7456% <100.0000%> (+0.1155%)` | :arrow_up: |
... and [7 files with indirect coverage
changes](https://app.codecov.io/gh/apache/groovy/pull/3077/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>
> InnerClasses attribute omits nested classes that are not in the current
> compilation unit
> ----------------------------------------------------------------------------------------
>
> Key: GROOVY-12544
> URL: https://issues.apache.org/jira/browse/GROOVY-12544
> Project: Groovy
> Issue Type: Improvement
> Components: Compiler
> Affects Versions: 4.0.33, 5.1.3, 6.0.0
> Reporter: Paul King
> Priority: Major
> Fix For: 7.x
>
>
> groovyc writes an {{InnerClasses}} entry for a referenced nested class only
> when that class is in the same compilation unit. A nested class read from the
> classpath, whether a pre-compiled Groovy class or a JDK class such as
> {{Map.Entry}}, gets no entry. javac writes an entry for every nested class
> referenced from the class.
> h3. Reproducer
> {code:groovy}
> // A.groovy
> class A { static class B {} }
> // C.groovy
> class C { def m() { new A.B() } }
> {code}
> {noformat}
> groovyc -d together A.groovy C.groovy
> groovyc -d first A.groovy
> groovyc -d second -cp first C.groovy
> {noformat}
> {{together/C.class}} contains:
> {noformat}
> InnerClasses:
> public static #34= #31 of #33; // B=class A$B of class A
> {noformat}
> {{second/C.class}} has no {{InnerClasses}} attribute. The extra constant-pool
> entries in the first case also shift later indices, so instructions can
> change too, e.g. {{ldc}} becomes {{ldc_w}}.
> For comparison, javac compiling
> {code:java}
> class Y { Object m(java.util.Map.Entry e) { return new
> java.util.AbstractMap.SimpleEntry(1, 2); } }
> {code}
> writes entries for both {{AbstractMap$SimpleEntry}} and {{Map$Entry}}. The
> equivalent Groovy class has none.
> h3. Why it matters
> * *Reproducible output:* compiling an unchanged source file gives different
> bytes depending on what else is in the compilation unit. Incremental builds
> recompile a subset of sources against the previous output, so unchanged
> classes come out different.
> * *Downstream recompilation:* in the Gradle project from GROOVY-10932
> (https://github.com/jdaugherty/groovy-trait-incremental-compile), with the
> GROOVY-10932 fix applied, the missing {{InnerClasses}} entry for a trait
> helper is the only remaining difference in the implementer. It is still
> enough to make the downstream project recompile. A control run where the
> class came out byte-identical left the downstream compile up to date. Trait
> helpers are only one case: any class referencing a nested type behaves the
> same way.
> * *Spec:* JVMS §4.7.6 expects every nested class referenced from the constant
> pool to be listed in {{InnerClasses}}.
> Runtime behaviour does not appear to be affected. Reflection on a nested
> class ({{getDeclaringClass}}, {{getSimpleName}}, ...) reads that class's own
> {{InnerClasses}} attribute, not the attribute of the classes that reference
> it.
> h3. Scripts
> Scripts behave the same way. A script body that references a nested class
> gets an {{InnerClasses}} entry only when that class is in the same
> compilation unit. Classes declared alongside the script body in a script file
> are top-level siblings rather than nested classes, so they need no entry and
> do not drift. Classes nested inside those siblings do drift, like any other
> nested class. No special handling for scripts is needed.
> h3. Possible approach
> * *Decompiler:* {{AsmDecompiler}} already reads a class's self-referencing
> {{InnerClasses}} entry to get its modifiers. It could also keep the outer
> class name and simple name in {{ClassStub}}.
> * *Class generator:* {{AsmClassGenerator.maybeInnerClassEntry}} currently
> writes an entry only when {{getOuterClass() != null}}, which holds only for
> {{InnerClassNode}}s. It could also write one for a member class read from the
> classpath, recursing through enclosing classes as {{makeInnerClassEntry}}
> does today (GROOVY-9842). For class nodes built by reflection,
> {{Class.getDeclaringClass()}} provides the same information.
> * *Keep {{getOuterClass()}} unchanged:* it should keep returning {{null}} for
> classpath classes. Name resolution, static type checking and access checks
> rely on it, so the change should stay confined to bytecode generation.
> h3. Compatibility
> This changes the bytes of most class files that reference a nested type,
> including commonly used JDK types such as {{Map.Entry}}. Behaviour is not
> expected to change, but builds comparing outputs across versions will see
> differences. I suggest master only, with a note in the release notes.
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
