[OpenJDK 2D-Dev] JDK 9 RFR of JDK-8042864 : Fix raw and unchecked warnings in javax.print

Joe Darcy joe.darcy at oracle.com
Mon May 19 18:54:48 UTC 2014


Hi Phil,

On 05/19/2014 11:36 AM, Phil Race wrote:
> Some of these seem to need a CCC eg ..
> In AttributeException
>
> public Class<?>[] getUnsupportedAttributes();
>
>  and ...
>
> http://cr.openjdk.java.net/~darcy/8042864.0/src/share/classes/javax/print/MimeType.java.sdiff.html
>
> public Map<String, String> getParameterMap() {

That is correct; there are a few API changes that need to go through 
ccc. However, they are just the usual sorts of changes when an API is 
generified; I was going to file the request once the code review process 
was a bit further along.

>
>
>
> http://cr.openjdk.java.net/~darcy/8042864.0/src/share/classes/javax/print/attribute/standard/DialogTypeSelection.java.sdiff.html
>
> < 113 public final Class getCategory() {
>
> > 113 public final Class<DialogTypeSelection> getCategory() {
>
>
> getCategory() is a super-class method ..
>
> So this seems wrong to me and I think|it should look like this ..|
>
> http://docs.oracle.com/javase/8/docs/api/javax/print/attribute/Attribute.html
> |Class 
> <http://docs.oracle.com/javase/8/docs/api/java/lang/Class.html><? 
> extends Attribute 
> <http://docs.oracle.com/javase/8/docs/api/javax/print/attribute/Attribute.html>>| 
> |getCategory 
> <http://docs.oracle.com/javase/8/docs/api/javax/print/attribute/Attribute.html#getCategory-->()
>
>
> |

Part of the generics language feature was covariant overrides, that is, 
when a subtype overrides a methods from a supertype, the return type of 
the subtype method can be a more-specific type.

That is the case here; DialogTypeSelection is a subtype of "? extends 
Attribute" so the getCategory method in DialogTypeSelection is a 
covariant override of the superclass method. Moreover, since the 
superclass uses the "? extends Attribute" wildcard, the intention is for 
subclasses to override this method when possible.

Thanks,

-Joe

> |
>
> -phil.
> |
> On 05/19/2014 09:35 AM, Joe Darcy wrote:
>> *ping*
>>
>> Any other comments?
>>
>> Thanks,
>>
>> -Joe
>>
>> On 05/15/2014 06:06 PM, Joe Darcy wrote:
>>> Hi Henry,
>>>
>>> On 05/15/2014 03:46 PM, Henry Jen wrote:
>>>> On 05/15/2014 12:07 PM, Joe Darcy wrote:
>>>>> Hello,
>>>>>
>>>>> Please review these change to fix
>>>>>
>>>>>      JDK-8042864 : Fix raw and unchecked warnings in javax.print
>>>>> http://cr.openjdk.java.net/~darcy/8042864.0/
>>>>>
>>>>> Patch below.
>>>>
>>>> Looks good to me, just nit-picking.
>>>>
>>>>>
>>>>> --- old/src/share/classes/javax/print/PrintServiceLookup.java 
>>>>> 2014-05-15
>>>>> 12:04:20.000000000 -0700
>>>>> +++ new/src/share/classes/javax/print/PrintServiceLookup.java 
>>>>> 2014-05-15
>>>>> 12:04:20.000000000 -0700
>>>>> @@ -208,7 +207,7 @@
>>>>>        */
>>>>>       public static boolean 
>>>>> registerServiceProvider(PrintServiceLookup
>>>>> sp) {
>>>>>           synchronized (PrintServiceLookup.class) {
>>>>> -            Iterator psIterator = getAllLookupServices().iterator();
>>>>> +            Iterator<?> psIterator = 
>>>>> getAllLookupServices().iterator();
>>>>>               while (psIterator.hasNext()) {
>>>>>                   try {
>>>>>                       Object lus = psIterator.next();
>>>>
>>>> We know this is Iterator<PrinterServiceLookup>, but this works.
>>>
>>> I can put the more precise type information in.
>>>
>>>>
>>>>> ---
>>>>> old/src/share/classes/javax/print/attribute/AttributeSetUtilities.java 
>>>>>
>>>>> 2014-05-15 12:04:22.000000000 -0700
>>>>> +++
>>>>> new/src/share/classes/javax/print/attribute/AttributeSetUtilities.java 
>>>>>
>>>>> 2014-05-15 12:04:22.000000000 -0700
>>>>> @@ -523,7 +523,7 @@
>>>>>       public static Class<?>
>>>>>           verifyAttributeCategory(Object object, Class<?> 
>>>>> interfaceName) {
>>>>>
>>>>> -        Class result = (Class) object;
>>>>> +        Class<?> result = (Class) object;
>>>>>           if (interfaceName.isAssignableFrom (result)) {
>>>>>               return result;
>>>>>           }
>>>>
>>>> Should the cast be (Class<?>) instead of (Class)?
>>>
>>> I think either is okay, but I'm fine to change this to Class<?>.
>>>
>>>>
>>>>> ---
>>>>> old/src/share/classes/javax/print/attribute/standard/DialogTypeSelection.java 
>>>>>
>>>>> 2014-05-15 12:04:24.000000000 -0700
>>>>> +++
>>>>> new/src/share/classes/javax/print/attribute/standard/DialogTypeSelection.java 
>>>>>
>>>>> 2014-05-15 12:04:23.000000000 -0700
>>>>> @@ -110,7 +110,7 @@
>>>>>        * @return  Printing attribute class (category), an instance 
>>>>> of class
>>>>>        *          {@link java.lang.Class java.lang.Class}.
>>>>>        */
>>>>> -    public final Class getCategory() {
>>>>> +    public final Class<DialogTypeSelection> getCategory() {
>>>>>           return DialogTypeSelection.class;
>>>>>       }
>>>>>
>>>>
>>>> Would this be too specific for this public API? <? extends 
>>>> Attribute> is defined in interface Attribute.
>>>
>>> Well, the javadoc does say that DialogTypeSelection.class is 
>>> returned. The DialogTypeSelection class does implement the Attribute 
>>> interface so all the typing works out fine as-is.
>>>
>>>>
>>>>
>>>>> ---
>>>>> old/src/share/classes/javax/print/attribute/standard/PrinterStateReasons.java 
>>>>>
>>>>> 2014-05-15 12:04:25.000000000 -0700
>>>>> +++
>>>>> new/src/share/classes/javax/print/attribute/standard/PrinterStateReasons.java 
>>>>>
>>>>> 2014-05-15 12:04:24.000000000 -0700
>>>>> @@ -242,16 +242,18 @@
>>>>>           extends AbstractSet<PrinterStateReason>
>>>>>       {
>>>>>           private Severity mySeverity;
>>>>> -        private Set myEntrySet;
>>>>> +        //
>>>>> +        private Set<Map.Entry<PrinterStateReason, Severity>> 
>>>>> myEntrySet;
>>>>>
>>>>> -        public PrinterStateReasonSet(Severity severity, Set 
>>>>> entrySet) {
>>>>> +        public PrinterStateReasonSet(Severity severity,
>>>>> + Set<Map.Entry<PrinterStateReason, Severity>> entrySet) {
>>>>>               mySeverity = severity;
>>>>>               myEntrySet = entrySet;
>>>>>           }
>>>>>
>>>>>           public int size() {
>>>>>               int result = 0;
>>>>> -            Iterator iter = iterator();
>>>>> +            Iterator<?> iter = iterator();
>>>>
>>>> We know it is Iterator<PrinterStateReason>.
>>>>
>>>>
>>>
>>> That was less clear when I addressed the warning on that line, but 
>>> the exact time works better.
>>>
>>> Updated webrev uploaded to:
>>>
>>> http://cr.openjdk.java.net/~darcy/8042864.1
>>>
>>> Thanks for the review,
>>>
>>> -Joe
>>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/2d-dev/attachments/20140519/dc3e2f96/attachment.html>


More information about the 2d-dev mailing list