From 254380d418817e8d68b208d97ec68967bfaed838 Mon Sep 17 00:00:00 2001 From: Denis Zharkov Date: Fri, 14 Dec 2018 12:57:50 +0300 Subject: [PATCH] Generate special bridges for removeAt/getOrDefault In case we extending some Map specialization with non-trivial type arguments, e.g. Map from Kotlin point-of-view it has "remove(String, String)" signature while in Java it's "remove(Object, Object)". So, we generate an override "remove(String, String)" in first Kotlin class of the hierarchy, which body delegates to "super.remove(Object, Object)" Also, we generate a final override for "remove(Object, Object)" to allow for Java inheritors choose only the version with String while overriding. The main idea of the fix is to make mayBeUsedAsSuperImplementation return true in case of PlatformDependent annotations. Otherwise, we weren't able to choose the impl from the java.util.Map as a delegate in our bridge. Another part of the fix is overriding `isDeclaration`: it was necessary because otherwise bridge-generation algorithm was assuming that there's already an actual declaration in the first sub-class (TestMap) in the test and we need to delegate to the latter instead of the method from the interface ^KT-26069 Fixed --- .../kotlin/codegen/JvmBridgesImpl.kt | 28 ++++-- .../jvm/annotations/jvmAnnotationUtil.kt | 4 + .../box/specialBuiltins/mapGetOrDefault.kt | 90 +++++++++++++++++++ .../codegen/BlackBoxCodegenTestGenerated.java | 5 ++ .../LightAnalysisModeTestGenerated.java | 5 ++ .../ir/IrBlackBoxCodegenTestGenerated.java | 5 ++ 6 files changed, 132 insertions(+), 5 deletions(-) create mode 100644 compiler/testData/codegen/box/specialBuiltins/mapGetOrDefault.kt diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/JvmBridgesImpl.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/JvmBridgesImpl.kt index f7e8fcb922a..9444d9c9137 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/JvmBridgesImpl.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/JvmBridgesImpl.kt @@ -5,13 +5,13 @@ package org.jetbrains.kotlin.codegen -import org.jetbrains.kotlin.backend.common.bridges.Bridge -import org.jetbrains.kotlin.backend.common.bridges.DescriptorBasedFunctionHandle -import org.jetbrains.kotlin.backend.common.bridges.generateBridges +import org.jetbrains.kotlin.backend.common.bridges.* import org.jetbrains.kotlin.config.JvmTarget +import org.jetbrains.kotlin.descriptors.CallableMemberDescriptor import org.jetbrains.kotlin.descriptors.FunctionDescriptor import org.jetbrains.kotlin.resolve.DescriptorUtils import org.jetbrains.kotlin.resolve.jvm.annotations.hasJvmDefaultAnnotation +import org.jetbrains.kotlin.resolve.jvm.annotations.hasPlatformDependentAnnotation class DescriptorBasedFunctionHandleForJvm( descriptor: FunctionDescriptor, @@ -27,14 +27,32 @@ class DescriptorBasedFunctionHandleForJvm( the method in the interface is abstract so we must not generate bridges for such cases. */ override val isAbstract: Boolean = super.isAbstract || isAbstractOnJvmIgnoringActualModality(jvmTarget, descriptor) + + override val mayBeUsedAsSuperImplementation: Boolean = + super.mayBeUsedAsSuperImplementation || descriptor.isJvmDefaultOrPlatformDependent() + + override val isDeclaration: Boolean = + descriptor.kind.isReal || needToGenerateDelegationToDefaultImpls(descriptor) +} + +private fun CallableMemberDescriptor.isJvmDefaultOrPlatformDependent() = + hasJvmDefaultAnnotation() || hasPlatformDependentAnnotation() + +private fun needToGenerateDelegationToDefaultImpls(descriptor: FunctionDescriptor): Boolean { + if (findInterfaceImplementation(descriptor) == null) return false + val overriddenFromInterface = findImplementationFromInterface(descriptor) ?: return false + + return !overriddenFromInterface.isJvmDefaultOrPlatformDependent() } /** - * @return return true for interface method not annotated with @JvmDefault + * @return return true for interface method not annotated with @JvmDefault or @PlatformDependent */ fun isAbstractOnJvmIgnoringActualModality(jvmTarget: JvmTarget, descriptor: FunctionDescriptor): Boolean { if (!DescriptorUtils.isInterface(descriptor.containingDeclaration)) return false - return jvmTarget == JvmTarget.JVM_1_6 || !descriptor.hasJvmDefaultAnnotation() + if (jvmTarget == JvmTarget.JVM_1_6) return true + + return !descriptor.isJvmDefaultOrPlatformDependent() } fun generateBridgesForFunctionDescriptorForJvm( diff --git a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/annotations/jvmAnnotationUtil.kt b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/annotations/jvmAnnotationUtil.kt index f41b9c26a56..aec5ade20df 100644 --- a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/annotations/jvmAnnotationUtil.kt +++ b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/annotations/jvmAnnotationUtil.kt @@ -10,6 +10,7 @@ import org.jetbrains.kotlin.descriptors.DeclarationDescriptor import org.jetbrains.kotlin.descriptors.PropertyDescriptor import org.jetbrains.kotlin.descriptors.annotations.Annotated import org.jetbrains.kotlin.descriptors.annotations.AnnotationDescriptor +import org.jetbrains.kotlin.descriptors.deserialization.PLATFORM_DEPENDENT_ANNOTATION_FQ_NAME import org.jetbrains.kotlin.load.java.JvmAbi.JVM_FIELD_ANNOTATION_FQ_NAME import org.jetbrains.kotlin.name.FqName import org.jetbrains.kotlin.resolve.DescriptorUtils @@ -45,6 +46,9 @@ fun DeclarationDescriptor.isCallableMemberWithJvmDefaultAnnotation(): Boolean = fun CallableMemberDescriptor.hasJvmDefaultAnnotation(): Boolean = DescriptorUtils.getDirectMember(this).annotations.hasAnnotation(JVM_DEFAULT_FQ_NAME) +fun CallableMemberDescriptor.hasPlatformDependentAnnotation(): Boolean = + DescriptorUtils.getDirectMember(this).annotations.hasAnnotation(PLATFORM_DEPENDENT_ANNOTATION_FQ_NAME) + private fun Annotated.findJvmSyntheticAnnotation(): AnnotationDescriptor? = annotations.findAnnotation(JVM_SYNTHETIC_ANNOTATION_FQ_NAME) ?: (this as? PropertyDescriptor)?.backingField?.annotations?.findAnnotation(JVM_SYNTHETIC_ANNOTATION_FQ_NAME) diff --git a/compiler/testData/codegen/box/specialBuiltins/mapGetOrDefault.kt b/compiler/testData/codegen/box/specialBuiltins/mapGetOrDefault.kt new file mode 100644 index 00000000000..355c9415261 --- /dev/null +++ b/compiler/testData/codegen/box/specialBuiltins/mapGetOrDefault.kt @@ -0,0 +1,90 @@ +// IGNORE_BACKEND: JVM_IR +// TARGET_BACKEND: JVM +// JVM_TARGET: 1.8 +// WITH_RUNTIME +// FULL_JDK + +// FILE: TestMap.java + +import java.util.Collection; +import java.util.Map; +import java.util.Set; + + +public class TestMap implements Map { + @Override + public int size() { + return 0; + } + + @Override + public boolean isEmpty() { + return false; + } + + @Override + public boolean containsKey(Object key) { + return false; + } + + @Override + public boolean containsValue(Object value) { + return false; + } + + @Override + public Object get(Object key) { + return null; + } + + + @Override + public Object put(String key, Object value) { + return null; + } + + @Override + public Object remove(Object key) { + return null; + } + + @Override + public void putAll(Map m) { + + } + + @Override + public void clear() { + + } + + + @Override + public Set keySet() { + return null; + } + + + @Override + public Collection values() { + return null; + } + + + @Override + public Set> entrySet() { + return null; + } +} + +// FILE: main.kt + +class MyMap: TestMap() + +fun box(): String { + val map = MyMap() + if (map.remove("aaa", 42)) return "fail 1" + if (map.getOrDefault("aaa", 42) != 42) return "fail 2" + + return "OK" +} diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java index 799a85a4693..9e638222e1a 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java @@ -22905,6 +22905,11 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest { runTest("compiler/testData/codegen/box/specialBuiltins/irrelevantRemoveAtOverride.kt"); } + @TestMetadata("mapGetOrDefault.kt") + public void testMapGetOrDefault() throws Exception { + runTest("compiler/testData/codegen/box/specialBuiltins/mapGetOrDefault.kt"); + } + @TestMetadata("maps.kt") public void testMaps() throws Exception { runTest("compiler/testData/codegen/box/specialBuiltins/maps.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java index be1d851999b..0da3a84a2fe 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java @@ -22905,6 +22905,11 @@ public class LightAnalysisModeTestGenerated extends AbstractLightAnalysisModeTes runTest("compiler/testData/codegen/box/specialBuiltins/irrelevantRemoveAtOverride.kt"); } + @TestMetadata("mapGetOrDefault.kt") + public void testMapGetOrDefault() throws Exception { + runTest("compiler/testData/codegen/box/specialBuiltins/mapGetOrDefault.kt"); + } + @TestMetadata("maps.kt") public void testMaps() throws Exception { runTest("compiler/testData/codegen/box/specialBuiltins/maps.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java index d5c590b278b..d080d3b9d65 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java @@ -22910,6 +22910,11 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes runTest("compiler/testData/codegen/box/specialBuiltins/irrelevantRemoveAtOverride.kt"); } + @TestMetadata("mapGetOrDefault.kt") + public void testMapGetOrDefault() throws Exception { + runTest("compiler/testData/codegen/box/specialBuiltins/mapGetOrDefault.kt"); + } + @TestMetadata("maps.kt") public void testMaps() throws Exception { runTest("compiler/testData/codegen/box/specialBuiltins/maps.kt");