Hi Vicente,

Looks ok from the changes required to integrate into the JDK.

Maybe the @SuppressWarnings warning annotations could be upstreamed?

Paul.


> On Apr 13, 2020, at 8:05 AM, Vicente Romero <vicente.rom...@oracle.com> 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/
> 

Reply via email to