RFR(s): 8160406: Collection.toArray() spec should be explicit about returning precisely an Object[]

[Martin Buchholz]

Wordsmithing is hard.  Which may be why I never tried to improve these
specs decades ago.

I would make one change.

+     * @param <T> the static component type of the array to contain the
collection

Just drop runtime/static

    * @param <T> the component type of the array to contain the collection

the "static" is "obvious" and the caller could in theory provide the T to
use.


On Wed, Nov 29, 2017 at 6:54 PM, Stuart Marks <stuart.marks at oracle.com>
wrote:

>
>
> On 11/29/17 5:20 PM, Martin Buchholz wrote:
>
>> Thanks.  This looks good, and finishes the effort started 12 years ago.
>> Apologies for not having made this spec change myself years ago.
>>
>> I tried to find a less JLSese way to wordsmith it.  Nearby spec talks
>> about the "runtime type" of the array. Maybe s/component/runtime component/
>> but I'm not sure that's actually better.
>>
>
> Good point. I looked through the specs of both toArray() methods to see if
> anything needed to be fixed up. Indeed, there is.
>
> Off-list, Claes Redestad pointed out that in the T[] method, there is the
> following:
>
>      * @param <T> the runtime type of the array to contain the collection
>
> This is in fact completely wrong. The array type is T[] and so T is not
> the type of the array, it's the component type. Furthermore, it's not the
> runtime component type, it's the static component type. This is pretty easy
> to illustrate. Suppose we have:
>
>     var list = List.of("a")
>     CharSequence[] csa = list.toArray((CharSequence[])new String[0])
>
> In the second line, the type variable T is CharSequence, but the runtime
> component type of the returned array is String. (Alternatively, the runtime
> type of the array is String[].)
>
> The specs here need to distinguish between the array's type and the
> array's component type, and also between the static component type and the
> runtime component type. It just goes to show that nothing is ever as simple
> as it seems!
>
> Here's what I did:
>
> * In a couple places, I substituted "runtime component type" and also made
> this be a link to the Class.getComponentType() method to make sure it's
> absolutely clear what's being talked about. The JLS describes the "element
> type" of an array and sometimes the "component" of an array; I chose
> "component type" because it's consistent with Class.getComponentType().
>
> * I fixed up the @param <T> doc to be "static component type".
>
> * I fixed up ArrayStoreException in the T[] method because it confused the
> array's type with the Collection elements' runtime types. I mirrored the
> wording from JLS 10.5 ArrayStoreException, which is framed around
> assignment compatibility.
>
> I left some statements that mentioned the "runtime type of the array"
> because I think they're correct as they stand. For example, "...a new array
> is allocated with the runtime type of the specified array...."
>
> Revised patch appended below. Overall it's not much bigger than the
> previous patch, but it did require a lot of close reading.
>
> Thanks,
>
> s'marks
>
>
>
>
>
> diff -r 9bb771005928 -r fb5434478123 src/java.base/share/classes/ja
> va/util/Collection.java
> --- a/src/java.base/share/classes/java/util/Collection.java     Tue Nov
> 28 17:14:30 2017 -0800
> +++ b/src/java.base/share/classes/java/util/Collection.java     Wed Nov
> 29 18:30:35 2017 -0800
> @@ -268,7 +268,8 @@
>       * Returns an array containing all of the elements in this collection.
>       * If this collection makes any guarantees as to what order its
> elements
>       * are returned by its iterator, this method must return the elements
> in
> -     * the same order.
> +     * the same order. The returned array's {@linkplain
> Class#getComponentType
> +     * runtime component type} is {@code Object}.
>       *
>       * <p>The returned array will be "safe" in that no references to it
> are
>       * maintained by this collection.  (In other words, this method must
> @@ -278,7 +279,8 @@
>       * <p>This method acts as bridge between array-based and
> collection-based
>       * APIs.
>       *
> -     * @return an array containing all of the elements in this collection
> +     * @return an array, whose {@linkplain Class#getComponentType runtime
> component
> +     * type} is {@code Object}, containing all of the elements in this
> collection
>       */
>      Object[] toArray();
>
> @@ -315,14 +317,14 @@
>       * Note that {@code toArray(new Object[0])} is identical in function
> to
>       * {@code toArray()}.
>       *
> -     * @param <T> the runtime type of the array to contain the collection
> +     * @param <T> the static component type of the array to contain the
> collection
>       * @param a the array into which the elements of this collection are
> to be
>       *        stored, if it is big enough; otherwise, a new array of the
> same
>       *        runtime type is allocated for this purpose.
>       * @return an array containing all of the elements in this collection
> -     * @throws ArrayStoreException if the runtime type of the specified
> array
> -     *         is not a supertype of the runtime type of every element in
> -     *         this collection
> +     * @throws ArrayStoreException if the runtime type of any element in
> this
> +     *         collection is not assignable to the {@linkplain
> Class#getComponentType
> +     *         runtime component type} of the specified array
>       * @throws NullPointerException if the specified array is null
>       */
>      <T> T[] toArray(T[] a);
>