From 3b9d90429b7d81a40c55a66d446dfe4f28075dde Mon Sep 17 00:00:00 2001 From: Alexander Udalov Date: Fri, 25 Sep 2015 18:13:19 +0300 Subject: [PATCH] Simplify local class name serialization, don't go through extension Also fix it for the case of a class in a non-default package --- .../codegen/JvmSerializerExtension.java | 20 +++--- .../codegen/serialization/JvmStringTable.kt | 68 +++++++++---------- .../builtins/BuiltInsSerializerExtension.kt | 2 +- .../serialization/SerializerExtension.java | 5 -- .../kotlin/serialization/StringTableImpl.java | 13 +--- .../serialization/local/deepInnerChain.kt | 2 + .../serialization/local/deepInnerChain.txt | 8 +-- .../AbstractLocalClassProtoTest.kt | 12 ++-- .../org/jetbrains/kotlin/name/ClassId.java | 12 +++- .../js/KotlinJavascriptSerializerExtension.kt | 2 +- 10 files changed, 66 insertions(+), 78 deletions(-) diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/JvmSerializerExtension.java b/compiler/backend/src/org/jetbrains/kotlin/codegen/JvmSerializerExtension.java index a6f8134f142..a785d53c391 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/JvmSerializerExtension.java +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/JvmSerializerExtension.java @@ -25,7 +25,10 @@ import org.jetbrains.kotlin.codegen.state.JetTypeMapper; import org.jetbrains.kotlin.descriptors.*; import org.jetbrains.kotlin.descriptors.annotations.AnnotationDescriptor; import org.jetbrains.kotlin.load.java.lazy.types.RawTypeCapabilities; -import org.jetbrains.kotlin.serialization.*; +import org.jetbrains.kotlin.serialization.AnnotationSerializer; +import org.jetbrains.kotlin.serialization.ProtoBuf; +import org.jetbrains.kotlin.serialization.SerializerExtension; +import org.jetbrains.kotlin.serialization.StringTable; import org.jetbrains.kotlin.serialization.deserialization.NameResolver; import org.jetbrains.kotlin.serialization.deserialization.descriptors.DeserializedPropertyDescriptor; import org.jetbrains.kotlin.serialization.deserialization.descriptors.DeserializedSimpleFunctionDescriptor; @@ -34,18 +37,17 @@ import org.jetbrains.kotlin.types.JetType; import org.jetbrains.org.objectweb.asm.Type; import org.jetbrains.org.objectweb.asm.commons.Method; -import static org.jetbrains.kotlin.codegen.AsmUtil.shortNameByAsmType; import static org.jetbrains.kotlin.codegen.JvmSerializationBindings.*; public class JvmSerializerExtension extends SerializerExtension { private final JvmSerializationBindings bindings; - private final JetTypeMapper typeMapper; - private final StringTable stringTable = new JvmStringTable(this); - private final AnnotationSerializer annotationSerializer = new AnnotationSerializer(stringTable); + private final StringTable stringTable; + private final AnnotationSerializer annotationSerializer; public JvmSerializerExtension(@NotNull JvmSerializationBindings bindings, @NotNull JetTypeMapper typeMapper) { this.bindings = bindings; - this.typeMapper = typeMapper; + this.stringTable = new JvmStringTable(typeMapper); + this.annotationSerializer = new AnnotationSerializer(stringTable); } @NotNull @@ -91,12 +93,6 @@ public class JvmSerializerExtension extends SerializerExtension { } } - @Override - @NotNull - public String getLocalClassName(@NotNull ClassDescriptor descriptor) { - return shortNameByAsmType(typeMapper.mapClass(descriptor)); - } - private void saveSignature(@NotNull CallableMemberDescriptor callable, @NotNull ProtoBuf.Callable.Builder proto) { SignatureSerializer signatureSerializer = new SignatureSerializer(); if (callable instanceof FunctionDescriptor) { diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/serialization/JvmStringTable.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/serialization/JvmStringTable.kt index 0603c954612..0f77f7b00ff 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/serialization/JvmStringTable.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/serialization/JvmStringTable.kt @@ -16,13 +16,12 @@ package org.jetbrains.kotlin.codegen.serialization +import org.jetbrains.kotlin.codegen.state.JetTypeMapper import org.jetbrains.kotlin.descriptors.ClassDescriptor -import org.jetbrains.kotlin.descriptors.ClassOrPackageFragmentDescriptor -import org.jetbrains.kotlin.descriptors.DeclarationDescriptor import org.jetbrains.kotlin.descriptors.PackageFragmentDescriptor import org.jetbrains.kotlin.load.kotlin.JvmNameResolver -import org.jetbrains.kotlin.resolve.descriptorUtil.classId -import org.jetbrains.kotlin.serialization.SerializerExtension +import org.jetbrains.kotlin.name.ClassId +import org.jetbrains.kotlin.name.FqName import org.jetbrains.kotlin.serialization.StringTable import org.jetbrains.kotlin.serialization.jvm.JvmProtoBuf import org.jetbrains.kotlin.serialization.jvm.JvmProtoBuf.StringTableTypes.Record @@ -30,7 +29,7 @@ import org.jetbrains.kotlin.types.ErrorUtils import java.io.OutputStream import java.util.* -class JvmStringTable(private val extension: SerializerExtension) : StringTable { +class JvmStringTable(private val typeMapper: JetTypeMapper) : StringTable { public val strings = ArrayList() private val records = ArrayList() private val map = HashMap() @@ -52,43 +51,35 @@ class JvmStringTable(private val extension: SerializerExtension) : StringTable { throw IllegalStateException("Cannot get FQ name of error class: " + descriptor) } - val string: String - val storeAsLiteral: Boolean + // We use the following format to encode ClassId: "pkg/Outer.Inner". + // It represents a unique name, but such names don't usually appear in the constant pool, so we're writing "Lpkg/Outer$Inner;" + // instead and an instruction to drop the first and the last character in this string and replace all '$' with '.'. + // This works most of the time, except in two rare cases: + // - the name of the class or any of its outer classes contains dollars. In this case we're just storing the described + // string literally: "pkg/Outer.Inner$with$dollars" + // - the class is local or nested in local. In this case we're also storing the literal string, and also storing the fact that + // this name represents a local class in a separate list - val parent = sequence(descriptor, DeclarationDescriptor::getContainingDeclaration).first { it !is ClassDescriptor } - if (parent is PackageFragmentDescriptor) { - val classId = descriptor.classId - val packageName = classId.packageFqName - val className = classId.relativeClassName.asString() - string = - if (packageName.isRoot) className - else packageName.asString().replace('.', '/') + "/" + className - - // If any of the class names contains '$', we can't simply replace all '$' with '.' upon deserialization. - // This case is rather rare, so we're storing a literal string - storeAsLiteral = '$' in string && classId.relativeClassName.pathSegments().any { '$' in it.asString() } - } - else { - storeAsLiteral = true - string = descriptor.localClassName() - } - - val isLocal = descriptor.containingDeclaration !is ClassOrPackageFragmentDescriptor + val classId = descriptor.classId + val string = classId.asString() map[string]?.let { recordedIndex -> - if (isLocal == (recordedIndex in localNames)) { + // If we already recorded such string, we only return its index if it's local and our name is local + // OR it's not local and our name is not local as well + if (classId.isLocal == (recordedIndex in localNames)) { return recordedIndex } } val index = strings.size() - if (isLocal) { + if (classId.isLocal) { localNames.add(index) } val record = Record.newBuilder() - if (storeAsLiteral) { + // If the class is local or any of its outer class names contains '$', store a literal string + if (classId.isLocal || '$' in string) { strings.add(string) // TODO: optimize, don't always store the operation record.setOperation(Record.Operation.NONE) @@ -103,7 +94,7 @@ class JvmStringTable(private val extension: SerializerExtension) : StringTable { } else { record.setOperation(Record.Operation.DESC_TO_CLASS_ID) - strings.add("L$string;") + strings.add("L${string.replace('.', '$')};") } } @@ -114,13 +105,18 @@ class JvmStringTable(private val extension: SerializerExtension) : StringTable { return index } - private fun ClassDescriptor.localClassName(): String { - val container = containingDeclaration - return when (container) { - is ClassDescriptor -> container.localClassName() + "." + name.asString() - else -> extension.getLocalClassName(this) + private val ClassDescriptor.classId: ClassId + get() { + val container = containingDeclaration + return when (container) { + is ClassDescriptor -> container.classId.createNestedClassId(name) + is PackageFragmentDescriptor -> ClassId(container.fqName, name) + else -> { + val fqName = FqName(typeMapper.mapClass(this).internalName.replace('/', '.')) + ClassId(fqName.parent(), FqName.topLevel(fqName.shortName()), /* isLocal = */ true) + } + } } - } override fun serializeTo(output: OutputStream) { with(JvmProtoBuf.StringTableTypes.newBuilder()) { diff --git a/compiler/builtins-serializer/src/org/jetbrains/kotlin/serialization/builtins/BuiltInsSerializerExtension.kt b/compiler/builtins-serializer/src/org/jetbrains/kotlin/serialization/builtins/BuiltInsSerializerExtension.kt index a69d8ad5471..08df4c04b92 100644 --- a/compiler/builtins-serializer/src/org/jetbrains/kotlin/serialization/builtins/BuiltInsSerializerExtension.kt +++ b/compiler/builtins-serializer/src/org/jetbrains/kotlin/serialization/builtins/BuiltInsSerializerExtension.kt @@ -22,7 +22,7 @@ import org.jetbrains.kotlin.resolve.scopes.DescriptorKindFilter import org.jetbrains.kotlin.serialization.* public class BuiltInsSerializerExtension : SerializerExtension() { - private val stringTable = StringTableImpl(this) + private val stringTable = StringTableImpl() private val annotationSerializer = AnnotationSerializer(stringTable) override fun getStringTable(): StringTable = stringTable diff --git a/compiler/serialization/src/org/jetbrains/kotlin/serialization/SerializerExtension.java b/compiler/serialization/src/org/jetbrains/kotlin/serialization/SerializerExtension.java index aef1fcb8e53..98cdd37aadb 100644 --- a/compiler/serialization/src/org/jetbrains/kotlin/serialization/SerializerExtension.java +++ b/compiler/serialization/src/org/jetbrains/kotlin/serialization/SerializerExtension.java @@ -46,9 +46,4 @@ public abstract class SerializerExtension { public void serializeType(@NotNull JetType type, @NotNull ProtoBuf.Type.Builder proto) { } - - @NotNull - public String getLocalClassName(@NotNull ClassDescriptor descriptor) { - throw new UnsupportedOperationException("Local classes are unsupported: " + descriptor); - } } diff --git a/compiler/serialization/src/org/jetbrains/kotlin/serialization/StringTableImpl.java b/compiler/serialization/src/org/jetbrains/kotlin/serialization/StringTableImpl.java index 11d9d8e1904..9daa5fe2095 100644 --- a/compiler/serialization/src/org/jetbrains/kotlin/serialization/StringTableImpl.java +++ b/compiler/serialization/src/org/jetbrains/kotlin/serialization/StringTableImpl.java @@ -61,11 +61,6 @@ public class StringTableImpl implements StringTable { private final Interner strings = new Interner(); private final Interner qualifiedNames = new Interner(); - private final SerializerExtension extension; - - public StringTableImpl(@NotNull SerializerExtension extension) { - this.extension = extension; - } public int getSimpleNameIndex(@NotNull Name name) { return getStringIndex(name.asString()); @@ -86,25 +81,21 @@ public class StringTableImpl implements StringTable { builder.setKind(QualifiedName.Kind.CLASS); DeclarationDescriptor containingDeclaration = descriptor.getContainingDeclaration(); - String shortName; if (containingDeclaration instanceof PackageFragmentDescriptor) { - shortName = descriptor.getName().asString(); FqName packageFqName = ((PackageFragmentDescriptor) containingDeclaration).getFqName(); if (!packageFqName.isRoot()) { builder.setParentQualifiedName(getPackageFqNameIndex(packageFqName)); } } else if (containingDeclaration instanceof ClassDescriptor) { - shortName = descriptor.getName().asString(); ClassDescriptor outerClass = (ClassDescriptor) containingDeclaration; builder.setParentQualifiedName(getFqNameIndex(outerClass)); } else { - builder.setKind(QualifiedName.Kind.LOCAL); - shortName = extension.getLocalClassName(descriptor); + throw new IllegalStateException("Cannot get FQ name of local class: " + descriptor); } - builder.setShortName(getStringIndex(shortName)); + builder.setShortName(getStringIndex(descriptor.getName().asString())); return qualifiedNames.intern(new FqNameProto(builder)); } diff --git a/compiler/testData/serialization/local/deepInnerChain.kt b/compiler/testData/serialization/local/deepInnerChain.kt index 5d059d89214..8480d3340db 100644 --- a/compiler/testData/serialization/local/deepInnerChain.kt +++ b/compiler/testData/serialization/local/deepInnerChain.kt @@ -1,3 +1,5 @@ +package test + // CLASS_NAME_SUFFIX: Deepest fun main() { diff --git a/compiler/testData/serialization/local/deepInnerChain.txt b/compiler/testData/serialization/local/deepInnerChain.txt index 856e8f1350a..1c1715a3f9d 100644 --- a/compiler/testData/serialization/local/deepInnerChain.txt +++ b/compiler/testData/serialization/local/deepInnerChain.txt @@ -1,7 +1,7 @@ local final inner class Deepest public constructor Deepest() -public final fun deep(): `DeepInnerChainKt$main$Local$Inner$prop$1$foo$1$DeepLocal` -public final fun deepest(): `DeepInnerChainKt$main$Local$Inner$prop$1$foo$1$DeepLocal`.Deepest? -public final fun inner(): `DeepInnerChainKt$main$Local`.Inner -public final fun local(): `DeepInnerChainKt$main$Local` +public final fun deep(): test.`DeepInnerChainKt$main$Local$Inner$prop$1$foo$1$DeepLocal` +public final fun deepest(): test.`DeepInnerChainKt$main$Local$Inner$prop$1$foo$1$DeepLocal`.Deepest? +public final fun inner(): test.`DeepInnerChainKt$main$Local`.Inner +public final fun local(): test.`DeepInnerChainKt$main$Local` diff --git a/compiler/tests/org/jetbrains/kotlin/serialization/AbstractLocalClassProtoTest.kt b/compiler/tests/org/jetbrains/kotlin/serialization/AbstractLocalClassProtoTest.kt index 0df8f40a4ad..fbd1246cba2 100644 --- a/compiler/tests/org/jetbrains/kotlin/serialization/AbstractLocalClassProtoTest.kt +++ b/compiler/tests/org/jetbrains/kotlin/serialization/AbstractLocalClassProtoTest.kt @@ -22,11 +22,11 @@ import org.jetbrains.kotlin.cli.jvm.compiler.EnvironmentConfigFiles import org.jetbrains.kotlin.cli.jvm.compiler.KotlinCoreEnvironment import org.jetbrains.kotlin.cli.jvm.config.getModuleName import org.jetbrains.kotlin.codegen.forTestCompile.ForTestCompileRuntime +import org.jetbrains.kotlin.descriptors.PackagePartProvider import org.jetbrains.kotlin.frontend.java.di.createContainerForTopDownAnalyzerForJvm import org.jetbrains.kotlin.incremental.components.LookupTracker import org.jetbrains.kotlin.jvm.compiler.LoadDescriptorUtil import org.jetbrains.kotlin.load.java.JvmAnnotationNames -import org.jetbrains.kotlin.descriptors.PackagePartProvider import org.jetbrains.kotlin.load.java.structure.reflect.classId import org.jetbrains.kotlin.resolve.jvm.TopDownAnalyzerFacadeForJVM import org.jetbrains.kotlin.resolve.lazy.declarations.FileBasedDeclarationProviderFactory @@ -38,20 +38,20 @@ import java.net.URLClassLoader public abstract class AbstractLocalClassProtoTest : TestCaseWithTmpdir() { protected fun doTest(filename: String) { val source = File(filename) - LoadDescriptorUtil.compileKotlinToDirAndGetAnalysisResult(listOf(source), tmpdir, getTestRootDisposable(), ConfigurationKind.ALL) + LoadDescriptorUtil.compileKotlinToDirAndGetAnalysisResult(listOf(source), tmpdir, testRootDisposable, ConfigurationKind.ALL) val classNameSuffix = InTextDirectivesUtils.findStringWithPrefixes(source.readText(), "// CLASS_NAME_SUFFIX: ") ?: error("CLASS_NAME_SUFFIX directive not found in test data") val classLoader = URLClassLoader(arrayOf(tmpdir.toURI().toURL()), ForTestCompileRuntime.runtimeAndReflectJarClassLoader()) - val classFile = tmpdir.listFiles().singleOrNull { it.getPath().endsWith("$classNameSuffix.class") } + val classFile = tmpdir.walkTopDown().singleOrNull { it.path.endsWith("$classNameSuffix.class") } ?: error("Local class with suffix `$classNameSuffix` is not found in: ${tmpdir.listFiles().toList()}") - val clazz = classLoader.loadClass(classFile.name.substringBeforeLast(".class")) + val clazz = classLoader.loadClass(classFile.relativeTo(tmpdir).substringBeforeLast(".class").replace('/', '.').replace('\\', '.')) assertHasAnnotationData(clazz) val environment = KotlinCoreEnvironment.createForTests( - getTestRootDisposable(), + testRootDisposable, JetTestUtils.compilerConfigurationForTests(ConfigurationKind.ALL, TestJdkKind.MOCK_JDK, tmpdir), EnvironmentConfigFiles.JVM_CONFIG_FILES ) @@ -79,7 +79,7 @@ public abstract class AbstractLocalClassProtoTest : TestCaseWithTmpdir() { private fun assertHasAnnotationData(clazz: Class<*>) { @Suppress("UNCHECKED_CAST") val annotation = clazz.getAnnotation( - clazz.getClassLoader().loadClass(JvmAnnotationNames.KOTLIN_CLASS.asString()) as Class + clazz.classLoader.loadClass(JvmAnnotationNames.KOTLIN_CLASS.asString()) as Class ) assert(annotation != null) { "KotlinClass annotation is not found for class $clazz" } diff --git a/core/descriptors/src/org/jetbrains/kotlin/name/ClassId.java b/core/descriptors/src/org/jetbrains/kotlin/name/ClassId.java index 8b11e881f83..54952802089 100644 --- a/core/descriptors/src/org/jetbrains/kotlin/name/ClassId.java +++ b/core/descriptors/src/org/jetbrains/kotlin/name/ClassId.java @@ -88,6 +88,15 @@ public final class ClassId { return new FqName(packageFqName.asString() + "." + relativeClassName.asString()); } + /** + * @return a string where packages are delimited by '/' and classes by '.', e.g. "kotlin/Map.Entry" + */ + @NotNull + public String asString() { + if (packageFqName.isRoot()) return relativeClassName.asString(); + return packageFqName.asString().replace('.', '/') + "/" + relativeClassName.asString(); + } + @Override public boolean equals(Object o) { if (this == o) return true; @@ -110,7 +119,6 @@ public final class ClassId { @Override public String toString() { - if (packageFqName.isRoot()) return "/" + relativeClassName; - return packageFqName.toString().replace('.', '/') + "/" + relativeClassName; + return packageFqName.isRoot() ? "/" + asString() : asString(); } } diff --git a/js/js.serializer/src/org/jetbrains/kotlin/serialization/js/KotlinJavascriptSerializerExtension.kt b/js/js.serializer/src/org/jetbrains/kotlin/serialization/js/KotlinJavascriptSerializerExtension.kt index 6325be7cc71..426fb3253c9 100644 --- a/js/js.serializer/src/org/jetbrains/kotlin/serialization/js/KotlinJavascriptSerializerExtension.kt +++ b/js/js.serializer/src/org/jetbrains/kotlin/serialization/js/KotlinJavascriptSerializerExtension.kt @@ -28,7 +28,7 @@ import org.jetbrains.kotlin.serialization.StringTableImpl import org.jetbrains.kotlin.types.JetType public class KotlinJavascriptSerializerExtension : SerializerExtension() { - private val stringTable = StringTableImpl(this) + private val stringTable = StringTableImpl() private val annotationSerializer = AnnotationSerializer(stringTable) override fun getStringTable() = stringTable