Re: JDK 9 RFR of JDK-8039109 : Fix unchecked and raw lint warnings in java.awt

[Phil Race]

I added 2-dev to have this recorded there since 13/20 files  in the 
webrev are 2D.

I didn't see any thing that's a definite problem although I don't
see what difference the following makes. How does it suppress a warning ?
 Collections.EMPTY_SET  = > Collections.emptySet()

I expect see you added the tmp var here as somewhere to hang the annotation
as I presume it must be on a declaration .. unfortunately a bit yucky tho'

 262             @SuppressWarnings("unchecked")
 263             Vector<TrayIcon> tmp = 
(Vector<TrayIcon>)AppContext.getAppContext().get(TrayIcon.class);
 264             icons = tmp;

-phil.

On 5/5/2014 3:22 PM, Jim Graham wrote:
I took a look at the j.a.image and j.a.geom files and everything 
looked fine.

            ...jim

On 5/4/14 9:44 AM, Joe Darcy wrote:
Hello,

Any further comments on the latest version

      http://cr.openjdk.java.net/~darcy/8039109.3/

Thanks,

-Joe

On 4/30/2014 4:04 PM, Joe Darcy wrote:
Hi Rémi,

Hi Joe,

in AWTKeyStroke, instead of

Class<AWTKeyStroke> clazz =
(Class<AWTKeyStroke>)AppContext.getAppContext().get(AWTKeyStroke.class); 


I should have the right type Class<? extends ...> (the class is a
subclass of AWTKeyStroke) and
do a classcheck at runtime when the class is extracted instead of 
later

Class<? extends AWTKeyStroke> clazz =
((Class<?>)AppContext.getAppContext().get(AWTKeyStroke.class)).asSubClass(AWTKeyStroke.class); 



As I'm not overly familiar with this code and the app context
mechanism, in the context of this lint removal exercise, I'd prefer to
leave the timing of the checks as they are today.


and I think the second @SuppressWarnings in getCachedStroke() is
unnecessary.

Right you are; I've removed the second @SuppressWarnings in the next
iteration of the patch.



in GraphicsEnvironment, the last line of your diff can be simplified,

ge = GraphicsEnvironment.class.cast(geCls.newInstance());

can be written

ge = (GraphicsEnvironment)geCls.newInstance();

which is more readable IMO but will also generate exactly the same
bytecode as the current implementation.

Yes, since the type being cast to is unchanging, it is not necessary
to do the cast using core reflection; updated in the next iteration.



in KeyboardFocusManager,

private Set<AWTKeyStroke>[] defaultFocusTraversalKeys = new Set[4];

should be

private Set<AWTKeyStroke>[] defaultFocusTraversalKeys = new Set<?>[4];

so @SuppressWarnings("rawtypes") is not needed.

Actually, javac objects to the code you've proposed:

src/share/classes/java/awt/KeyboardFocusManager.java:352: error:
incompatible types: Set<?>[] cannot be converted to Set<AWTKeyStroke>[]
    private Set<AWTKeyStroke>[] defaultFocusTraversalKeys = new
Set<?>[4];

(IIRC, I ran into this when I was first putting the changeset 
together.)



in DragGestureEvent, the newEvents should be a List<? extends
InputEvent>,
and @SuppressWarnings("rawtypes") should be a
@SuppressWarnings("unchecked")

For the purposes of this clean up effort, I'm not eager to take on
more extensive updates to DragGestureEvent than adding the one
@SuppressWarnings. I would encourage the awt team to consider the
update you've suggested.


in RenderableImageOp,
getRenderableSources() should return a Vector of RenderableImage,

       public Vector<RenderableImage> getSources() {
          getRenderableSources();
       }
          private Vector<RenderableImage> getRenderableSources() {
          Vector<RenderableImage sources = null;

Petr made the same observation and I've addressed that in the second
iteration of the patch:

    http://cr.openjdk.java.net/~darcy/8039109.2/


all other modifications are OK for me.

Newest iteration at:

    http://cr.openjdk.java.net/~darcy/8039109.3/

Thanks for the review,

-Joe