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

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

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

Stuart Marks
Hi all,

Please review this small spec change / clarification regarding
Collection.toArray(). The runtime type of the array returned has always been
intended to be exactly Object[] and not an array of some subtype. This
requirement is actually implied by the spec already, but in the wrong place; the
spec for

     <T> T[] toArray​(T[] a)

says

> Note that toArray(new Object[0]) is identical in function to toArray().

Clearly, this should also be specified on toArray() itself.

Patch is below.

Thanks,

s'marks



diff -r 9bb771005928 -r c3d5e370e06f
src/java.base/share/classes/java/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 14:29:14
2017 -0800
@@ -268,7 +268,7 @@
       * 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 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 +278,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 component type is {@code Object}, containing all
+     * of the elements in this collection
       */
      Object[] toArray();

Reply | Threaded
Open this post in threaded view
|

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

Paul Sandoz
+1

Paul.

> On 29 Nov 2017, at 16:18, Stuart Marks <[hidden email]> wrote:
>
> Hi all,
>
> Please review this small spec change / clarification regarding Collection.toArray(). The runtime type of the array returned has always been intended to be exactly Object[] and not an array of some subtype. This requirement is actually implied by the spec already, but in the wrong place; the spec for
>
>    <T> T[] toArray​(T[] a)
>
> says
>
>> Note that toArray(new Object[0]) is identical in function to toArray().
>
> Clearly, this should also be specified on toArray() itself.
>
> Patch is below.
>
> Thanks,
>
> s'marks
>
>
>
> diff -r 9bb771005928 -r c3d5e370e06f src/java.base/share/classes/java/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 14:29:14 2017 -0800
> @@ -268,7 +268,7 @@
>      * 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 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 +278,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 component type is {@code Object}, containing all
> +     * of the elements in this collection
>      */
>     Object[] toArray();
>

Reply | Threaded
Open this post in threaded view
|

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

Martin Buchholz-3
In reply to this post by Stuart Marks
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.

On Wed, Nov 29, 2017 at 4:18 PM, Stuart Marks <[hidden email]>
wrote:

> Hi all,
>
> Please review this small spec change / clarification regarding
> Collection.toArray(). The runtime type of the array returned has always
> been intended to be exactly Object[] and not an array of some subtype. This
> requirement is actually implied by the spec already, but in the wrong
> place; the spec for
>
>     <T> T[] toArray​(T[] a)
>
> says
>
> Note that toArray(new Object[0]) is identical in function to toArray().
>>
>
> Clearly, this should also be specified on toArray() itself.
>
> Patch is below.
>
> Thanks,
>
> s'marks
>
>
>
> diff -r 9bb771005928 -r c3d5e370e06f 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 14:29:14 2017 -0800
> @@ -268,7 +268,7 @@
>       * 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 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 +278,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 component type is {@code Object},
> containing all
> +     * of the elements in this collection
>       */
>      Object[] toArray();
>
>
Reply | Threaded
Open this post in threaded view
|

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

Stuart Marks


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/java/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);
Reply | Threaded
Open this post in threaded view
|

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

Martin Buchholz-3
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 <[hidden email]>
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);
>
Reply | Threaded
Open this post in threaded view
|

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

Stuart Marks


On 11/29/17 7:28 PM, Martin Buchholz wrote:
> 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.

OK. This makes it read a bit better. We emphasize the runtime type in the other
places where it's important, which is probably provides a sufficient distinction.

Thanks,

s'marks