From 55d1fdfbc29ac872c9ef095a70e059cd30edc7d9 Mon Sep 17 00:00:00 2001 From: Dmitry Petrov Date: Tue, 23 Nov 2021 10:05:09 +0300 Subject: [PATCH] JVM_IR prevent clash of unsubstituted special bridges --- .../FirBlackBoxCodegenTestGenerated.java | 6 ++ .../backend/jvm/lower/BridgeLowering.kt | 37 ++++++---- .../box/collections/specialBridgeForGet.kt | 18 +++++ .../specialBridges/specialBridgeForGet.kt | 13 ++++ .../specialBridges/specialBridgeForGet.txt | 64 +++++++++++++++++ .../specialBridges/specialBridgeForGet_ir.txt | 70 +++++++++++++++++++ .../codegen/BlackBoxCodegenTestGenerated.java | 6 ++ .../codegen/BytecodeListingTestGenerated.java | 6 ++ .../IrBlackBoxCodegenTestGenerated.java | 6 ++ .../IrBytecodeListingTestGenerated.java | 6 ++ .../LightAnalysisModeTestGenerated.java | 5 ++ .../NativeExtBlackBoxTestGenerated.java | 6 ++ 12 files changed, 230 insertions(+), 13 deletions(-) create mode 100644 compiler/testData/codegen/box/collections/specialBridgeForGet.kt create mode 100644 compiler/testData/codegen/bytecodeListing/specialBridges/specialBridgeForGet.kt create mode 100644 compiler/testData/codegen/bytecodeListing/specialBridges/specialBridgeForGet.txt create mode 100644 compiler/testData/codegen/bytecodeListing/specialBridges/specialBridgeForGet_ir.txt diff --git a/compiler/fir/fir2ir/tests-gen/org/jetbrains/kotlin/test/runners/codegen/FirBlackBoxCodegenTestGenerated.java b/compiler/fir/fir2ir/tests-gen/org/jetbrains/kotlin/test/runners/codegen/FirBlackBoxCodegenTestGenerated.java index f6b5593d261..5c6d7418e46 100644 --- a/compiler/fir/fir2ir/tests-gen/org/jetbrains/kotlin/test/runners/codegen/FirBlackBoxCodegenTestGenerated.java +++ b/compiler/fir/fir2ir/tests-gen/org/jetbrains/kotlin/test/runners/codegen/FirBlackBoxCodegenTestGenerated.java @@ -6676,6 +6676,12 @@ public class FirBlackBoxCodegenTestGenerated extends AbstractFirBlackBoxCodegenT runTest("compiler/testData/codegen/box/collections/removeOverriddenInJava_Map.kt"); } + @Test + @TestMetadata("specialBridgeForGet.kt") + public void testSpecialBridgeForGet() throws Exception { + runTest("compiler/testData/codegen/box/collections/specialBridgeForGet.kt"); + } + @Test @TestMetadata("strList.kt") public void testStrList() throws Exception { diff --git a/compiler/ir/backend.jvm/lower/src/org/jetbrains/kotlin/backend/jvm/lower/BridgeLowering.kt b/compiler/ir/backend.jvm/lower/src/org/jetbrains/kotlin/backend/jvm/lower/BridgeLowering.kt index 2db3d8ff10f..872a768cc4f 100644 --- a/compiler/ir/backend.jvm/lower/src/org/jetbrains/kotlin/backend/jvm/lower/BridgeLowering.kt +++ b/compiler/ir/backend.jvm/lower/src/org/jetbrains/kotlin/backend/jvm/lower/BridgeLowering.kt @@ -219,7 +219,7 @@ internal class BridgeLowering(val context: JvmBackendContext) : FileLoweringPass if (specialBridge != null) { // If the current function overrides a special bridge then it's possible that we already generated a final // bridge methods in a superclass. - blacklist += irFunction.overriddenFinalSpecialBridgeSignatures() + blacklist += irFunction.allOverridden().flatMapTo(arrayListOf()) { it.getSpecialBridgeSignatures() } fun getSpecialBridgeTargetAddingExtraBridges(): IrSimpleFunction { // We only generate a special bridge method if it does not clash with a final method in a superclass or the current method @@ -279,6 +279,7 @@ internal class BridgeLowering(val context: JvmBackendContext) : FileLoweringPass // (where a Kotlin class implements a read-only collection interface and extends a Java collection class). val unsubstitutedSpecialBridge = specialBridge.unsubstitutedSpecialBridge if (unsubstitutedSpecialBridge != null && + unsubstitutedSpecialBridge.signature !in blacklist && irClass.functions.none { it.isClashingWithPotentialBridge(irFunction.name, unsubstitutedSpecialBridge.signature) } ) { blacklist += unsubstitutedSpecialBridge.signature @@ -364,19 +365,29 @@ internal class BridgeLowering(val context: JvmBackendContext) : FileLoweringPass private val IrSimpleFunction.specialBridgeOrNull: SpecialBridge? get() = context.bridgeLoweringCache.computeSpecialBridge(this) - private fun IrSimpleFunction.overriddenFinalSpecialBridgeSignatures(): List = - allOverridden().mapNotNullTo(mutableListOf()) { - // Ignore special bridges in interfaces or Java classes. While we never generate special bridges in Java - // classes, we may generate special bridges in interfaces for methods annotated with @JvmDefault. - // However, these bridges are not final and are thus safe to override. - // - // This matches the behavior of the JVM backend, but it's probably a bad idea since this is an - // opportunity for a Java and Kotlin implementation of the same interface to go out of sync. - if (it.parentAsClass.isInterface || it.isFromJava()) - null - else - it.specialBridgeOrNull?.signature?.takeIf { bridgeSignature -> bridgeSignature != it.jvmMethod } + private fun IrSimpleFunction.getSpecialBridgeSignatures(): List { + // Ignore special bridges in interfaces or Java classes. While we never generate special bridges in Java + // classes, we may generate special bridges in interfaces for methods annotated with @JvmDefault. + // However, these bridges are not final and are thus safe to override. + // This matches the behavior of the JVM backend, but it's probably a bad idea since this is an + // opportunity for a Java and Kotlin implementation of the same interface to go out of sync. + + if (this.parentAsClass.isInterface || this.isFromJava()) + return emptyList() + val specialBridge = this.specialBridgeOrNull + ?: return emptyList() + + val result = SmartList() + val jvmMethod = this.jvmMethod + if (specialBridge.signature != jvmMethod) { + result.add(specialBridge.signature) } + val unsubstitutedSpecialBridge = specialBridge.unsubstitutedSpecialBridge + if (unsubstitutedSpecialBridge != null && unsubstitutedSpecialBridge.signature != jvmMethod) { + result.add(unsubstitutedSpecialBridge.signature) + } + return result + } // List of special bridge methods which were not implemented in Kotlin superclasses. private fun IrSimpleFunction.overriddenSpecialBridges(): List { diff --git a/compiler/testData/codegen/box/collections/specialBridgeForGet.kt b/compiler/testData/codegen/box/collections/specialBridgeForGet.kt new file mode 100644 index 00000000000..a46d7aa14fa --- /dev/null +++ b/compiler/testData/codegen/box/collections/specialBridgeForGet.kt @@ -0,0 +1,18 @@ +// TARGET_BACKEND: JVM +// WITH_STDLIB +// FULL_JDK + +abstract class AMap1(private val m: Map) : Map by m + +interface Value2 + +abstract class AMap2(m: Map) : AMap1(m) + +class C(val value: String): Value2 + +class CMap(m: Map) : AMap2(m) + +fun box(): String { + val cmap = CMap(mapOf("1" to C("OK"))) + return cmap["1"]!!.value +} diff --git a/compiler/testData/codegen/bytecodeListing/specialBridges/specialBridgeForGet.kt b/compiler/testData/codegen/bytecodeListing/specialBridges/specialBridgeForGet.kt new file mode 100644 index 00000000000..a8ce4efac64 --- /dev/null +++ b/compiler/testData/codegen/bytecodeListing/specialBridges/specialBridgeForGet.kt @@ -0,0 +1,13 @@ +// TARGET_BACKEND: JVM +// WITH_STDLIB +// FULL_JDK + +abstract class AMap1(private val m: Map) : Map by m + +interface Value2 + +abstract class AMap2(m: Map) : AMap1(m) + +class C(val value: String): Value2 + +class Map3(m: Map) : AMap2(m) diff --git a/compiler/testData/codegen/bytecodeListing/specialBridges/specialBridgeForGet.txt b/compiler/testData/codegen/bytecodeListing/specialBridges/specialBridgeForGet.txt new file mode 100644 index 00000000000..48fc1773957 --- /dev/null +++ b/compiler/testData/codegen/bytecodeListing/specialBridges/specialBridgeForGet.txt @@ -0,0 +1,64 @@ +@kotlin.Metadata +public abstract class AMap1 { + // source: 'specialBridgeForGet.kt' + private final field m: java.util.Map + public method (@org.jetbrains.annotations.NotNull p0: java.util.Map): void + public method clear(): void + public method compute(p0: java.lang.Object, p1: java.util.function.BiFunction): java.lang.Object + public method computeIfAbsent(p0: java.lang.Object, p1: java.util.function.Function): java.lang.Object + public method computeIfPresent(p0: java.lang.Object, p1: java.util.function.BiFunction): java.lang.Object + public method containsKey(p0: java.lang.Object): boolean + public method containsValue(p0: java.lang.Object): boolean + public bridge final method entrySet(): java.util.Set + public @org.jetbrains.annotations.Nullable method get(p0: java.lang.Object): java.lang.Object + public @org.jetbrains.annotations.NotNull method getEntries(): java.util.Set + public @org.jetbrains.annotations.NotNull method getKeys(): java.util.Set + public method getSize(): int + public @org.jetbrains.annotations.NotNull method getValues(): java.util.Collection + public method isEmpty(): boolean + public bridge final method keySet(): java.util.Set + public method merge(p0: java.lang.Object, p1: java.lang.Object, p2: java.util.function.BiFunction): java.lang.Object + public method put(p0: java.lang.Object, p1: java.lang.Object): java.lang.Object + public method putAll(p0: java.util.Map): void + public method putIfAbsent(p0: java.lang.Object, p1: java.lang.Object): java.lang.Object + public method remove(p0: java.lang.Object): java.lang.Object + public method remove(p0: java.lang.Object, p1: java.lang.Object): boolean + public method replace(p0: java.lang.Object, p1: java.lang.Object): java.lang.Object + public method replace(p0: java.lang.Object, p1: java.lang.Object, p2: java.lang.Object): boolean + public method replaceAll(p0: java.util.function.BiFunction): void + public bridge final method size(): int + public bridge final method values(): java.util.Collection +} + +@kotlin.Metadata +public abstract class AMap2 { + // source: 'specialBridgeForGet.kt' + public method (@org.jetbrains.annotations.NotNull p0: java.util.Map): void + public bridge final method containsKey(p0: java.lang.Object): boolean + public bridge method containsKey(p0: java.lang.String): boolean + public bridge method containsValue(p0: Value2): boolean + public bridge final method containsValue(p0: java.lang.Object): boolean + public bridge final method get(p0: java.lang.Object): java.lang.Object + public bridge method get(p0: java.lang.String): Value2 + public bridge final method getOrDefault(p0: java.lang.Object, p1: java.lang.Object): java.lang.Object + public bridge method getOrDefault(p0: java.lang.String, p1: Value2): Value2 +} + +@kotlin.Metadata +public final class C { + // source: 'specialBridgeForGet.kt' + private final @org.jetbrains.annotations.NotNull field value: java.lang.String + public method (@org.jetbrains.annotations.NotNull p0: java.lang.String): void + public final @org.jetbrains.annotations.NotNull method getValue(): java.lang.String +} + +@kotlin.Metadata +public final class Map3 { + // source: 'specialBridgeForGet.kt' + public method (@org.jetbrains.annotations.NotNull p0: java.util.Map): void +} + +@kotlin.Metadata +public interface Value2 { + // source: 'specialBridgeForGet.kt' +} diff --git a/compiler/testData/codegen/bytecodeListing/specialBridges/specialBridgeForGet_ir.txt b/compiler/testData/codegen/bytecodeListing/specialBridges/specialBridgeForGet_ir.txt new file mode 100644 index 00000000000..31578c511fa --- /dev/null +++ b/compiler/testData/codegen/bytecodeListing/specialBridges/specialBridgeForGet_ir.txt @@ -0,0 +1,70 @@ +@kotlin.Metadata +public abstract class AMap1 { + // source: 'specialBridgeForGet.kt' + private final @org.jetbrains.annotations.NotNull field m: java.util.Map + public method (@org.jetbrains.annotations.NotNull p0: java.util.Map): void + public method clear(): void + public method compute(p0: java.lang.Object, p1: java.util.function.BiFunction): java.lang.Object + public method computeIfAbsent(p0: java.lang.Object, p1: java.util.function.Function): java.lang.Object + public method computeIfPresent(p0: java.lang.Object, p1: java.util.function.BiFunction): java.lang.Object + public method containsKey(p0: java.lang.Object): boolean + public method containsValue(p0: java.lang.Object): boolean + public bridge final method entrySet(): java.util.Set + public @org.jetbrains.annotations.Nullable method get(p0: java.lang.Object): java.lang.Object + public @org.jetbrains.annotations.NotNull method getEntries(): java.util.Set + public @org.jetbrains.annotations.NotNull method getKeys(): java.util.Set + public method getSize(): int + public @org.jetbrains.annotations.NotNull method getValues(): java.util.Collection + public method isEmpty(): boolean + public bridge final method keySet(): java.util.Set + public method merge(p0: java.lang.Object, p1: java.lang.Object, p2: java.util.function.BiFunction): java.lang.Object + public method put(p0: java.lang.Object, p1: java.lang.Object): java.lang.Object + public method putAll(p0: java.util.Map): void + public method putIfAbsent(p0: java.lang.Object, p1: java.lang.Object): java.lang.Object + public method remove(p0: java.lang.Object): java.lang.Object + public method remove(p0: java.lang.Object, p1: java.lang.Object): boolean + public method replace(p0: java.lang.Object, p1: java.lang.Object): java.lang.Object + public method replace(p0: java.lang.Object, p1: java.lang.Object, p2: java.lang.Object): boolean + public method replaceAll(p0: java.util.function.BiFunction): void + public bridge final method size(): int + public bridge final method values(): java.util.Collection +} + +@kotlin.Metadata +public abstract class AMap2 { + // source: 'specialBridgeForGet.kt' + public method (@org.jetbrains.annotations.NotNull p0: java.util.Map): void + public bridge final method containsKey(p0: java.lang.Object): boolean + public bridge method containsKey(p0: java.lang.String): boolean + public bridge method containsValue(p0: Value2): boolean + public bridge final method containsValue(p0: java.lang.Object): boolean + public bridge final method get(p0: java.lang.Object): Value2 + public synthetic bridge final method get(p0: java.lang.Object): java.lang.Object + public bridge method get(p0: java.lang.String): Value2 + public bridge final method getOrDefault(p0: java.lang.Object, p1: Value2): Value2 + public synthetic bridge final method getOrDefault(p0: java.lang.Object, p1: java.lang.Object): java.lang.Object + public bridge method getOrDefault(p0: java.lang.String, p1: Value2): Value2 +} + +@kotlin.Metadata +public final class C { + // source: 'specialBridgeForGet.kt' + private final @org.jetbrains.annotations.NotNull field value: java.lang.String + public method (@org.jetbrains.annotations.NotNull p0: java.lang.String): void + public final @org.jetbrains.annotations.NotNull method getValue(): java.lang.String +} + +@kotlin.Metadata +public final class Map3 { + // source: 'specialBridgeForGet.kt' + public method (@org.jetbrains.annotations.NotNull p0: java.util.Map): void + public bridge final method get(p0: java.lang.Object): C + public bridge method get(p0: java.lang.String): C + public bridge final method getOrDefault(p0: java.lang.Object, p1: C): C + public bridge method getOrDefault(p0: java.lang.String, p1: C): C +} + +@kotlin.Metadata +public interface Value2 { + // source: 'specialBridgeForGet.kt' +} diff --git a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/BlackBoxCodegenTestGenerated.java b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/BlackBoxCodegenTestGenerated.java index ba30dffb431..ed6b79b847f 100644 --- a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/BlackBoxCodegenTestGenerated.java +++ b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/BlackBoxCodegenTestGenerated.java @@ -6604,6 +6604,12 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest { runTest("compiler/testData/codegen/box/collections/removeOverriddenInJava_Map.kt"); } + @Test + @TestMetadata("specialBridgeForGet.kt") + public void testSpecialBridgeForGet() throws Exception { + runTest("compiler/testData/codegen/box/collections/specialBridgeForGet.kt"); + } + @Test @TestMetadata("strList.kt") public void testStrList() throws Exception { diff --git a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/BytecodeListingTestGenerated.java b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/BytecodeListingTestGenerated.java index 3bb03df737f..5df23ba0479 100644 --- a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/BytecodeListingTestGenerated.java +++ b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/BytecodeListingTestGenerated.java @@ -2296,6 +2296,12 @@ public class BytecodeListingTestGenerated extends AbstractBytecodeListingTest { runTest("compiler/testData/codegen/bytecodeListing/specialBridges/removeAtTwoSpecialBridges.kt"); } + @Test + @TestMetadata("specialBridgeForGet.kt") + public void testSpecialBridgeForGet() throws Exception { + runTest("compiler/testData/codegen/bytecodeListing/specialBridges/specialBridgeForGet.kt"); + } + @Test @TestMetadata("unsignedArray.kt") public void testUnsignedArray() throws Exception { diff --git a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrBlackBoxCodegenTestGenerated.java b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrBlackBoxCodegenTestGenerated.java index 463719a56f7..a05b726ba82 100644 --- a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrBlackBoxCodegenTestGenerated.java +++ b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrBlackBoxCodegenTestGenerated.java @@ -6676,6 +6676,12 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes runTest("compiler/testData/codegen/box/collections/removeOverriddenInJava_Map.kt"); } + @Test + @TestMetadata("specialBridgeForGet.kt") + public void testSpecialBridgeForGet() throws Exception { + runTest("compiler/testData/codegen/box/collections/specialBridgeForGet.kt"); + } + @Test @TestMetadata("strList.kt") public void testStrList() throws Exception { diff --git a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrBytecodeListingTestGenerated.java b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrBytecodeListingTestGenerated.java index 0ba0f2f031d..669350a8dc8 100644 --- a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrBytecodeListingTestGenerated.java +++ b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrBytecodeListingTestGenerated.java @@ -2344,6 +2344,12 @@ public class IrBytecodeListingTestGenerated extends AbstractIrBytecodeListingTes runTest("compiler/testData/codegen/bytecodeListing/specialBridges/removeAtTwoSpecialBridges.kt"); } + @Test + @TestMetadata("specialBridgeForGet.kt") + public void testSpecialBridgeForGet() throws Exception { + runTest("compiler/testData/codegen/bytecodeListing/specialBridges/specialBridgeForGet.kt"); + } + @Test @TestMetadata("unsignedArray.kt") public void testUnsignedArray() throws Exception { diff --git a/compiler/tests-gen/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java b/compiler/tests-gen/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java index 8dda270e6d9..78faa1b4f6d 100644 --- a/compiler/tests-gen/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java +++ b/compiler/tests-gen/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java @@ -5755,6 +5755,11 @@ public class LightAnalysisModeTestGenerated extends AbstractLightAnalysisModeTes runTest("compiler/testData/codegen/box/collections/removeOverriddenInJava_Map.kt"); } + @TestMetadata("specialBridgeForGet.kt") + public void testSpecialBridgeForGet() throws Exception { + runTest("compiler/testData/codegen/box/collections/specialBridgeForGet.kt"); + } + @TestMetadata("strList.kt") public void testStrList() throws Exception { runTest("compiler/testData/codegen/box/collections/strList.kt"); diff --git a/native/native.tests/tests-gen/org/jetbrains/kotlin/konan/blackboxtest/NativeExtBlackBoxTestGenerated.java b/native/native.tests/tests-gen/org/jetbrains/kotlin/konan/blackboxtest/NativeExtBlackBoxTestGenerated.java index d68e9660402..fc3a9fe228d 100644 --- a/native/native.tests/tests-gen/org/jetbrains/kotlin/konan/blackboxtest/NativeExtBlackBoxTestGenerated.java +++ b/native/native.tests/tests-gen/org/jetbrains/kotlin/konan/blackboxtest/NativeExtBlackBoxTestGenerated.java @@ -6750,6 +6750,12 @@ public class NativeExtBlackBoxTestGenerated extends AbstractNativeBlackBoxTest { runTest("compiler/testData/codegen/box/collections/removeOverriddenInJava_Map.kt"); } + @Test + @TestMetadata("specialBridgeForGet.kt") + public void testSpecialBridgeForGet() throws Exception { + runTest("compiler/testData/codegen/box/collections/specialBridgeForGet.kt"); + } + @Test @TestMetadata("strList.kt") public void testStrList() throws Exception {