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

[Andrew Hughes]

----- Original Message -----
> 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!
> 
> > 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) ?
> 
> > 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.

Well, it's a bigger problem with HotSpot where the compiler is outside
the remit of the OpenJDK build, but enabling -Werror by default increases
the chance of build breakage when no changes have even been made to the
source code.  I've lost count of the number of times we've had to patch
HotSpot due to -Werror failures.  It's happened twice this summer alone.

I'm not so much worried about us encountering failures (we'd probably
still turn it on during development anyway), but it makes OpenJDK less
approachable and puts off potential new developers.

With javac, as the compiler is usually built prior to the jdk build, it's
less this issue and more the implicit compilation issue Stuart mentioned
i.e. javac tries to compile a file outside util/jar which has not yet
been declared "warning free".  This problem will disappear as the entire
source code base is cleaned up.

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

-- 
Andrew :)

Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)

PGP Key: 248BDC07 (https://keys.indymedia.org/)
Fingerprint = EC5A 1F5E C0AD 1D15 8F1F  8F91 3B96 A578 248B DC07