azalea/kotlin-fork
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix serialization of type parameters inner generic class

Browse Source 
The call to `createTopLevel` instead of `create` (which creates
serializers for outer classes properly, with correct type parameter
contexts) caused MetadataSerializer to write type parameter metadata
incorrectly.  For example, in the following case:

    class A<E> {
        inner class B<T, E> { ... }
    }

A's type parameter E would get id 0, and B's type parameters T and E
would get ids 0 and 1. This is a problem because ids are supposed to be
unique for each class including its outer classes, and deserializer,
decompiler and stub builder rely on this assumption.

JVM metadata is unaffected because `create` is called correctly there,
see MemberCodegen#generateKotlinClassMetadataAnnotation

 #KT-24944 Fixed
This commit is contained in:
Alexander Udalov
2018-06-19 15:55:46 +02:00
parent c05285a23c
commit 9309570752
11 changed files with 97 additions and 39 deletions
Download Patch File Download Diff File Expand all files Collapse all files
build-common/src/org/jetbrains/kotlin/incremental/JavaClassesTrackerImpl.kt
+2 -5
View File
@@ -35,7 +35,6 @@ import org.jetbrains.kotlin.resolve.source.PsiSourceElement
import org.jetbrains.kotlin.serialization.DescriptorSerializer
import org.jetbrains.kotlin.serialization.deserialization.getClassId
import org.jetbrains.kotlin.util.PerformanceCounter
import org.jetbrains.kotlin.util.getExceptionMessage
import org.jetbrains.kotlin.utils.addToStdlib.safeAs
import org.jetbrains.kotlin.utils.sure
import java.io.DataInput
@@ -96,10 +95,8 @@ fun JavaClassDescriptor.convertToProto(): SerializedJavaClassWithSource {
val file = javaSourceFile.sure { "convertToProto should only be called for source based classes" }
val extension = JavaClassesSerializerExtension()
val serializer = DescriptorSerializer.createTopLevel(extension)
val classProto: ProtoBuf.Class
try {
classProto = serializer.classProto(this).build()
val classProto = try {
DescriptorSerializer.create(this, extension).classProto(this).build()
} catch (e: Exception) {
throw IllegalStateException(
"Error during writing proto for descriptor: ${DescriptorRenderer.DEBUG_TEXT.render(this)}\n" +
compiler/cli/src/org/jetbrains/kotlin/cli/metadata/MetadataSerializer.kt
+1 -1
View File
@@ -158,7 +158,7 @@ open class MetadataSerializer(private val dependOnOldBuiltIns: Boolean) {
}
private fun serializeClass(classDescriptor: ClassDescriptor) {
val classProto = DescriptorSerializer.createTopLevel(extension).classProto(classDescriptor).build()
val classProto = DescriptorSerializer.create(classDescriptor, extension).classProto(classDescriptor).build()
proto.addClass_(classProto)
serializeClasses(classDescriptor.unsubstitutedInnerClassesScope.getContributedDescriptors(DescriptorKindFilter.CLASSIFIERS))
compiler/psi/src/org/jetbrains/kotlin/psi/stubs/KotlinStubVersions.kt
+2 -2
View File
@@ -36,9 +36,9 @@ object KotlinStubVersions {
// BuiltIn stub version should be increased if changes are made to builtIn stub building subsystem (org.jetbrains.kotlin.idea.decompiler.builtIns)
// Increasing this version will lead to reindexing of all builtIn files (see KotlinBuiltInFileType).
const val BUILTIN_STUB_VERSION = BINARY_STUB_VERSION + 1
const val BUILTIN_STUB_VERSION = BINARY_STUB_VERSION + 2
// JS stub version should be increased if changes are made to js stub building subsystem (org.jetbrains.kotlin.idea.decompiler.js)
// Increasing this version will lead to reindexing of js binary files (see KotlinJavaScriptMetaFileType).
const val JS_STUB_VERSION = BINARY_STUB_VERSION + 1
const val JS_STUB_VERSION = BINARY_STUB_VERSION + 2
}
compiler/testData/multiplatform/innerGenericClass/common.kt
Vendored
+9
View File
@@ -0,0 +1,9 @@
class A<E> {
inner class B<T, E> {
fun getAE() = this@A.getAE()
fun getBT(): T? = null
fun getBE(): E? = null
}
fun getAE(): E? = null
}
compiler/testData/multiplatform/innerGenericClass/common2.kt
Vendored
+11
View File
@@ -0,0 +1,11 @@
fun test(): String {
val b = A<String>().B<Int, Double>()
val x: String? = b.getAE()
val y: Int? = b.getBT()
val z: Double? = b.getBE()
// This line is needed to ensure that B.getAE's return type is not an error type; if it was, this line would compile with no errors
b.getAE().unresolved()
return "$x$y$z"
}
compiler/testData/multiplatform/innerGenericClass/output.txt
Vendored
+10
View File
@@ -0,0 +1,10 @@
-- Common --
Exit code: OK
Output:
-- Common (2) --
Exit code: COMPILATION_ERROR
Output:
compiler/testData/multiplatform/innerGenericClass/common2.kt:8:15: error: unresolved reference: unresolved
b.getAE().unresolved()
^
compiler/tests/org/jetbrains/kotlin/multiplatform/AbstractMultiPlatformIntegrationTest.kt
+36 -23
View File
@@ -33,44 +33,57 @@ import java.nio.file.Paths
abstract class AbstractMultiPlatformIntegrationTest : KtUsefulTestCase() {
fun doTest(directoryPath: String) {
val root = File(directoryPath).apply { assert(exists()) }
val commonSrc = File(root, "common.kt")
val jsSrc = File(root, "js.kt")
val jvmSrc = File(root, "jvm.kt")
val commonSrc = File(root, "common.kt").apply { assert(exists()) }
val jsSrc = File(root, "js.kt").takeIf(File::exists)
val jvmSrc = File(root, "jvm.kt").takeIf(File::exists)
// TODO: consider inventing a more clever scheme
val jvm2Src = File(root, "jvm2.kt")
val common2Src = File(root, "common2.kt").takeIf(File::exists)
val jvm2Src = File(root, "jvm2.kt").takeIf(File::exists)
val tmpdir = KotlinTestUtils.tmpDir(getTestName(true))
val optionalStdlibCommon =
if (InTextDirectivesUtils.isDirectiveDefined(commonSrc.readText(), "WITH_RUNTIME"))
arrayOf("-cp", findStdlibCommon().absolutePath)
else emptyArray()
if (InTextDirectivesUtils.isDirectiveDefined(commonSrc.readText(), "WITH_RUNTIME"))
arrayOf("-cp", findStdlibCommon().absolutePath)
else emptyArray()
val commonDest = File(tmpdir, "common").absolutePath
val jvmDest = File(tmpdir, "jvm").absolutePath
val jsDest = File(File(tmpdir, "js"), "output.js").absolutePath
val jvm2Dest = File(tmpdir, "jvm2").absolutePath
val jvmDest = File(tmpdir, "jvm").absolutePath.takeIf { jvmSrc != null }
val jsDest = File(File(tmpdir, "js"), "output.js").absolutePath.takeIf { jsSrc != null }
val common2Dest = File(tmpdir, "common2").absolutePath.takeIf { common2Src != null }
val jvm2Dest = File(tmpdir, "jvm2").absolutePath.takeIf { jvm2Src != null }
val result = buildString {
appendln("-- Common --")
appendln(K2MetadataCompiler().compile(listOf(commonSrc), "-d", commonDest, *optionalStdlibCommon))
if (jvmSrc.exists()) {
if (jvmSrc != null) {
appendln()
appendln("-- JVM --")
appendln(K2JVMCompiler().compileBothWays(commonSrc, jvmSrc, "-d", jvmDest))
appendln(K2JVMCompiler().compileBothWays(commonSrc, jvmSrc, "-d", jvmDest!!))
}
if (jsSrc.exists()) {
if (jsSrc != null) {
appendln()
appendln("-- JS --")
appendln(K2JSCompiler().compileBothWays(commonSrc, jsSrc, "-output", jsDest))
appendln(K2JSCompiler().compileBothWays(commonSrc, jsSrc, "-output", jsDest!!))
}
if (jvm2Src.exists()) {
if (common2Src != null) {
appendln()
appendln("-- Common (2) --")
appendln(K2MetadataCompiler().compile(listOf(common2Src), "-d", common2Dest!!, "-cp", commonDest, *optionalStdlibCommon))
}
if (jvm2Src != null) {
appendln()
appendln("-- JVM (2) --")
appendln(K2JVMCompiler().compile(listOf(jvm2Src), "-d", jvm2Dest, "-cp", listOf(commonDest, jvmDest).joinToString(File.pathSeparator)))
appendln(
K2JVMCompiler().compile(
listOf(jvm2Src), "-d", jvm2Dest!!,
"-cp", listOfNotNull(commonDest, common2Dest, jvmDest).joinToString(File.pathSeparator)
)
)
}
}
@@ -89,17 +102,17 @@ abstract class AbstractMultiPlatformIntegrationTest : KtUsefulTestCase() {
private fun CLICompiler<*>.compileBothWays(commonSource: File, platformSource: File, vararg mainArguments: String): String {
val configurations = listOf(
listOf(platformSource, commonSource),
listOf(commonSource, platformSource)
listOf(platformSource, commonSource),
listOf(commonSource, platformSource)
)
val (platformFirst, commonFirst) = configurations.map { compile(it, *mainArguments) }
if (platformFirst != commonFirst) {
assertEquals(
"Compilation results are different when compiling [platform-specific, common] compared to when compiling [common, platform-specific]",
"// Compiling [platform-specific, common]\n\n$platformFirst",
"// Compiling [common, platform-specific]\n\n$commonFirst"
"Compilation results are different when compiling [platform-specific, common] compared to when compiling [common, platform-specific]",
"// Compiling [platform-specific, common]\n\n$platformFirst",
"// Compiling [common, platform-specific]\n\n$commonFirst"
)
}
return platformFirst
@@ -107,8 +120,8 @@ abstract class AbstractMultiPlatformIntegrationTest : KtUsefulTestCase() {
private fun CLICompiler<*>.compile(sources: List<File>, vararg mainArguments: String): String = buildString {
val (output, exitCode) = AbstractCliTest.executeCompilerGrabOutput(
this@compile,
sources.map(File::getAbsolutePath) + listOf("-Xmulti-platform") + mainArguments + loadExtraArguments(sources)
this@compile,
sources.map(File::getAbsolutePath) + listOf("-Xmulti-platform") + mainArguments + loadExtraArguments(sources)
)
appendln("Exit code: $exitCode")
appendln("Output:")
compiler/tests/org/jetbrains/kotlin/multiplatform/MultiPlatformIntegrationTestGenerated.java
Generated
+18
View File
@@ -89,6 +89,11 @@ public class MultiPlatformIntegrationTestGenerated extends AbstractMultiPlatform
runTest("compiler/testData/multiplatform/inlineClasses/");
}
@TestMetadata("innerGenericClass")
public void testInnerGenericClass() throws Exception {
runTest("compiler/testData/multiplatform/innerGenericClass/");
}
@TestMetadata("jsNameClash")
public void testJsNameClash() throws Exception {
runTest("compiler/testData/multiplatform/jsNameClash/");
@@ -558,6 +563,19 @@ public class MultiPlatformIntegrationTestGenerated extends AbstractMultiPlatform
}
}
@TestMetadata("compiler/testData/multiplatform/innerGenericClass")
@TestDataPath("$PROJECT_ROOT")
@RunWith(JUnit3RunnerWithInners.class)
public static class InnerGenericClass extends AbstractMultiPlatformIntegrationTest {
private void runTest(String testDataFilePath) throws Exception {
KotlinTestUtils.runTest(this::doTest, TargetBackend.ANY, testDataFilePath);
}
public void testAllFilesPresentInInnerGenericClass() throws Exception {
KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/multiplatform/innerGenericClass"), Pattern.compile("^([^\\.]+)$"), TargetBackend.ANY, true);
}
}
@TestMetadata("compiler/testData/multiplatform/jsNameClash")
@TestDataPath("$PROJECT_ROOT")
@RunWith(JUnit3RunnerWithInners.class)
compiler/util/src/org/jetbrains/kotlin/utils/KotlinJavascriptMetadataUtils.kt
+1 -1
View File
@@ -32,7 +32,7 @@ class JsMetadataVersion(vararg numbers: Int) : BinaryVersion(*numbers) {
companion object {
@JvmField
val INSTANCE = JsMetadataVersion(1, 2, 1)
val INSTANCE = JsMetadataVersion(1, 2, 2)
@JvmField
val INVALID_VERSION = JsMetadataVersion()
core/metadata/src/org/jetbrains/kotlin/metadata/builtins/BuiltInsBinaryVersion.kt
+1 -1
View File
@@ -18,7 +18,7 @@ class BuiltInsBinaryVersion(vararg numbers: Int) : BinaryVersion(*numbers) {
companion object {
@JvmField
val INSTANCE = BuiltInsBinaryVersion(1, 0, 2)
val INSTANCE = BuiltInsBinaryVersion(1, 0, 3)
@JvmField
val INVALID_VERSION = BuiltInsBinaryVersion()
js/js.serializer/src/org/jetbrains/kotlin/serialization/js/KotlinJavascriptSerializationUtil.kt
+6 -6
View File
@@ -18,7 +18,6 @@ package org.jetbrains.kotlin.serialization.js
import org.jetbrains.kotlin.config.AnalysisFlag
import org.jetbrains.kotlin.config.LanguageVersionSettings
import org.jetbrains.kotlin.config.isPreRelease
import org.jetbrains.kotlin.descriptors.*
import org.jetbrains.kotlin.incremental.components.LookupTracker
import org.jetbrains.kotlin.incremental.components.NoLookupLocation
@@ -197,15 +196,16 @@ object KotlinJavascriptSerializationUtil {
}
val fileRegistry = KotlinFileRegistry()
val serializerExtension = KotlinJavascriptSerializerExtension(fileRegistry)
val serializer = DescriptorSerializer.createTopLevel(serializerExtension)
val extension = KotlinJavascriptSerializerExtension(fileRegistry)
val classDescriptors = scope.filterIsInstance<ClassDescriptor>().sortedBy { it.fqNameSafe.asString() }
fun serializeClasses(descriptors: Collection<DeclarationDescriptor>) {
fun serializeClass(classDescriptor: ClassDescriptor) {
if (skip(classDescriptor)) return
val classProto = serializer.classProto(classDescriptor).build() ?: error("Class not serialized: $classDescriptor")
val classProto =
DescriptorSerializer.create(classDescriptor, extension).classProto(classDescriptor).build()
?: error("Class not serialized: $classDescriptor")
builder.addClass_(classProto)
serializeClasses(classDescriptor.unsubstitutedInnerClassesScope.getContributedDescriptors())
}
@@ -219,10 +219,10 @@ object KotlinJavascriptSerializationUtil {
serializeClasses(classDescriptors)
val stringTable = serializerExtension.stringTable
val stringTable = extension.stringTable
val members = scope.filterNot(skip)
builder.`package` = serializer.packagePartProto(fqName, members).build()
builder.`package` = DescriptorSerializer.createTopLevel(extension).packagePartProto(fqName, members).build()
builder.setExtension(
JsProtoBuf.packageFragmentFiles,