From 1b98723b3ff87097427c63d44cd8fa6441106f74 Mon Sep 17 00:00:00 2001 From: Alexander Udalov Date: Wed, 11 Aug 2021 22:44:51 +0200 Subject: [PATCH] JVM IR: fix incorrect detection of interface method impls in Java The problem in the test case was that `JImpl.entrySet` was detected by ReplaceDefaultImplsOverriddenSymbols as a class fake override, which overrides non-abstract interface method. Thus, overriddenSymbols of `MyMap.entrySet` were changed in `ReplaceDefaultImplsOverriddenSymbols.visitSimpleFunction`. Later, BridgeLowering tried to determine which special bridges to generate for `MyMap.`, and for that it inspected existing methods and their overrides. Normally we would generate the following special bridge: getEntries()Ljava/util/Set; invokespecial JImpl.entrySet()Ljava/util/Set; However, because of incorrect overrides, the generated class method was selected here instead of the expected `Map.`: https://github.com/JetBrains/kotlin/blob/06001fc0919c814e757caa494891619882fae15f/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/lower/BridgeLowering.kt#L282 and since the JVM signature of the generated class method is the same as that of the fake override in MyMap, we never got to generating the special bridge. The solution is to skip Java classes when looking for class methods which override non-abstract interface methods. This logic only makes sense for Kotlin code, because only Kotlin has DefaultImpls, and the requirement to generate non-abstract fake overrides of interface methods as actual methods in the bytecode, delegating to DefaultImpls. #KT-48167 Fixed --- .../FirBlackBoxCodegenTestGenerated.java | 6 ++ ...nheritedDefaultMethodsOnClassesLowering.kt | 4 +- .../javaMapWithCustomEntries.kt | 81 +++++++++++++++++++ .../implementsJavaMapWithCustomEntries.kt | 75 +++++++++++++++++ .../implementsJavaMapWithCustomEntries.txt | 13 +++ .../implementsJavaMapWithCustomEntries_ir.txt | 13 +++ .../codegen/BlackBoxCodegenTestGenerated.java | 6 ++ .../codegen/BytecodeListingTestGenerated.java | 6 ++ .../IrBlackBoxCodegenTestGenerated.java | 6 ++ .../IrBytecodeListingTestGenerated.java | 6 ++ .../LightAnalysisModeTestGenerated.java | 5 ++ 11 files changed, 220 insertions(+), 1 deletion(-) create mode 100644 compiler/testData/codegen/box/specialBuiltins/javaMapWithCustomEntries.kt create mode 100644 compiler/testData/codegen/bytecodeListing/specialBridges/signatures/implementsJavaMapWithCustomEntries.kt create mode 100644 compiler/testData/codegen/bytecodeListing/specialBridges/signatures/implementsJavaMapWithCustomEntries.txt create mode 100644 compiler/testData/codegen/bytecodeListing/specialBridges/signatures/implementsJavaMapWithCustomEntries_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 fa5f3821021..761d1d9b219 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 @@ -40578,6 +40578,12 @@ public class FirBlackBoxCodegenTestGenerated extends AbstractFirBlackBoxCodegenT runTest("compiler/testData/codegen/box/specialBuiltins/irrelevantRemoveAtOverride.kt"); } + @Test + @TestMetadata("javaMapWithCustomEntries.kt") + public void testJavaMapWithCustomEntries() throws Exception { + runTest("compiler/testData/codegen/box/specialBuiltins/javaMapWithCustomEntries.kt"); + } + @Test @TestMetadata("mapGetOrDefault.kt") public void testMapGetOrDefault() throws Exception { diff --git a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/lower/InheritedDefaultMethodsOnClassesLowering.kt b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/lower/InheritedDefaultMethodsOnClassesLowering.kt index 0c934061a5d..ea9e12274c1 100644 --- a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/lower/InheritedDefaultMethodsOnClassesLowering.kt +++ b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/lower/InheritedDefaultMethodsOnClassesLowering.kt @@ -262,7 +262,9 @@ private class InterfaceObjectCallsLowering(val context: JvmBackendContext) : IrE */ internal fun IrSimpleFunction.findInterfaceImplementation(jvmDefaultMode: JvmDefaultMode): IrSimpleFunction? { if (!isFakeOverride) return null - parent.let { if (it is IrClass && it.isJvmInterface) return null } + + val parent = parent + if (parent is IrClass && (parent.isJvmInterface || parent.isFromJava())) return null val implementation = resolveFakeOverride(toSkip = ::isDefaultImplsBridge) ?: return null diff --git a/compiler/testData/codegen/box/specialBuiltins/javaMapWithCustomEntries.kt b/compiler/testData/codegen/box/specialBuiltins/javaMapWithCustomEntries.kt new file mode 100644 index 00000000000..55bb4b454bc --- /dev/null +++ b/compiler/testData/codegen/box/specialBuiltins/javaMapWithCustomEntries.kt @@ -0,0 +1,81 @@ +// TARGET_BACKEND: JVM +// JVM_TARGET: 1.8 +// FILE: box.kt + +class MyMap : JImpl() + +fun box(): String { + val a = MyMap() + a.put(42, "OK") + return a.entries.iterator().next().value +} + +// FILE: J.java + +import java.util.*; + +public interface J extends Map { + @Override + default Set> entrySet() { + return myEntrySet(); + } + + Set> myEntrySet(); +} + +// FILE: JImpl.java + +import java.util.*; + +public class JImpl implements J { + private final Map delegate = new HashMap<>(); + + @Override + public Set> myEntrySet() { + return delegate.entrySet(); + } + @Override + public int size() { + return delegate.size(); + } + @Override + public boolean isEmpty() { + return delegate.isEmpty(); + } + @Override + public boolean containsKey(Object key) { + return delegate.containsKey(key); + } + @Override + public boolean containsValue(Object value) { + return delegate.containsValue(value); + } + @Override + public V get(Object key) { + return delegate.get(key); + } + @Override + public V put(K key, V value) { + return delegate.put(key, value); + } + @Override + public V remove(Object key) { + return delegate.remove(key); + } + @Override + public void putAll(Map m) { + delegate.putAll(m); + } + @Override + public void clear() { + delegate.clear(); + } + @Override + public Set keySet() { + return delegate.keySet(); + } + @Override + public Collection values() { + return delegate.values(); + } +} diff --git a/compiler/testData/codegen/bytecodeListing/specialBridges/signatures/implementsJavaMapWithCustomEntries.kt b/compiler/testData/codegen/bytecodeListing/specialBridges/signatures/implementsJavaMapWithCustomEntries.kt new file mode 100644 index 00000000000..e1507b0a74e --- /dev/null +++ b/compiler/testData/codegen/bytecodeListing/specialBridges/signatures/implementsJavaMapWithCustomEntries.kt @@ -0,0 +1,75 @@ +// WITH_SIGNATURES +// JVM_TARGET: 1.8 +// FILE: test.kt + +class MyMap : JImpl() + +// FILE: J.java + +import java.util.*; + +public interface J extends Map { + @Override + default Set> entrySet() { + return myEntrySet(); + } + + Set> myEntrySet(); +} + +// FILE: JImpl.java + +import java.util.*; + +public class JImpl implements J { + private final Map delegate = new HashMap<>(); + + @Override + public Set> myEntrySet() { + return delegate.entrySet(); + } + @Override + public int size() { + return delegate.size(); + } + @Override + public boolean isEmpty() { + return delegate.isEmpty(); + } + @Override + public boolean containsKey(Object key) { + return delegate.containsKey(key); + } + @Override + public boolean containsValue(Object value) { + return delegate.containsValue(value); + } + @Override + public V get(Object key) { + return delegate.get(key); + } + @Override + public V put(K key, V value) { + return delegate.put(key, value); + } + @Override + public V remove(Object key) { + return delegate.remove(key); + } + @Override + public void putAll(Map m) { + delegate.putAll(m); + } + @Override + public void clear() { + delegate.clear(); + } + @Override + public Set keySet() { + return delegate.keySet(); + } + @Override + public Collection values() { + return delegate.values(); + } +} diff --git a/compiler/testData/codegen/bytecodeListing/specialBridges/signatures/implementsJavaMapWithCustomEntries.txt b/compiler/testData/codegen/bytecodeListing/specialBridges/signatures/implementsJavaMapWithCustomEntries.txt new file mode 100644 index 00000000000..93d411d00f3 --- /dev/null +++ b/compiler/testData/codegen/bytecodeListing/specialBridges/signatures/implementsJavaMapWithCustomEntries.txt @@ -0,0 +1,13 @@ +@kotlin.Metadata +public final class<LJImpl;> MyMap { + // source: 'test.kt' + public bridge final <()Ljava/util/Collection;> method values(): java.util.Collection + public bridge final <()Ljava/util/Set;>;> method entrySet(): java.util.Set + public bridge final <()Ljava/util/Set;> method keySet(): java.util.Set + public method (): void + public bridge method getEntries(): java.util.Set + public bridge method getKeys(): java.util.Set + public bridge method getSize(): int + public bridge method getValues(): java.util.Collection + public bridge final method size(): int +} diff --git a/compiler/testData/codegen/bytecodeListing/specialBridges/signatures/implementsJavaMapWithCustomEntries_ir.txt b/compiler/testData/codegen/bytecodeListing/specialBridges/signatures/implementsJavaMapWithCustomEntries_ir.txt new file mode 100644 index 00000000000..a9e0d3fd0f2 --- /dev/null +++ b/compiler/testData/codegen/bytecodeListing/specialBridges/signatures/implementsJavaMapWithCustomEntries_ir.txt @@ -0,0 +1,13 @@ +@kotlin.Metadata +public final class<LJImpl;> MyMap { + // source: 'test.kt' + public bridge <()Ljava/util/Collection;> method getValues(): java.util.Collection + public bridge final <()Ljava/util/Collection;> method values(): java.util.Collection + public bridge <()Ljava/util/Set;> method getKeys(): java.util.Set + public bridge <()Ljava/util/Set;>;> method getEntries(): java.util.Set + public bridge final <()Ljava/util/Set;>;> method entrySet(): java.util.Set + public bridge final <()Ljava/util/Set;> method keySet(): java.util.Set + public method (): void + public bridge method getSize(): int + public bridge final method size(): int +} 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 c53e030adbf..21d311c3914 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 @@ -40416,6 +40416,12 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest { runTest("compiler/testData/codegen/box/specialBuiltins/irrelevantRemoveAtOverride.kt"); } + @Test + @TestMetadata("javaMapWithCustomEntries.kt") + public void testJavaMapWithCustomEntries() throws Exception { + runTest("compiler/testData/codegen/box/specialBuiltins/javaMapWithCustomEntries.kt"); + } + @Test @TestMetadata("mapGetOrDefault.kt") public void testMapGetOrDefault() 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 357ba2165f3..bade89058b3 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 @@ -2255,6 +2255,12 @@ public class BytecodeListingTestGenerated extends AbstractBytecodeListingTest { runTest("compiler/testData/codegen/bytecodeListing/specialBridges/signatures/implementsJavaMap.kt"); } + @Test + @TestMetadata("implementsJavaMapWithCustomEntries.kt") + public void testImplementsJavaMapWithCustomEntries() throws Exception { + runTest("compiler/testData/codegen/bytecodeListing/specialBridges/signatures/implementsJavaMapWithCustomEntries.kt"); + } + @Test @TestMetadata("implementsMap.kt") public void testImplementsMap() 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 a4f7bb0060a..3137e0fa81b 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 @@ -40578,6 +40578,12 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes runTest("compiler/testData/codegen/box/specialBuiltins/irrelevantRemoveAtOverride.kt"); } + @Test + @TestMetadata("javaMapWithCustomEntries.kt") + public void testJavaMapWithCustomEntries() throws Exception { + runTest("compiler/testData/codegen/box/specialBuiltins/javaMapWithCustomEntries.kt"); + } + @Test @TestMetadata("mapGetOrDefault.kt") public void testMapGetOrDefault() 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 b712a346fb9..36bcf55f67f 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 @@ -2303,6 +2303,12 @@ public class IrBytecodeListingTestGenerated extends AbstractIrBytecodeListingTes runTest("compiler/testData/codegen/bytecodeListing/specialBridges/signatures/implementsJavaMap.kt"); } + @Test + @TestMetadata("implementsJavaMapWithCustomEntries.kt") + public void testImplementsJavaMapWithCustomEntries() throws Exception { + runTest("compiler/testData/codegen/bytecodeListing/specialBridges/signatures/implementsJavaMapWithCustomEntries.kt"); + } + @Test @TestMetadata("implementsMap.kt") public void testImplementsMap() 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 125aa5f894b..c5d5f53cfc0 100644 --- a/compiler/tests-gen/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java +++ b/compiler/tests-gen/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java @@ -32423,6 +32423,11 @@ public class LightAnalysisModeTestGenerated extends AbstractLightAnalysisModeTes runTest("compiler/testData/codegen/box/specialBuiltins/irrelevantRemoveAtOverride.kt"); } + @TestMetadata("javaMapWithCustomEntries.kt") + public void testJavaMapWithCustomEntries() throws Exception { + runTest("compiler/testData/codegen/box/specialBuiltins/javaMapWithCustomEntries.kt"); + } + @TestMetadata("mapGetOrDefault.kt") public void testMapGetOrDefault() throws Exception { runTest("compiler/testData/codegen/box/specialBuiltins/mapGetOrDefault.kt");