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

[Dan Xu]

Hi David,

Please see my response below. Thanks!


On 08/23/2012 06:53 PM, David Holmes wrote:
> Hi Dan,
>
> On 24/08/2012 7:46 AM, Dan Xu wrote:
>> Please review the fix of CR 7193406 at
>> http://cr.openjdk.java.net/~dxu/7193406/webrev/.
>>
>> It cleans up warnings in the following java files.
>>
>> src/share/classes/java/io/FilePermission.java
>
> I'm surprised that you need this:
>
>  426     @SuppressWarnings("fallthrough")
>  ...
>  436         switch (actions) {
>  437             case SecurityConstants.FILE_READ_ACTION:
>  438                 return READ;
>
> If this generates a fallthrough warning then I think whatever tool 
> generates that warning needs fixing!

@SuppressWarnings("fallthrough") is put to suppress warnings generated 
by another switch/case statements

             while (i >= matchlen && !seencomma) {
                 switch(a[i-matchlen]) {
                 case ',':
                     seencomma = true;
                     /*FALLTHROUGH*/
                 case ' ': case '\r': case '\n':
                 case '\f': case '\t':
                     break;
                 default:
                     throw new IllegalArgumentException(
                             "invalid permission: " + actions);
                 }
                 i--;
             }

It is at the end of this method, getMask(String).

The change of switch/case statements starting from line 436, as you 
pointed out, does not generate a fallthrough warning.

>
>> src/share/classes/java/util/ArrayDeque.java
>> src/share/classes/java/util/Arrays.java
>> src/share/classes/java/util/Collections.java
>> src/share/classes/java/util/HashMap.java
>> src/share/classes/java/util/JumboEnumSet.java
>> src/share/classes/java/util/PriorityQueue.java
>> src/share/classes/java/util/PropertyResourceBundle.java
>
> Here:
>
> !         @SuppressWarnings({ "unchecked", "rawtypes" })
> !         Map<String,Object> result = new HashMap(properties);
> !         lookup = result;
>
> why do you keep the raw type instead of using HashMap<>(properties) ?

If I change to use diamond operator, the compiler will give me the 
following error,

    ../../../src/share/classes/java/util/PropertyResourceBundle.java:132: error:
    incompatible types: HashMap<Object,Object> cannot be converted to
    Map<String,Object>
             Map<String,Object> result = new HashMap<>(properties);
                                         ^
    1 error


I think it is because the type of properties is 
Hashtable<Object,Object>, which does not fit into new 
HashMap<String,Object>() constructor.

>
>> src/share/classes/java/util/jar/JarVerifier.java
>> src/share/classes/java/util/jar/Pack200.java
>> src/share/classes/sun/util/PreHashedMap.java
>>
>>
>>
>> And it enables fatal warning flag in the following make file.
>>
>> make/java/jar/Makefile
>
> I'm not sure what Andrew's objection is to this change as it only 
> affects how java/util/jar classes get compiled and is consistent with 
> the usage in all the other Makefiles. As far as I can see you can 
> override this on the make invocation anyway.
>
> I'm unclear how this is handled in the new build.
>
>> 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!
>
> As Andrew stated this is a package-private class so I don't see any 
> issue.
>
> Cheers,
> David
>
>> - Dan


Thanks for your comments.

-Dan