RFR (JDK13/java.xml) 8224157: BCEL: update to version 6.3.1

[Joe Wang]

On 6/21/19, 11:59 AM, Daniel Fuchs wrote:
> Hi Joe,
>
> I promise, I will not grumble about the formatting and
> weird white spaces [grumble grumble]...

Someone at BCEL needs to re-format the source with a modern IDE ;-)

>
> http://cr.openjdk.java.net/~joehw/jdk13/8224157/webrev/src/java.xml/share/classes/com/sun/org/apache/bcel/internal/classfile/ConstantUtf8.java.frames.html 
>
>
> Is the new import really needed here?

No, good catch.
>
> http://cr.openjdk.java.net/~joehw/jdk13/8224157/webrev/src/java.xml/share/classes/com/sun/org/apache/bcel/internal/classfile/JavaClass.java.frames.html 
>
>
>  232                 if (!dir.isDirectory()) {
>
> I see use of SecuritySupport was dropped here and I think I agree,
> as I don't see why isDirectory() would require it if mkdirs() doesn't.

I honestly don't remember why I added that in the previous update. But 
you're right, and all test run proved it.

>
>
> http://cr.openjdk.java.net/~joehw/jdk13/8224157/webrev/src/java.xml/share/classes/com/sun/org/apache/bcel/internal/classfile/LineNumberTable.java.udiff.html 
>
>
> Are the changes in toString() actually correct?

No, it's not. Good catch again! The change in the last update was correct
>
> Otherwise I believe the rest looks mostly OK.

Thanks Daniel!  Here's an updated webrev:

Full webrev (for the record)
http://cr.openjdk.java.net/~joehw/jdk13/8224157/webrev_02/

A short version of webrev_02 that includes the only files mentioned in 
this review:
http://cr.openjdk.java.net/~joehw/jdk13/8224157/webrev_02_short/


I'm re-running all tests.

Best regards,
Joe

>
> best regards,
>
> -- daniel
>
> On 20/06/2019 23:45, Joe Wang wrote:
>> Please review an update to BCEL 6.3.1. This changeset will go into 
>> JDK13 if approved, 14 if not.
>>
>> 1. Format
>>      The changeset looks big, the majority of the changes however 
>> were only different in format (i.e. Const.java). Unlike previous 
>> updates, I'm leaving the format as they are in the upstream source so 
>> that they won't show up in future updates. The only change I made 
>> were those that had extremely long lines.
>>
>> 2. Exclusions
>>      Since the BCEL component is for xml transform only, several 
>> classes, that were not in the JDK, were excluded. Among them, 
>> JavaWrapper.java for example can be problematic as it may use an 
>> user-specified classloader to load arbitrary classes. It and related 
>> classes were therefore excluded.
>>
>> 3. Warnings
>>      Warnings were the main reason for the changes made to the 
>> original source. It has been done in the previous update. For this 
>> update therefore, I only had to re-apply them after making copies of 
>> the upstream source. Still, I updated the LastModified field to 
>> indicate a modification to the original source.
>>
>> 4. Deprecated fields to private
>>      Deprecated fields in the original source were changed to private 
>> ones in previous update. The changes are inherited in this update. 
>> Again, the LastModified fields are also updated.
>>
>> 5. Test
>>      Since the update does not affect the usage of the BCEL 
>> component, it is essential to pass all of the existing tests. I've 
>> run the tests multiple times and noted that all of the XML functional 
>> and unit tests passed, so were JCK XML tests. I've also done a 
>> comparison between builds before and after applying the BCEL update, 
>> and found no change in performance.
>>
>> JBS: https://bugs.openjdk.java.net/browse/JDK-8224157
>> webrevs: http://cr.openjdk.java.net/~joehw/jdk13/8224157/webrev/
>>
>> Thanks,
>> Joe
>>
>