RFR: 8294972: Convert jdk.jlink internal plugins to use the Classfile API [v6]
Adam Sotona
asotona at openjdk.org
Wed Mar 15 16:43:00 UTC 2023
On Tue, 14 Mar 2023 16:34:59 GMT, Chen Liang <liach at openjdk.org> wrote:
>> 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.
It has been redundant, now more effective `withMethodBody` method is used.
> 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.
The whole syntax used in `SystemModulesPlugin` deserved a refresh. `loadInstruction(ReferenceType, 0)` is a generic pattern used before more convenient `aload(0)` has been added to `CodeBuilder`.
Now it is fixed, thanks.
> 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);
Applied, thanks.
-------------
PR: https://git.openjdk.org/jdk/pull/12944
More information about the core-libs-dev
mailing list