Re: RFR: JDK-8241627: Updating ASM to 8.0 for JDK 15

[Vicente Romero]

Hi Chris,

On 4/20/20 4:43 AM, Chris Hegarty wrote:

I skimmed over the webrev, the changes seem fine.

thanks for the review

Thanks for updating the record serialization tests that use ASM.

sure np,

-Chris.

Vicente

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/