From 058cc9def269e2691f5a4773f1f8579c14863279 Mon Sep 17 00:00:00 2001
From: Alexander Korepanov <alexander.korepanov@jetbrains.com>
Date: Tue, 20 Jun 2023 20:41:43 +0200
Subject: [PATCH] [JS IR] Rework HashSet

^KT-59001
---
 .../js/src/kotlin/collections/HashSet.kt      | 61 ++++++++-----------
 .../src/kotlin/collections/LinkedHashSet.kt   | 39 +++++-------
 2 files changed, 41 insertions(+), 59 deletions(-)

diff --git a/libraries/stdlib/js/src/kotlin/collections/HashSet.kt b/libraries/stdlib/js/src/kotlin/collections/HashSet.kt
index 83b44a82e3e..aaaf0ed0387 100644
--- a/libraries/stdlib/js/src/kotlin/collections/HashSet.kt
+++ b/libraries/stdlib/js/src/kotlin/collections/HashSet.kt
@@ -1,5 +1,5 @@
 /*
- * Copyright 2010-2018 JetBrains s.r.o. and Kotlin Programming Language contributors.
+ * Copyright 2010-2023 JetBrains s.r.o. and Kotlin Programming Language contributors.
  * Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file.
  */
 /*
@@ -16,21 +16,30 @@ package kotlin.collections
 // have to make sure mutating methods check `checkIsMutable`.
 public actual open class HashSet<E> : AbstractMutableSet<E>, MutableSet<E> {
 
-    internal val map: HashMap<E, Any>
+    internal val internalMap: InternalMap<E, Boolean>
+
+    /**
+     * Internal constructor to specify the underlying map.
+     * This is used by LinkedHashSet and stringSetOf().
+     *
+     * @param map underlying map to use.
+     */
+    internal constructor(map: InternalMap<E, Boolean>) {
+        internalMap = map
+    }
 
     /**
      * Creates a new empty [HashSet].
      */
-    actual constructor() {
-        map = HashMap<E, Any>()
-    }
+    actual constructor() : this(InternalHashMap())
 
     /**
      * Creates a new [HashSet] filled with the elements of the specified collection.
      */
-    actual constructor(elements: Collection<E>) {
-        map = HashMap<E, Any>(elements.size)
-        addAll(elements)
+    actual constructor(elements: Collection<E>) : this(InternalHashMap(elements.size)) {
+        for (element in elements) {
+            internalMap.put(element, true)
+        }
     }
 
     /**
@@ -47,9 +56,7 @@ public actual open class HashSet<E> : AbstractMutableSet<E>, MutableSet<E> {
      *
      * @throws IllegalArgumentException if [initialCapacity] is negative or [loadFactor] is non-positive.
      */
-    actual constructor(initialCapacity: Int, loadFactor: Float) {
-        map = HashMap<E, Any>(initialCapacity, loadFactor)
-    }
+    actual constructor(initialCapacity: Int, loadFactor: Float) : this(InternalHashMap(initialCapacity, loadFactor))
 
     /**
      * Creates a new empty [HashSet] with the specified initial capacity.
@@ -65,39 +72,23 @@ public actual open class HashSet<E> : AbstractMutableSet<E>, MutableSet<E> {
      */
     actual constructor(initialCapacity: Int) : this(initialCapacity, 1.0f)
 
-    /**
-     * Protected constructor to specify the underlying map. This is used by
-     * LinkedHashSet.
-
-     * @param map underlying map to use.
-     */
-    internal constructor(map: HashMap<E, Any>) {
-        this.map = map
-    }
-
     actual override fun add(element: E): Boolean {
-        val old = map.put(element, this)
-        return old == null
+        return internalMap.put(element, true) == null
     }
 
     actual override fun clear() {
-        map.clear()
+        internalMap.clear()
     }
 
-//    public override fun clone(): Any {
-//        return HashSet<E>(this)
-//    }
+    actual override operator fun contains(element: E): Boolean = internalMap.contains(element)
 
-    actual override operator fun contains(element: E): Boolean = map.containsKey(element)
+    actual override fun isEmpty(): Boolean = internalMap.size == 0
 
-    actual override fun isEmpty(): Boolean = map.isEmpty()
+    actual override fun iterator(): MutableIterator<E> = internalMap.keysIterator()
 
-    actual override fun iterator(): MutableIterator<E> = map.keys.iterator()
-
-    actual override fun remove(element: E): Boolean = map.remove(element) != null
-
-    actual override val size: Int get() = map.size
+    actual override fun remove(element: E): Boolean = internalMap.remove(element) != null
 
+    actual override val size: Int get() = internalMap.size
 }
 
 /**
@@ -105,5 +96,5 @@ public actual open class HashSet<E> : AbstractMutableSet<E>, MutableSet<E> {
  * which elements the keys as properties of JS object without hashing them.
  */
 public fun stringSetOf(vararg elements: String): HashSet<String> {
-    return HashSet(stringMapOf<Any>()).apply { addAll(elements) }
+    return HashSet<String>(InternalStringMap()).apply { addAll(elements) }
 }
