From 3e926999620eaad6fa3affc35f06b2ed34a7d419 Mon Sep 17 00:00:00 2001 From: SvyatoslavScherbina Date: Mon, 9 Nov 2020 10:50:22 +0300 Subject: [PATCH] Fix MutableMap.entries.contains(Map.Entry) #KT-42428 Fixed. --- .../backend.native/tests/build.gradle | 1 + .../collections/KT42428Test.kt | 56 +++++++++++++++++++ .../main/kotlin/kotlin/collections/HashMap.kt | 45 +++++++++++---- 3 files changed, 91 insertions(+), 11 deletions(-) create mode 100644 kotlin-native/backend.native/tests/stdlib_external/collections/KT42428Test.kt diff --git a/kotlin-native/backend.native/tests/build.gradle b/kotlin-native/backend.native/tests/build.gradle index a7ad979658d..52a44e6f0e1 100644 --- a/kotlin-native/backend.native/tests/build.gradle +++ b/kotlin-native/backend.native/tests/build.gradle @@ -4837,6 +4837,7 @@ task override_konan_properties0(type: KonanDriverTest) { KotlinNativeTestKt.createTest(project, 'stdlibTest', KonanGTest) { task -> def sources = UtilsKt.getFilesToCompile(project, [ 'build/stdlib_external/stdlib', 'stdlib_external/utils.kt', + 'stdlib_external/collections', 'stdlib_external/jsCollectionFactoriesActuals.kt', 'stdlib_external/text/StringEncodingTestNative.kt'], [ 'build/stdlib_external/stdlib/test/internalAnnotations.kt' ]) diff --git a/kotlin-native/backend.native/tests/stdlib_external/collections/KT42428Test.kt b/kotlin-native/backend.native/tests/stdlib_external/collections/KT42428Test.kt new file mode 100644 index 00000000000..ef09a06c18b --- /dev/null +++ b/kotlin-native/backend.native/tests/stdlib_external/collections/KT42428Test.kt @@ -0,0 +1,56 @@ +/* + * Copyright 2010-2020 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. + */ + +package test.collections + +import kotlin.test.* + +// TODO: consider moving to common stdlib tests. +class KT42428Test { + + private val listOfLetterIndexPairs = ('a'..'z').withIndex().map { (i, c) -> "$c" to i } + + private val mapOfLetterToIndex = listOfLetterIndexPairs.toMap() + + @Test fun testListOfPairsToMapEntriesContainsMapEntry() { + testMapEntriesContainsMapEntry(listOfLetterIndexPairs.toMap(), "h", 7) + } + + @Test fun testMapToMutableMapEntriesContainsMapEntry() { + testMapEntriesContainsMapEntry(mapOfLetterToIndex.toMutableMap(), "h", 7) + } + + @Test fun testHashMapEntriesContainsMapEntry() { + testMapEntriesContainsMapEntry(HashMap(mapOfLetterToIndex), "h", 7) + } + + @Test fun testLinkedHashMapEntriesContainsMapEntry() { + testMapEntriesContainsMapEntry(LinkedHashMap(mapOfLetterToIndex), "h", 7) + } + + // Based on https://youtrack.jetbrains.com/issue/KT-42428. + private fun testMapEntriesContainsMapEntry(map: Map, key: String, value: Int) { + data class SimpleEntry(override val key: K, override val value: V) : Map.Entry { + + override fun equals(other: Any?): Boolean = + other is Map.Entry<*, *> && key == other.key && value == other.value + + override fun hashCode(): Int = key.hashCode() xor value.hashCode() + + override fun toString(): String = "$key=$value" + } + + + assertTrue(map.keys.contains(key)) + + // This one requires special efforts to make it work this way. + // map.entries can in fact be `MutableSet`, + // which [contains] method takes [MutableEntry], so the compiler may generate special bridge + // returning false for values that aren't [MutableEntry] (including [SimpleEntry]). + assertTrue(map.entries.contains(SimpleEntry(key, value))) + + assertTrue(map.entries.toSet().contains(SimpleEntry(key, value))) + } +} diff --git a/kotlin-native/runtime/src/main/kotlin/kotlin/collections/HashMap.kt b/kotlin-native/runtime/src/main/kotlin/kotlin/collections/HashMap.kt index 22384f69b74..d9f52a67aec 100644 --- a/kotlin-native/runtime/src/main/kotlin/kotlin/collections/HashMap.kt +++ b/kotlin-native/runtime/src/main/kotlin/kotlin/collections/HashMap.kt @@ -630,31 +630,54 @@ internal class HashMapValues internal constructor( } } -internal class HashMapEntrySet internal constructor( +/** + * Note: intermediate class with [E] `: Map.Entry` is required to support + * [contains] for values that are [Map.Entry] but not [MutableMap.MutableEntry], + * and probably same for other functions. + * This is important because an instance of this class can be used as a result of [Map.entries], + * which should support [contains] for [Map.Entry]. + * For example, this happens when upcasting [MutableMap] to [Map]. + * + * The compiler enables special type-safe barriers to methods like [contains], which has [UnsafeVariance]. + * Changing type from [MutableMap.MutableEntry] to [E] makes the compiler generate barriers checking that + * argument `is` [E] (so technically `is` [Map.Entry]) instead of `is` [MutableMap.MutableEntry]. + * + * See also [KT-42248](https://youtrack.jetbrains.com/issue/KT-42428). + */ +internal abstract class HashMapEntrySetBase> internal constructor( val backing: HashMap -) : MutableSet>, kotlin.native.internal.KonanSet>, AbstractMutableSet>() { +) : MutableSet, kotlin.native.internal.KonanSet, AbstractMutableSet() { override val size: Int get() = backing.size override fun isEmpty(): Boolean = backing.isEmpty() - override fun contains(element: MutableMap.MutableEntry): Boolean = backing.containsEntry(element) - override fun getElement(element: MutableMap.MutableEntry): MutableMap.MutableEntry? = backing.getEntry(element) + override fun contains(element: E): Boolean = backing.containsEntry(element) + override fun getElement(element: E): E? = getEntry(element) + protected abstract fun getEntry(element: Map.Entry): E? override fun clear() = backing.clear() - override fun add(element: MutableMap.MutableEntry): Boolean = throw UnsupportedOperationException() - override fun addAll(elements: Collection>): Boolean = throw UnsupportedOperationException() - override fun remove(element: MutableMap.MutableEntry): Boolean = backing.removeEntry(element) - override fun iterator(): MutableIterator> = backing.entriesIterator() - override fun containsAll(elements: Collection>): Boolean = backing.containsAllEntries(elements) + override fun add(element: E): Boolean = throw UnsupportedOperationException() + override fun addAll(elements: Collection): Boolean = throw UnsupportedOperationException() + override fun remove(element: E): Boolean = backing.removeEntry(element) + override fun containsAll(elements: Collection): Boolean = backing.containsAllEntries(elements) - override fun removeAll(elements: Collection>): Boolean { + override fun removeAll(elements: Collection): Boolean { backing.checkIsMutable() return super.removeAll(elements) } - override fun retainAll(elements: Collection>): Boolean { + override fun retainAll(elements: Collection): Boolean { backing.checkIsMutable() return super.retainAll(elements) } } +internal class HashMapEntrySet internal constructor( + backing: HashMap +) : HashMapEntrySetBase>(backing) { + + override fun getEntry(element: Map.Entry): MutableMap.MutableEntry? = backing.getEntry(element) + + override fun iterator(): MutableIterator> = backing.entriesIterator() +} + // This hash map keeps insertion order. actual typealias LinkedHashMap = HashMap \ No newline at end of file