JVM IR: rename private fields for properties with same name

This is needed to get rid of the code that appends "$companion" to
properties moved from companion, because it caused inconsistencies in
the ABI and in JVM signatures stored in the metadata

compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/JvmLower.kt

@@ -49,6 +49,7 @@ internal val jvmPhases = namedIrFilePhase(
             constPhase then
             propertiesToFieldsPhase then
             propertiesPhase then
+            renameFieldsPhase then
             annotationPhase then
 
             jvmDefaultArgumentStubPhase then

compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/lower/MoveCompanionObjectFieldsLowering.kt

@@ -31,7 +31,6 @@ import org.jetbrains.kotlin.ir.util.isObject
 import org.jetbrains.kotlin.ir.visitors.IrElementTransformerVoid
 import org.jetbrains.kotlin.ir.visitors.transformChildrenVoid
 import org.jetbrains.kotlin.load.java.JvmAbi.JVM_FIELD_ANNOTATION_FQ_NAME
-import org.jetbrains.kotlin.name.Name
 
 internal val moveCompanionObjectFieldsPhase = makeIrFilePhase(
     ::MoveCompanionObjectFieldsLowering,
@@ -169,19 +168,12 @@ private class MoveCompanionObjectFieldsLowering(val context: CommonBackendContex
     }
 
     private fun createStaticBackingField(oldField: IrField, propertyParent: IrClass, fieldParent: IrClass): IrField {
-        val newName = if (fieldParent == propertyParent ||
-            oldField.hasAnnotation(JVM_FIELD_ANNOTATION_FQ_NAME) ||
-            oldField.correspondingProperty?.isConst == true
-        )
-            oldField.name
-        else
-            Name.identifier(oldField.name.toString() + "\$companion")
         val descriptor = WrappedFieldDescriptor(oldField.descriptor.annotations, oldField.descriptor.source)
         val field = IrFieldImpl(
             oldField.startOffset, oldField.endOffset,
             IrDeclarationOrigin.PROPERTY_BACKING_FIELD,
             IrFieldSymbolImpl(descriptor),
-            newName, oldField.type, oldField.visibility,
+            oldField.name, oldField.type, oldField.visibility,
             isFinal = oldField.isFinal,
             isExternal = oldField.isExternal,
             isStatic = true

compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/lower/RenameFieldsLowering.kt

@@ -0,0 +1,129 @@
+/*
+ * Copyright 2010-2019 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license
+ * that can be found in the license/LICENSE.txt file.
+ */
+
+package org.jetbrains.kotlin.backend.jvm.lower
+
+import org.jetbrains.kotlin.backend.common.CommonBackendContext
+import org.jetbrains.kotlin.backend.common.FileLoweringPass
+import org.jetbrains.kotlin.backend.common.descriptors.WrappedFieldDescriptor
+import org.jetbrains.kotlin.backend.common.phaser.makeIrFilePhase
+import org.jetbrains.kotlin.ir.IrElement
+import org.jetbrains.kotlin.ir.IrStatement
+import org.jetbrains.kotlin.ir.declarations.IrDeclarationParent
+import org.jetbrains.kotlin.ir.declarations.IrField
+import org.jetbrains.kotlin.ir.declarations.IrFile
+import org.jetbrains.kotlin.ir.declarations.impl.IrFieldImpl
+import org.jetbrains.kotlin.ir.expressions.IrExpression
+import org.jetbrains.kotlin.ir.expressions.IrGetField
+import org.jetbrains.kotlin.ir.expressions.IrSetField
+import org.jetbrains.kotlin.ir.expressions.impl.IrGetFieldImpl
+import org.jetbrains.kotlin.ir.expressions.impl.IrSetFieldImpl
+import org.jetbrains.kotlin.ir.symbols.IrFieldSymbol
+import org.jetbrains.kotlin.ir.symbols.impl.IrFieldSymbolImpl
+import org.jetbrains.kotlin.ir.util.hasAnnotation
+import org.jetbrains.kotlin.ir.visitors.IrElementTransformerVoid
+import org.jetbrains.kotlin.ir.visitors.IrElementVisitorVoid
+import org.jetbrains.kotlin.ir.visitors.acceptChildrenVoid
+import org.jetbrains.kotlin.ir.visitors.acceptVoid
+import org.jetbrains.kotlin.load.java.JvmAbi
+import org.jetbrains.kotlin.name.Name
+
+internal val renameFieldsPhase = makeIrFilePhase(
+    ::RenameFieldsLowering,
+    name = "RenameFields",
+    description = "Rename private fields (including fields copied from companion object) to avoid JVM declaration clash"
+)
+
+private class RenameFieldsLowering(val context: CommonBackendContext) : FileLoweringPass {
+    override fun lower(irFile: IrFile) {
+        val collector = FieldNameCollector()
+        irFile.acceptVoid(collector)
+
+        val newNames = mutableMapOf<IrField, Name>()
+        for ((_, fields) in collector.nameToField) {
+            if (fields.size < 2) continue
+
+            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 }) {
+                val oldName = field.name
+                val newName = if (count == 0) oldName else Name.identifier(oldName.asString() + "$$count")
+                count++
+
+                // TODO: check visibility instead of annotation
+                if (field.isJvmField) continue
+
+                newNames[field] = newName
+            }
+        }
+
+        val renamer = FieldRenamer(newNames)
+        irFile.transform(renamer, null)
+
+        irFile.transform(FieldAccessTransformer(renamer.newSymbols), null)
+    }
+
+    private val IrField.isJvmField: Boolean
+        get() = hasAnnotation(JvmAbi.JVM_FIELD_ANNOTATION_FQ_NAME)
+}
+
+private class FieldNameCollector : IrElementVisitorVoid {
+    val nameToField = mutableMapOf<Pair<IrDeclarationParent, Name>, MutableList<IrField>>()
+
+    override fun visitElement(element: IrElement) {
+        element.acceptChildrenVoid(this)
+    }
+
+    override fun visitField(declaration: IrField) {
+        nameToField.getOrPut(declaration.parent to declaration.name) { mutableListOf() }.add(declaration)
+    }
+}
+
+private class FieldRenamer(private val newNames: Map<IrField, Name>) : IrElementTransformerVoid() {
+    val newSymbols = mutableMapOf<IrField, IrFieldSymbol>()
+
+    override fun visitField(declaration: IrField): IrStatement {
+        val newName = newNames[declaration] ?: return super.visitField(declaration)
+
+        val descriptor = WrappedFieldDescriptor()
+        val symbol = IrFieldSymbolImpl(descriptor)
+        return IrFieldImpl(
+            declaration.startOffset, declaration.endOffset, declaration.origin, symbol, newName,
+            declaration.type, declaration.visibility, declaration.isFinal, declaration.isExternal, declaration.isStatic
+        ).also {
+            descriptor.bind(it)
+            it.parent = declaration.parent
+            it.initializer = declaration.initializer?.transform(this, null)
+
+            newSymbols[declaration] = symbol
+        }
+    }
+}
+
+private class FieldAccessTransformer(private val oldToNew: Map<IrField, IrFieldSymbol>) : IrElementTransformerVoid() {
+    override fun visitGetField(expression: IrGetField): IrExpression {
+        val newSymbol = oldToNew[expression.symbol.owner] ?: return super.visitGetField(expression)
+
+        return IrGetFieldImpl(
+            expression.startOffset, expression.endOffset, newSymbol, expression.type,
+            expression.receiver?.transform(this, null),
+            expression.origin, expression.superQualifierSymbol
+        )
+    }
+
+    override fun visitSetField(expression: IrSetField): IrExpression {
+        val newSymbol = oldToNew[expression.symbol.owner] ?: return super.visitSetField(expression)
+
+        return IrSetFieldImpl(
+            expression.startOffset, expression.endOffset, newSymbol,
+            expression.receiver?.transform(this, null),
+            expression.value.transform(this, null),
+            expression.type,
+            expression.origin, expression.superQualifierSymbol
+        )
+    }
+}

compiler/testData/codegen/box/fieldRename/delegates.kt

@@ -16,14 +16,21 @@ public open class TestDelegate<T: Any>(private val initializer: () -> T) {
     }
 }
 
-class A {}
+class A
-class B {}
+class B
+class C
+class D
 
-public val A.s: String by TestDelegate( {"OK2"})
+public val A.s: String by TestDelegate({"A"})
-public val B.s: String by TestDelegate( {"OK"})
+public val B.s: String by TestDelegate({"B"})
+public val C.s: String by TestDelegate({"C"})
+public val D.s: String by TestDelegate({"D"})
 
 fun box() : String {
-  if (A().s != "OK2") return "fail1"
+    if (A().s != "A") return "Fail A"
+    if (B().s != "B") return "Fail B"
+    if (C().s != "C") return "Fail C"
+    if (D().s != "D") return "Fail D"
 
-  return B().s
+    return "OK"
 }

compiler/testData/codegen/box/fieldRename/jvmFieldNoClash1.kt

@@ -0,0 +1,23 @@
+// TARGET_BACKEND: JVM
+// WITH_RUNTIME
+
+// In the old JVM backend, FieldOwnerContext is sensitive to the order of properties which it invents name for. Companion object properties
+// are usually the first, so A.Companion.x here gets the name "x". After that it tries to invent a new name for A.x but fails because
+// @JvmField-annotated properties cannot be renamed, which leads to a JVM declaration clash error.
+// IGNORE_BACKEND: JVM
+
+class A {
+    @JvmField
+    val x = "outer"
+
+    companion object {
+        val x = "companion"
+    }
+}
+
+fun box(): String {
+    if (A().x != "outer") return "Fail outer"
+    if (A.x != "companion") return "Fail companion"
+
+    return "OK"
+}

compiler/testData/codegen/box/fieldRename/jvmFieldNoClash2.kt

@@ -0,0 +1,23 @@
+// TARGET_BACKEND: JVM
+// WITH_RUNTIME
+
+class A {
+    val x = "outer"
+    val y = "outer"
+
+    companion object {
+        @JvmField
+        val x = "companion"
+
+        const val y = "companion"
+    }
+}
+
+fun box(): String {
+    if (A().x != "outer") return "Fail outer x"
+    if (A().y != "outer") return "Fail outer y"
+    if (A.x != "companion") return "Fail companion x"
+    if (A.y != "companion") return "Fail companion y"
+
+    return "OK"
+}

compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java

@@ -10630,6 +10630,16 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest {
         public void testGenericPropertyWithItself() throws Exception {
             runTest("compiler/testData/codegen/box/fieldRename/genericPropertyWithItself.kt");
         }
+
+        @TestMetadata("jvmFieldNoClash1.kt")
+        public void testJvmFieldNoClash1() throws Exception {
+            runTest("compiler/testData/codegen/box/fieldRename/jvmFieldNoClash1.kt");
+        }
+
+        @TestMetadata("jvmFieldNoClash2.kt")
+        public void testJvmFieldNoClash2() throws Exception {
+            runTest("compiler/testData/codegen/box/fieldRename/jvmFieldNoClash2.kt");
+        }
     }
 
     @TestMetadata("compiler/testData/codegen/box/finally")

compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java

@@ -10630,6 +10630,16 @@ public class LightAnalysisModeTestGenerated extends AbstractLightAnalysisModeTes
         public void testGenericPropertyWithItself() throws Exception {
             runTest("compiler/testData/codegen/box/fieldRename/genericPropertyWithItself.kt");
         }
+
+        @TestMetadata("jvmFieldNoClash1.kt")
+        public void testJvmFieldNoClash1() throws Exception {
+            runTest("compiler/testData/codegen/box/fieldRename/jvmFieldNoClash1.kt");
+        }
+
+        @TestMetadata("jvmFieldNoClash2.kt")
+        public void testJvmFieldNoClash2() throws Exception {
+            runTest("compiler/testData/codegen/box/fieldRename/jvmFieldNoClash2.kt");
+        }
     }
 
     @TestMetadata("compiler/testData/codegen/box/finally")

compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java

@@ -10630,6 +10630,16 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes
         public void testGenericPropertyWithItself() throws Exception {
             runTest("compiler/testData/codegen/box/fieldRename/genericPropertyWithItself.kt");
         }
+
+        @TestMetadata("jvmFieldNoClash1.kt")
+        public void testJvmFieldNoClash1() throws Exception {
+            runTest("compiler/testData/codegen/box/fieldRename/jvmFieldNoClash1.kt");
+        }
+
+        @TestMetadata("jvmFieldNoClash2.kt")
+        public void testJvmFieldNoClash2() throws Exception {
+            runTest("compiler/testData/codegen/box/fieldRename/jvmFieldNoClash2.kt");
+        }
     }
 
     @TestMetadata("compiler/testData/codegen/box/finally")