Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[KT-20070] Add the bridge for indirectly overridden of Map.entries #5382

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import org.jetbrains.kotlin.backend.common.phaser.PhaseDescription
import org.jetbrains.kotlin.backend.jvm.*
import org.jetbrains.kotlin.backend.jvm.ir.*
import org.jetbrains.kotlin.backend.jvm.mapping.MethodSignatureMapper
import org.jetbrains.kotlin.builtins.StandardNames
import org.jetbrains.kotlin.codegen.AsmUtil
import org.jetbrains.kotlin.descriptors.DescriptorVisibilities
import org.jetbrains.kotlin.descriptors.Modality
Expand Down Expand Up @@ -315,8 +316,8 @@ internal class BridgeLowering(val context: JvmBackendContext) : ClassLoweringPas

// Generate common bridges
val generated = mutableMapOf<Method, Bridge>()

for (override in irFunction.allOverridden()) {
val allOverridden = irFunction.allOverridden()
for (override in allOverridden) {
if (override.isFakeOverride) continue

val signature = override.jvmMethod
Expand All @@ -328,6 +329,47 @@ internal class BridgeLowering(val context: JvmBackendContext) : ClassLoweringPas
}
}

/**
* See KT-20070.
*
* Check if a method indirectly overridden [kotlin.collections.Map.entries].
* If [irFunction] directly override [kotlin.collections.Map.entries], no issue here.
* If an overridden chain like this: A1.entries -> A2.entries -> ... -> An.entries -> Map.entries
* ("->" represents "override"), and all "entries" functions in from 'A2' to 'An' are FAKEOVERRIDE,
* the issue occurs.
* To solve this, we add a hardcoded bridge "getEntries()Ljava/util/Set;" here.
*
* This issue occurred because we forgot to add "getEntries()Ljava/tilt/Set;". The
* bridge method for [kotlin.collections.Map.entries] is "entrySet()Ljava/tilt/Set;"
* (see [org.jetbrains.kotlin.load.java.BuiltinSpecialProperties.PROPERTY_FQ_NAME_TO_JVM_GETTER_NAME_MAP]).
* But when the above situation occurs, we also need a "getEntries()Ljava/util/Set;".
* So the hardcoded bridge will not affect other situations.
*/
val entriesGetterName = "<get-entries>"
val mapEntriesName = StandardNames.FqNames.map.child(Name.identifier("<get-entries>"))
val entriesGetters = allOverridden.filter {
it.kotlinFqName != mapEntriesName && it.name.asString() == entriesGetterName
}
val overrideMapEntries =
// Check not directly override [kotlin.collections.Map.entries]
irFunction.overriddenSymbols.none { it.owner.kotlinFqName == mapEntriesName }
// Check all override in chain is FAKEOVERRIDE
&& entriesGetters.isNotEmpty()
&& entriesGetters.all { it.isFakeOverride }
// Check if really override [kotlin.collections.Map.entries]
&& allOverridden.any { it.kotlinFqName == mapEntriesName }
if (overrideMapEntries) {
val override = irFunction.overriddenSymbols.first {
it.owner.isFakeOverride && it.owner.name.asString() == entriesGetterName
}.owner
val signature = override.jvmMethod
if (targetMethod != signature && signature !in blacklist) {
val bridge = generated.getOrPut(signature) {
Bridge(override, signature)
}
bridge.overriddenSymbols += override.symbol
}
}
if (generated.isEmpty())
return

Expand Down
19 changes: 19 additions & 0 deletions compiler/testData/codegen/box/specialBuiltins/kt20070_1.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// WITH_STDLIB
// TARGET_BACKEND: JVM

class KtMap : AbstractMap<String, String>() {
override val entries: HashSet<Map.Entry<String, String>>
get() = HashSet<Map.Entry<String, String>>().apply {
add(object : Map.Entry<String, String> {
override val key: String
get() = "O"
override val value: String
get() = "K"
})
}
}

fun box(): String {
val entry = KtMap().entries.first()
return entry.key + entry.value
}
46 changes: 46 additions & 0 deletions compiler/testData/codegen/box/specialBuiltins/kt20070_2.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
// WITH_STDLIB
// TARGET_BACKEND: JVM
abstract class MyMap : AbstractMap<String, String>() {
override val keys: Set<String>
get() = TODO("Not yet implemented")
override val size: Int
get() = TODO("Not yet implemented")
override val values: Collection<String>
get() = TODO("Not yet implemented")

override fun isEmpty(): Boolean {
TODO("Not yet implemented")
}

override fun get(key: String): String? {
TODO("Not yet implemented")
}

override fun containsValue(value: String): Boolean {
TODO("Not yet implemented")
}

override fun containsKey(key: String): Boolean {
TODO("Not yet implemented")
}

}

abstract class MyMap1 : MyMap(), Map<String, String>
abstract class MyMap2 : MyMap1()
class KtMap : MyMap2() {
override val entries: HashSet<Map.Entry<String, String>>
get() = HashSet<Map.Entry<String, String>>().apply {
add(object : Map.Entry<String, String> {
override val key: String
get() = "O"
override val value: String
get() = "K"
})
}
}

fun box(): String {
val entry = KtMap().entries.first()
return entry.key + entry.value
}
49 changes: 49 additions & 0 deletions compiler/testData/codegen/box/specialBuiltins/kt20070_3.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
// WITH_STDLIB
// TARGET_BACKEND: JVM
// FILE: MyMap2.java
abstract public class MyMap2 extends MyMap1 {}

// FILE: main.kt
abstract class MyMap : AbstractMap<String, String>() {
override val keys: Set<String>
get() = TODO("Not yet implemented")
override val size: Int
get() = TODO("Not yet implemented")
override val values: Collection<String>
get() = TODO("Not yet implemented")

override fun isEmpty(): Boolean {
TODO("Not yet implemented")
}

override fun get(key: String): String? {
TODO("Not yet implemented")
}

override fun containsValue(value: String): Boolean {
TODO("Not yet implemented")
}

override fun containsKey(key: String): Boolean {
TODO("Not yet implemented")
}

}

abstract class MyMap1 : MyMap(), Map<String, String>
class KtMap : MyMap2() {
override val entries: HashSet<Map.Entry<String, String>>
get() = HashSet<Map.Entry<String, String>>().apply {
add(object : Map.Entry<String, String> {
override val key: String
get() = "O"
override val value: String
get() = "K"
})
}
}

fun box(): String {
val entry = KtMap().entries.first()
return entry.key + entry.value
}

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading