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

[Joe Wang]

On 6/24/19, 2:23 AM, Daniel Fuchs wrote:
> On 21/06/2019 21:20, Joe Wang wrote:
>> 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/
>>
>
> Thanks Joe!
>
> The updated files look good.
> Thumbs up!

Thanks!

>
> Minor comment - that you might want to propagate upstream:

The hashCode impl is indeed inconsistent with equals. Looking at the 
issue [1], by using Objects.equals, they were fixing a possible NPE. 
Either case (the original or the use of Objects.equals) does not affect 
java.xml's specific usage of BCEL. The original JAXP team used to be 
part of Apache projects, but I myself hasn't been. While we would like 
it to be perfect (in format, and in issues such as this), I think we 
have to live with it, acknowledging it but knowing the edge case does 
not affect our usage.

Keeping the code in sync with the BCEL (not perfect) source is in our 
interest to produce smaller changeset in any future updates. Would you 
be okay with the current webrevs?

[1] https://issues.apache.org/jira/browse/BCEL-297

Best regards,
Joe

>
>          @Override
> -        public boolean equals(final Object o1, final Object o2) {
> +        public boolean equals( final Object o1, final Object o2 ) {
>              final JavaClass THIS = (JavaClass) o1;
>              final JavaClass THAT = (JavaClass) o2;
> -            return THIS.getClassName().equals(THAT.getClassName());
> +            return Objects.equals(THIS.getClassName(), 
> THAT.getClassName());
>          }
>
> +
>          @Override
> -        public int hashCode(final Object o) {
> +        public int hashCode( final Object o ) {
>              final JavaClass THIS = (JavaClass) o;
>              return THIS.getClassName().hashCode();
>          }
>
> I've seen that idiom at a few places: equals was modified to
> use Objects.equals(), but hashCode was not.
>
> 1. equals will throw if it is passed a null THIS or THAT (is
>    that intented?)
> 2. equals seems to think that getClassName() can return null,
>    whereas hashCode obviously expect that it will be non null.
>
> I cannot say whether that's right or wrong, but I can smell something...
> Maybe having a look at the issue that triggered the use of
> Objects::equals could shed some light on this. It doesn't seem worse
> than what was there before - it's just a bit odd.
>
>
> best regards,
>
> -- daniel