From 4dcf50d483a1a5fcf645ecd4be8235dd502d8e20 Mon Sep 17 00:00:00 2001 From: Alexander Udalov Date: Thu, 29 Sep 2022 00:57:29 +0200 Subject: [PATCH] Fix reflection for repeated annotations with array arguments This is related to 8308f5d7d3 (KT-44977). It turns out that it's necessary to create `ArrayValue` with a precomputed type not only in case of annotations stored directly in Kotlin metadata (i.e. annotations on types and type parameters), but also for arguments of repeated annotations. The reason is that repeated annotations are stored in an array themselves, as an argument to another (container) annotation. The annotation-reading code unwraps this array to load repeated annotations as individual instances of `AnnotationDescriptor`, but in contrast to loading normal annotations, it doesn't set `source` to the reflection object, it uses `NO_SOURCE` (see BinaryClassAnnotationAndConstantLoaderImpl.AbstractAnnotationArgumentVisitor.visitArray). So when kotlin-reflect is later asked for the arguments of the annotation, it needs to recreate the reflection object from `AnnotationDescriptor` in `util.kt`. Doing so requires knowing the array type, which is now present because we're creating a `TypedArrayValue` (previously `DeserializedArrayValue`) in places where arrays are read as annotation arguments. Alternative solution would be to fix source passed to `AnnotationDescriptorImpl` in the annotation-reading code, but that seems a bit messy because the code is very abstract and is used in lots of other places. #KT-53279 Fixed --- .../FirBlackBoxCodegenTestGenerated.java | 12 ++++ .../repeatable/kt53279_explicitContainer.kt | 62 +++++++++++++++++++ .../repeatable/kt53279_implicitContainer.kt | 50 +++++++++++++++ .../IrBlackBoxCodegenTestGenerated.java | 12 ++++ ...aryClassAnnotationAndConstantLoaderImpl.kt | 4 +- .../resolve/constants/ConstantValueFactory.kt | 35 ++++++----- .../resolve/constants/constantValues.kt | 4 +- .../deserialization/AnnotationDeserializer.kt | 2 +- .../deserialization/DeserializedArrayValue.kt | 12 ---- .../src/kotlin/reflect/jvm/internal/util.kt | 3 +- 10 files changed, 164 insertions(+), 32 deletions(-) create mode 100644 compiler/testData/codegen/box/reflection/annotations/repeatable/kt53279_explicitContainer.kt create mode 100644 compiler/testData/codegen/box/reflection/annotations/repeatable/kt53279_implicitContainer.kt delete mode 100644 core/deserialization/src/org/jetbrains/kotlin/serialization/deserialization/DeserializedArrayValue.kt diff --git a/compiler/fir/fir2ir/tests-gen/org/jetbrains/kotlin/test/runners/codegen/FirBlackBoxCodegenTestGenerated.java b/compiler/fir/fir2ir/tests-gen/org/jetbrains/kotlin/test/runners/codegen/FirBlackBoxCodegenTestGenerated.java index 3d11c82997b..d21690b5ffa 100644 --- a/compiler/fir/fir2ir/tests-gen/org/jetbrains/kotlin/test/runners/codegen/FirBlackBoxCodegenTestGenerated.java +++ b/compiler/fir/fir2ir/tests-gen/org/jetbrains/kotlin/test/runners/codegen/FirBlackBoxCodegenTestGenerated.java @@ -41743,6 +41743,18 @@ public class FirBlackBoxCodegenTestGenerated extends AbstractFirBlackBoxCodegenT runTest("compiler/testData/codegen/box/reflection/annotations/repeatable/kt49335.kt"); } + @Test + @TestMetadata("kt53279_explicitContainer.kt") + public void testKt53279_explicitContainer() throws Exception { + runTest("compiler/testData/codegen/box/reflection/annotations/repeatable/kt53279_explicitContainer.kt"); + } + + @Test + @TestMetadata("kt53279_implicitContainer.kt") + public void testKt53279_implicitContainer() throws Exception { + runTest("compiler/testData/codegen/box/reflection/annotations/repeatable/kt53279_implicitContainer.kt"); + } + @Test @TestMetadata("nonRepeatedAnnotationWithItsContainer.kt") public void testNonRepeatedAnnotationWithItsContainer() throws Exception { diff --git a/compiler/testData/codegen/box/reflection/annotations/repeatable/kt53279_explicitContainer.kt b/compiler/testData/codegen/box/reflection/annotations/repeatable/kt53279_explicitContainer.kt new file mode 100644 index 00000000000..66719b4c98e --- /dev/null +++ b/compiler/testData/codegen/box/reflection/annotations/repeatable/kt53279_explicitContainer.kt @@ -0,0 +1,62 @@ +// TARGET_BACKEND: JVM_IR +// JVM_TARGET: 1.8 +// FULL_JDK +// WITH_REFLECT + +// Android doesn't have @Repeatable before API level 24, so findAnnotations can't unpack repeatable annotations. +// IGNORE_BACKEND: ANDROID + +import kotlin.test.assertEquals +import kotlin.reflect.full.findAnnotations + +@JvmRepeatable(ArrayOfInt.ContainerOfInt::class) +annotation class ArrayOfInt(val ints: IntArray = []) { + annotation class ContainerOfInt(val value: Array) +} + +@JvmRepeatable(ArrayOfString.ContainerOfString::class) +annotation class ArrayOfString(val strings: Array = []) { + annotation class ContainerOfString(val value: Array) +} + +@JvmRepeatable(ArrayOfEnum.ContainerOfEnum::class) +annotation class ArrayOfEnum(val enums: Array = []) { + annotation class ContainerOfEnum(val value: Array) +} + +@JvmRepeatable(ArrayOfAnnotation.ContainerOfAnnotation::class) +annotation class ArrayOfAnnotation(val annotations: Array = []) { + annotation class ContainerOfAnnotation(val value: Array) +} + +class C { + @ArrayOfInt([1]) + @ArrayOfInt([2]) + val arrayOfInt = "" + + @ArrayOfString(["a"]) + @ArrayOfString(["b"]) + val arrayOfString = "" + + @ArrayOfEnum([DeprecationLevel.WARNING]) + @ArrayOfEnum([DeprecationLevel.ERROR]) + val arrayOfEnum = "" + + @ArrayOfAnnotation([ArrayOfString(arrayOf("a"))]) + @ArrayOfAnnotation([ArrayOfString(arrayOf("b"))]) + val arrayOfAnnotation = "" +} + +fun box(): String { + assertEquals("[1, 2]", C::arrayOfInt.findAnnotations().map { it.ints.single() }.toString()) + assertEquals("[a, b]", C::arrayOfString.findAnnotations().map { it.strings.single() }.toString()) + assertEquals("[WARNING, ERROR]", C::arrayOfEnum.findAnnotations().map { it.enums.single() }.toString()) + assertEquals( + "[a, b]", + C::arrayOfAnnotation.findAnnotations().map { + it.annotations.single().strings.single() + }.toString() + ) + + return "OK" +} diff --git a/compiler/testData/codegen/box/reflection/annotations/repeatable/kt53279_implicitContainer.kt b/compiler/testData/codegen/box/reflection/annotations/repeatable/kt53279_implicitContainer.kt new file mode 100644 index 00000000000..2ff97488e8c --- /dev/null +++ b/compiler/testData/codegen/box/reflection/annotations/repeatable/kt53279_implicitContainer.kt @@ -0,0 +1,50 @@ +// TARGET_BACKEND: JVM_IR +// JVM_TARGET: 1.8 +// FULL_JDK +// WITH_REFLECT + +import kotlin.test.assertEquals + +@Repeatable +annotation class ArrayOfInt(val ints: IntArray = []) + +@Repeatable +annotation class ArrayOfString(val strings: Array = []) + +@Repeatable +annotation class ArrayOfEnum(val enums: Array = []) + +@Repeatable +annotation class ArrayOfAnnotation(val annotations: Array = []) + +class C { + @ArrayOfInt([1]) + @ArrayOfInt([2]) + val arrayOfInt = "" + + @ArrayOfString(["a"]) + @ArrayOfString(["b"]) + val arrayOfString = "" + + @ArrayOfEnum([DeprecationLevel.WARNING]) + @ArrayOfEnum([DeprecationLevel.ERROR]) + val arrayOfEnum = "" + + @ArrayOfAnnotation([ArrayOfString(arrayOf("a"))]) + @ArrayOfAnnotation([ArrayOfString(arrayOf("b"))]) + val arrayOfAnnotation = "" +} + +fun box(): String { + assertEquals("[1, 2]", C::arrayOfInt.annotations.filterIsInstance().map { it.ints.single() }.toString()) + assertEquals("[a, b]", C::arrayOfString.annotations.filterIsInstance().map { it.strings.single() }.toString()) + assertEquals("[WARNING, ERROR]", C::arrayOfEnum.annotations.filterIsInstance().map { it.enums.single() }.toString()) + assertEquals( + "[a, b]", + C::arrayOfAnnotation.annotations.filterIsInstance().map { + it.annotations.single().strings.single() + }.toString() + ) + + return "OK" +} diff --git a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrBlackBoxCodegenTestGenerated.java b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrBlackBoxCodegenTestGenerated.java index c76b458b986..354a4323361 100644 --- a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrBlackBoxCodegenTestGenerated.java +++ b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrBlackBoxCodegenTestGenerated.java @@ -41743,6 +41743,18 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes runTest("compiler/testData/codegen/box/reflection/annotations/repeatable/kt49335.kt"); } + @Test + @TestMetadata("kt53279_explicitContainer.kt") + public void testKt53279_explicitContainer() throws Exception { + runTest("compiler/testData/codegen/box/reflection/annotations/repeatable/kt53279_explicitContainer.kt"); + } + + @Test + @TestMetadata("kt53279_implicitContainer.kt") + public void testKt53279_implicitContainer() throws Exception { + runTest("compiler/testData/codegen/box/reflection/annotations/repeatable/kt53279_implicitContainer.kt"); + } + @Test @TestMetadata("nonRepeatedAnnotationWithItsContainer.kt") public void testNonRepeatedAnnotationWithItsContainer() throws Exception { diff --git a/core/descriptors.jvm/src/org/jetbrains/kotlin/load/kotlin/BinaryClassAnnotationAndConstantLoaderImpl.kt b/core/descriptors.jvm/src/org/jetbrains/kotlin/load/kotlin/BinaryClassAnnotationAndConstantLoaderImpl.kt index ddb60d4a250..02e13c390cd 100644 --- a/core/descriptors.jvm/src/org/jetbrains/kotlin/load/kotlin/BinaryClassAnnotationAndConstantLoaderImpl.kt +++ b/core/descriptors.jvm/src/org/jetbrains/kotlin/load/kotlin/BinaryClassAnnotationAndConstantLoaderImpl.kt @@ -50,7 +50,7 @@ class BinaryClassAnnotationAndConstantLoaderImpl( initializer } - return ConstantValueFactory.createConstantValue(normalizedValue) + return ConstantValueFactory.createConstantValue(normalizedValue, module) } override fun transformToUnsignedConstant(constant: ConstantValue<*>): ConstantValue<*>? { @@ -216,7 +216,7 @@ class BinaryClassAnnotationAndConstantLoaderImpl( } private fun createConstant(name: Name?, value: Any?): ConstantValue<*> { - return ConstantValueFactory.createConstantValue(value) + return ConstantValueFactory.createConstantValue(value, module) ?: ErrorValue.create("Unsupported annotation argument: $name") } diff --git a/core/descriptors/src/org/jetbrains/kotlin/resolve/constants/ConstantValueFactory.kt b/core/descriptors/src/org/jetbrains/kotlin/resolve/constants/ConstantValueFactory.kt index 73046b40c28..636986f2da0 100644 --- a/core/descriptors/src/org/jetbrains/kotlin/resolve/constants/ConstantValueFactory.kt +++ b/core/descriptors/src/org/jetbrains/kotlin/resolve/constants/ConstantValueFactory.kt @@ -18,13 +18,15 @@ package org.jetbrains.kotlin.resolve.constants import org.jetbrains.kotlin.builtins.KotlinBuiltIns import org.jetbrains.kotlin.builtins.PrimitiveType +import org.jetbrains.kotlin.descriptors.ModuleDescriptor import org.jetbrains.kotlin.types.KotlinType import org.jetbrains.kotlin.types.TypeUtils object ConstantValueFactory { - fun createArrayValue(value: List>, type: KotlinType) = ArrayValue(value) { type } + fun createArrayValue(value: List>, type: KotlinType): ArrayValue = + TypedArrayValue(value, type) - fun createConstantValue(value: Any?): ConstantValue<*>? { + fun createConstantValue(value: Any?, module: ModuleDescriptor? = null): ConstantValue<*>? { return when (value) { is Byte -> ByteValue(value) is Short -> ShortValue(value) @@ -35,14 +37,14 @@ object ConstantValueFactory { is Double -> DoubleValue(value) is Boolean -> BooleanValue(value) is String -> StringValue(value) - is ByteArray -> createArrayValue(value.toList(), PrimitiveType.BYTE) - is ShortArray -> createArrayValue(value.toList(), PrimitiveType.SHORT) - is IntArray -> createArrayValue(value.toList(), PrimitiveType.INT) - is LongArray -> createArrayValue(value.toList(), PrimitiveType.LONG) - is CharArray -> createArrayValue(value.toList(), PrimitiveType.CHAR) - is FloatArray -> createArrayValue(value.toList(), PrimitiveType.FLOAT) - is DoubleArray -> createArrayValue(value.toList(), PrimitiveType.DOUBLE) - is BooleanArray -> createArrayValue(value.toList(), PrimitiveType.BOOLEAN) + is ByteArray -> createArrayValue(value.toList(), module, PrimitiveType.BYTE) + is ShortArray -> createArrayValue(value.toList(), module, PrimitiveType.SHORT) + is IntArray -> createArrayValue(value.toList(), module, PrimitiveType.INT) + is LongArray -> createArrayValue(value.toList(), module, PrimitiveType.LONG) + is CharArray -> createArrayValue(value.toList(), module, PrimitiveType.CHAR) + is FloatArray -> createArrayValue(value.toList(), module, PrimitiveType.FLOAT) + is DoubleArray -> createArrayValue(value.toList(), module, PrimitiveType.DOUBLE) + is BooleanArray -> createArrayValue(value.toList(), module, PrimitiveType.BOOLEAN) null -> NullValue() else -> null } @@ -58,10 +60,15 @@ object ConstantValueFactory { } } - private fun createArrayValue(value: List<*>, componentType: PrimitiveType): ArrayValue = - ArrayValue(value.toList().mapNotNull(this::createConstantValue)) { module -> - module.builtIns.getPrimitiveArrayKotlinType(componentType) - } + private fun createArrayValue(value: List<*>, module: ModuleDescriptor?, componentType: PrimitiveType): ArrayValue { + val elements = value.toList().mapNotNull(this::createConstantValue) + return if (module != null) + TypedArrayValue(elements, module.builtIns.getPrimitiveArrayKotlinType(componentType)) + else + ArrayValue(elements) { + it.builtIns.getPrimitiveArrayKotlinType(componentType) + } + } fun createIntegerConstantValue( value: Long, diff --git a/core/descriptors/src/org/jetbrains/kotlin/resolve/constants/constantValues.kt b/core/descriptors/src/org/jetbrains/kotlin/resolve/constants/constantValues.kt index 94ca8b2878e..61b1246f29a 100644 --- a/core/descriptors/src/org/jetbrains/kotlin/resolve/constants/constantValues.kt +++ b/core/descriptors/src/org/jetbrains/kotlin/resolve/constants/constantValues.kt @@ -29,8 +29,8 @@ import org.jetbrains.kotlin.name.Name import org.jetbrains.kotlin.resolve.DescriptorUtils import org.jetbrains.kotlin.resolve.descriptorUtil.classId import org.jetbrains.kotlin.types.* -import org.jetbrains.kotlin.types.error.ErrorUtils import org.jetbrains.kotlin.types.error.ErrorTypeKind +import org.jetbrains.kotlin.types.error.ErrorUtils import org.jetbrains.kotlin.types.typeUtil.replaceArgumentsWithStarProjections abstract class ConstantValue(open val value: T) { @@ -69,6 +69,8 @@ open class ArrayValue( override fun accept(visitor: AnnotationArgumentVisitor, data: D) = visitor.visitArrayValue(this, data) } +class TypedArrayValue(value: List>, val type: KotlinType) : ArrayValue(value, { type }) + class BooleanValue(value: Boolean) : ConstantValue(value) { override fun getType(module: ModuleDescriptor) = module.builtIns.booleanType override fun accept(visitor: AnnotationArgumentVisitor, data: D) = visitor.visitBooleanValue(this, data) diff --git a/core/deserialization/src/org/jetbrains/kotlin/serialization/deserialization/AnnotationDeserializer.kt b/core/deserialization/src/org/jetbrains/kotlin/serialization/deserialization/AnnotationDeserializer.kt index 6d40e601010..cc022e51197 100644 --- a/core/deserialization/src/org/jetbrains/kotlin/serialization/deserialization/AnnotationDeserializer.kt +++ b/core/deserialization/src/org/jetbrains/kotlin/serialization/deserialization/AnnotationDeserializer.kt @@ -83,7 +83,7 @@ class AnnotationDeserializer(private val module: ModuleDescriptor, private val n Type.CLASS -> KClassValue(nameResolver.getClassId(value.classId), value.arrayDimensionCount) Type.ENUM -> EnumValue(nameResolver.getClassId(value.classId), nameResolver.getName(value.enumValueId)) Type.ANNOTATION -> AnnotationValue(deserializeAnnotation(value.annotation, nameResolver)) - Type.ARRAY -> DeserializedArrayValue( + Type.ARRAY -> ConstantValueFactory.createArrayValue( value.arrayElementList.map { resolveValue(builtIns.anyType, it, nameResolver) }, expectedType ) diff --git a/core/deserialization/src/org/jetbrains/kotlin/serialization/deserialization/DeserializedArrayValue.kt b/core/deserialization/src/org/jetbrains/kotlin/serialization/deserialization/DeserializedArrayValue.kt deleted file mode 100644 index ceb2227948a..00000000000 --- a/core/deserialization/src/org/jetbrains/kotlin/serialization/deserialization/DeserializedArrayValue.kt +++ /dev/null @@ -1,12 +0,0 @@ -/* - * Copyright 2010-2021 JetBrains s.r.o. and Kotlin Programming Language contributors. - * 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.serialization.deserialization - -import org.jetbrains.kotlin.resolve.constants.ArrayValue -import org.jetbrains.kotlin.resolve.constants.ConstantValue -import org.jetbrains.kotlin.types.KotlinType - -class DeserializedArrayValue(value: List>, val type: KotlinType) : ArrayValue(value, { type }) diff --git a/core/reflection.jvm/src/kotlin/reflect/jvm/internal/util.kt b/core/reflection.jvm/src/kotlin/reflect/jvm/internal/util.kt index 9f27364dd39..d7e37d16842 100644 --- a/core/reflection.jvm/src/kotlin/reflect/jvm/internal/util.kt +++ b/core/reflection.jvm/src/kotlin/reflect/jvm/internal/util.kt @@ -45,7 +45,6 @@ import org.jetbrains.kotlin.resolve.descriptorUtil.annotationClass import org.jetbrains.kotlin.resolve.descriptorUtil.classId import org.jetbrains.kotlin.resolve.isInlineClassType import org.jetbrains.kotlin.serialization.deserialization.DeserializationContext -import org.jetbrains.kotlin.serialization.deserialization.DeserializedArrayValue import org.jetbrains.kotlin.serialization.deserialization.MemberDeserializer import java.lang.reflect.Type import kotlin.jvm.internal.FunctionReference @@ -188,7 +187,7 @@ private fun ConstantValue<*>.toRuntimeValue(classLoader: ClassLoader): Any? = wh } private fun ArrayValue.arrayToRuntimeValue(classLoader: ClassLoader): Any? { - val type = (this as? DeserializedArrayValue)?.type ?: return null + val type = (this as? TypedArrayValue)?.type ?: return null val values = value.map { it.toRuntimeValue(classLoader) } return when (KotlinBuiltIns.getPrimitiveArrayElementType(type)) {