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

[Dan Xu]

On 08/23/2012 05:17 PM, Andrew Hughes wrote:
>
> ----- Original Message -----
>>
>> ----- Original Message -----
>>> Please review the fix of CR 7193406 at
>>> http://cr.openjdk.java.net/~dxu/7193406/webrev/.
>>>
>> snip...
>>
>>> And it enables fatal warning flag in the following make file.
>>>
>>>      make/java/jar/Makefile
>>>
>> Please don't do this, at least not unconditionally.
>>
>> At the very least, these should not be forced on if the user
>> has explicitly built with them set to false.  If I set
>> JAVAC_WARNINGS_FATAL=false, I don't expect part of the build
>> to ignore this and use -Werror, possibly then causing build
>> failures.
>>
>> This is especially bad with both on as a new warning may be
>> introduced into javac which then breaks everyone's build.
>>
>> I already have a patch I intend to upstream which fixes the
>> other cases of this I found.  I'd prefer we didn't have another.
>>
>>> In FilePermission.java file, I make one change to its method
>>> signature,
>>>
>>>      public Enumeration elements()  ==> public
>>>      Enumeration<Permission>
>>>      elements()
>>>
>>>
>>> I am not sure whether it will cause an issue of backward
>>> compatibility.
>>> Please advise. Thanks!
>>>
>> It's in a package-private class so I doubt it.
>>
>> Even if it wasn't, a new major release is the perfect time to fix
>> these issues.
>>
>>> - Dan
>>>
> I should also point out that the annotation:
>
>      @SuppressWarnings("unchecked")
>      private void readObject(ObjectInputStream in) throws IOException,
>
> could be moved down to the actual problematic assignment:
>
>          Vector<Permission> permissions = (Vector<Permission>)gfields.get("permissions", null);
>
> so that warnings aren't suppressed throughout the method.
>
Andrew,

Thanks for your suggestions. I have revoked the change to the make file 
and added the changes to readObject method, which did not draw my 
attention as it already suppresses all warnings inside the method. And 
we should place @SuppressWarnings at narrowest possible scope. Thanks!

-Dan