RFR: 8294962: java.base jdk.internal.module package uses ASM to modify and write module-info.class [v3]

[liach]

On Fri, 10 Mar 2023 08:46:16 GMT, Adam Sotona <asotona at openjdk.org> wrote:

>> 8294962: java.base jdk.internal.module package uses ASM to modify and write module-info.class.
>> This patch converts it to use Classfile API.
>> 
>> Please review.
>> Thanks,
>> Adam
>
> Adam Sotona has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 198 commits:
> 
>  - ModuleInfoWriter fixes
>  - ModuleInfoExtender comment fix
>  - Merge branch 'master' into JDK-8294962-internal-module
>  - Update src/java.base/share/classes/jdk/internal/module/ModuleInfoExtender.java
>    
>    Co-authored-by: Mandy Chung <mandy.chung at oracle.com>
>  - Merge branch 'master' into JDK-8294962-internal-module
>  - fixed jdk.internal.classfile.java.lang.constant package name
>  - Merge branch 'JDK-8294982' into JDK-8294962
>  - removed obsolete javadoc from implementation classes
>  - minor fix in CodeBuilder and added test cases to LDCTest
>  - EntryMap::nextPowerOfTwo delegates to Long:numberOfLeadingZeros
>  - ... and 188 more: https://git.openjdk.org/jdk/compare/e26cc526...9e366fbd

src/java.base/share/classes/jdk/internal/module/ModuleInfoWriter.java line 88:

> 86:                 ModuleAttribute.of(ModuleDesc.of(md.name()), mb -> {
> 87:                     mb.moduleFlags(md.modifiers().stream()
> 88:                             .map(mm -> MODULE_MODS_TO_FLAGS.getOrDefault(mm, 0))

Should we use `mapToInt` instead of a plain `map`? Same for the other flags below.

src/java.base/share/classes/jdk/internal/module/ModuleInfoWriter.java line 124:

> 122: 
> 123:                     // uses
> 124:                     md.uses().stream().map(sn -> ClassDesc.of(sn)).forEach(mb::uses);

map(ClassDesc::of).forEach(...

src/java.base/share/classes/jdk/internal/module/ModuleInfoWriter.java line 150:

> 148: 
> 149:                     // write ModuleTarget attribute if there is a target platform
> 150:                     if (target != null && target.targetPlatform().length() > 0) {

`!target.targetPlatform().isEmpty()` is probably better than a length check.

-------------

PR: https://git.openjdk.org/jdk/pull/11368