15

I'm trying to decide what to do every time I get a Java heap pollution warning when using parameterized varargs such as in

public static <T> LinkedList<T> list(T... elements) {
    ...
}

It seems to me that if I am confident not to be using some weird casts in my methods, I should just use @SafeVarargs and move on. But is this correct, or do I need to be more careful? Is there apparently correct code that is actually not safe when using parameterized varargs?

Reading about the subject, I notice that the provided examples are quite artificial. For example, the Java documentation shows the following faulty method:

public static void faultyMethod(List<String>... l) {
    Object[] objectArray = l;     // Valid
    objectArray[0] = Arrays.asList(42);
    String s = l[0].get(0);       // ClassCastException thrown here
}

which is didactic but pretty unrealistic; experienced programmers are not likely to write code doing stuff like this. Another example is

Pair<String, String>[] method(Pair<String, String>... lists) { 
   Object[] objs = lists; 
   objs[0] = new Pair<String, String>("x", "y");  
   objs[1] = new Pair<Long, Long>(0L, 0L);  // corruption !!! 
   return lists; 
} 

which is again pretty obviously mixing types in an unrealistic way.

So, are there more subtle cases in which heap pollution happens under parameterized varargs? Am I justified in using @SafeVarargs if I am not casting variables in a way that loses typing information, or mixes types incorrectly? In other words, am I justified in treating this warning as a not very important formality?

Pika Supports Ukraine
  • 3,612
  • 10
  • 26
  • 42
user118967
  • 4,895
  • 5
  • 33
  • 54

2 Answers2

9

Good question. This has bothered me quite a while too. There are two things here - you don't care about the actual runtime type of the elements within the array, like the example that you have shown:

public static <T> LinkedList<T> list(T... elements) {
    // suppose you iterate over them and add
}

This is where @SafeVarargs is well, safe.

And the second one is where you DO care about the runtime type of the elements within the array (even if so by accident). Arrays, in java, can not be generic, so you can not create a type T [] ts = new T[10], but you can declare a type T[] ts... and because arrays are covariant you can cast an Object[] to a T[] - if you know the types match.

All this becomes interesting when you pass a generic array:

// create a single element "generic" array
static <T> T[] singleElement(T elem) {
    @SuppressWarnings("unchecked")
    T[] array = (T[]) new Object[] { elem };
    return self(array);
}

// @SafeVarargs
static <T> T[] self(T... ts) {
    return ts;
}

Invoking this with Integer[] ints = singleElement(1); looks perfectly legal, but will break at runtime, this is where placing @SafeVarargs would be unsafe.

It will break because that cast (T[]) is actually useless and does not enforce any compile time checks. Even if you rewrote that method as:

 static <T> T[] singleElement(T elem) {
    @SuppressWarnings("unchecked")
    T[] array = (T[]) new Object[]{elem};
    System.out.println(array.getClass());
    return array;
}

it would still not work.

Eugene
  • 117,005
  • 15
  • 201
  • 306
  • I am revisiting this today and something else occurred to me: isn't the problem here the incorrect cast rather than the invocation of `self`? It seems that once the compiler knows an array to be `T[]`, invoking `self` on it is safe; the problem happened before, when the user told the compiler the compiler the array is `T[]` when it really is not. – user118967 Nov 06 '18 at 19:01
  • @user118967 that cast is incorrect as long as you return that generic array to the caller, like in the examples I showed. As a matter of fact that cast is actually fake, no compile time checks are performed in such cases. The invocation of self what just an example btw, look at my last example that I've recently edited - you don't need self to actually prove that this is broken. – Eugene Nov 10 '18 at 19:14
  • That's a very interesting example, but I would not say that the problem was using `@SafeVarargs`. It seems the issue is expecting `singleElement` to return an `Integer[]` just because `T` was instantiated to `Integer` (and indeed the problem remains even when `@SafeVarargs` is not present, in your last example). So I would conclude that the answer to the original question is indeed that `@SafeVarargs` is essentially a formality, since it will not hide errors likely to happen. – user118967 Nov 11 '18 at 21:40
  • @user118967 I never said the problem is using `@SafeVarargs`. If you read it's documentation: **A programmer assertion that...** .... **at call sites** - this is something that a compiler can't prove, but you can help it. Either it is correct or not, it's an entirely different story. And remember, this is needed so that only at call sites there would not be any warnings. I don't understand what is not clear here, actually. – Eugene Nov 12 '18 at 04:45
3

To declare generic arrays T[] in Java is problematic because their type is not known at compile time and as a consequence they can be misused, as the examples in the question show. So the Java compiler issues warnings whenever this is done.

For example, if we declare a generic array as in

T[] tArray = (T[]) new Object[] { 42 };

we get an "unchecked cast" warning.

Besides such casts, the only other way of introducing a generic array into a program is by using a generic varargs. For example, in

void bar() {
    foo(new Integer[]{ 42 })
}

void foo(T... args) {
}

Again here a generic array is being introduced, but in a different way than an unchecked cast, so it gets its own specific warning to make sure the user is not misusing it.

Indeed, as long as one is not converting the array to an array of a different type, it seems that using @SafeVarargs should be safe to use, barring atypical type conversions.

user118967
  • 4,895
  • 5
  • 33
  • 54
  • 1
    what do you mean is not known at runtime, no generic type at all is known at runtime (almost), so why is this special to arrays? Your first example only works because arrays are covariant and that cast can work, but it will only work when `T` is `Object` and at runtime this is what happens actually. This is what I meant that the cast is fake. How about this example? `static T[] create(T elem) { T[] array = (T[]) new Object[]{elem}; return array; } static class Foo { }` – Eugene Nov 12 '18 at 05:22