On Sun, 9 Apr 2023 02:28:37 GMT, Tingjun Yuan <d...@openjdk.org> wrote:

>> In the current implementation of `String.join(CharSequence, Iterable)`, the 
>> temp array `elems` is always initialized with a length of 8. It will cause 
>> many array recreations when the `Iterable` contains more than 8 elements. 
>> Furthermore, it's very common that an `Iterable` is also a `Collection`. So 
>> if the `Iterable` is an instance of `Collection`, the initial length of the 
>> array can be `((Collection<?>)elements).size()`. It will not change the 
>> current behavior even if the `Collection` is modified asynchronously.
>> 
>> I don't know whether this change requires a CSR request.
>
> Tingjun Yuan has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Add benchmark

If you really don't trust a collection, then we can't do anything.

Can copying the results of `toArray` ensure accuracy and security? It has too 
many possible problems. Maybe the size of the array is wrong, maybe it forgot 
to copy the contents of the collection and all it returns is an array full of 
nulls.

To put it one step further, is its iterator necessarily correct? Perhaps its 
iterator implementation is also incorrect:


class BadList implements List<Object> {
    private Object[] array;

    // ...

    public Iterator<Object> iterator(){
        return new Iterator<Object>() {
            int i = 0;

            public boolean hasNext() {
                return true;
            }

            public Object next() {
                array[i++] = null;
                return new Object(); // crazy implementation
            }
        };
    }
}


But who cares? Since its implementation is incorrect, it is normal for it to 
suffer for itself. We only need to prevent errors from being leaked to other 
places, rather than defending against all errors.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/13383#issuecomment-1501390568

Reply via email to