Review Request: 7193406 - Clean-up JDK Build Warnings in java.util, java.io (original) (raw)
Andrew Hughes ahughes at redhat.com
Fri Aug 24 09:49:30 UTC 2012
- Previous message: Review Request: 7193406 - Clean-up JDK Build Warnings in java.util, java.io
- Next message: Review Request: 7193406 - Clean-up JDK Build Warnings in java.util, java.io
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
----- 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.FILEREADACTION: 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 > 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
- Previous message: Review Request: 7193406 - Clean-up JDK Build Warnings in java.util, java.io
- Next message: Review Request: 7193406 - Clean-up JDK Build Warnings in java.util, java.io
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]