From 20cbceb56d24e3fcdff8e7db4168d31cc5df4dcd Mon Sep 17 00:00:00 2001 From: Denis Zharkov Date: Tue, 24 Nov 2015 13:42:46 +0300 Subject: [PATCH] Add temporary hack for wildcards in Collections By default we would render 'MutableCollection.addAll(Collection)' as '(LCollection;)' (without wildcard) because String is final and effectively it's the same as '(LCollection;)'. But that's wrong signature in a sense that java.util.Collection has different signature: '(LCollection)'. Actually the problem is much wider than collections, it concerns any Java code that uses Kotlin classes with covariant parameters without '? extends E' wildcards. Temporary solution is just to hardcode/enumerate builtin methods with special signature. --- .../kotlin/codegen/builtinSpecialBridges.kt | 8 +++- .../kotlin/codegen/state/JetTypeMapper.java | 13 +++++- .../kotlin/codegen/state/typeMappingUtil.kt | 20 +++++++++- .../collections/readOnlyList/J.java | 13 ++++++ .../collections/readOnlyList/readOnlyList.kt | 40 +++++++++++++++++++ .../collections/strList/strList.kt | 2 +- .../BlackBoxWithJavaCodegenTestGenerated.java | 6 +++ .../kotlin/load/java/specialBuiltinMembers.kt | 34 +--------------- .../kotlin/resolve/DescriptorUtils.kt | 27 +++++++++++++ 9 files changed, 124 insertions(+), 39 deletions(-) create mode 100644 compiler/testData/codegen/boxWithJava/collections/readOnlyList/J.java create mode 100644 compiler/testData/codegen/boxWithJava/collections/readOnlyList/readOnlyList.kt diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/builtinSpecialBridges.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/builtinSpecialBridges.kt index 781b69b7cd6..22a4893490e 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/builtinSpecialBridges.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/builtinSpecialBridges.kt @@ -19,16 +19,20 @@ package org.jetbrains.kotlin.codegen import org.jetbrains.kotlin.backend.common.bridges.DescriptorBasedFunctionHandle import org.jetbrains.kotlin.backend.common.bridges.findAllReachableDeclarations import org.jetbrains.kotlin.backend.common.bridges.findConcreteSuperDeclaration -import org.jetbrains.kotlin.descriptors.* -import org.jetbrains.kotlin.load.java.* +import org.jetbrains.kotlin.descriptors.CallableMemberDescriptor +import org.jetbrains.kotlin.descriptors.FunctionDescriptor +import org.jetbrains.kotlin.descriptors.Modality +import org.jetbrains.kotlin.load.java.BuiltinMethodsWithSpecialGenericSignature import org.jetbrains.kotlin.load.java.BuiltinMethodsWithSpecialGenericSignature.getSpecialSignatureInfo import org.jetbrains.kotlin.load.java.descriptors.JavaClassDescriptor +import org.jetbrains.kotlin.load.java.getOverriddenBuiltinWithDifferentJvmDescriptor import org.jetbrains.kotlin.psi.KtElement import org.jetbrains.kotlin.psi.KtPsiUtil import org.jetbrains.kotlin.resolve.BindingContext import org.jetbrains.kotlin.resolve.DescriptorUtils import org.jetbrains.kotlin.resolve.calls.callUtil.getParentCall import org.jetbrains.kotlin.resolve.calls.callUtil.getResolvedCall +import org.jetbrains.kotlin.resolve.descriptorUtil.firstOverridden import org.jetbrains.kotlin.utils.singletonOrEmptyList import java.util.* diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/state/JetTypeMapper.java b/compiler/backend/src/org/jetbrains/kotlin/codegen/state/JetTypeMapper.java index 92bb56c02c5..3e3a91663c9 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/state/JetTypeMapper.java +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/state/JetTypeMapper.java @@ -1200,7 +1200,18 @@ public class JetTypeMapper { @NotNull CallableDescriptor callableDescriptor ) { sw.writeParameterType(kind); - mapType(type, sw, TypeMappingMode.getOptimalModeForValueParameter(type)); + + TypeMappingMode typeMappingMode; + + if (TypeMappingUtil.isMethodWithDeclarationSiteWildcards(callableDescriptor) && !type.getArguments().isEmpty()) { + typeMappingMode = TypeMappingMode.GENERIC_TYPE; // Render all wildcards + } + else { + typeMappingMode = TypeMappingMode.getOptimalModeForValueParameter(type); + } + + mapType(type, sw, typeMappingMode); + sw.writeParameterTypeEnd(); } diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/state/typeMappingUtil.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/state/typeMappingUtil.kt index f0389b82211..09a0974ae16 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/state/typeMappingUtil.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/state/typeMappingUtil.kt @@ -18,8 +18,11 @@ package org.jetbrains.kotlin.codegen.state import org.jetbrains.kotlin.builtins.KotlinBuiltIns -import org.jetbrains.kotlin.descriptors.ClassDescriptor -import org.jetbrains.kotlin.descriptors.TypeParameterDescriptor +import org.jetbrains.kotlin.descriptors.* +import org.jetbrains.kotlin.name.FqName +import org.jetbrains.kotlin.resolve.descriptorUtil.firstOverridden +import org.jetbrains.kotlin.resolve.descriptorUtil.fqNameOrNull +import org.jetbrains.kotlin.resolve.descriptorUtil.propertyIfAccessor import org.jetbrains.kotlin.types.KotlinType import org.jetbrains.kotlin.types.Variance @@ -67,3 +70,16 @@ public fun getEffectiveVariance(parameterVariance: Variance, projectionKind: Var return Variance.OUT_VARIANCE } +val CallableDescriptor.isMethodWithDeclarationSiteWildcards: Boolean + get() { + if (this !is CallableMemberDescriptor) return false + return firstOverridden { + METHODS_WITH_DECLARATION_SITE_WILDCARDS.containsRaw(it.propertyIfAccessor.fqNameOrNull()) + } != null + } + +private val METHODS_WITH_DECLARATION_SITE_WILDCARDS = setOf( + FqName("kotlin.MutableCollection.addAll"), + FqName("kotlin.MutableList.addAll"), + FqName("kotlin.MutableMap.putAll") +) diff --git a/compiler/testData/codegen/boxWithJava/collections/readOnlyList/J.java b/compiler/testData/codegen/boxWithJava/collections/readOnlyList/J.java new file mode 100644 index 00000000000..cfc22995761 --- /dev/null +++ b/compiler/testData/codegen/boxWithJava/collections/readOnlyList/J.java @@ -0,0 +1,13 @@ +import java.util.*; + +public class J { + + private static class MyList extends KList {} + + public static String foo() { + Collection collection = new MyList(); + if (!collection.contains("ABCDE")) return "fail 1"; + if (!collection.containsAll(Arrays.asList(1, 2, 3))) return "fail 2"; + return "OK"; + } +} \ No newline at end of file diff --git a/compiler/testData/codegen/boxWithJava/collections/readOnlyList/readOnlyList.kt b/compiler/testData/codegen/boxWithJava/collections/readOnlyList/readOnlyList.kt new file mode 100644 index 00000000000..fbe4312ae11 --- /dev/null +++ b/compiler/testData/codegen/boxWithJava/collections/readOnlyList/readOnlyList.kt @@ -0,0 +1,40 @@ +open class KList : List { + override val size: Int + get() = throw UnsupportedOperationException() + + override fun isEmpty(): Boolean { + throw UnsupportedOperationException() + } + override fun contains(o: E) = true + override fun containsAll(c: Collection) = true + + override fun iterator(): Iterator { + throw UnsupportedOperationException() + } + + override fun get(index: Int): E { + throw UnsupportedOperationException() + } + + override fun indexOf(element: E): Int { + throw UnsupportedOperationException() + } + + override fun lastIndexOf(element: E): Int { + throw UnsupportedOperationException() + } + + override fun listIterator(): ListIterator { + throw UnsupportedOperationException() + } + + override fun listIterator(index: Int): ListIterator { + throw UnsupportedOperationException() + } + + override fun subList(fromIndex: Int, toIndex: Int): List { + throw UnsupportedOperationException() + } +} + +fun box() = J.foo() \ No newline at end of file diff --git a/compiler/testData/codegen/boxWithJava/collections/strList/strList.kt b/compiler/testData/codegen/boxWithJava/collections/strList/strList.kt index 00bcaff8d94..508b521c719 100644 --- a/compiler/testData/codegen/boxWithJava/collections/strList/strList.kt +++ b/compiler/testData/codegen/boxWithJava/collections/strList/strList.kt @@ -1,4 +1,4 @@ -open class KList : MutableList { +abstract class KList : MutableList { override val size: Int get() = throw UnsupportedOperationException() diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/generated/BlackBoxWithJavaCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/generated/BlackBoxWithJavaCodegenTestGenerated.java index fc0e4a632b2..1c84d1296d4 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/generated/BlackBoxWithJavaCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/generated/BlackBoxWithJavaCodegenTestGenerated.java @@ -287,6 +287,12 @@ public class BlackBoxWithJavaCodegenTestGenerated extends AbstractBlackBoxCodege doTestWithJava(fileName); } + @TestMetadata("readOnlyList") + public void testReadOnlyList() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/boxWithJava/collections/readOnlyList/"); + doTestWithJava(fileName); + } + @TestMetadata("removeAtInt") public void testRemoveAtInt() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/boxWithJava/collections/removeAtInt/"); diff --git a/core/descriptor.loader.java/src/org/jetbrains/kotlin/load/java/specialBuiltinMembers.kt b/core/descriptor.loader.java/src/org/jetbrains/kotlin/load/java/specialBuiltinMembers.kt index 1f7990eb438..bd53631b93c 100644 --- a/core/descriptor.loader.java/src/org/jetbrains/kotlin/load/java/specialBuiltinMembers.kt +++ b/core/descriptor.loader.java/src/org/jetbrains/kotlin/load/java/specialBuiltinMembers.kt @@ -27,13 +27,8 @@ import org.jetbrains.kotlin.load.java.descriptors.JavaClassDescriptor import org.jetbrains.kotlin.name.FqName import org.jetbrains.kotlin.name.Name import org.jetbrains.kotlin.resolve.DescriptorUtils -import org.jetbrains.kotlin.resolve.descriptorUtil.builtIns -import org.jetbrains.kotlin.resolve.descriptorUtil.fqNameSafe -import org.jetbrains.kotlin.resolve.descriptorUtil.fqNameUnsafe -import org.jetbrains.kotlin.resolve.descriptorUtil.module +import org.jetbrains.kotlin.resolve.descriptorUtil.* import org.jetbrains.kotlin.types.checker.TypeCheckingProcedure -import org.jetbrains.kotlin.utils.DFS -import org.jetbrains.kotlin.utils.addToStdlib.check object BuiltinSpecialProperties { private val PROPERTY_FQ_NAME_TO_JVM_GETTER_NAME_MAP = mapOf( @@ -280,33 +275,6 @@ private fun CallableMemberDescriptor.isFromBuiltins(): Boolean { this.module == this.builtIns.builtInsModule } -private val CallableMemberDescriptor.propertyIfAccessor: CallableMemberDescriptor - get() = if (this is PropertyAccessorDescriptor) correspondingProperty else this - -private fun CallableDescriptor.fqNameOrNull(): FqName? = fqNameUnsafe.check { it.isSafe }?.toSafe() - -public fun CallableMemberDescriptor.firstOverridden( - predicate: (CallableMemberDescriptor) -> Boolean -): CallableMemberDescriptor? { - var result: CallableMemberDescriptor? = null - return DFS.dfs(listOf(this), - object : DFS.Neighbors { - override fun getNeighbors(current: CallableMemberDescriptor?): Iterable { - return current?.overriddenDescriptors ?: emptyList() - } - }, - object : DFS.AbstractNodeHandler() { - override fun beforeChildren(current: CallableMemberDescriptor) = result == null - override fun afterChildren(current: CallableMemberDescriptor) { - if (result == null && predicate(current)) { - result = current - } - } - override fun result(): CallableMemberDescriptor? = result - } - ) -} - public fun CallableMemberDescriptor.isFromJavaOrBuiltins() = isFromJava || isFromBuiltins() private fun Map.getInversedShortNamesMap(): Map> = diff --git a/core/descriptors/src/org/jetbrains/kotlin/resolve/DescriptorUtils.kt b/core/descriptors/src/org/jetbrains/kotlin/resolve/DescriptorUtils.kt index 167dd385741..b65e7c3f4a1 100644 --- a/core/descriptors/src/org/jetbrains/kotlin/resolve/DescriptorUtils.kt +++ b/core/descriptors/src/org/jetbrains/kotlin/resolve/DescriptorUtils.kt @@ -31,6 +31,7 @@ import org.jetbrains.kotlin.resolve.DescriptorUtils import org.jetbrains.kotlin.resolve.constants.EnumValue import org.jetbrains.kotlin.types.KotlinType import org.jetbrains.kotlin.utils.DFS +import org.jetbrains.kotlin.utils.addToStdlib.check public fun ClassDescriptor.getClassObjectReferenceTarget(): ClassDescriptor = getCompanionObjectDescriptor() ?: this @@ -197,3 +198,29 @@ public val DeclarationDescriptor.parentsWithSelf: Sequence get() = parentsWithSelf.drop(1) +val CallableMemberDescriptor.propertyIfAccessor: CallableMemberDescriptor + get() = if (this is PropertyAccessorDescriptor) correspondingProperty else this + +fun CallableDescriptor.fqNameOrNull(): FqName? = fqNameUnsafe.check { it.isSafe }?.toSafe() + +public fun CallableMemberDescriptor.firstOverridden( + predicate: (CallableMemberDescriptor) -> Boolean +): CallableMemberDescriptor? { + var result: CallableMemberDescriptor? = null + return DFS.dfs(listOf(this), + object : DFS.Neighbors { + override fun getNeighbors(current: CallableMemberDescriptor?): Iterable { + return current?.overriddenDescriptors ?: emptyList() + } + }, + object : DFS.AbstractNodeHandler() { + override fun beforeChildren(current: CallableMemberDescriptor) = result == null + override fun afterChildren(current: CallableMemberDescriptor) { + if (result == null && predicate(current)) { + result = current + } + } + override fun result(): CallableMemberDescriptor? = result + } + ) +}