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

[Adam Sotona]

On Fri, 10 Mar 2023 14:28:09 GMT, liach <duke at openjdk.org> wrote:

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

yes, `mapToInt` seems to be better, thanks.

> 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(...

fixed, thanks.

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

fixed, thanks.

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

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