Skip to content

Commit 3794c25

Browse files
qurbonzodaqurbonzoda
authored
Merge pull request #33 from Kotlin/guava-test-suite
Support fail-fast behaviour in iterators and add tests from guava
2 parents dd792b7 + a61c5ce commit 3794c25

25 files changed

+1113
-152
lines changed

‎kotlinx-collections-immutable/build.gradle

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ dependencies {
4444
project('tests') {
4545
dependencies {
4646
compile project(path: ':kotlinx-collections-immutable', configuration: 'shadow')
47+
testCompile 'com.google.guava:guava-testlib:18.0'
4748
}
4849
}
4950

‎kotlinx-collections-immutable/src/main/kotlin/kotlinx/collections/immutable/implementations/immutableList/PersistentVector.kt

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -250,10 +250,9 @@ internal class PersistentVector<E>(private val root: Array<Any?>,
250250
return newRoot
251251
}
252252

253-
// Could be val bufferLastIndex = root.indexOfLast { it != null }, but that isn't optimized enough
254253
var bufferLastIndex = MAX_BUFFER_SIZE_MINUS_ONE
255-
while (root[bufferLastIndex] == null) {
256-
bufferLastIndex -= 1
254+
if (root[bufferLastIndex] == null) {
255+
bufferLastIndex = indexSegment(rootSize() - 1, shift)
257256
}
258257

259258
val newRoot = root.copyOf(MAX_BUFFER_SIZE)

‎kotlinx-collections-immutable/src/main/kotlin/kotlinx/collections/immutable/implementations/immutableList/PersistentVectorBuilder.kt

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -299,8 +299,8 @@ class PersistentVectorBuilder<E>(private var vector: PersistentList<E>,
299299
}
300300

301301
var bufferLastIndex = MAX_BUFFER_SIZE_MINUS_ONE
302-
while (root[bufferLastIndex] == null) {
303-
bufferLastIndex -= 1
302+
if (root[bufferLastIndex] == null) {
303+
bufferLastIndex = indexSegment(rootSize() - 1, shift)
304304
}
305305

306306
val mutableRoot = makeMutable(root)
@@ -380,6 +380,10 @@ class PersistentVectorBuilder<E>(private var vector: PersistentList<E>,
380380
checkElementIndex(index, size)
381381
if (rootSize() <= index) {
382382
val mutableTail = makeMutable(tail)
383+
384+
// Creating new tail implies structural change.
385+
if (mutableTail !== tail) { modCount++ }
386+
383387
val tailIndex = index and MAX_BUFFER_SIZE_MINUS_ONE
384388
val oldElement = mutableTail[tailIndex]
385389
mutableTail[tailIndex] = element
@@ -399,6 +403,12 @@ class PersistentVectorBuilder<E>(private var vector: PersistentList<E>,
399403
val mutableRoot = makeMutable(root)
400404

401405
if (shift == 0) {
406+
// Creating new leaf implies structural change.
407+
// Actually, while descending to this leaf several nodes could be recreated.
408+
// However, this builder is exclusive owner of this leaf iff it is exclusive owner of all leaf's ancestors.
409+
// Hence, checking recreation of this leaf is enough to determine if a structural change occurred.
410+
if (mutableRoot !== root) { modCount++ }
411+
402412
oldElementCarry.value = mutableRoot[bufferIndex]
403413
mutableRoot[bufferIndex] = e
404414
return mutableRoot

‎kotlinx-collections-immutable/src/main/kotlin/kotlinx/collections/immutable/implementations/immutableMap/PersistentHashMapBuilder.kt

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,14 @@ internal class PersistentHashMapBuilder<K, V>(private var map: PersistentHashMap
2424
internal var marker = Marker()
2525
internal var node = map.node
2626
internal var operationResult: V? = null
27+
internal var modCount = 0
28+
29+
// Size change implies structural changes.
2730
override var size = map.size
31+
set(value) {
32+
field = value
33+
modCount++
34+
}
2835

2936
override fun build(): PersistentHashMap<K, V> {
3037
map = if (node === map.node) {

‎kotlinx-collections-immutable/src/main/kotlin/kotlinx/collections/immutable/implementations/immutableMap/PersistentHashMapBuilderContentIterators.kt

Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -43,30 +43,31 @@ internal abstract class PersistentHashMapBuilderBaseIterator<K, V, T>(private va
4343
path: Array<TrieNodeBaseIterator<K, V, T>>)
4444
: MutableIterator<T>, PersistentHashMapBaseIterator<K, V, T>(builder.node, path) {
4545

46-
private var lastKey: K? = null
46+
private var lastIteratedKey: K? = null
4747
private var nextWasInvoked = false
48+
private var expectedModCount = builder.modCount
4849

4950
override fun next(): T {
50-
lastKey = currentKey()
51+
checkForComodification()
52+
lastIteratedKey = currentKey()
5153
nextWasInvoked = true
5254
return super.next()
5355
}
5456

5557
override fun remove() {
56-
if (!nextWasInvoked) {
57-
throw NoSuchElementException()
58-
}
58+
checkNextWasInvoked()
5959
if (hasNext()) {
6060
val currentKey = currentKey()
6161

62-
builder.remove(lastKey)
62+
builder.remove(lastIteratedKey)
6363
resetPath(currentKey.hashCode(), builder.node, currentKey, 0)
6464
} else {
65-
builder.remove(lastKey)
65+
builder.remove(lastIteratedKey)
6666
}
6767

68-
lastKey = null
68+
lastIteratedKey = null
6969
nextWasInvoked = false
70+
expectedModCount = builder.modCount
7071
}
7172

7273
private fun resetPath(keyHash: Int, node: TrieNode<*, *>, key: K, pathIndex: Int) {
@@ -76,13 +77,13 @@ internal abstract class PersistentHashMapBuilderBaseIterator<K, V, T>(private va
7677
if (node.hasDataAt(keyPosition)) { // key is directly in buffer
7778
val keyIndex = node.keyDataIndex(keyPosition)
7879

79-
assert(node.keyAtIndex(keyIndex) == key)
80+
// assert(node.keyAtIndex(keyIndex) == key)
8081

8182
path[pathIndex].reset(node.buffer, 2 * Integer.bitCount(node.dataMap), keyIndex)
8283
return
8384
}
8485

85-
assert(node.hasNodeAt(keyPosition)) // key is in node
86+
// assert(node.hasNodeAt(keyPosition)) // key is in node
8687

8788
val nodeIndex = node.keyNodeIndex(keyPosition)
8889
val targetNode = node.nodeAtIndex(nodeIndex)
@@ -96,6 +97,16 @@ internal abstract class PersistentHashMapBuilderBaseIterator<K, V, T>(private va
9697
resetPath(keyHash, targetNode, key, pathIndex + 1)
9798
}
9899
}
100+
101+
private fun checkNextWasInvoked() {
102+
if (!nextWasInvoked)
103+
throw IllegalStateException()
104+
}
105+
106+
private fun checkForComodification() {
107+
if (builder.modCount != expectedModCount)
108+
throw ConcurrentModificationException()
109+
}
99110
}
100111

101112
internal class PersistentHashMapBuilderEntriesIterator<K, V>(builder: PersistentHashMapBuilder<K, V>)

‎kotlinx-collections-immutable/src/main/kotlin/kotlinx/collections/immutable/implementations/immutableMap/PersistentHashMapContentIterators.kt

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,7 @@ internal abstract class PersistentHashMapBaseIterator<K, V, T>(node: TrieNode<K,
134134
}
135135

136136
protected fun currentKey(): K {
137+
checkHasNext()
137138
return path[pathLastIndex].currentKey()
138139
}
139140

@@ -142,11 +143,16 @@ internal abstract class PersistentHashMapBaseIterator<K, V, T>(node: TrieNode<K,
142143
}
143144

144145
override fun next(): T {
145-
assert(hasNext())
146+
checkHasNext()
146147
val result = path[pathLastIndex].next()
147148
ensureNextEntryIsReady()
148149
return result
149150
}
151+
152+
private fun checkHasNext() {
153+
if (!hasNext())
154+
throw NoSuchElementException()
155+
}
150156
}
151157

152158
internal class PersistentHashMapEntriesIterator<K, V>(node: TrieNode<K, V>)

‎kotlinx-collections-immutable/src/main/kotlin/kotlinx/collections/immutable/implementations/immutableMap/TrieNode.kt

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ internal class TrieNode<K, V>(var dataMap: Int,
1919
return dataMap and position != 0
2020
}
2121

22-
internal fun hasNodeAt(position: Int): Boolean {
22+
private fun hasNodeAt(position: Int): Boolean {
2323
return nodeMap and position != 0
2424
}
2525

@@ -31,7 +31,7 @@ internal class TrieNode<K, V>(var dataMap: Int,
3131
return buffer.size - 1 - Integer.bitCount(nodeMap and (position - 1))
3232
}
3333

34-
internal fun keyAtIndex(keyIndex: Int): K {
34+
private fun keyAtIndex(keyIndex: Int): K {
3535
return buffer[keyIndex] as K
3636
}
3737

@@ -84,11 +84,14 @@ internal class TrieNode<K, V>(var dataMap: Int,
8484
private fun mutableUpdateValueAtIndex(keyIndex: Int, value: V, mutator: PersistentHashMapBuilder<K, V>): TrieNode<K, V> {
8585
// assert(buffer[keyIndex + 1] !== value)
8686

87-
mutator.operationResult = buffer[keyIndex + 1] as V
87+
// If the [mutator] is exclusive owner of this node, update value at specified index in-place.
8888
if (marker === mutator.marker) {
8989
buffer[keyIndex + 1] = value
9090
return this
9191
}
92+
// Structural change due to node replacement.
93+
mutator.modCount++
94+
// Create new node with updated value at specified index.
9295
val newBuffer = buffer.copyOf()
9396
newBuffer[keyIndex + 1] = value
9497
return TrieNode(dataMap, nodeMap, newBuffer, mutator.marker)
@@ -103,6 +106,8 @@ internal class TrieNode<K, V>(var dataMap: Int,
103106
}
104107

105108
private fun mutableUpdateNodeAtIndex(nodeIndex: Int, newNode: TrieNode<K, V>, mutatorMarker: Marker): TrieNode<K, V> {
109+
// assert(buffer[nodeIndex] !== newNode)
110+
106111
if (marker === mutatorMarker) {
107112
buffer[nodeIndex] = newNode
108113
return this
@@ -286,19 +291,26 @@ internal class TrieNode<K, V>(var dataMap: Int,
286291
}
287292

288293
private fun mutableCollisionPut(key: K, value: V, mutator: PersistentHashMapBuilder<K, V>): TrieNode<K, V> {
294+
// Check if there is an entry with the specified key.
289295
for (i in 0 until buffer.size step ENTRY_SIZE) {
290-
if (key == buffer[i]) {
296+
if (key == buffer[i]) { // found entry with the specified key
291297
mutator.operationResult = buffer[i + 1] as V
292298

299+
// If the [mutator] is exclusive owner of this node, update value of the entry in-place.
293300
if (marker === mutator.marker) {
294301
buffer[i + 1] = value
295302
return this
296303
}
304+
305+
// Structural change due to node replacement.
306+
mutator.modCount++
307+
// Create new node with updated entry value.
297308
val newBuffer = buffer.copyOf()
298309
newBuffer[i + 1] = value
299310
return TrieNode(0, 0, newBuffer, mutator.marker)
300311
}
301312
}
313+
// Create new collision node with the specified entry added to it.
302314
mutator.size++
303315
val newBuffer = bufferPutDataAtIndex(0, key, value)
304316
return TrieNode(0, 0, newBuffer, mutator.marker)
@@ -388,9 +400,9 @@ internal class TrieNode<K, V>(var dataMap: Int,
388400
val keyIndex = keyDataIndex(keyPosition)
389401

390402
if (key == keyAtIndex(keyIndex)) {
403+
modification.value = UPDATE_VALUE
391404
if (valueAtKeyIndex(keyIndex) === value) { return this }
392405

393-
modification.value = UPDATE_VALUE
394406
return updateValueAtIndex(keyIndex, value)
395407
}
396408
modification.value = PUT_KEY_VALUE
@@ -421,10 +433,9 @@ internal class TrieNode<K, V>(var dataMap: Int,
421433
val keyIndex = keyDataIndex(keyPosition)
422434

423435
if (key == keyAtIndex(keyIndex)) {
424-
if (valueAtKeyIndex(keyIndex) === value) { // needs discussion
425-
mutator.operationResult = value
426-
return this
427-
}
436+
mutator.operationResult = valueAtKeyIndex(keyIndex)
437+
if (valueAtKeyIndex(keyIndex) === value) { return this }
438+
428439
return mutableUpdateValueAtIndex(keyIndex, value, mutator)
429440
}
430441
mutator.size++

‎kotlinx-collections-immutable/src/main/kotlin/kotlinx/collections/immutable/implementations/immutableSet/PersistentHashSetBuilder.kt

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,14 @@ internal class Marker
2323
internal class PersistentHashSetBuilder<E>(private var set: PersistentHashSet<E>) : AbstractMutableSet<E>(), PersistentSet.Builder<E> {
2424
internal var marker = Marker()
2525
internal var node = set.node
26+
internal var modCount = 0
27+
28+
// Size change implies structural changes.
2629
override var size = set.size
30+
set(value) {
31+
field = value
32+
modCount++
33+
}
2734

2835
override fun build(): PersistentSet<E> {
2936
set = if (node === set.node) {

‎kotlinx-collections-immutable/src/main/kotlin/kotlinx/collections/immutable/implementations/immutableSet/PersistentHashSetMutableIterator.kt

Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -18,31 +18,32 @@ package kotlinx.collections.immutable.implementations.immutableSet
1818

1919
internal class PersistentHashSetMutableIterator<E>(private val builder: PersistentHashSetBuilder<E>)
2020
: PersistentHashSetIterator<E>(builder.node), MutableIterator<E> {
21-
var lastReturned: E? = null
22-
var nextWasInvoked = false
21+
private var lastIteratedElement: E? = null
22+
private var nextWasInvoked = false
23+
private var expectedModCount = builder.modCount
2324

2425
override fun next(): E {
26+
checkForComodification()
2527
val next = super.next()
26-
lastReturned = next
28+
lastIteratedElement = next
2729
nextWasInvoked = true
2830
return next
2931
}
3032

3133
override fun remove() {
32-
if (!nextWasInvoked) {
33-
throw NoSuchElementException()
34-
}
34+
checkNextWasInvoked()
3535
if (hasNext()) {
3636
val currentElement = currentElement()
3737

38-
assert(builder.remove(lastReturned))
38+
assert(builder.remove(lastIteratedElement))
3939
resetPath(currentElement.hashCode(), builder.node, currentElement, 0)
4040
} else {
41-
assert(builder.remove(lastReturned))
41+
assert(builder.remove(lastIteratedElement))
4242
}
4343

44-
lastReturned = null
44+
lastIteratedElement = null
4545
nextWasInvoked = false
46+
expectedModCount = builder.modCount
4647
}
4748

4849
private fun resetPath(hashCode: Int, node: TrieNode<*>, element: E, pathIndex: Int) {
@@ -63,12 +64,22 @@ internal class PersistentHashSetMutableIterator<E>(private val builder: Persiste
6364
if (cell is TrieNode<*>) {
6465
resetPath(hashCode, cell, element, pathIndex + 1)
6566
} else {
66-
assert(cell == element)
67+
// assert(cell == element)
6768
pathLastIndex = pathIndex
6869
}
6970
}
7071

7172
private fun isCollision(node: TrieNode<*>): Boolean {
7273
return node.bitmap == 0
7374
}
75+
76+
private fun checkNextWasInvoked() {
77+
if (!nextWasInvoked)
78+
throw IllegalStateException()
79+
}
80+
81+
private fun checkForComodification() {
82+
if (builder.modCount != expectedModCount)
83+
throw ConcurrentModificationException()
84+
}
7485
}

0 commit comments

Comments
 (0)