[OpenJDK 2D-Dev] JDK 9 RFR of JDK-8033624: Fix raw and unchecked lint warnings in sun.font

Phil Race philip.race at oracle.com
Fri Feb 7 00:13:51 UTC 2014


Comments/questions on the changes :-

Font2D.java


108     protected Reference<FontStrike> lastFontStrike = new SoftReference<>(null);
...


320 lastFontStrike = new SoftReference<>(strike);

So the diamond operator can be used in this 2nd case ? Apparently so.
I vaguely remember that it used to only be able to infer the type in the first case ?
Did it get updated or did I just not realise ??


There are a few places including these :-
-----
AttributeValues.java

  Map<TextAttribute, ?> imStyles = hl.getStyle();

FontResolver.java


226     public Font getFont(int index,
  227                         Map<? extends AttributedCharacterIterator.Attribute, ?> attributes) {

----

where its not obvious how you decided the type to use.
I know a lot of this code and that they look reasonable but I wouldn't have been confident
that there weren't places where we allowed some wider type to be used in these maps.
How did you decide on the types and how did you verify it ?


>      @SuppressWarnings("unchecked")

Can you show what thevarious  warnings looked like and say why they needed to be suppressed rather than fixed ?
Is it anywhere you found it to tricky/impossible to get rid of a cast ?

FontScaler.java :

Is the whole somewhat ugly re-writing of this so that you have a single expression
to which to apply  @SuppressWarnings ?

If you are going to do that anyway was it worth it ?  Can't it just be applied
to the static initialiser block ?

And at the very least there should be spaces before "?" and ":" to make it
easier to read.


StrikeCache.java:

Disposer.addReference((Reference<Object>)(Reference)this, disposer);

Why does this look like a cast of an already cast variable ? Looks weird to me.


SunFontManager.java

lines 2310-2314 look much > 80 chars. We keep all these files <=80 chars wide

I find it tricky to catch all those in review but please be conscious of it.
I suspect line 2644 in the same file is also too long.Also 3066, 3068, 3128, 3157..
please fix all of these and any others I may have overlooked in this or other files.

And don't forget - please re-generate against client, or ask me to push there for you.

-phil.

On 2/5/2014 1:51 AM, Alan Bateman wrote:
> On 05/02/2014 07:41, Joe Darcy wrote:
>> Hello,
>>
>> Please review this change which resolves ~200 raw and unchecked 
>> warnings in sun.font. (Afterward, the code is also free the of the 
>> "cast" warning.)
>>
>>     JDK-8033624 : Fix raw and unchecked lint warnings in sun.font
>> http://cr.openjdk.java.net/~darcy/8033624.0/
> I've skimmed through this and I don't see anything obviously wrong. 
> There are a @SuppressWarnings("unchecked") on methods where I wonder 
> if the scope could be reduced but this requires deeper knowledge of 
> this area.
>
> -Alan.




More information about the 2d-dev mailing list