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

Daniel Fuchs daniel.fuchs at oracle.com
Mon Jun 24 09:23:26 UTC 2019


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!

Minor comment - that you might want to propagate upstream:

          @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


More information about the core-libs-dev mailing list