From e22594acde040d13ec88dea1957d3be011ecd1e2 Mon Sep 17 00:00:00 2001 From: Alexander Udalov Date: Thu, 28 Mar 2019 19:05:35 +0100 Subject: [PATCH] JVM IR: slightly change field renaming convention implementation Prefer to rename fields from the class, not from the companion, to be more in line with the old backend's behavior. This has no effect on the behavior of current tests but removes differences in metadata (since metadata has information about every property->field mapping) in some of them --- .../backend/jvm/lower/RenameFieldsLowering.kt | 15 ++++++++++++--- .../codegen/box/fieldRename/jvmFieldNoClash2.kt | 9 +++++++++ .../classObject/rename/delegatedAndDelegated.kt | 1 - 3 files changed, 21 insertions(+), 4 deletions(-) diff --git a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/lower/RenameFieldsLowering.kt b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/lower/RenameFieldsLowering.kt index 3426966c7f6..58bef2cd792 100644 --- a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/lower/RenameFieldsLowering.kt +++ b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/lower/RenameFieldsLowering.kt @@ -48,9 +48,18 @@ private class RenameFieldsLowering(val context: CommonBackendContext) : FileLowe var count = 0 // We never rename JvmField properties, since they are public ABI. Therefore we consider the JvmField-annotated property first, - // in order to make sure it'll claim its original name - // TODO: also do not rename const properties - for (field in fields.sortedByDescending { it.isJvmField }) { + // in order to make sure it'll claim its original name. + // If there are non-static and static (moved from companion) fields with the same name, we try to make static properties retain + // their original names first, since this is what the old JVM backend did. However this can easily be changed without any + // major binary compatibility consequences (modulo access to private members via Java reflection). + for (field in fields.sortedBy { + when { + // TODO: also do not rename const properties + it.isJvmField -> 0 + it.isStatic -> 1 + else -> 2 + } + }) { val oldName = field.name val newName = if (count == 0) oldName else Name.identifier(oldName.asString() + "$$count") count++ diff --git a/compiler/testData/codegen/box/fieldRename/jvmFieldNoClash2.kt b/compiler/testData/codegen/box/fieldRename/jvmFieldNoClash2.kt index 261fd2a5def..153f5411ae9 100644 --- a/compiler/testData/codegen/box/fieldRename/jvmFieldNoClash2.kt +++ b/compiler/testData/codegen/box/fieldRename/jvmFieldNoClash2.kt @@ -1,5 +1,8 @@ // TARGET_BACKEND: JVM // WITH_RUNTIME +// FULL_JDK + +import java.lang.reflect.Modifier class A { val x = "outer" @@ -19,5 +22,11 @@ fun box(): String { if (A.x != "companion") return "Fail companion x" if (A.y != "companion") return "Fail companion y" + if (!Modifier.isStatic(A::class.java.getDeclaredField("x").modifiers)) + return "Fail: A.x should be static" + + if (Modifier.isStatic(A::class.java.getDeclaredField("x$1").modifiers)) + return "Fail: A.x$1 should not be static" + return "OK" } diff --git a/compiler/testData/writeFlags/property/classObject/rename/delegatedAndDelegated.kt b/compiler/testData/writeFlags/property/classObject/rename/delegatedAndDelegated.kt index fe9144a45dd..c4bd477fa31 100644 --- a/compiler/testData/writeFlags/property/classObject/rename/delegatedAndDelegated.kt +++ b/compiler/testData/writeFlags/property/classObject/rename/delegatedAndDelegated.kt @@ -1,4 +1,3 @@ -// IGNORE_BACKEND: JVM_IR import kotlin.reflect.KProperty public open class TestDelegate(private val initializer: () -> T) {