diff --git a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/JvmLower.kt b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/JvmLower.kt index 503b34be87b..eb8f396f8e6 100644 --- a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/JvmLower.kt +++ b/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 diff --git a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/lower/MoveCompanionObjectFieldsLowering.kt b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/lower/MoveCompanionObjectFieldsLowering.kt index 2dd538055b0..d9dbd288e9d 100644 --- a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/lower/MoveCompanionObjectFieldsLowering.kt +++ b/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 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 new file mode 100644 index 00000000000..7f87b9ad6cd --- /dev/null +++ b/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() + 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, MutableList>() + + 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) : IrElementTransformerVoid() { + val newSymbols = mutableMapOf() + + 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) : 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 + ) + } +} diff --git a/compiler/testData/codegen/box/fieldRename/delegates.kt b/compiler/testData/codegen/box/fieldRename/delegates.kt index 8655ab1feaa..7d54b865533 100644 --- a/compiler/testData/codegen/box/fieldRename/delegates.kt +++ b/compiler/testData/codegen/box/fieldRename/delegates.kt @@ -16,14 +16,21 @@ public open class TestDelegate(private val initializer: () -> T) { } } -class A {} -class B {} +class A +class B +class C +class D -public val A.s: String by TestDelegate( {"OK2"}) -public val B.s: String by TestDelegate( {"OK"}) +public val A.s: String by TestDelegate({"A"}) +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" } diff --git a/compiler/testData/codegen/box/fieldRename/jvmFieldNoClash1.kt b/compiler/testData/codegen/box/fieldRename/jvmFieldNoClash1.kt new file mode 100644 index 00000000000..3a3fd51a02a --- /dev/null +++ b/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" +} diff --git a/compiler/testData/codegen/box/fieldRename/jvmFieldNoClash2.kt b/compiler/testData/codegen/box/fieldRename/jvmFieldNoClash2.kt new file mode 100644 index 00000000000..261fd2a5def --- /dev/null +++ b/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" +} diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java index e82e446b0f6..c9c5d2b8953 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java +++ b/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") diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java index d295b2cfa4a..e56cdfd5697 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java +++ b/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") diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java index 9a1d8c3a563..9853232f9c4 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java +++ b/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")