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

Reply via email to