diff --git a/libraries/stdlib/js/src/kotlin/collections/LinkedHashSet.kt b/libraries/stdlib/js/src/kotlin/collections/LinkedHashSet.kt
index 190c3d91b0d..2e7c3c36c76 100644
--- a/libraries/stdlib/js/src/kotlin/collections/LinkedHashSet.kt
+++ b/libraries/stdlib/js/src/kotlin/collections/LinkedHashSet.kt
@@ -12,30 +12,20 @@ package kotlin.collections
 /**
  * The implementation of the [MutableSet] interface, backed by a [LinkedHashMap] instance.
  *
- * This implementation preserves the insertion order of elements during the iteration.
+ * The insertion order is preserved natively by the HashSet implementation.
  */
 public actual open class LinkedHashSet<E> : HashSet<E>, MutableSet<E> {
-    private companion object {
-        private val Empty = LinkedHashSet<Nothing>(0).also {
-            (it.map as LinkedHashMap<Nothing, Any>).build()
-        }
-    }
-
-    internal constructor(map: LinkedHashMap<E, Any>) : super(map)
-
     /**
      * Creates a new empty [LinkedHashSet].
      */
-    actual constructor() : super(LinkedHashMap<E, Any>())
+    actual constructor() : super()
 
     /**
      * Creates a new [LinkedHashSet] filled with the elements of the specified collection.
      *
      * The iteration order of elements in the created set is the same as in the specified collection.
      */
-    actual constructor(elements: Collection<E>) : super(LinkedHashMap<E, Any>()) {
-        addAll(elements)
-    }
+    actual constructor(elements: Collection<E>) : super(elements)
 
     /**
      * Creates a new empty [LinkedHashSet] with the specified initial capacity and load factor.
@@ -51,7 +41,7 @@ public actual open class LinkedHashSet<E> : HashSet<E>, MutableSet<E> {
      *
      * @throws IllegalArgumentException if [initialCapacity] is negative or [loadFactor] is non-positive.
      */
-    actual constructor(initialCapacity: Int, loadFactor: Float) : super(LinkedHashMap<E, Any>(initialCapacity, loadFactor))
+    actual constructor(initialCapacity: Int, loadFactor: Float) : super(initialCapacity, loadFactor)
 
     /**
      * Creates a new empty [LinkedHashSet] with the specified initial capacity.
@@ -67,18 +57,19 @@ public actual open class LinkedHashSet<E> : HashSet<E>, MutableSet<E> {
      */
     actual constructor(initialCapacity: Int) : this(initialCapacity, 1.0f)
 
-    @PublishedApi
-    internal fun build(): Set<E> {
-        (map as LinkedHashMap<E, Any>).build()
-        return if (size > 0) this else Empty
+    internal constructor(internalMap: InternalMap<E, Boolean>) : super(internalMap)
+
+    private object EmptyHolder {
+        val value = LinkedHashSet(InternalHashMap<Nothing, Boolean>(0).also { it.build() })
     }
 
-    internal override fun checkIsMutable(): Unit = map.checkIsMutable()
-
-//    public override fun clone(): Any {
-//        return LinkedHashSet(this)
-//    }
+    @PublishedApi
+    internal fun build(): Set<E> {
+        internalMap.build()
+        return if (size > 0) this else EmptyHolder.value
+    }
 
+    override fun checkIsMutable() = internalMap.checkIsMutable()
 }
 
 /**
@@ -86,5 +77,5 @@ public actual open class LinkedHashSet<E> : HashSet<E>, MutableSet<E> {
  * which elements the keys as properties of JS object without hashing them.
  */
 public fun linkedStringSetOf(vararg elements: String): LinkedHashSet<String> {
-    return LinkedHashSet(linkedStringMapOf<Any>()).apply { addAll(elements) }
+    return LinkedHashSet<String>(InternalStringMap()).apply { addAll(elements) }
 }