Possible bug from ImmutableList.of(exactly 13 elements)

[opi...@google.com]

I think I found a possible bug in doing an ImmutableList.of(array1,array2.....array13). It builds and works with < 12 arrays and >13 arrays. It also works for ImmutableList.of(singualrElement1,singualrElement1......singualrElement13). Refere to cl/257906805

[opi...@google.com]

It works if you explicitly put in the type of the array, the cl has been updated to show the difference of the two cases: http://shortn/_duH1QDf3OS 

[David Beaumont]

I suspect this might be unavoidable when using the "extended varargs" pattern.

The signature of any method like:

  foo(T e1, ..., T eN, T... remaining)

Is ambiguous if there is only one "remaining" element and T is itself an array.

By default it's treated as the varargs directly rather than being wrapped into a varargs array.

By explicitly typing the method, you tell the compiler to treat it as an element and wrap it as varargs.

I think the ambiguity comes due to have Java erases generic type information at compilation time, and the way that array typing is handled in Java.

There's probably something in the Java spec about when to treat arguments as vararg arrays or vararg elements.

I doubt there's any way in the library to get around this ambiguity (a similar thing happens with Flogger too).

[Joachim Durchholz]

Am 13.07.19 um 06:29 schrieb 'David Beaumont' via guava-discuss:

> I suspect this might be unavoidable when using the "extended varargs"
> pattern.
>
> The signature of any method like:
>   foo(T e1, ..., T eN, T... remaining)
>
> Is ambiguous if there is only one "remaining" element and T is itself an
> array.

In particular if it's an array U[] where U is T or a supertype.
There are rules for this situation, and the caller can use a type cast
to select whether the U[] should be treated as a single parameter or as
varargs, but nobody cares to remember them (I certainly don't).

Since this is a source of subtle bugs, I for myself have come to the
conclusion that overloading foo(T... varargs) with foo(T e1, T... es)
etc. is an API misdesign.
I found foo(List<T>) to be an acceptable substitute:
foo(Arrays.asList(t1, t2, t3, ...)) clearly expresses the intent at the
caller side, and if that pattern comes up often enough in real-world
code, JIT makers should start optimizing it away (if they haven't done
so already).

Things *still* can go wrong if T itself is List or a superclass.
I avoid varargs in that case.

Just my 2c of API design tinkering results; YMMV.

Regards,
Jo

[David Beaumont]

While it is not ideal, it does have its uses. In Flogger this pattern lets us avoid allocating and populating many trillions of small arrays (since log statements have a fairly exponential curve regarding how many parameters people pass).

The number of people passing 10 or more log arguments is something like 0.001% of all log statements, and the number of people who would pass exactly 11 arguments with the 11th argument being an Object[] is (last time we checked) zero in all of Google's enormous Java corpus.

In Flogger's case, the downside to getting it "wrong" is also minimized because the "wrong" case would see an array being treated like its contents, which would typically look as if there were too many arguments passed into the log statement, but Flogger's response to that is to print the arguments anyway.

It is a shame this wrinkle exists in the way varargs is defined, but I wouldn't go as far as saying you shouldn't use this pattern; just be aware that it has this drawback.

David

[muntazir ulla khan]