From a5c0f11d60d39e1537f5a250f862aebf4f424d74 Mon Sep 17 00:00:00 2001 From: Ilya Gorbunov Date: Thu, 1 Sep 2016 00:30:39 +0300 Subject: [PATCH] Split AbstractCollection, List, Set in JS to readonly Abstract[Collection] and mutable AbstractMutable[Collection]. Update compatibility type aliases. --- .../core/collections/AbstractCollection.kt | 23 +----- .../src/core/collections/AbstractList.kt | 70 +++---------------- .../src/core/collections/AbstractMap.kt | 6 +- .../src/core/collections/AbstractSet.kt | 13 +--- .../src/core/collections/ArrayList.kt | 6 +- .../src/core/collections/HashMap.kt | 2 +- .../src/core/collections/HashSet.kt | 2 +- .../src/core/collections/LinkedHashMap.kt | 2 +- .../src/core/collections/LinkedHashSet.kt | 2 +- js/js.libraries/src/core/javautil.kt | 4 +- .../rtti/cases/collectionClassesIsCheck.kt | 9 ++- .../kotlin/collections/AbstractCollection.kt | 4 +- .../src/kotlin/collections/AbstractList.kt | 8 +-- .../src/kotlin/collections/AbstractSet.kt | 22 ++++-- .../reference-public-api/kotlin-stdlib.txt | 8 +-- 15 files changed, 56 insertions(+), 125 deletions(-) diff --git a/js/js.libraries/src/core/collections/AbstractCollection.kt b/js/js.libraries/src/core/collections/AbstractCollection.kt index d1d6978ca4a..68c3ab0aad4 100644 --- a/js/js.libraries/src/core/collections/AbstractCollection.kt +++ b/js/js.libraries/src/core/collections/AbstractCollection.kt @@ -17,10 +17,7 @@ package kotlin.collections -public abstract class AbstractCollection protected constructor() : MutableCollection { - - abstract override val size: Int - abstract override fun iterator(): MutableIterator +public abstract class AbstractMutableCollection protected constructor() : AbstractCollection(), MutableCollection { override fun add(element: E): Boolean = throw UnsupportedOperationException("Add not supported on this collection") @@ -37,16 +34,6 @@ public abstract class AbstractCollection protected constructor() : MutableCol override fun isEmpty(): Boolean = size == 0 - override fun contains(element: E): Boolean { - for (e in this) { - if (e == element) return true - } - return false - } - - override fun containsAll(elements: Collection): Boolean = elements.all { contains(it) } - - override fun addAll(elements: Collection): Boolean { var modified = false for (element in elements) { @@ -66,13 +53,7 @@ public abstract class AbstractCollection protected constructor() : MutableCol } } - abstract override fun hashCode(): Int - - abstract override fun equals(other: Any?): Boolean - - override fun toString(): String = joinToString(", ", "[", "]") { - if (it === this) "(this Collection)" else it.toString() - } + // TODO: move somehow to AbstractCollection protected open fun toArray(): Array = copyToArrayImpl(this) diff --git a/js/js.libraries/src/core/collections/AbstractList.kt b/js/js.libraries/src/core/collections/AbstractList.kt index cd391306b55..5646e5929e2 100644 --- a/js/js.libraries/src/core/collections/AbstractList.kt +++ b/js/js.libraries/src/core/collections/AbstractList.kt @@ -21,10 +21,7 @@ package kotlin.collections -public abstract class AbstractList protected constructor() : AbstractCollection(), MutableList { - abstract override val size: Int - abstract override fun get(index: Int): E - +public abstract class AbstractMutableList protected constructor() : AbstractMutableCollection(), MutableList { protected var modCount: Int = 0 override fun add(index: Int, element: E): Unit = throw UnsupportedOperationException("Add not supported on this list") @@ -94,10 +91,10 @@ public abstract class AbstractList protected constructor() : AbstractCollecti if (other === this) return true if (other !is List<*>) return false - return orderedEquals(this, other) + return AbstractList.orderedEquals(this, other) } - override fun hashCode(): Int = orderedHashCode(this) + override fun hashCode(): Int = AbstractList.orderedHashCode(this) private open inner class IteratorImpl : MutableIterator { @@ -132,7 +129,7 @@ public abstract class AbstractList protected constructor() : AbstractCollecti private inner class ListIteratorImpl(index: Int) : IteratorImpl(), MutableListIterator { init { - checkPositionIndex(index, this@AbstractList.size) + AbstractList.checkPositionIndex(index, this@AbstractMutableList.size) this.index = index } @@ -157,33 +154,33 @@ public abstract class AbstractList protected constructor() : AbstractCollecti override fun set(element: E) { require(last != -1) - this@AbstractList[last] = element + this@AbstractMutableList[last] = element } } - private class SubList(private val list: AbstractList, private val fromIndex: Int, toIndex: Int) : AbstractList() { + private class SubList(private val list: AbstractMutableList, private val fromIndex: Int, toIndex: Int) : AbstractMutableList() { private var _size: Int = 0 init { - checkRangeIndexes(fromIndex, toIndex, list.size) + AbstractList.checkRangeIndexes(fromIndex, toIndex, list.size) this._size = toIndex - fromIndex } override fun add(index: Int, element: E) { - checkPositionIndex(index, _size) + AbstractList.checkPositionIndex(index, _size) list.add(fromIndex + index, element) _size++ } override fun get(index: Int): E { - checkElementIndex(index, _size) + AbstractList.checkElementIndex(index, _size) return list[fromIndex + index] } override fun removeAt(index: Int): E { - checkElementIndex(index, _size) + AbstractList.checkElementIndex(index, _size) val result = list.removeAt(fromIndex + index) _size-- @@ -191,7 +188,7 @@ public abstract class AbstractList protected constructor() : AbstractCollecti } override fun set(index: Int, element: E): E { - checkElementIndex(index, _size) + AbstractList.checkElementIndex(index, _size) return list.set(fromIndex + index, element) } @@ -199,49 +196,4 @@ public abstract class AbstractList protected constructor() : AbstractCollecti override val size: Int get() = _size } - companion object { - internal fun checkElementIndex(index: Int, size: Int) { - if (index < 0 || index >= size) { - throw IndexOutOfBoundsException("index: $index, size: $size") - } - } - - internal fun checkPositionIndex(index: Int, size: Int) { - if (index < 0 || index > size) { - throw IndexOutOfBoundsException("index: $index, size: $size") - } - } - - internal fun checkRangeIndexes(start: Int, end: Int, size: Int) { - if (start < 0 || end > size) { - throw IndexOutOfBoundsException("fromIndex: $start, toIndex: $end, size: $size") - } - if (start > end) { - throw IllegalArgumentException("fromIndex: $start > toIndex: $end") - } - } - - internal fun orderedHashCode(c: Collection<*>): Int { - var hashCode = 1 - for (e in c) { - hashCode = 31 * hashCode + (e?.hashCode() ?: 0) - hashCode = hashCode or 0 // make sure we don't overflow - } - return hashCode - } - - internal fun orderedEquals(c: Collection<*>, other: Collection<*>): Boolean { - if (c.size != other.size) return false - - val otherIterator = other.iterator() - for (elem in c) { - val elemOther = otherIterator.next() - if (elem != elemOther) { - return false - } - } - return true - } - } - } diff --git a/js/js.libraries/src/core/collections/AbstractMap.kt b/js/js.libraries/src/core/collections/AbstractMap.kt index e40eac380ef..c57759be10f 100644 --- a/js/js.libraries/src/core/collections/AbstractMap.kt +++ b/js/js.libraries/src/core/collections/AbstractMap.kt @@ -20,7 +20,7 @@ package kotlin.collections -abstract class AbstractMap protected constructor() : MutableMap { +public abstract class AbstractMap protected constructor() : MutableMap { /** * A mutable [Map.Entry] shared by several [Map] implementations. @@ -102,7 +102,7 @@ abstract class AbstractMap protected constructor() : MutableMap { override val keys: MutableSet get() { - return object : AbstractSet() { + return object : AbstractMutableSet() { override fun clear() { this@AbstractMap.clear() } @@ -149,7 +149,7 @@ abstract class AbstractMap protected constructor() : MutableMap { private fun toString(o: Any?): String = if (o === this) "(this Map)" else o.toString() override val values: MutableCollection get() { - return object : AbstractCollection() { + return object : AbstractMutableCollection() { override fun clear() = this@AbstractMap.clear() override operator fun contains(element: V): Boolean = containsValue(element) diff --git a/js/js.libraries/src/core/collections/AbstractSet.kt b/js/js.libraries/src/core/collections/AbstractSet.kt index 2c375b0fa8a..aa9bb6b7926 100644 --- a/js/js.libraries/src/core/collections/AbstractSet.kt +++ b/js/js.libraries/src/core/collections/AbstractSet.kt @@ -15,21 +15,14 @@ */ package kotlin.collections -abstract class AbstractSet protected constructor() : AbstractCollection(), MutableSet { +public abstract class AbstractMutableSet protected constructor() : AbstractMutableCollection(), MutableSet { override fun equals(other: Any?): Boolean { if (other === this) return true if (other !is Set<*>) return false - if (other.size != size) return false - return containsAll(other) + return AbstractSet.setEquals(this, other) } - override fun hashCode(): Int { - var hashCode = 0 - for (element in this) { - hashCode = (hashCode + (element?.hashCode() ?: 0)) or 0 - } - return hashCode - } + override fun hashCode(): Int = AbstractSet.unorderedHashCode(this) } \ No newline at end of file diff --git a/js/js.libraries/src/core/collections/ArrayList.kt b/js/js.libraries/src/core/collections/ArrayList.kt index cf132428e2b..ea36b6ec489 100644 --- a/js/js.libraries/src/core/collections/ArrayList.kt +++ b/js/js.libraries/src/core/collections/ArrayList.kt @@ -16,7 +16,7 @@ package kotlin.collections -public open class ArrayList internal constructor(private var array: Array) : AbstractList(), RandomAccess { +public open class ArrayList internal constructor(private var array: Array) : AbstractMutableList(), RandomAccess { public constructor(capacity: Int = 0) : this(emptyArray()) {} public constructor(elements: Collection) : this(elements.toTypedArray()) {} @@ -103,10 +103,10 @@ public open class ArrayList internal constructor(private var array: Array : AbstractMap { - private inner class EntrySet : AbstractSet>() { + private inner class EntrySet : AbstractMutableSet>() { override fun clear() { this@HashMap.clear() diff --git a/js/js.libraries/src/core/collections/HashSet.kt b/js/js.libraries/src/core/collections/HashSet.kt index 0055624a3bd..fde000be732 100644 --- a/js/js.libraries/src/core/collections/HashSet.kt +++ b/js/js.libraries/src/core/collections/HashSet.kt @@ -21,7 +21,7 @@ package kotlin.collections -open class HashSet : AbstractSet { +public open class HashSet : AbstractMutableSet { private val map: HashMap diff --git a/js/js.libraries/src/core/collections/LinkedHashMap.kt b/js/js.libraries/src/core/collections/LinkedHashMap.kt index 04e6f9fd243..0a2f4c94233 100644 --- a/js/js.libraries/src/core/collections/LinkedHashMap.kt +++ b/js/js.libraries/src/core/collections/LinkedHashMap.kt @@ -40,7 +40,7 @@ open class LinkedHashMap : HashMap, Map { internal var prev: ChainEntry? = null } - private inner class EntrySet : AbstractSet>() { + private inner class EntrySet : AbstractMutableSet>() { private inner class EntryIterator : MutableIterator> { // The last entry that was returned from this iterator. diff --git a/js/js.libraries/src/core/collections/LinkedHashSet.kt b/js/js.libraries/src/core/collections/LinkedHashSet.kt index ee9f3847e0f..246078498b3 100644 --- a/js/js.libraries/src/core/collections/LinkedHashSet.kt +++ b/js/js.libraries/src/core/collections/LinkedHashSet.kt @@ -20,7 +20,7 @@ package kotlin.collections -open class LinkedHashSet : HashSet { +public open class LinkedHashSet : HashSet { internal constructor(map: LinkedHashMap) : super(map) diff --git a/js/js.libraries/src/core/javautil.kt b/js/js.libraries/src/core/javautil.kt index 42e61c6c5f1..343bae3be1d 100644 --- a/js/js.libraries/src/core/javautil.kt +++ b/js/js.libraries/src/core/javautil.kt @@ -34,8 +34,8 @@ public class Date() { // TODO: Deprecate with replacement public typealias RandomAccess = kotlin.collections.RandomAccess -public typealias AbstractCollection = kotlin.collections.AbstractCollection -public typealias AbstractList = kotlin.collections.AbstractList +public typealias AbstractCollection = kotlin.collections.AbstractMutableCollection +public typealias AbstractList = kotlin.collections.AbstractMutableList public typealias ArrayList = kotlin.collections.ArrayList public typealias HashSet = kotlin.collections.HashSet public typealias LinkedHashSet = kotlin.collections.LinkedHashSet diff --git a/js/js.translator/testData/rtti/cases/collectionClassesIsCheck.kt b/js/js.translator/testData/rtti/cases/collectionClassesIsCheck.kt index 1a21971672e..213733db3f4 100644 --- a/js/js.translator/testData/rtti/cases/collectionClassesIsCheck.kt +++ b/js/js.translator/testData/rtti/cases/collectionClassesIsCheck.kt @@ -1,15 +1,13 @@ // KT-2468 ArrayList is List or HashSet is Set fails in generated JS code package foo -import java.util.* - class A fun checkAbstractList(obj: Any) { - assertTrue(obj is AbstractList<*>, "checkAbstractList: is AbstractList") + assertTrue(obj is AbstractMutableList<*>, "checkAbstractList: is AbstractMutableList") assertTrue(obj is MutableList<*>, "checkAbstractList: is MutableList") assertTrue(obj is List<*>, "checkAbstractList: is List") - assertTrue(obj is AbstractCollection<*>, "checkAbstractList: is AbstractCollection") + assertTrue(obj is AbstractMutableCollection<*>, "checkAbstractList: is AbstractMutableCollection") assertTrue(obj is MutableCollection<*>, "checkAbstractList: is MutableCollection") assertTrue(obj is Collection<*>, "checkAbstractList: is Collection") assertTrue(obj is MutableIterable<*>, "checkAbstractList: is MutableIterable") @@ -25,7 +23,8 @@ fun checkArrayList(obj: Any) { fun checkHashSet(obj: Any) { assertTrue(obj is HashSet<*>, "checkHashSet: is HashSet") - assertTrue(obj is AbstractCollection<*>, "checkHashSet: is AbstractCollection") + assertTrue(obj is AbstractMutableSet<*>, "checkHashSet: is AbstractMutableSet") + assertTrue(obj is AbstractMutableCollection<*>, "checkHashSet: is AbstractMutableCollection") assertTrue(obj is MutableCollection<*>, "checkHashSet: is MutableCollection") assertTrue(obj is Collection<*>, "checkHashSet: is Collection") assertTrue(obj is MutableIterable<*>, "checkHashSet: is MutableIterable") diff --git a/libraries/stdlib/src/kotlin/collections/AbstractCollection.kt b/libraries/stdlib/src/kotlin/collections/AbstractCollection.kt index 5ff2a502bb5..e23d3e383c9 100644 --- a/libraries/stdlib/src/kotlin/collections/AbstractCollection.kt +++ b/libraries/stdlib/src/kotlin/collections/AbstractCollection.kt @@ -15,14 +15,14 @@ */ package kotlin.collections -public abstract class AbstractCollection : Collection { +public abstract class AbstractCollection protected constructor() : Collection { abstract override val size: Int abstract override fun iterator(): Iterator override fun contains(element: @UnsafeVariance E): Boolean = any { it == element } override fun containsAll(elements: Collection<@UnsafeVariance E>): Boolean = - elements.all(this::contains) + elements.all { contains(it) } // use when js will support bound refs: elements.all(this::contains) override fun isEmpty(): Boolean = size == 0 diff --git a/libraries/stdlib/src/kotlin/collections/AbstractList.kt b/libraries/stdlib/src/kotlin/collections/AbstractList.kt index 230449f0f2c..7f5dd2f3c89 100644 --- a/libraries/stdlib/src/kotlin/collections/AbstractList.kt +++ b/libraries/stdlib/src/kotlin/collections/AbstractList.kt @@ -20,7 +20,7 @@ package kotlin.collections -public abstract class AbstractList : AbstractCollection(), List { +public abstract class AbstractList protected constructor() : AbstractCollection(), List { abstract override val size: Int abstract override fun get(index: Int): E @@ -62,7 +62,7 @@ public abstract class AbstractList : AbstractCollection(), List { override fun hashCode(): Int = orderedHashCode(this) - internal open inner class IteratorImpl : Iterator { + private open inner class IteratorImpl : Iterator { /** the index of the item that will be returned on the next call to [next]`()` */ protected var index = 0 /** the index of the item that was returned on the previous call to [next]`()` @@ -83,7 +83,7 @@ public abstract class AbstractList : AbstractCollection(), List { /** * Implementation of `MutableListIterator` for abstract lists. */ - internal open inner class ListIteratorImpl(index: Int) : IteratorImpl(), ListIterator { + private open inner class ListIteratorImpl(index: Int) : IteratorImpl(), ListIterator { init { checkPositionIndex(index, this@AbstractList.size) @@ -104,7 +104,7 @@ public abstract class AbstractList : AbstractCollection(), List { override fun previousIndex(): Int = index - 1 } - companion object { + internal companion object { internal fun checkElementIndex(index: Int, size: Int) { if (index < 0 || index >= size) { throw IndexOutOfBoundsException("index: $index, size: $size") diff --git a/libraries/stdlib/src/kotlin/collections/AbstractSet.kt b/libraries/stdlib/src/kotlin/collections/AbstractSet.kt index 5f91b809ffd..deec185ed56 100644 --- a/libraries/stdlib/src/kotlin/collections/AbstractSet.kt +++ b/libraries/stdlib/src/kotlin/collections/AbstractSet.kt @@ -20,16 +20,24 @@ public abstract class AbstractSet protected constructor() : AbstractColle override fun equals(other: Any?): Boolean { if (other === this) return true if (other !is Set<*>) return false - if (other.size != size) return false - return containsAll(other) + return setEquals(this, other) } - override fun hashCode(): Int { - var hashCode = 0 - for (element in this) { - hashCode = (hashCode + (element?.hashCode() ?: 0)) or 0 + override fun hashCode(): Int = unorderedHashCode(this) + + internal companion object { + internal fun unorderedHashCode(c: Collection<*>): Int { + var hashCode = 0 + for (element in c) { + hashCode = (hashCode + (element?.hashCode() ?: 0)) or 0 + } + return hashCode + } + + internal fun setEquals(c: Set<*>, other: Set<*>): Boolean { + if (c.size != other.size) return false + return c.containsAll(other) } - return hashCode } } \ No newline at end of file diff --git a/libraries/tools/binary-compatibility-validator/reference-public-api/kotlin-stdlib.txt b/libraries/tools/binary-compatibility-validator/reference-public-api/kotlin-stdlib.txt index cdf3228147c..b3ddcaa30ed 100644 --- a/libraries/tools/binary-compatibility-validator/reference-public-api/kotlin-stdlib.txt +++ b/libraries/tools/binary-compatibility-validator/reference-public-api/kotlin-stdlib.txt @@ -67,7 +67,7 @@ public final class kotlin/_Assertions { } public abstract class kotlin/collections/AbstractCollection : java/util/Collection, kotlin/jvm/internal/markers/KMappedMarker { - public fun ()V + protected fun ()V public fun add (Ljava/lang/Object;)Z public fun addAll (Ljava/util/Collection;)Z public fun clear ()V @@ -99,7 +99,7 @@ public abstract class kotlin/collections/AbstractIterator : java/util/Iterator, public abstract class kotlin/collections/AbstractList : kotlin/collections/AbstractCollection, java/util/List, kotlin/jvm/internal/markers/KMappedMarker { public static final field Companion Lkotlin/collections/AbstractList$Companion; - public fun ()V + protected fun ()V public fun add (ILjava/lang/Object;)V public fun add (Ljava/lang/Object;)Z public fun addAll (ILjava/util/Collection;)Z @@ -125,10 +125,8 @@ public abstract class kotlin/collections/AbstractList : kotlin/collections/Abstr public fun toArray ([Ljava/lang/Object;)[Ljava/lang/Object; } -public final class kotlin/collections/AbstractList$Companion { -} - public abstract class kotlin/collections/AbstractSet : kotlin/collections/AbstractCollection, java/util/Set, kotlin/jvm/internal/markers/KMappedMarker { + public static final field Companion Lkotlin/collections/AbstractSet$Companion; protected fun ()V public fun add (Ljava/lang/Object;)Z public fun addAll (Ljava/util/Collection;)Z