Review Request: 7193406 - Clean-up JDK Build Warnings in java.util, java.io

[Dan Xu]

On 08/29/2012 08:27 AM, Joe Darcy wrote:
> Hello,
>
> On 8/29/2012 1:48 AM, Rémi Forax wrote:
>> On 08/29/2012 08:33 AM, Kurchi Subhra Hazra wrote:
>>> Thanks for cleaning up those spaces Dan. The changes look fine.
>>> Sorry for the extra trouble!
>>>
>>> - Kurchi
>>>
>>> On 8/28/12 10:22 PM, Dan Xu wrote:
>>>> It is funny. :) I have searched all source codes under jdk and 
>>>> removed spaces for the similar cases.
>>>>
>>>> Please review the new version of change at, 
>>>> http://cr.openjdk.java.net/~dxu/7193406/webrev.03/.
>>>>
>>>> Thanks for your comment!
>>>>
>>>> -Dan
>>
>> Hi Dan,
>>
>> In PreHashedMap, the code should be
>>
>>   Map.Entry<?,?> that = (Map.Entry<?,?>)ob;
>>
>> so the @SuppressWarnings is not needed anymore.
>>
>> And in java.util.Arrays, asList() should not be annotated with 
>> @SuppressWarnings because it is already annotated with SafeVarargs, 
>> so the compiler should not report the varargs warning.
>>
>> I have CC to compiler-dev to be sure that this is a compiler error.
>>
>> cheers,
>> Rémi
>>
>
> I've spoken to Stuart about this last point.  The @SafeVarargs covers 
> uses of Array.asList and covers the declaration of the asList method 
> itself.  However, it does not cover calls to the constructor inside 
> the asList method, a constructor which itself is *not* @SafeVarargs.  
> Solutions include
>
> * @SuppressWarnings("varargs") on the asList method
> * @SuppressWarnings("varargs")  on new temporary variable for that 
> purpose (with possible class file size consequences)
> * @SafeVarargs on the ArrayList constructor
>
> Regards,
>
> -Joe
The first change to the PreHashedMap is right, and I will update my fix.

For the second comment, as Joe mentioned, it is necessary. Otherwise, 
the compiler will give the following warnings.

    ../../../src/share/classes/java/util/Arrays.java:2837: warning:
    [varargs] Varargs method could cause heap pollution from
    non-reifiable varargs parameter a
             return new ArrayList<>(a);
                                    ^
    ../../../src/share/classes/java/util/Arrays.java:2837: warning:
    [varargs] Varargs method could cause heap pollution from
    non-reifiable varargs parameter a
             return new ArrayList<>(a);

                                ^

The first and second options works fine. But if I follow the third 
option to add @SafeVarargs on the ArrayList constructor, I get the 
following error.

    ../../../src/share/classes/java/util/Arrays.java:2849: error:
    Invalid SafeVarargs annotation. Method ArrayList(E[]) is not a
    varargs method.
             ArrayList(E[] array) {
             ^
       where E is a type-variable:
         E extends Object declared in class ArrayList


So I tried further to change the constructor to a varargs method and add 
the @SafeVarargs annotation, but the compiler still gives me warnings 
like this,

    ../../../src/share/classes/java/util/Arrays.java:2837: warning:
    [varargs] Varargs method could cause heap pollution from
    non-reifiable varargs parameter a
             return new ArrayList<>(a);
                                    ^
    ../../../src/share/classes/java/util/Arrays.java:2837: warning:
    [varargs] Varargs method could cause heap pollution from
    non-reifiable varargs parameter a
             return new ArrayList<>(a);
                                    ^
    ../../../src/share/classes/java/util/Arrays.java:2852: warning:
    [varargs] Varargs method could cause heap pollution from
    non-reifiable varargs parameter array
                 a = array;
                     ^
    3 warnings



I think the current change is good for this one. Thanks for your comments.

-Dan