From cdb77039471952b55b24f1ec29f1baaf0f8bd94b Mon Sep 17 00:00:00 2001 From: Alexander Udalov Date: Thu, 14 Nov 2019 17:04:38 +0100 Subject: [PATCH] JVM IR: fix VerifyError on annotated annotation properties The problem was that we tried to generate an `$annotations` method for a property declared in an annotation class. That method is final and has a body, which is not allowed in annotation classes. Now we're generating this method in the separate `$DefaultImpls` class as for properties in interfaces. Note that the added test still doesn't find any annotations because the proper support is needed in reflection (KT-22463). Currently it only checks that no VerifyError happens. --- .../jvm/descriptors/JvmDeclarationFactory.kt | 3 +- .../backend/jvm/lower/InterfaceLowering.kt | 69 ++++++++++++------- .../box/annotations/annotationProperty.kt | 13 ++++ .../codegen/BlackBoxCodegenTestGenerated.java | 5 ++ .../LightAnalysisModeTestGenerated.java | 5 ++ .../ir/IrBlackBoxCodegenTestGenerated.java | 5 ++ 6 files changed, 74 insertions(+), 26 deletions(-) create mode 100644 compiler/testData/codegen/box/annotations/annotationProperty.kt diff --git a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/descriptors/JvmDeclarationFactory.kt b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/descriptors/JvmDeclarationFactory.kt index d44bf433124..e7dca35b82f 100644 --- a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/descriptors/JvmDeclarationFactory.kt +++ b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/descriptors/JvmDeclarationFactory.kt @@ -9,6 +9,7 @@ import org.jetbrains.kotlin.backend.common.DescriptorsToIrRemapper import org.jetbrains.kotlin.backend.common.ir.* import org.jetbrains.kotlin.backend.jvm.JvmLoweredDeclarationOrigin import org.jetbrains.kotlin.backend.jvm.codegen.MethodSignatureMapper +import org.jetbrains.kotlin.backend.jvm.codegen.isJvmInterface import org.jetbrains.kotlin.builtins.CompanionObjectMapping.isMappedIntrinsicCompanionObject import org.jetbrains.kotlin.descriptors.ClassKind import org.jetbrains.kotlin.descriptors.Modality @@ -135,7 +136,7 @@ class JvmDeclarationFactory( fun getDefaultImplsFunction(interfaceFun: IrSimpleFunction): IrSimpleFunction { val parent = interfaceFun.parentAsClass - assert(parent.isInterface) { "Parent of ${interfaceFun.dump()} should be interface" } + assert(parent.isJvmInterface) { "Parent of ${interfaceFun.dump()} should be interface" } return defaultImplsMethods.getOrPut(interfaceFun) { val defaultImpls = getDefaultImplsClass(interfaceFun.parentAsClass) diff --git a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/lower/InterfaceLowering.kt b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/lower/InterfaceLowering.kt index ca89155898b..312aacc70d0 100644 --- a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/lower/InterfaceLowering.kt +++ b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/lower/InterfaceLowering.kt @@ -12,7 +12,9 @@ import org.jetbrains.kotlin.backend.common.ir.passTypeArgumentsFrom import org.jetbrains.kotlin.backend.common.lower.createIrBuilder import org.jetbrains.kotlin.backend.jvm.JvmBackendContext import org.jetbrains.kotlin.backend.jvm.JvmLoweredDeclarationOrigin +import org.jetbrains.kotlin.backend.jvm.codegen.isJvmInterface import org.jetbrains.kotlin.backend.jvm.ir.hasJvmDefault +import org.jetbrains.kotlin.descriptors.ClassKind import org.jetbrains.kotlin.descriptors.Modality import org.jetbrains.kotlin.descriptors.Visibilities import org.jetbrains.kotlin.ir.UNDEFINED_OFFSET @@ -41,11 +43,28 @@ internal class InterfaceLowering(val context: JvmBackendContext) : IrElementTran private val removedFunctions = hashMapOf() override fun lower(irClass: IrClass) { - if (!irClass.isInterface) return + if (!irClass.isJvmInterface) return + + when (irClass.kind) { + ClassKind.INTERFACE -> handleInterface(irClass) + ClassKind.ANNOTATION_CLASS -> handleAnnotationClass(irClass) + else -> return + } + + irClass.declarations.removeAll { + it is IrFunction && removedFunctions.containsKey(it.symbol) + } val defaultImplsIrClass = context.declarationFactory.getDefaultImplsClass(irClass) - val members = defaultImplsIrClass.declarations + if (defaultImplsIrClass.declarations.isNotEmpty()) { + irClass.declarations.add(defaultImplsIrClass) + } + // Update IrElements (e.g., IrCalls) to point to the new functions. + irClass.transformChildrenVoid(this) + } + + private fun handleInterface(irClass: IrClass) { // There are 6 cases for functions on interfaces: loop@ for (function in irClass.functions) { when { @@ -78,7 +97,7 @@ internal class InterfaceLowering(val context: JvmBackendContext) : IrElementTran && !implementation.isMethodOfAny() && (!implementation.hasJvmDefault() || context.state.jvmDefaultMode.isCompatibility) ) { - delegateInheritedDefaultImplementationToDefaultImpls(function, implementation, defaultImplsIrClass) + delegateInheritedDefaultImplementationToDefaultImpls(function, implementation) } } @@ -89,8 +108,7 @@ internal class InterfaceLowering(val context: JvmBackendContext) : IrElementTran Visibilities.isPrivate(function.visibility) || (function.origin == IrDeclarationOrigin.FUNCTION_FOR_DEFAULT_PARAMETER && !function.hasJvmDefault()) || function.origin == JvmLoweredDeclarationOrigin.SYNTHETIC_METHOD_FOR_PROPERTY_ANNOTATIONS -> { - val defaultImpl = createDefaultImpl(function, members) - removedFunctions[function.symbol] = defaultImpl.symbol + removedFunctions[function.symbol] = createDefaultImpl(function).symbol } /** @@ -98,7 +116,7 @@ internal class InterfaceLowering(val context: JvmBackendContext) : IrElementTran * an abstract stub is left. */ !function.hasJvmDefault() -> { - createDefaultImpl(function, members) + createDefaultImpl(function) function.body = null //TODO reset modality to abstract } @@ -107,7 +125,7 @@ internal class InterfaceLowering(val context: JvmBackendContext) : IrElementTran * 5) _With_ @JvmDefault, we move and bridge if in compatibility mode, ... */ context.state.jvmDefaultMode.isCompatibility -> { - val defaultImpl = createDefaultImpl(function, members) + val defaultImpl = createDefaultImpl(function) function.body = IrExpressionBodyImpl(createDelegatingCall(defaultImpl, function)) } @@ -115,9 +133,7 @@ internal class InterfaceLowering(val context: JvmBackendContext) : IrElementTran } } - irClass.declarations.removeAll { - it is IrFunction && removedFunctions.containsKey(it.symbol) - } + val defaultImplsIrClass = context.declarationFactory.getDefaultImplsClass(irClass) // Move metadata for local delegated properties from the interface to DefaultImpls, since this is where kotlin-reflect looks for it. val localDelegatedProperties = context.localDelegatedProperties[irClass.attributeOwnerId as IrClass] @@ -135,17 +151,24 @@ internal class InterfaceLowering(val context: JvmBackendContext) : IrElementTran delegatedPropertyArray.parent = defaultImplsIrClass delegatedPropertyArray.initializer?.patchDeclarationParents(defaultImplsIrClass) } - - if (defaultImplsIrClass.declarations.isNotEmpty()) irClass.declarations.add(defaultImplsIrClass) - // Update IrElements (e.g., IrCalls) to point to the new functions. - irClass.transformChildrenVoid(this) } - private fun createDefaultImpl(function: IrSimpleFunction, members: MutableList): IrSimpleFunction = - context.declarationFactory.getDefaultImplsFunction(function).also { - it.body = function.body?.patchDeclarationParents(it) - copyBodyToStatic(function, it) - members.add(it) + private fun handleAnnotationClass(irClass: IrClass) { + // We produce $DefaultImpls for annotation classes only to move $annotations methods (for property annotations) there. + val annotationsMethods = + irClass.functions.filter { it.origin == JvmLoweredDeclarationOrigin.SYNTHETIC_METHOD_FOR_PROPERTY_ANNOTATIONS } + if (annotationsMethods.none()) return + + for (function in annotationsMethods) { + removedFunctions[function.symbol] = createDefaultImpl(function).symbol + } + } + + private fun createDefaultImpl(function: IrSimpleFunction): IrSimpleFunction = + context.declarationFactory.getDefaultImplsFunction(function).also { newFunction -> + newFunction.body = function.body?.patchDeclarationParents(newFunction) + copyBodyToStatic(function, newFunction) + newFunction.parentAsClass.declarations.add(newFunction) } private fun createDelegatingCall(defaultImpls: IrFunction, interfaceMethod: IrFunction): IrCall { @@ -168,15 +191,11 @@ internal class InterfaceLowering(val context: JvmBackendContext) : IrElementTran } } - private fun delegateInheritedDefaultImplementationToDefaultImpls( - fakeOverride: IrSimpleFunction, - implementation: IrSimpleFunction, - defaultImpls: IrClass - ) { + private fun delegateInheritedDefaultImplementationToDefaultImpls(fakeOverride: IrSimpleFunction, implementation: IrSimpleFunction) { val defaultImplFun = context.declarationFactory.getDefaultImplsFunction(implementation) val irFunction = context.declarationFactory.getDefaultImplsFunction(fakeOverride) - defaultImpls.declarations.add(irFunction) + irFunction.parentAsClass.declarations.add(irFunction) context.createIrBuilder(irFunction.symbol, UNDEFINED_OFFSET, UNDEFINED_OFFSET).apply { irFunction.body = irBlockBody { +irReturn(createDelegatingCall(defaultImplFun, irFunction)) diff --git a/compiler/testData/codegen/box/annotations/annotationProperty.kt b/compiler/testData/codegen/box/annotations/annotationProperty.kt new file mode 100644 index 00000000000..7490db2c7a8 --- /dev/null +++ b/compiler/testData/codegen/box/annotations/annotationProperty.kt @@ -0,0 +1,13 @@ +// TARGET_BACKEND: JVM +// WITH_RUNTIME + +@Target(AnnotationTarget.PROPERTY) +annotation class Anno(val value: String) + +annotation class M(@Anno("OK") val result: Int) + +fun box(): String = + M::class.java.getAnnotation(Anno::class.java)?.value + // TODO: fix KT-22463 and enable this test + // ?: "Fail: no annotation" + ?: "OK" diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java index 9a85d0330ee..c3aab349ab1 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java @@ -56,6 +56,11 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest { runTest("compiler/testData/codegen/box/annotations/annotatedObjectLiteral.kt"); } + @TestMetadata("annotationProperty.kt") + public void testAnnotationProperty() throws Exception { + runTest("compiler/testData/codegen/box/annotations/annotationProperty.kt"); + } + @TestMetadata("annotationWithKotlinProperty.kt") public void testAnnotationWithKotlinProperty() throws Exception { runTest("compiler/testData/codegen/box/annotations/annotationWithKotlinProperty.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java index 426dda2d402..27f89252ed0 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java @@ -61,6 +61,11 @@ public class LightAnalysisModeTestGenerated extends AbstractLightAnalysisModeTes runTest("compiler/testData/codegen/box/annotations/annotatedObjectLiteral.kt"); } + @TestMetadata("annotationProperty.kt") + public void testAnnotationProperty() throws Exception { + runTest("compiler/testData/codegen/box/annotations/annotationProperty.kt"); + } + @TestMetadata("annotationWithKotlinProperty.kt") public void testAnnotationWithKotlinProperty() throws Exception { runTest("compiler/testData/codegen/box/annotations/annotationWithKotlinProperty.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java index 23d3379c3c4..e6cf5de2f1f 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java @@ -56,6 +56,11 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes runTest("compiler/testData/codegen/box/annotations/annotatedObjectLiteral.kt"); } + @TestMetadata("annotationProperty.kt") + public void testAnnotationProperty() throws Exception { + runTest("compiler/testData/codegen/box/annotations/annotationProperty.kt"); + } + @TestMetadata("annotationWithKotlinProperty.kt") public void testAnnotationWithKotlinProperty() throws Exception { runTest("compiler/testData/codegen/box/annotations/annotationWithKotlinProperty.kt");