On Tue, 14 Oct 2025 23:12:34 GMT, Chen Liang <[email protected]> wrote:
>> One of the goals of ClassFile API is to avoid updating the copy of ASM in >> the JDK (now moved to the test library) to support future class file formats. >> >> However, some tests in hotspot turn out to parse latest class files, usually >> produced by the javac in the JDK under test, to transform them to inject >> desired bytecode patterns. If we keep these tests, we must keep maintaining >> the ASM library to accept all current class files, which will be costly with >> the upcoming project Valhalla. >> >> To avoid maintaining ASM down the road, we can either: >> 1. Migrate the transformation to ClassFile API >> 2. Set source and release version in javac flags to produce stable bytecode >> >> I recommend migrating to ClassFile API; javac has a deprecation policy, that >> in the future, old source and target versions will no longer be supported, >> and we would still need another port at that time. > > Chen Liang has updated the pull request with a new target base due to a merge > or a rebase. The incremental webrev excludes the unrelated changes brought in > by the merge/rebase. The pull request contains 16 additional commits since > the last revision: > > - Merge branch 'master' of https://github.com/openjdk/jdk into > fix/asm-test-upgrade > - Ioi review > - Merge branch 'master' of https://github.com/openjdk/jdk into > fix/asm-test-upgrade > - Merge branch 'master' of https://github.com/openjdk/jdk into > fix/asm-test-upgrade > - Merge branch 'master' of https://github.com/openjdk/jdk into > fix/asm-test-upgrade > - Merge branch 'master' of https://github.com/openjdk/jdk into > fix/asm-test-upgrade > - Merge branch 'fix/asm-test-upgrade' of github.com:liachmodded/jdk into > fix/asm-test-upgrade > - Update test/hotspot/jtreg/compiler/calls/common/InvokeDynamicPatcher.java > > Co-authored-by: Andrew Haley <[email protected]> > - Variable name improvements > - Merge branch 'master' of https://github.com/openjdk/jdk into > fix/asm-test-upgrade > - ... and 6 more: https://git.openjdk.org/jdk/compare/e7527539...25972f2d test/hotspot/jtreg/serviceability/jvmti/RedefineClasses/RedefineAnnotations.java line 94: > 92: public void accept(ClassBuilder builder, ClassElement > element) { > 93: if (element instanceof FieldModel field && > field.fieldName().stringValue().startsWith("dummy")) { > 94: // Remove dummy field Q: Is this dummy field removed or added? Seems to be a mismatch between the comment and the real action. test/hotspot/jtreg/serviceability/jvmti/RedefineClasses/RedefineRetransform/RedefineRetransform.java line 102: > 100: // Extracts ClassVersion values from the provided class bytes. > 101: private static int getClassBytesVersion(byte[] classBytes) { > 102: ClassModel clazz = ClassFile.of().parse(classBytes); Nit: Better to make it consistent with the case below and call it `classModel` instead of `clazz`. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/25124#discussion_r2431212827 PR Review Comment: https://git.openjdk.org/jdk/pull/25124#discussion_r2431217408
