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

Rémi Forax forax at univ-mlv.fr
Thu Aug 30 12:06:04 UTC 2012

On 08/29/2012 09:11 PM, Dan Xu wrote:

On 08/29/2012 08:27 AM, Joe Darcy wrote: Hello,

On 8/29/2012 1:48 AM, Rémi Forax wrote: On 08/29/2012 08:33 AM, Kurchi Subhra Hazra wrote: Thanks for cleaning up those spaces Dan. The changes look fine. Sorry for the extra trouble!

- Kurchi On 8/28/12 10:22 PM, Dan Xu wrote: It is funny. :) I have searched all source codes under jdk and removed spaces for the similar cases.

Please review the new version of change at, http://cr.openjdk.java.net/~dxu/7193406/webrev.03/. Thanks for your comment! -Dan Hi Dan, In PreHashedMap, the code should be Map.Entry that = (Map.Entry)ob; so the @SuppressWarnings is not needed anymore. And in java.util.Arrays, asList() should not be annotated with @SuppressWarnings because it is already annotated with SafeVarargs, so the compiler should not report the varargs warning. I have CC to compiler-dev to be sure that this is a compiler error. cheers, Rémi I've spoken to Stuart about this last point. The @SafeVarargs covers uses of Array.asList and covers the declaration of the asList method itself. However, it does not cover calls to the constructor inside the asList method, a constructor which itself is not @SafeVarargs. Solutions include * @SuppressWarnings("varargs") on the asList method * @SuppressWarnings("varargs") on new temporary variable for that purpose (with possible class file size consequences) * @SafeVarargs on the ArrayList constructor Regards, -Joe The first change to the PreHashedMap is right, and I will update my fix. For the second comment, as Joe mentioned, it is necessary. Otherwise, the compiler will give the following warnings. ../../../src/share/classes/java/util/Arrays.java:2837: warning: [varargs] Varargs method could cause heap pollution from non-reifiable varargs parameter a return new ArrayList<>(a); ^ ../../../src/share/classes/java/util/Arrays.java:2837: warning: [varargs] Varargs method could cause heap pollution from non-reifiable varargs parameter a return new ArrayList<>(a); ^ The first and second options works fine. But if I follow the third option to add @SafeVarargs on the ArrayList constructor, I get the following error. ../../../src/share/classes/java/util/Arrays.java:2849: error: Invalid SafeVarargs annotation. Method ArrayList(E[]) is not a varargs method. ArrayList(E[] array) { ^ where E is a type-variable: E extends Object declared in class ArrayList So I tried further to change the constructor to a varargs method and add the @SafeVarargs annotation, but the compiler still gives me warnings like this, ../../../src/share/classes/java/util/Arrays.java:2837: warning: [varargs] Varargs method could cause heap pollution from non-reifiable varargs parameter a return new ArrayList<>(a); ^ ../../../src/share/classes/java/util/Arrays.java:2837: warning: [varargs] Varargs method could cause heap pollution from non-reifiable varargs parameter a return new ArrayList<>(a); ^ ../../../src/share/classes/java/util/Arrays.java:2852: warning: [varargs] Varargs method could cause heap pollution from non-reifiable varargs parameter array a = array; ^ 3 warnings I think the current change is good for this one. Thanks for your comments. -Dan

@Joe, So the question is whenever @SafeVarargs change the status of the variable 'a' to consider it as a safe use of a varargs. Given the fact that the user annotate the method with @SafeVarargs, the interpretation is that @SafeVarargs is only something for methods that call asList() but it has no effect on the body of the method asList().

That make sense.

cheers, Rémi

More information about the core-libs-dev mailing list