On Fri, 2 Oct 2020 17:16:03 GMT, Nir Lisker <nlis...@openjdk.org> wrote:

>> I looked at the fix for 
>> [JDK-8093144](https://bugs.openjdk.java.net/browse/JDK-8093144), and the 
>> reason BitSet was
>> introduced was to ensure that the elements are removed from this List in 
>> reverse order (prior to that fix, they were
>> removed in forward order with the loop index being messed up). This patch 
>> preserves the correct behavior, and just
>> looks to be a better fix for that earlier problem.  I do recommend running 
>> the failing test case from
>> [JDK-8093144](https://bugs.openjdk.java.net/browse/JDK-8093144) to verify no 
>> regressions, but it looks like a good and
>> safe fix to me.  @yososs I marked this discussion thread as unresolved 
>> mainly to make this comment, but also because
>> you didn't fix the spacing suggested by @nlisker -- please do that.
>
>> the reason BitSet was introduced was to ensure that the elements are removed 
>> from this List in reverse order (prior to
>> that fix, they were removed in forward order with the loop index being 
>> messed up).
> 
> But why do they need to be removed in reverse order to begin with? The super 
> implementation does forward removal, or
> just use `removeIf`.

It might not matter any more (presuming that it was done correctly), but it 
seems safer to leave the logic as-is to
match the existing behavior.

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

PR: https://git.openjdk.java.net/jfx/pull/305

Reply via email to