Re: 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