RFR: 8328111: Convert Runtime tests to use the ClassFile API instead of ASM [v20]

Oussama Louati duke at openjdk.org
Tue May 28 16:30:22 UTC 2024


On Tue, 28 May 2024 15:12:55 GMT, Chen Liang <liach at openjdk.org> wrote:

>> Oussama Louati has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   resolve review comments
>
> test/hotspot/jtreg/runtime/ConstantPool/BadMethodHandles.java line 50:
> 
>> 48:     private static final ClassDesc CD_System = ClassDesc.of("java.lang.System");
>> 49:     private static final ClassDesc CD_PrintStream = ClassDesc.of("java.io.PrintStream");
>> 50:     private static final ClassDesc CD_LPrintStream = ClassDesc.ofDescriptor("Ljava/io/PrintStream;");
> 
> This is the same as `CD_PrintStream`, so no need for two constants.

Thank you, changes applied.

> test/hotspot/jtreg/runtime/ConstantPool/BadMethodHandles.java line 154:
> 
>> 152: 
>> 153:         DirectMethodHandleDesc handle = MethodHandleDesc.of(DirectMethodHandleDesc.Kind.VIRTUAL,
>> 154:                 ClassDesc.of("InvokeBasicref"), "runInvokeBasicM", "()V");
> 
> Suggestion:
> 
>                 CD_MethodHandle, "invokeBasic", "([Ljava/lang/Object;)Ljava/lang/Object;");

Changes applied, thank you

> test/hotspot/jtreg/runtime/ConstantPool/IntfMethod.java line 72:
> 
>> 70:                                 cob -> cob
>> 71:                                         .aload(0)
>> 72:                                         .invokespecial(cob.constantPool().interfaceMethodRefEntry(ClassDesc.of("C"), "f1", MTD_void))
> 
> Suggestion:
> 
>                                         .invokespecial(ClassDesc.of("C"), "f1", MTD_void, true)

Thank you, done

> test/hotspot/jtreg/runtime/ConstantPool/IntfMethod.java line 78:
> 
>> 76:                         .withMethodBody("testStaticClass", MTD_void, ACC_PUBLIC,
>> 77:                                 cob -> cob
>> 78:                                         .invokestatic(cob.constantPool().interfaceMethodRefEntry(ClassDesc.of("C"), "f2", MTD_void))
> 
> Suggestion:
> 
>                                         .invokestatic(ClassDesc.of("C"), "f2", MTD_void, true)

Done

> test/hotspot/jtreg/runtime/verifier/TestANewArray.java line 110:
> 
>> 108:         classCName = "classCName_" + cfv + "_" + testDimension264;
>> 109: 
>> 110:         return ClassFile.of(ClassFile.StackMapsOption.DROP_STACK_MAPS).build(ClassDesc.of(classCName),
> 
> Suggestion:
> 
>         return ClassFile.of().build(ClassDesc.of(classCName),
> 
> The original ClassWriter has `COMPUTE_FRAMES` flag.

here we are testing anewarray instruction with 254, 255 & 264 dimensions to verify JVM Spec, so we don't need stackmaps for the class file as it won't allow us to create array type descriptor with more than 255 dimensions.

> test/hotspot/jtreg/runtime/verifier/TestANewArray.java line 123:
> 
>> 121:                                     cob -> cob
>> 122:                                             .bipush(1)
>> 123:                                             .anewarray(cob.constantPool().classEntry(cob.constantPool().utf8Entry(arrayType)))
> 
> Suggestion:
> 
>                                             .anewarray(ClassDesc.ofDescriptor(arrayType))

throws an illegalArgs exception otherwise.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18271#discussion_r1617562747
PR Review Comment: https://git.openjdk.org/jdk/pull/18271#discussion_r1617566989
PR Review Comment: https://git.openjdk.org/jdk/pull/18271#discussion_r1617570713
PR Review Comment: https://git.openjdk.org/jdk/pull/18271#discussion_r1617570427
PR Review Comment: https://git.openjdk.org/jdk/pull/18271#discussion_r1617581616
PR Review Comment: https://git.openjdk.org/jdk/pull/18271#discussion_r1617583681


More information about the hotspot-runtime-dev mailing list