[OpenJDK 2D-Dev] Request for review: 7196571, 7196572, 7196573: javac warnings cleanup from Adopt OpenJDK bugathon

Martijn Verburg martijnverburg at gmail.com
Tue Sep 25 15:12:40 UTC 2012

Hi all,

Apologies for the long response, as with many of you JavaOne prep
seems to get in the way of doing interesting coding work!

On 12 September 2012 20:03, Stuart Marks <stuart.marks at oracle.com> wrote:
> On 9/11/12 1:41 PM, Martijn Verburg wrote:
>>> Also for both of these cases, you added .asSubClass(..).
>>> Now when I look at the new code there, it looks a lot busier, all to
>>> avoid one simple (ImageWriterSpi) cast. Is it really worth it ?
>>> And if the change is made, maybe you should check for ClassCastException.
>>> I guess we already vulnerable to that, although its rather unlikely to
>>> occur
>>> ...
>> I'm more than happy to follow your lead on this one. I'll revert those
>> two, we can
>> always visit it again later with fresh eyes (I'm not that convinced that
>> using asSubClass(..) is the way to go here either).
> Using Class.forName(...).asSubClass(...) is a reasonable type-safe way of
> getting a Class<T> given a string name. However, it does change the location
> of any exception that might get thrown, and there might be a subtle
> interaction with the IIORegistry, so it might indeed be best to leave these
> untouched for the moment.

OK, left untouched in that case.

> I looked through the webrevs and I noticed a couple minor things...
> *
> http://cr.openjdk.java.net/~art/Bugathon-2012/webrev.j2d-print/src/share/classes/sun/print/RasterPrinterJob.java.sdiff.html
>  - lines 508-509 can be joined
>  - lines 1311-1312 should probably be broken after the = operator,
>    for consistency with other cases here
>  - lines 1320-1321 can be joined
>  - line 1910 can remove redundant parentheses

I've fixed these and sent Artem an updated patch for his webrev

>>> Don't forget you need a second reviewer for all the changes.
>> OK, thanks - if another reviewer is following this thread then any
>> assistance would be most welcome :-)
> I'm not sure I count as a second reviewer. I did look quickly through all
> the changes, though, and the only issues I noticed were the ones I mentioned
> above.

Hopefully that's all that's required then :-)


More information about the 2d-dev mailing list