On Sun, 28 Jun 2026 19:23:05 GMT, Daniel Gredler <[email protected]> wrote:

> Update code in the package `sun.font` to make use of the `instanceof` pattern 
> variable. Includes `equals` methods and a few methods using 
> `ClassCastException` for flow control.
> 
> While looking through this code, I've noticed a few pre-existing suspicious 
> differences between `equals` and `hashCode` implementations, but have not 
> made any functional changes to fix this (best to address separately, if 
> needed).
> 
> Tests checked locally on Linux, Windows and macOS:
> - make test TEST="jtreg:test/jdk/java/awt/font"
> - make test TEST="jtreg:test/jdk/java/awt/Graphics2D/DrawString"
> 
> ---------
> - [x] I confirm that I make this contribution in accordance with the [OpenJDK 
> Interim AI Policy](https://openjdk.org/legal/ai).

I ran all our automated tests on this.

src/java.desktop/macosx/classes/sun/font/CFont.java line 310:

> 308:          }
> 309:          return o instanceof Font2D other &&
> 310:              other.getStyle() == this.getStyle();

I'm not sure what value this adds in this case and the existing code is easier 
to follow .. but up to you ...

src/java.desktop/share/classes/sun/font/CompositeFont.java line 305:

> 303:                     Font2D f2d = fm.findFont2D(componentNames[slot],
> 304:                                                style,
> 305:                                                
> FontManager.PHYSICAL_FALLBACK);

This is OK, since I doubt it makes any difference today, but back in early JDKs 
I believe it was cheaper to just do the cast in cases where it was extremely 
unlikely you'd need to catch an exception and this was in an early start up 
code path.

src/java.desktop/share/classes/sun/font/Decoration.java line 179:

> 177:             return o instanceof DecorationImpl other &&
> 178:                 swapColors == other.swapColors &&
> 179:                 strikethrough == other.strikethrough &&

This looks semantically equivalent should be OK, and this code is older than 
java.util.Objects

src/java.desktop/share/classes/sun/font/FontUtilities.java line 502:

> 500:             return fuir;
> 501:         }
> 502:         PhysicalFont physicalFont = (PhysicalFont)font2D;

I had to think a bit about this. And create a little test case. So like the 
compiler works out you can only  reach here if is a physicalFont so 
(unexpectedly to me) widens the scope of the instanceof variable.

-------------

Marked as reviewed by prr (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/31702#pullrequestreview-4708570949
PR Review Comment: https://git.openjdk.org/jdk/pull/31702#discussion_r3591241877
PR Review Comment: https://git.openjdk.org/jdk/pull/31702#discussion_r3591128536
PR Review Comment: https://git.openjdk.org/jdk/pull/31702#discussion_r3591143626
PR Review Comment: https://git.openjdk.org/jdk/pull/31702#discussion_r3591202452

Reply via email to