I skimmed over the webrev, the changes seem fine. Thanks for updating the record serialization tests that use ASM.
-Chris. > On 17 Apr 2020, at 21:28, Vicente Romero <vicente.rom...@oracle.com> wrote: > > ping, any other comment? are we ready to push this change? > > Vicente > > On 4/13/20 11:05 AM, Vicente Romero wrote: >> Hi all, >> >> Please review this patch that updates the ASM version to appear in JDK15 >> (from 7.0 to 8.0). Version 8.0 is compatible with JDK14 plus it has several >> experimental APIs to cover records and sealed types. We have also updated a >> number of tests (mostly record oriented tests) to use the updated APIs. >> Thanks to Igor Ignatyev for his help in particular for finding the source >> and fixing an issue that was provoking a failure at test: >> >> open/test/jdk/jdk/jfr/jvm/TestLogOutput.java >> >> This is the only substantial change that makes our copy of ASM different >> from the original, apart from several @SuppressWarnings that were added for >> the build to pass. The patch from Igor is this: >> >> diff -r b2ca6a37c16b >> src/java.base/share/classes/jdk/internal/org/objectweb/asm/Constants.java >> --- >> a/src/java.base/share/classes/jdk/internal/org/objectweb/asm/Constants.java >> Fri Apr 10 07:04:27 2020 -0700 >> +++ >> b/src/java.base/share/classes/jdk/internal/org/objectweb/asm/Constants.java >> Fri Apr 10 17:14:59 2020 -0700 >> @@ -222,15 +222,15 @@ >> } >> >> static boolean isWhitelisted(final String internalName) { >> - if (!internalName.startsWith("org/objectweb/asm/")) { >> + if (!internalName.startsWith("jdk/internal/org/objectweb/asm/")) { >> return false; >> } >> String member = >> "(Annotation|Class|Field|Method|Module|RecordComponent|Signature)"; >> return internalName.contains("Test$") >> || Pattern.matches( >> - "org/objectweb/asm/util/Trace" + member + >> "Visitor(\\$.*)?", internalName) >> + "jdk/internal/org/objectweb/asm/util/Trace" + member + "Visitor(\\$.*)?", >> internalName) >> || Pattern.matches( >> - "org/objectweb/asm/util/Check" + member + >> "Adapter(\\$.*)?", internalName); >> + "jdk/internal/org/objectweb/asm/util/Check" + member + "Adapter(\\$.*)?", >> internalName); >> } >> >> static void checkIsPreview(final InputStream classInputStream) { >> >> >> And it is basically adapting our copy to the different package prefix we use >> compared to the original ASM code. I have prepared two webrevs one at [1] >> which has the full change and one at [2] for which I have removed a lot of >> javadoc from the full webrev for reviewers that prefer less javadoc. >> >> Thanks, >> Vicente >> >> PS, thanks to Remi for fixing in a very short time a bug I found in ASM >> related to the parsing of Record attributes >> >> [1] http://cr.openjdk.java.net/~vromero/8241627/full/webrev.01/ >> [2] http://cr.openjdk.java.net/~vromero/8241627/no_javadoc/webrev.01/ >> >