Skip to content

Ghost element when applying multiples minus operations on a PersistentHashSet #219

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 14 commits into from
May 8, 2025
Merged
10 changes: 5 additions & 5 deletions core/commonMain/src/implementations/immutableSet/TrieNode.kt
Original file line number Diff line number Diff line change
Expand Up @@ -621,17 +621,17 @@ internal class TrieNode<E>(
val realSize = realBitMap.countOneBits()
return when {
realBitMap == 0 -> EMPTY
// single values are kept only on root level
realSize == 1 && shift != 0 -> when (val single = mutableNode.buffer[mutableNode.indexOfCellAt(realBitMap)]) {
is TrieNode<*> -> TrieNode<E>(realBitMap, arrayOf(single), mutator.ownership)
else -> single
}
realBitMap == bitmap -> {
when {
mutableNode.elementsIdentityEquals(this) -> this
else -> mutableNode
}
}
// single values are kept only on root level
realSize == 1 && shift != 0 -> when (val single = mutableNode.buffer[mutableNode.indexOfCellAt(realBitMap)]) {
is TrieNode<*> -> TrieNode<E>(realBitMap, arrayOf(single), mutator.ownership)
else -> single
}
else -> {
// clean up all the EMPTYs in the resulting buffer
val realBuffer = arrayOfNulls<Any>(realSize)
Expand Down
29 changes: 29 additions & 0 deletions core/commonTest/src/contract/set/PersistentHashSetTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,10 @@

package tests.contract.set

import kotlinx.collections.immutable.implementations.immutableSet.PersistentHashSet
import kotlinx.collections.immutable.persistentHashSetOf
import kotlinx.collections.immutable.minus
import kotlinx.collections.immutable.toPersistentHashSet
import kotlin.test.Test
import kotlin.test.assertEquals
import kotlin.test.assertTrue
Expand All @@ -27,4 +30,30 @@ class PersistentHashSetTest {
assertEquals(set2, builder.build().toSet())
assertEquals(set2, builder.build())
}

/**
* Test from issue: https://github.com/Kotlin/kotlinx.collections.immutable/issues/144
*/
@Test
fun `removing multiple batches should leave only remaining elements`() {
val firstBatch = listOf(4554, 9380, 4260, 6602)
val secondBatch = listOf(1188, 14794)
val extraElement = 7450

val set = firstBatch.plus(secondBatch).plus(extraElement).toPersistentHashSet()
val result = set.minus(firstBatch.toPersistentHashSet()).minus(secondBatch)
assertEquals(1, result.size)
assertEquals(extraElement, result.first())
}

@Test
fun `after removing elements from one collision the remaining one element must be promoted to the root`() {
val set1: PersistentHashSet<Int> = persistentHashSetOf(0, 32768, 65536) as PersistentHashSet<Int>
val set2: PersistentHashSet<Int> = persistentHashSetOf(0, 32768) as PersistentHashSet<Int>

val expected = persistentHashSetOf(65536)
val actual = set1 - set2

assertEquals(expected, actual)
}
}