RFR: 6187113: DefaultListSelectionModel.removeIndexInterval(0, Integer.MAX_VALUE) fails [v6]
Alexey Ivanov
aivanov at openjdk.org
Wed Oct 19 13:22:01 UTC 2022
On Tue, 18 Oct 2022 20:01:22 GMT, Alexey Ivanov <aivanov at openjdk.org> wrote:
>> Prasanta Sadhukhan has updated the pull request incrementally with one additional commit since the last revision:
>>
>> insertIndexInterval fix. Add more subtests
>
> src/java.desktop/share/classes/javax/swing/DefaultListSelectionModel.java line 721:
>
>> 719: setState(i, value.get(gapLength));
>> 720: break;
>> 721: }
>
> This does not work. It must not break. With this code, 5 of my tests fail.
>
> For example,
>
>
> selectionModel.setSelectionInterval(0, Integer.MAX_VALUE);
> selectionModel.removeIndexInterval(0, Integer.MAX_VALUE);
>
> must result in empty selection. All the possible indexes were selected, you removed all the possible indexes (so that “the list” contains no elements at all). This test case fails either way, unfortunately: `selectionModel.isSelectedIndex(0)` still returns `true`.
>
> This edge case where `rmMinIndex = 0` and `rmMaxIndex = Integer.MAX_VALUE` should probably be short-circuited: `setState(i, false)` for all the elements. Other values are handled correctly by my suggested code, as far as I can see.
>
> This test case:
>
> selectionModel.setSelectionInterval(0, Integer.MAX_VALUE);
> selectionModel.removeIndexInterval(1, Integer.MAX_VALUE);
>
> passed before and now it fails again. Here, `selectionModel.isSelectedIndex(0)` must be `true`.
If we handle the edge case separately, it looks better and the first test case as well as other tests for `remove` pass successfully:
public void removeIndexInterval(int index0, int index1)
{
if (index0 < 0 || index1 < 0) {
throw new IndexOutOfBoundsException("index is negative");
}
int rmMinIndex = Math.min(index0, index1);
int rmMaxIndex = Math.max(index0, index1);
if (rmMinIndex == 0 && rmMaxIndex == Integer.MAX_VALUE) {
for (int i = Integer.MAX_VALUE; i >= 0; i--) {
setState(i, false);
}
// min and max are updated automatically by the for-loop
// TODO Update anchor and lead
return;
}
int gapLength = (rmMaxIndex - rmMinIndex) + 1;
/* Shift the entire bitset to the left to close the index0, index1
* gap.
*/
for(int i = rmMinIndex; i >= 0 && i <= maxIndex; i++) {
setState(i, (i <= Integer.MAX_VALUE - gapLength)
&& (i + gapLength >= minIndex)
&& value.get(i + gapLength));
}
// The rest of the method
}
-------------
PR: https://git.openjdk.org/jdk/pull/10409
More information about the client-libs-dev
mailing list