On Tue, 14 Mar 2023 15:27:50 GMT, Adam Sotona <[email protected]> wrote:
>> jdk.jlink internal plugins are heavily using ASM
>>
>> This patch converts ASM calls to Classfile API.
>>
>> Please review.
>> Thanks,
>> Adam
>
> Adam Sotona has updated the pull request incrementally with three additional
> commits since the last revision:
>
> - long lines wrapped
> - StripJavaDebugAttributesPlugin transformation fixed
> - VersionPropsPlugin transformation fixed
src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/SystemModulesPlugin.java
line 614:
> 612: private void genConstructor(ClassBuilder clb) {
> 613: clb.withMethod("<init>", MethodTypeDesc.of(CD_void),
> 614: ACC_PUBLIC, mb -> mb.withFlags(ACC_PUBLIC).withCode(
> cob -> {
Why `withFlags(ACC_PUBLIC)` when the flags is already given in `withMethod`'s
3rd argument? Same for occurrences below.
src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/SystemModulesPlugin.java
line 615:
> 613: clb.withMethod("<init>", MethodTypeDesc.of(CD_void),
> 614: ACC_PUBLIC, mb -> mb.withFlags(ACC_PUBLIC).withCode(
> cob -> {
> 615: cob.loadInstruction(TypeKind.ReferenceType, 0);
What's the basis for using `loadInstruction(ReferenceType, 0)` over `aload(0)`,
etc.? I think, at least for constructors, we won't have other TypeKinds, so we
can safely use `aload(0)` `return_()`, and this is what the original ASM code
used.
src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/SystemModulesPlugin.java
line 894:
> 892: // use Set.of(Object[]) when there are more than 2 elements
> 893: // use Set.of(Object) or Set.of(Object, Object) when fewer
> 894: if (size > 2) {
Set.of can take up to 10 arguments, but this may be off topic.
src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/SystemModulesPlugin.java
line 916:
> 914: sb.append("Ljava/lang/Object;");
> 915: }
> 916: sb.append(")Ljava/util/Set;");
The construction of the Set.of method type should move away from StringBuilder.
It can be as:
var mtdArgs = new ClassDesc[size];
Arrays.fill(mtdArgs, CD_Object);
MethodTypeDesc mtd = MethodTypeDesc.of(CD_Set, mtdArgs);
-------------
PR: https://git.openjdk.org/jdk/pull/12944
