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/
>> 
> 

Reply via email to