From a5a69c509962bf203bf36c06932a6d66eff3ae7f Mon Sep 17 00:00:00 2001 From: Alexander Udalov Date: Thu, 1 Mar 2018 17:14:31 +0100 Subject: [PATCH] Remove dependency of ClassMapperLite on JavaToKotlinClassMap There are two reasons for this. First, this class will be used in the metadata reading library which should not have dependencies on lots of compiler stuff JavaToKotlinClassMap depends on. Second, it was easy to accidentally break the deserialization in old compilers by adding another mapping to JavaToKotlinClassMap. This was possible because ClassMapperLite is used to decide whether or not the JVM signature is "trivial" and should be written to the metadata (at JvmSerializerExtension.SignatureSerializer.requiresSignature). If the signature is trivial but mentions a type added in JavaToKotlinClassMap in the new compiler, the old compiler will not be able to load the signature correctly. See the comment on ClassMapperLite for more information --- .../serialization/JvmSerializerExtension.java | 2 +- ...tBinaryClassAnnotationAndConstantLoader.kt | 2 +- .../serialization/jvm/ClassMapperLite.kt | 87 +++++++++++++------ .../serialization/jvm/JvmProtoBufUtil.kt | 2 +- 4 files changed, 65 insertions(+), 28 deletions(-) diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/serialization/JvmSerializerExtension.java b/compiler/backend/src/org/jetbrains/kotlin/codegen/serialization/JvmSerializerExtension.java index 09521a38338..0618c793034 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/serialization/JvmSerializerExtension.java +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/serialization/JvmSerializerExtension.java @@ -265,7 +265,7 @@ public class JvmSerializerExtension extends SerializerExtension { ClassifierDescriptor classifier = type.getConstructor().getDeclarationDescriptor(); if (!(classifier instanceof ClassDescriptor)) return null; ClassId classId = DescriptorUtilsKt.getClassId((ClassDescriptor) classifier); - return classId == null ? null : ClassMapperLite.mapClass(classId); + return classId == null ? null : ClassMapperLite.mapClass(classId.asString()); } @NotNull diff --git a/core/descriptors.jvm/src/org/jetbrains/kotlin/load/kotlin/AbstractBinaryClassAnnotationAndConstantLoader.kt b/core/descriptors.jvm/src/org/jetbrains/kotlin/load/kotlin/AbstractBinaryClassAnnotationAndConstantLoader.kt index e7f680e911c..9742e45b470 100644 --- a/core/descriptors.jvm/src/org/jetbrains/kotlin/load/kotlin/AbstractBinaryClassAnnotationAndConstantLoader.kt +++ b/core/descriptors.jvm/src/org/jetbrains/kotlin/load/kotlin/AbstractBinaryClassAnnotationAndConstantLoader.kt @@ -120,7 +120,7 @@ abstract class AbstractBinaryClassAnnotationAndConstantLoader { val signature = MemberSignature.fromFieldNameAndDesc( container.nameResolver.getString(proto.name), - ClassMapperLite.mapClass((container as ProtoContainer.Class).classId) + ClassMapperLite.mapClass((container as ProtoContainer.Class).classId.asString()) ) return findClassAndLoadMemberAnnotations(container, signature) } diff --git a/core/descriptors.jvm/src/org/jetbrains/kotlin/serialization/jvm/ClassMapperLite.kt b/core/descriptors.jvm/src/org/jetbrains/kotlin/serialization/jvm/ClassMapperLite.kt index d18d9654da7..f280b11492e 100644 --- a/core/descriptors.jvm/src/org/jetbrains/kotlin/serialization/jvm/ClassMapperLite.kt +++ b/core/descriptors.jvm/src/org/jetbrains/kotlin/serialization/jvm/ClassMapperLite.kt @@ -16,37 +16,74 @@ package org.jetbrains.kotlin.serialization.jvm -import org.jetbrains.kotlin.builtins.KotlinBuiltIns -import org.jetbrains.kotlin.name.ClassId -import org.jetbrains.kotlin.platform.JavaToKotlinClassMap -import org.jetbrains.kotlin.resolve.jvm.JvmPrimitiveType - // The purpose of this class is to map Kotlin classes to JVM bytecode desc strings, as KotlinTypeMapper does in the backend. // It's used as an optimization during serialization/deserialization: if there's no JVM signature for a method/property/constructor, // it means that the JVM signature should be trivially computable from the Kotlin signature with this class. // It's not required to support everything in KotlinTypeMapper, but the more it does, the more we save on JVM signatures in proto metadata. -// Note that improving the behavior of this class may break binary compatibility of code compiled by Kotlin, because it may make +// +// WARNING: improving the behavior of this class MAY BREAK BINARY COMPATIBILITY of code compiled by Kotlin, because it may make // the new compiler skip writing the signatures it now thinks are trivial, and the old compiler would recreate them incorrectly. object ClassMapperLite { + // Kotlin ClassId -> JVM desc + // e.g. "kotlin.IntArray" -> "[I" + // "kotlin.String.Companion" -> "Lkotlin/jvm/internal/StringCompanionObject" + // "kotlin/collections/Map.Entry" -> "Ljava/util/Map$Entry" + private val map: Map = mutableMapOf().apply { + val primitives = listOf( + "Boolean", "Z", + "Char", "C", + "Byte", "B", + "Short", "S", + "Int", "I", + "Float", "F", + "Long", "J", + "Double", "D" + ) + + for (i in primitives.indices step 2) { + put("kotlin/${primitives[i]}", primitives[i + 1]) + put("kotlin/${primitives[i]}Array", "[${primitives[i + 1]}") + } + + put("kotlin/Unit", "V") + + fun add(kotlinSimpleName: String, javaInternalName: String) { + put("kotlin/$kotlinSimpleName", "L$javaInternalName;") + } + + add("Any", "java/lang/Object") + add("Nothing", "java/lang/Void") + add("Annotation", "java/lang/annotation/Annotation") + + for (klass in listOf("String", "CharSequence", "Throwable", "Cloneable", "Number", "Comparable", "Enum")) { + add(klass, "java/lang/$klass") + } + + for (klass in listOf("Iterator", "Collection", "List", "Set", "Map", "ListIterator")) { + add("collections/$klass", "java/util/$klass") + add("collections/Mutable$klass", "java/util/$klass") + } + + add("collections/Iterable", "java/lang/Iterable") + add("collections/MutableIterable", "java/lang/Iterable") + add("collections/Map.Entry", "java/util/Map\$Entry") + add("collections/MutableMap.MutableEntry", "java/util/Map\$Entry") + + for (i in 0..22) { + add("Function$i", "kotlin/jvm/functions/Function$i") + add("reflect/KFunction$i", "kotlin/reflect/KFunction") + } + + for (klass in listOf("Char", "Byte", "Short", "Int", "Float", "Long", "Double", "String", "Enum")) { + add("$klass.Companion", "kotlin/jvm/internal/${klass}CompanionObject") + } + } + + /** + * @param classId the name of the class in the format: "org/foo/bar/Test.Inner" + */ @JvmStatic - fun mapClass(classId: ClassId): String { - val internalName = classId.asString().replace('.', '$') - val simpleName = internalName.removePrefix("kotlin/") - if (simpleName != internalName) { - for (jvmPrimitive in JvmPrimitiveType.values()) { - val primitiveType = jvmPrimitive.primitiveType - if (simpleName == primitiveType.typeName.asString()) return jvmPrimitive.desc - if (simpleName == primitiveType.arrayTypeName.asString()) return "[" + jvmPrimitive.desc - } - - if (simpleName == KotlinBuiltIns.FQ_NAMES.unit.shortName().asString()) return "V" - } - - val javaClassId = JavaToKotlinClassMap.mapKotlinToJava(classId.asSingleFqName().toUnsafe()) - if (javaClassId != null) { - return "L" + javaClassId.asString().replace('.', '$') + ";" - } - - return "L$internalName;" + fun mapClass(classId: String): String { + return map[classId] ?: "L${classId.replace('.', '$')};" } } diff --git a/core/descriptors.jvm/src/org/jetbrains/kotlin/serialization/jvm/JvmProtoBufUtil.kt b/core/descriptors.jvm/src/org/jetbrains/kotlin/serialization/jvm/JvmProtoBufUtil.kt index f66f4c390d6..3e284350b5d 100644 --- a/core/descriptors.jvm/src/org/jetbrains/kotlin/serialization/jvm/JvmProtoBufUtil.kt +++ b/core/descriptors.jvm/src/org/jetbrains/kotlin/serialization/jvm/JvmProtoBufUtil.kt @@ -117,6 +117,6 @@ object JvmProtoBufUtil { data class PropertySignature(val name: String, val desc: String) private fun mapTypeDefault(type: ProtoBuf.Type, nameResolver: NameResolver): String? { - return if (type.hasClassName()) ClassMapperLite.mapClass(nameResolver.getClassId(type.className)) else null + return if (type.hasClassName()) ClassMapperLite.mapClass(nameResolver.getQualifiedClassName(type.className)) else null } }