On Tue, 15 Aug 2023 14:08:08 GMT, John Hendrikx <jhendr...@openjdk.org> wrote:
>> modules/javafx.graphics/src/main/java/com/sun/javafx/css/PseudoClassState.java
>> line 65:
>>
>>> 63: /** {@inheritDoc} */
>>> 64: @Override
>>> 65: public <T> T[] toArray(T[] a) {
>>
>> You have mentioned that this was a bug but hard to reproduce.
>> Could you add a test that somehow calls `toArray` that fails before your
>> patch and passes now?
>
> It's definitely a bug, but it would only appear if you had bit sets which
> required more than 1 long to store their data **and** you called `toArray`.
> It would then throw an `ArrayIndexOutOfBoundsException` in this line:
>
> final long state = getBits()[index];
>
> This is because `index` is used for two unrelated purposes:
> - the index into the array of longs
> - the index of the entry in the resulting array
>
> If the outer loop ever needs two passes, it again wants to read a `long`
> using `getBits()[index]` but as `index` was advanced (multiple times) while
> filling entries in the result array, it won't be the correct index at all.
>
> Anyway, I'm not sure what the value would be of a test. Since `toArray` is
> no longer implemented by us but by `AbstractSet`, I'd just be testing the
> `AbstractSet` code.
Okay, thanks for the clarification.
So if I get it right, the removal of `toArray` doesn't have to do with the bug,
but with the fact that with the immutable set it is not longer required?
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1076#discussion_r1294681403
