[OpenJDK 2D-Dev] JDK 9 RFR of JDK-8039109 : Fix unchecked and raw lint warnings in java.awt

Phil Race philip.race at oracle.com
Mon May 5 22:51:45 UTC 2014


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
>>>
>>




More information about the 2d-dev mailing list