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

[Joe Wang]

Thanks Lance!

Joe

On 6/21/19, 1:59 PM, Lance Andersen wrote:
> The revised webrev looks fine Joe
>> On Jun 21, 2019, at 4:20 PM, Joe Wang <huizhe.wang at oracle.com 
>> <mailto:huizhe.wang at oracle.com>> wrote:
>>
>>
>>
>> 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 
>>> <http://cr.openjdk.java.net/%7Ejoehw/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 
>>> <http://cr.openjdk.java.net/%7Ejoehw/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 
>>> <http://cr.openjdk.java.net/%7Ejoehw/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/ 
>> <http://cr.openjdk.java.net/%7Ejoehw/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
>>>>
>>>
>
> <http://oracle.com/us/design/oracle-email-sig-198324.gif>
> <http://oracle.com/us/design/oracle-email-sig-198324.gif><http://oracle.com/us/design/oracle-email-sig-198324.gif>
> <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance 
> Andersen| Principal Member of Technical Staff | +1.781.442.2037
> Oracle Java Engineering
> 1 Network Drive
> Burlington, MA 01803
> Lance.Andersen at oracle.com <mailto:Lance.Andersen at oracle.com>
>
>
>