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

[Rémi Forax]

On 08/29/2012 09:11 PM, Dan Xu wrote:
>
> 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
>
>
>

@Joe,
So the question is whenever @SafeVarargs change the status of the 
variable 'a'
to consider it as a safe use of a varargs.
Given the fact that the user annotate the method with @SafeVarargs,
the interpretation is that @SafeVarargs is only something for methods 
that call asList()
but it has no effect on the body of the method asList().

That make sense.

cheers,
Rémi