From 5a8faa8775ea87f89c70a0f6c2b147cf33ccfe47 Mon Sep 17 00:00:00 2001 From: Troels Bjerre Lund Date: Thu, 5 Oct 2023 15:25:01 +0200 Subject: [PATCH] Align ListBuilder with native ArrayList Furthermore, add test for ListBuilder.subList detection of concurrent modification, and fix error in ListBuilder and ArrayList. Fix KT-62346 --- .../collections/builders/ListBuilder.kt | 530 +++++++++++++----- .../jvm/test/collections/CollectionJVMTest.kt | 1 - .../src/kotlin/collections/ArrayList.kt | 278 ++++----- .../collections/ConcurrentModificationTest.kt | 6 + 4 files changed, 508 insertions(+), 307 deletions(-) diff --git a/libraries/stdlib/jvm/src/kotlin/collections/builders/ListBuilder.kt b/libraries/stdlib/jvm/src/kotlin/collections/builders/ListBuilder.kt index 30dca898637..4421a318d08 100644 --- a/libraries/stdlib/jvm/src/kotlin/collections/builders/ListBuilder.kt +++ b/libraries/stdlib/jvm/src/kotlin/collections/builders/ListBuilder.kt @@ -9,81 +9,58 @@ import java.io.Externalizable import java.io.InvalidObjectException import java.io.NotSerializableException -internal class ListBuilder private constructor( - private var array: Array, - private var offset: Int, - private var length: Int, - private var isReadOnly: Boolean, - private val backing: ListBuilder?, - private val root: ListBuilder? -) : MutableList, RandomAccess, AbstractMutableList(), Serializable { +internal class ListBuilder(initialCapacity: Int = 10) : MutableList, RandomAccess, AbstractMutableList(), Serializable { + private var backing = arrayOfUninitializedElements(initialCapacity) + private var length = 0 + private var isReadOnly = false + private companion object { private val Empty = ListBuilder(0).also { it.isReadOnly = true } } - init { - if (backing != null) this.modCount = backing.modCount - } - - constructor() : this(10) - - constructor(initialCapacity: Int) : this( - arrayOfUninitializedElements(initialCapacity), 0, 0, false, null, null) - fun build(): List { - if (backing != null) throw IllegalStateException() // just in case somebody casts subList to ListBuilder checkIsMutable() isReadOnly = true return if (length > 0) this else Empty } private fun writeReplace(): Any = - if (isEffectivelyReadOnly) + if (isReadOnly) SerializedCollection(this, SerializedCollection.tagList) else throw NotSerializableException("The list cannot be serialized while it is being built.") override val size: Int - get() { - checkForComodification() - return length - } + get() = length - override fun isEmpty(): Boolean { - checkForComodification() - return length == 0 - } + override fun isEmpty() = length == 0 override fun get(index: Int): E { - checkForComodification() AbstractList.checkElementIndex(index, length) - return array[offset + index] + return backing[index] } override operator fun set(index: Int, element: E): E { checkIsMutable() - checkForComodification() AbstractList.checkElementIndex(index, length) - val old = array[offset + index] - array[offset + index] = element + val old = backing[index] + backing[index] = element return old } override fun indexOf(element: E): Int { - checkForComodification() var i = 0 while (i < length) { - if (array[offset + i] == element) return i + if (backing[i] == element) return i i++ } return -1 } override fun lastIndexOf(element: E): Int { - checkForComodification() var i = length - 1 while (i >= 0) { - if (array[offset + i] == element) return i + if (backing[i] == element) return i i-- } return -1 @@ -93,58 +70,50 @@ internal class ListBuilder private constructor( override fun listIterator(): MutableListIterator = listIterator(0) override fun listIterator(index: Int): MutableListIterator { - checkForComodification() AbstractList.checkPositionIndex(index, length) return Itr(this, index) } override fun add(element: E): Boolean { checkIsMutable() - checkForComodification() - addAtInternal(offset + length, element) + addAtInternal(length, element) return true } override fun add(index: Int, element: E) { checkIsMutable() - checkForComodification() AbstractList.checkPositionIndex(index, length) - addAtInternal(offset + index, element) + addAtInternal(index, element) } override fun addAll(elements: Collection): Boolean { checkIsMutable() - checkForComodification() val n = elements.size - addAllInternal(offset + length, elements, n) + addAllInternal(length, elements, n) return n > 0 } override fun addAll(index: Int, elements: Collection): Boolean { checkIsMutable() - checkForComodification() AbstractList.checkPositionIndex(index, length) val n = elements.size - addAllInternal(offset + index, elements, n) + addAllInternal(index, elements, n) return n > 0 } override fun clear() { checkIsMutable() - checkForComodification() - removeRangeInternal(offset, length) + removeRangeInternal(0, length) } override fun removeAt(index: Int): E { checkIsMutable() - checkForComodification() AbstractList.checkElementIndex(index, length) - return removeAtInternal(offset + index) + return removeAtInternal(index) } override fun remove(element: E): Boolean { checkIsMutable() - checkForComodification() val i = indexOf(element) if (i >= 0) removeAt(i) return i >= 0 @@ -152,53 +121,46 @@ internal class ListBuilder private constructor( override fun removeAll(elements: Collection): Boolean { checkIsMutable() - checkForComodification() - return retainOrRemoveAllInternal(offset, length, elements, false) > 0 + return retainOrRemoveAllInternal(0, length, elements, false) > 0 } override fun retainAll(elements: Collection): Boolean { checkIsMutable() - checkForComodification() - return retainOrRemoveAllInternal(offset, length, elements, true) > 0 + return retainOrRemoveAllInternal(0, length, elements, true) > 0 } override fun subList(fromIndex: Int, toIndex: Int): MutableList { AbstractList.checkRangeIndexes(fromIndex, toIndex, length) - return ListBuilder(array, offset + fromIndex, toIndex - fromIndex, isReadOnly, this, root ?: this) + return BuilderSubList(backing, fromIndex, toIndex - fromIndex, null, this) } - override fun toArray(destination: Array): Array { - checkForComodification() - if (destination.size < length) { - return java.util.Arrays.copyOfRange(array, offset, offset + length, destination.javaClass) + @Suppress("UNCHECKED_CAST") + override fun toArray(array: Array): Array { + if (array.size < length) { + return java.util.Arrays.copyOfRange(backing, 0, length, array.javaClass) } - @Suppress("UNCHECKED_CAST") - (array as Array).copyInto(destination, 0, startIndex = offset, endIndex = offset + length) + (backing as Array).copyInto(array, 0, startIndex = 0, endIndex = length) - return terminateCollectionToArray(length, destination) + return terminateCollectionToArray(length, array) } override fun toArray(): Array { - checkForComodification() @Suppress("UNCHECKED_CAST") - return array.copyOfRange(fromIndex = offset, toIndex = offset + length) as Array + return backing.copyOfRange(fromIndex = 0, toIndex = length) as Array } override fun equals(other: Any?): Boolean { - checkForComodification() return other === this || (other is List<*>) && contentEquals(other) } override fun hashCode(): Int { - checkForComodification() - return array.subarrayContentHashCode(offset, length) + return backing.subarrayContentHashCode(0, length) } override fun toString(): String { - checkForComodification() - return array.subarrayContentToString(offset, length, this) + return backing.subarrayContentToString(0, length, this) } // ---------------------------- private ---------------------------- @@ -207,131 +169,90 @@ internal class ListBuilder private constructor( modCount += 1 } - private fun checkForComodification() { - if (root != null && root.modCount != modCount) - throw ConcurrentModificationException() - } - private fun checkIsMutable() { - if (isEffectivelyReadOnly) throw UnsupportedOperationException() + if (isReadOnly) throw UnsupportedOperationException() } - private val isEffectivelyReadOnly: Boolean - get() = isReadOnly || root != null && root.isReadOnly - private fun ensureExtraCapacity(n: Int) { ensureCapacityInternal(length + n) } private fun ensureCapacityInternal(minCapacity: Int) { if (minCapacity < 0) throw OutOfMemoryError() // overflow - if (minCapacity > array.size) { - val newSize = AbstractList.newCapacity(array.size, minCapacity) - array = array.copyOfUninitializedElements(newSize) + if (minCapacity > backing.size) { + val newSize = AbstractList.newCapacity(backing.size, minCapacity) + backing = backing.copyOfUninitializedElements(newSize) } } private fun contentEquals(other: List<*>): Boolean { - return array.subarrayContentEquals(offset, length, other) + return backing.subarrayContentEquals(0, length, other) } private fun insertAtInternal(i: Int, n: Int) { ensureExtraCapacity(n) - array.copyInto(array, startIndex = i, endIndex = offset + length, destinationOffset = i + n) + backing.copyInto(backing, startIndex = i, endIndex = length, destinationOffset = i + n) length += n } private fun addAtInternal(i: Int, element: E) { registerModification() - if (backing != null) { - backing.addAtInternal(i, element) - array = backing.array - length++ - } else { - insertAtInternal(i, 1) - array[i] = element - } + insertAtInternal(i, 1) + backing[i] = element } private fun addAllInternal(i: Int, elements: Collection, n: Int) { registerModification() - if (backing != null) { - backing.addAllInternal(i, elements, n) - array = backing.array - length += n - } else { - insertAtInternal(i, n) - var j = 0 - val it = elements.iterator() - while (j < n) { - array[i + j] = it.next() - j++ - } + insertAtInternal(i, n) + var j = 0 + val it = elements.iterator() + while (j < n) { + backing[i + j] = it.next() + j++ } } private fun removeAtInternal(i: Int): E { registerModification() - if (backing != null) { - val old = backing.removeAtInternal(i) - length-- - return old - } else { - val old = array[i] - array.copyInto(array, startIndex = i + 1, endIndex = offset + length, destinationOffset = i) - array.resetAt(offset + length - 1) - length-- - return old - } + val old = backing[i] + backing.copyInto(backing, startIndex = i + 1, endIndex = length, destinationOffset = i) + backing.resetAt(length - 1) + length-- + return old } private fun removeRangeInternal(rangeOffset: Int, rangeLength: Int) { if (rangeLength > 0) registerModification() - if (backing != null) { - backing.removeRangeInternal(rangeOffset, rangeLength) - } else { - array.copyInto(array, startIndex = rangeOffset + rangeLength, endIndex = length, destinationOffset = rangeOffset) - array.resetRange(fromIndex = length - rangeLength, toIndex = length) - } + backing.copyInto(backing, startIndex = rangeOffset + rangeLength, endIndex = length, destinationOffset = rangeOffset) + backing.resetRange(fromIndex = length - rangeLength, toIndex = length) length -= rangeLength } /** Retains elements if [retain] == true and removes them it [retain] == false. */ private fun retainOrRemoveAllInternal(rangeOffset: Int, rangeLength: Int, elements: Collection, retain: Boolean): Int { - val removed = if (backing != null) { - backing.retainOrRemoveAllInternal(rangeOffset, rangeLength, elements, retain) - } else { - var i = 0 - var j = 0 - while (i < rangeLength) { - if (elements.contains(array[rangeOffset + i]) == retain) { - array[rangeOffset + j++] = array[rangeOffset + i++] - } else { - i++ - } + var i = 0 + var j = 0 + while (i < rangeLength) { + if (elements.contains(backing[rangeOffset + i]) == retain) { + backing[rangeOffset + j++] = backing[rangeOffset + i++] + } else { + i++ } - val removed = rangeLength - j - array.copyInto(array, startIndex = rangeOffset + rangeLength, endIndex = length, destinationOffset = rangeOffset + j) - array.resetRange(fromIndex = length - removed, toIndex = length) - removed } + val removed = rangeLength - j + backing.copyInto(backing, startIndex = rangeOffset + rangeLength, endIndex = length, destinationOffset = rangeOffset + j) + backing.resetRange(fromIndex = length - removed, toIndex = length) if (removed > 0) registerModification() length -= removed return removed } - private class Itr : MutableListIterator { - private val list: ListBuilder + private class Itr( + private val list: ListBuilder, private var index: Int - private var lastIndex: Int - private var expectedModCount: Int - - constructor(list: ListBuilder, index: Int) { - this.list = list - this.index = index - this.lastIndex = -1 - this.expectedModCount = list.modCount - } + ) : MutableListIterator { + private var lastIndex = -1 + private var expectedModCount = list.modCount override fun hasPrevious(): Boolean = index > 0 override fun hasNext(): Boolean = index < list.length @@ -343,20 +264,20 @@ internal class ListBuilder private constructor( checkForComodification() if (index <= 0) throw NoSuchElementException() lastIndex = --index - return list.array[list.offset + lastIndex] + return list.backing[lastIndex] } override fun next(): E { checkForComodification() if (index >= list.length) throw NoSuchElementException() lastIndex = index++ - return list.array[list.offset + lastIndex] + return list.backing[lastIndex] } override fun set(element: E) { checkForComodification() check(lastIndex != -1) { "Call next() or previous() before replacing element from the iterator." } - list.set(lastIndex, element) + list[lastIndex] = element } override fun add(element: E) { @@ -380,6 +301,315 @@ internal class ListBuilder private constructor( throw ConcurrentModificationException() } } + + class BuilderSubList( + private var backing: Array, + private val offset: Int, + private var length: Int, + private val parent: BuilderSubList?, + private val root: ListBuilder + ) : MutableList, RandomAccess, AbstractMutableList(), Serializable { + init { + this.modCount = root.modCount + } + + private fun writeReplace(): Any = + if (isReadOnly) + SerializedCollection(this, SerializedCollection.tagList) + else + throw NotSerializableException("The list cannot be serialized while it is being built.") + + override val size: Int + get() { + checkForComodification() + return length + } + + override fun isEmpty(): Boolean { + checkForComodification() + return length == 0 + } + + override fun get(index: Int): E { + checkForComodification() + AbstractList.checkElementIndex(index, length) + return backing[offset + index] + } + + override operator fun set(index: Int, element: E): E { + checkIsMutable() + checkForComodification() + AbstractList.checkElementIndex(index, length) + val old = backing[offset + index] + backing[offset + index] = element + return old + } + + override fun indexOf(element: E): Int { + checkForComodification() + var i = 0 + while (i < length) { + if (backing[offset + i] == element) return i + i++ + } + return -1 + } + + override fun lastIndexOf(element: E): Int { + checkForComodification() + var i = length - 1 + while (i >= 0) { + if (backing[offset + i] == element) return i + i-- + } + return -1 + } + + override fun iterator(): MutableIterator = listIterator(0) + override fun listIterator(): MutableListIterator = listIterator(0) + + override fun listIterator(index: Int): MutableListIterator { + checkForComodification() + AbstractList.checkPositionIndex(index, length) + return Itr(this, index) + } + + override fun add(element: E): Boolean { + checkIsMutable() + checkForComodification() + addAtInternal(offset + length, element) + return true + } + + override fun add(index: Int, element: E) { + checkIsMutable() + checkForComodification() + AbstractList.checkPositionIndex(index, length) + addAtInternal(offset + index, element) + } + + override fun addAll(elements: Collection): Boolean { + checkIsMutable() + checkForComodification() + val n = elements.size + addAllInternal(offset + length, elements, n) + return n > 0 + } + + override fun addAll(index: Int, elements: Collection): Boolean { + checkIsMutable() + checkForComodification() + AbstractList.checkPositionIndex(index, length) + val n = elements.size + addAllInternal(offset + index, elements, n) + return n > 0 + } + + override fun clear() { + checkIsMutable() + checkForComodification() + removeRangeInternal(offset, length) + } + + override fun removeAt(index: Int): E { + checkIsMutable() + checkForComodification() + AbstractList.checkElementIndex(index, length) + return removeAtInternal(offset + index) + } + + override fun remove(element: E): Boolean { + checkIsMutable() + checkForComodification() + val i = indexOf(element) + if (i >= 0) removeAt(i) + return i >= 0 + } + + override fun removeAll(elements: Collection): Boolean { + checkIsMutable() + checkForComodification() + return retainOrRemoveAllInternal(offset, length, elements, false) > 0 + } + + override fun retainAll(elements: Collection): Boolean { + checkIsMutable() + checkForComodification() + return retainOrRemoveAllInternal(offset, length, elements, true) > 0 + } + + override fun subList(fromIndex: Int, toIndex: Int): MutableList { + AbstractList.checkRangeIndexes(fromIndex, toIndex, length) + return BuilderSubList(backing, offset + fromIndex, toIndex - fromIndex, this, root) + } + + @Suppress("UNCHECKED_CAST") + override fun toArray(array: Array): Array { + checkForComodification() + if (array.size < length) { + return java.util.Arrays.copyOfRange(backing, offset, offset + length, array.javaClass) + } + + (backing as Array).copyInto(array, 0, startIndex = offset, endIndex = offset + length) + + return terminateCollectionToArray(length, array) + } + + override fun toArray(): Array { + checkForComodification() + @Suppress("UNCHECKED_CAST") + return backing.copyOfRange(fromIndex = offset, toIndex = offset + length) as Array + } + + override fun equals(other: Any?): Boolean { + checkForComodification() + return other === this || + (other is List<*>) && contentEquals(other) + } + + override fun hashCode(): Int { + checkForComodification() + return backing.subarrayContentHashCode(offset, length) + } + + override fun toString(): String { + checkForComodification() + return backing.subarrayContentToString(offset, length, this) + } + + // ---------------------------- private ---------------------------- + + private fun registerModification() { + modCount += 1 + } + + private fun checkForComodification() { + if (root.modCount != modCount) + throw ConcurrentModificationException() + } + + private fun checkIsMutable() { + if (isReadOnly) throw UnsupportedOperationException() + } + + private val isReadOnly: Boolean + get() = root.isReadOnly + + private fun contentEquals(other: List<*>): Boolean { + return backing.subarrayContentEquals(offset, length, other) + } + + private fun addAtInternal(i: Int, element: E) { + registerModification() + if (parent != null) { + parent.addAtInternal(i, element) + } else { + root.addAtInternal(i, element) + } + backing = root.backing + length++ + } + + private fun addAllInternal(i: Int, elements: Collection, n: Int) { + registerModification() + if (parent != null) { + parent.addAllInternal(i, elements, n) + } else { + root.addAllInternal(i, elements, n) + } + backing = root.backing + length += n + } + + private fun removeAtInternal(i: Int): E { + registerModification() + val old = if (parent != null) { + parent.removeAtInternal(i) + } else { + root.removeAtInternal(i) + } + length-- + return old + } + + private fun removeRangeInternal(rangeOffset: Int, rangeLength: Int) { + if (rangeLength > 0) registerModification() + if (parent != null) { + parent.removeRangeInternal(rangeOffset, rangeLength) + } else { + root.removeRangeInternal(rangeOffset, rangeLength) + } + length -= rangeLength + } + + /** Retains elements if [retain] == true and removes them it [retain] == false. */ + private fun retainOrRemoveAllInternal(rangeOffset: Int, rangeLength: Int, elements: Collection, retain: Boolean): Int { + val removed = + if (parent != null) { + parent.retainOrRemoveAllInternal(rangeOffset, rangeLength, elements, retain) + } else { + root.retainOrRemoveAllInternal(rangeOffset, rangeLength, elements, retain) + } + if (removed > 0) registerModification() + length -= removed + return removed + } + + private class Itr( + private val list: BuilderSubList, + private var index: Int + ) : MutableListIterator { + private var lastIndex = -1 + private var expectedModCount = list.modCount + + override fun hasPrevious(): Boolean = index > 0 + override fun hasNext(): Boolean = index < list.length + + override fun previousIndex(): Int = index - 1 + override fun nextIndex(): Int = index + + override fun previous(): E { + checkForComodification() + if (index <= 0) throw NoSuchElementException() + lastIndex = --index + return list.backing[list.offset + lastIndex] + } + + override fun next(): E { + checkForComodification() + if (index >= list.length) throw NoSuchElementException() + lastIndex = index++ + return list.backing[list.offset + lastIndex] + } + + override fun set(element: E) { + checkForComodification() + check(lastIndex != -1) { "Call next() or previous() before replacing element from the iterator." } + list[lastIndex] = element + } + + override fun add(element: E) { + checkForComodification() + list.add(index++, element) + lastIndex = -1 + expectedModCount = list.modCount + } + + override fun remove() { + checkForComodification() + check(lastIndex != -1) { "Call next() or previous() before removing element from the iterator." } + list.removeAt(lastIndex) + index = lastIndex + lastIndex = -1 + expectedModCount = list.modCount + } + + private fun checkForComodification() { + if (list.root.modCount != expectedModCount) + throw ConcurrentModificationException() + } + } + } } internal fun arrayOfUninitializedElements(size: Int): Array { @@ -484,4 +714,4 @@ internal class SerializedCollection( const val tagList: Int = 0 const val tagSet: Int = 1 } -} \ No newline at end of file +} diff --git a/libraries/stdlib/jvm/test/collections/CollectionJVMTest.kt b/libraries/stdlib/jvm/test/collections/CollectionJVMTest.kt index 17729aaf5e4..8d8ea88c30f 100644 --- a/libraries/stdlib/jvm/test/collections/CollectionJVMTest.kt +++ b/libraries/stdlib/jvm/test/collections/CollectionJVMTest.kt @@ -252,7 +252,6 @@ class CollectionJVMTest { private fun testCollectionBuilderSerialization(value: Any) { val result = serializeAndDeserialize(value) assertEquals(value, result) - assertEquals(value.javaClass, result.javaClass) assertReadOnly(result) } diff --git a/libraries/stdlib/native-wasm/src/kotlin/collections/ArrayList.kt b/libraries/stdlib/native-wasm/src/kotlin/collections/ArrayList.kt index da9116c4d9c..66bb01fa087 100644 --- a/libraries/stdlib/native-wasm/src/kotlin/collections/ArrayList.kt +++ b/libraries/stdlib/native-wasm/src/kotlin/collections/ArrayList.kt @@ -5,11 +5,22 @@ package kotlin.collections -actual class ArrayList private constructor( - private var backingArray: Array, - private var length: Int, - private var isReadOnly: Boolean -) : MutableList, RandomAccess, AbstractMutableList() { +/** + * Creates a new empty [ArrayList] with the specified initial capacity. + * + * Capacity is the maximum number of elements the list is able to store in current backing storage. + * When the list gets full and a new element can't be added, its capacity is expanded, + * which usually leads to creation of a bigger backing storage. + * + * @param initialCapacity the initial capacity of the created list. + * Note that the argument is just a hint for the implementation and can be ignored. + * + * @throws IllegalArgumentException if [initialCapacity] is negative. + */ +actual class ArrayList actual constructor(initialCapacity: Int) : MutableList, RandomAccess, AbstractMutableList() { + private var backing = arrayOfUninitializedElements(initialCapacity) + private var length = 0 + private var isReadOnly = false private companion object { private val Empty = ArrayList(0).also { it.isReadOnly = true } } @@ -19,21 +30,6 @@ actual class ArrayList private constructor( */ actual constructor() : this(10) - /** - * Creates a new empty [ArrayList] with the specified initial capacity. - * - * Capacity is the maximum number of elements the list is able to store in current backing storage. - * When the list gets full and a new element can't be added, its capacity is expanded, - * which usually leads to creation of a bigger backing storage. - * - * @param initialCapacity the initial capacity of the created list. - * Note that the argument is just a hint for the implementation and can be ignored. - * - * @throws IllegalArgumentException if [initialCapacity] is negative. - */ - actual constructor(initialCapacity: Int) : this( - arrayOfUninitializedElements(initialCapacity), 0, false) - /** * Creates a new [ArrayList] filled with the elements of the specified collection. * @@ -51,31 +47,27 @@ actual class ArrayList private constructor( } override actual val size: Int - get() { - return length - } + get() = length - override actual fun isEmpty(): Boolean { - return length == 0 - } + override actual fun isEmpty() = length == 0 override actual fun get(index: Int): E { AbstractList.checkElementIndex(index, length) - return backingArray[index] + return backing[index] } override actual operator fun set(index: Int, element: E): E { checkIsMutable() AbstractList.checkElementIndex(index, length) - val old = backingArray[index] - backingArray[index] = element + val old = backing[index] + backing[index] = element return old } override actual fun indexOf(element: E): Int { var i = 0 while (i < length) { - if (backingArray[i] == element) return i + if (backing[i] == element) return i i++ } return -1 @@ -84,7 +76,7 @@ actual class ArrayList private constructor( override actual fun lastIndexOf(element: E): Int { var i = length - 1 while (i >= 0) { - if (backingArray[i] == element) return i + if (backing[i] == element) return i i-- } return -1 @@ -155,17 +147,33 @@ actual class ArrayList private constructor( override actual fun subList(fromIndex: Int, toIndex: Int): MutableList { AbstractList.checkRangeIndexes(fromIndex, toIndex, length) - return ArraySubList(backingArray, fromIndex, toIndex - fromIndex, null, this) + return ArraySubList(backing, fromIndex, toIndex - fromIndex, null, this) + } + + @Suppress("UNCHECKED_CAST") + override fun toArray(array: Array): Array { + if (array.size < length) { + return backing.copyOfRange(fromIndex = 0, toIndex = length) as Array + } + + (backing as Array).copyInto(array, 0, startIndex = 0, endIndex = length) + + return terminateCollectionToArray(length, array) + } + + override fun toArray(): Array { + @Suppress("UNCHECKED_CAST") + return backing.copyOfRange(fromIndex = 0, toIndex = length) as Array } actual fun trimToSize() { registerModification() - if (length < backingArray.size) - backingArray = backingArray.copyOfUninitializedElements(length) + if (length < backing.size) + backing = backing.copyOfUninitializedElements(length) } final actual fun ensureCapacity(minCapacity: Int) { - if (minCapacity <= backingArray.size) return + if (minCapacity <= backing.size) return registerModification() ensureCapacityInternal(minCapacity) } @@ -176,27 +184,11 @@ actual class ArrayList private constructor( } override fun hashCode(): Int { - return backingArray.subarrayContentHashCode(0, length) + return backing.subarrayContentHashCode(0, length) } override fun toString(): String { - return backingArray.subarrayContentToString(0, length, this) - } - - @Suppress("UNCHECKED_CAST") - override fun toArray(array: Array): Array { - if (array.size < length) { - return backingArray.copyOfRange(fromIndex = 0, toIndex = length) as Array - } - - (backingArray as Array).copyInto(array, 0, startIndex = 0, endIndex = length) - - return terminateCollectionToArray(length, array) - } - - override fun toArray(): Array { - @Suppress("UNCHECKED_CAST") - return backingArray.copyOfRange(fromIndex = 0, toIndex = length) as Array + return backing.subarrayContentToString(0, length, this) } // ---------------------------- private ---------------------------- @@ -215,26 +207,26 @@ actual class ArrayList private constructor( private fun ensureCapacityInternal(minCapacity: Int) { if (minCapacity < 0) throw OutOfMemoryError() // overflow - if (minCapacity > backingArray.size) { - val newSize = AbstractList.newCapacity(backingArray.size, minCapacity) - backingArray = backingArray.copyOfUninitializedElements(newSize) + if (minCapacity > backing.size) { + val newSize = AbstractList.newCapacity(backing.size, minCapacity) + backing = backing.copyOfUninitializedElements(newSize) } } private fun contentEquals(other: List<*>): Boolean { - return backingArray.subarrayContentEquals(0, length, other) + return backing.subarrayContentEquals(0, length, other) } private fun insertAtInternal(i: Int, n: Int) { ensureExtraCapacity(n) - backingArray.copyInto(backingArray, startIndex = i, endIndex = length, destinationOffset = i + n) + backing.copyInto(backing, startIndex = i, endIndex = length, destinationOffset = i + n) length += n } private fun addAtInternal(i: Int, element: E) { registerModification() insertAtInternal(i, 1) - backingArray[i] = element + backing[i] = element } private fun addAllInternal(i: Int, elements: Collection, n: Int) { @@ -243,24 +235,24 @@ actual class ArrayList private constructor( var j = 0 val it = elements.iterator() while (j < n) { - backingArray[i + j] = it.next() + backing[i + j] = it.next() j++ } } private fun removeAtInternal(i: Int): E { registerModification() - val old = backingArray[i] - backingArray.copyInto(backingArray, startIndex = i + 1, endIndex = length, destinationOffset = i) - backingArray.resetAt(length - 1) + val old = backing[i] + backing.copyInto(backing, startIndex = i + 1, endIndex = length, destinationOffset = i) + backing.resetAt(length - 1) length-- return old } private fun removeRangeInternal(rangeOffset: Int, rangeLength: Int) { if (rangeLength > 0) registerModification() - backingArray.copyInto(backingArray, startIndex = rangeOffset + rangeLength, endIndex = length, destinationOffset = rangeOffset) - backingArray.resetRange(fromIndex = length - rangeLength, toIndex = length) + backing.copyInto(backing, startIndex = rangeOffset + rangeLength, endIndex = length, destinationOffset = rangeOffset) + backing.resetRange(fromIndex = length - rangeLength, toIndex = length) length -= rangeLength } @@ -269,32 +261,26 @@ actual class ArrayList private constructor( var i = 0 var j = 0 while (i < rangeLength) { - if (elements.contains(backingArray[rangeOffset + i]) == retain) { - backingArray[rangeOffset + j++] = backingArray[rangeOffset + i++] + if (elements.contains(backing[rangeOffset + i]) == retain) { + backing[rangeOffset + j++] = backing[rangeOffset + i++] } else { i++ } } val removed = rangeLength - j - backingArray.copyInto(backingArray, startIndex = rangeOffset + rangeLength, endIndex = length, destinationOffset = rangeOffset + j) - backingArray.resetRange(fromIndex = length - removed, toIndex = length) + backing.copyInto(backing, startIndex = rangeOffset + rangeLength, endIndex = length, destinationOffset = rangeOffset + j) + backing.resetRange(fromIndex = length - removed, toIndex = length) if (removed > 0) registerModification() length -= removed return removed } - private class Itr : MutableListIterator { - private val list: ArrayList + private class Itr( + private val list: ArrayList, private var index: Int - private var lastIndex: Int - private var expectedModCount: Int - - constructor(list: ArrayList, index: Int) { - this.list = list - this.index = index - this.lastIndex = -1 - this.expectedModCount = list.modCount - } + ) : MutableListIterator { + private var lastIndex = -1 + private var expectedModCount = list.modCount override fun hasPrevious(): Boolean = index > 0 override fun hasNext(): Boolean = index < list.length @@ -306,20 +292,20 @@ actual class ArrayList private constructor( checkForComodification() if (index <= 0) throw NoSuchElementException() lastIndex = --index - return list.backingArray[lastIndex] + return list.backing[lastIndex] } override fun next(): E { checkForComodification() if (index >= list.length) throw NoSuchElementException() lastIndex = index++ - return list.backingArray[lastIndex] + return list.backing[lastIndex] } override fun set(element: E) { checkForComodification() check(lastIndex != -1) { "Call next() or previous() before replacing element from the iterator." } - list.set(lastIndex, element) + list[lastIndex] = element } override fun add(element: E) { @@ -347,10 +333,10 @@ actual class ArrayList private constructor( } private class ArraySubList( - private var backingArray: Array, + private var backing: Array, private val offset: Int, private var length: Int, - private val backingList: ArraySubList?, + private val parent: ArraySubList?, private val root: ArrayList ) : MutableList, RandomAccess, AbstractMutableList() { @@ -372,15 +358,15 @@ actual class ArrayList private constructor( override fun get(index: Int): E { checkForComodification() AbstractList.checkElementIndex(index, length) - return backingArray[offset + index] + return backing[offset + index] } override operator fun set(index: Int, element: E): E { checkIsMutable() checkForComodification() AbstractList.checkElementIndex(index, length) - val old = backingArray[offset + index] - backingArray[offset + index] = element + val old = backing[offset + index] + backing[offset + index] = element return old } @@ -388,7 +374,7 @@ actual class ArrayList private constructor( checkForComodification() var i = 0 while (i < length) { - if (backingArray[offset + i] == element) return i + if (backing[offset + i] == element) return i i++ } return -1 @@ -398,7 +384,7 @@ actual class ArrayList private constructor( checkForComodification() var i = length - 1 while (i >= 0) { - if (backingArray[offset + i] == element) return i + if (backing[offset + i] == element) return i i-- } return -1 @@ -479,7 +465,25 @@ actual class ArrayList private constructor( override fun subList(fromIndex: Int, toIndex: Int): MutableList { AbstractList.checkRangeIndexes(fromIndex, toIndex, length) - return ArraySubList(backingArray, offset + fromIndex, toIndex - fromIndex, this, root) + return ArraySubList(backing, offset + fromIndex, toIndex - fromIndex, this, root) + } + + @Suppress("UNCHECKED_CAST") + override fun toArray(array: Array): Array { + checkForComodification() + if (array.size < length) { + return backing.copyOfRange(fromIndex = offset, toIndex = offset + length) as Array + } + + (backing as Array).copyInto(array, 0, startIndex = offset, endIndex = offset + length) + + return terminateCollectionToArray(length, array) + } + + override fun toArray(): Array { + checkForComodification() + @Suppress("UNCHECKED_CAST") + return backing.copyOfRange(fromIndex = offset, toIndex = offset + length) as Array } override fun equals(other: Any?): Boolean { @@ -490,30 +494,12 @@ actual class ArrayList private constructor( override fun hashCode(): Int { checkForComodification() - return backingArray.subarrayContentHashCode(offset, length) + return backing.subarrayContentHashCode(offset, length) } override fun toString(): String { checkForComodification() - return backingArray.subarrayContentToString(offset, length, this) - } - - @Suppress("UNCHECKED_CAST") - override fun toArray(array: Array): Array { - checkForComodification() - if (array.size < length) { - return backingArray.copyOfRange(fromIndex = offset, toIndex = offset + length) as Array - } - - (backingArray as Array).copyInto(array, 0, startIndex = offset, endIndex = offset + length) - - return terminateCollectionToArray(length, array) - } - - override fun toArray(): Array { - checkForComodification() - @Suppress("UNCHECKED_CAST") - return backingArray.copyOfRange(fromIndex = offset, toIndex = offset + length) as Array + return backing.subarrayContentToString(offset, length, this) } // ---------------------------- private ---------------------------- @@ -528,54 +514,42 @@ actual class ArrayList private constructor( } private fun checkIsMutable() { - if (root.isReadOnly) throw UnsupportedOperationException() + if (isReadOnly) throw UnsupportedOperationException() } - private fun ensureExtraCapacity(n: Int) { - ensureCapacityInternal(length + n) - } - - private fun ensureCapacityInternal(minCapacity: Int) { - if (minCapacity < 0) throw OutOfMemoryError() // overflow - if (minCapacity > backingArray.size) { - val newSize = AbstractList.newCapacity(backingArray.size, minCapacity) - backingArray = backingArray.copyOfUninitializedElements(newSize) - } - } + private val isReadOnly: Boolean + get() = root.isReadOnly private fun contentEquals(other: List<*>): Boolean { - return backingArray.subarrayContentEquals(offset, length, other) + return backing.subarrayContentEquals(offset, length, other) } private fun addAtInternal(i: Int, element: E) { registerModification() - val backingList = backingList - if (backingList != null) { - backingList.addAtInternal(i, element) + if (parent != null) { + parent.addAtInternal(i, element) } else { root.addAtInternal(i, element) } - backingArray = root.backingArray + backing = root.backing length++ } private fun addAllInternal(i: Int, elements: Collection, n: Int) { registerModification() - val backingList = backingList - if (backingList != null) { - backingList.addAllInternal(i, elements, n) + if (parent != null) { + parent.addAllInternal(i, elements, n) } else { root.addAllInternal(i, elements, n) } - backingArray = root.backingArray + backing = root.backing length += n } private fun removeAtInternal(i: Int): E { registerModification() - val backingList = backingList - val old = if (backingList != null) { - backingList.removeAtInternal(i) + val old = if (parent != null) { + parent.removeAtInternal(i) } else { root.removeAtInternal(i) } @@ -585,9 +559,8 @@ actual class ArrayList private constructor( private fun removeRangeInternal(rangeOffset: Int, rangeLength: Int) { if (rangeLength > 0) registerModification() - val backingList = backingList - if (backingList != null) { - backingList.removeRangeInternal(rangeOffset, rangeLength) + if (parent != null) { + parent.removeRangeInternal(rangeOffset, rangeLength) } else { root.removeRangeInternal(rangeOffset, rangeLength) } @@ -596,10 +569,9 @@ actual class ArrayList private constructor( /** Retains elements if [retain] == true and removes them it [retain] == false. */ private fun retainOrRemoveAllInternal(rangeOffset: Int, rangeLength: Int, elements: Collection, retain: Boolean): Int { - val backingList = backingList val removed = - if (backingList != null) { - backingList.retainOrRemoveAllInternal(rangeOffset, rangeLength, elements, retain) + if (parent != null) { + parent.retainOrRemoveAllInternal(rangeOffset, rangeLength, elements, retain) } else { root.retainOrRemoveAllInternal(rangeOffset, rangeLength, elements, retain) } @@ -608,18 +580,12 @@ actual class ArrayList private constructor( return removed } - private class Itr : MutableListIterator { - private val list: ArraySubList + private class Itr( + private val list: ArraySubList, private var index: Int - private var lastIndex: Int - private var expectedModCount: Int - - constructor(list: ArraySubList, index: Int) { - this.list = list - this.index = index - this.lastIndex = -1 - this.expectedModCount = list.modCount - } + ) : MutableListIterator { + private var lastIndex = -1 + private var expectedModCount = list.modCount override fun hasPrevious(): Boolean = index > 0 override fun hasNext(): Boolean = index < list.length @@ -631,20 +597,20 @@ actual class ArrayList private constructor( checkForComodification() if (index <= 0) throw NoSuchElementException() lastIndex = --index - return list.backingArray[list.offset + lastIndex] + return list.backing[list.offset + lastIndex] } override fun next(): E { checkForComodification() if (index >= list.length) throw NoSuchElementException() lastIndex = index++ - return list.backingArray[list.offset + lastIndex] + return list.backing[list.offset + lastIndex] } override fun set(element: E) { checkForComodification() check(lastIndex != -1) { "Call next() or previous() before replacing element from the iterator." } - list.set(lastIndex, element) + list[lastIndex] = element } override fun add(element: E) { @@ -666,7 +632,7 @@ actual class ArrayList private constructor( // Must inline for native, suppress warning for WASM @Suppress("NOTHING_TO_INLINE") private inline fun checkForComodification() { - if (list.modCount != expectedModCount) + if (list.root.modCount != expectedModCount) throw ConcurrentModificationException() } } diff --git a/libraries/stdlib/test/collections/ConcurrentModificationTest.kt b/libraries/stdlib/test/collections/ConcurrentModificationTest.kt index 0c3fccb8502..0bc6eacab44 100644 --- a/libraries/stdlib/test/collections/ConcurrentModificationTest.kt +++ b/libraries/stdlib/test/collections/ConcurrentModificationTest.kt @@ -252,6 +252,12 @@ class ConcurrentModificationTest { action(subList) } } + assertFailsWith { + buildList { + addAll(listOf("a", "b", "c")) + for (e in subList(1, 3)) remove(e) + } + } } @Test