diff --git a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/deserialization/AbstractAnnotationDeserializer.kt b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/deserialization/AbstractAnnotationDeserializer.kt index c7c6af35dbf..39c1f677354 100644 --- a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/deserialization/AbstractAnnotationDeserializer.kt +++ b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/deserialization/AbstractAnnotationDeserializer.kt @@ -142,29 +142,7 @@ abstract class AbstractAnnotationDeserializer( ) } // ARRAY -> { -// val expectedIsArray = KotlinBuiltIns.isArray(expectedType) || KotlinBuiltIns.isPrimitiveArray(expectedType) -// val arrayElements = value.arrayElementList -// -// val actualArrayType = -// if (arrayElements.isNotEmpty()) { -// val actualElementType = resolveArrayElementType(arrayElements.first(), nameResolver) -// builtIns.getPrimitiveArrayKotlinTypeByPrimitiveKotlinType(actualElementType) -// ?: builtIns.getArrayType(Variance.INVARIANT, actualElementType) -// } else { -// // In the case of empty array, no element has the element type, so we fall back to the expected type, if any. -// // This is not very accurate when annotation class has been changed without recompiling clients, -// // but should not in fact matter because the value is empty anyway -// if (expectedIsArray) expectedType else builtIns.getArrayType(Variance.INVARIANT, builtIns.anyType) -// } -// -// val expectedElementType = builtIns.getArrayElementType(if (expectedIsArray) expectedType else actualArrayType) -// -// ConstantValueFactory.createArrayValue( -// arrayElements.map { -// resolveValue(expectedElementType, it, nameResolver) -// }, -// actualArrayType -// ) +// TODO: see AnnotationDeserializer // } // else -> error("Unsupported annotation argument type: ${value.type} (expected $expectedType)") else -> return null @@ -173,4 +151,4 @@ abstract class AbstractAnnotationDeserializer( } private fun const(kind: IrConstKind, value: T) = FirConstExpressionImpl(session, null, kind, value) -} \ No newline at end of file +} diff --git a/compiler/testData/codegen/box/reflection/annotations/onTypes/classLiteralWithExpectedType.kt b/compiler/testData/codegen/box/reflection/annotations/onTypes/classLiteralWithExpectedType.kt new file mode 100644 index 00000000000..aa457f531f9 --- /dev/null +++ b/compiler/testData/codegen/box/reflection/annotations/onTypes/classLiteralWithExpectedType.kt @@ -0,0 +1,24 @@ +// TARGET_BACKEND: JVM +// WITH_REFLECT + +import kotlin.reflect.KClass +import kotlin.test.assertEquals + +@Target(AnnotationTarget.TYPE) +annotation class Anno( + val k1: KClass, + val k2: KClass, + val ka: Array> +) + +fun f(): @Anno(String::class, CharSequence::class, [Double::class, Long::class, Int::class]) Unit {} + +fun box(): String { + assertEquals( + "[@Anno(k1=class java.lang.String, k2=interface java.lang.CharSequence, " + + "ka=[class java.lang.Double, class java.lang.Long, class java.lang.Integer])]", + ::f.returnType.annotations.toString() + ) + + return "OK" +} diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java index 9647bf73a32..9e4cac8cba0 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java @@ -20008,6 +20008,11 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest { KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/codegen/box/reflection/annotations/onTypes"), Pattern.compile("^(.+)\\.kt$"), TargetBackend.JVM, true); } + @TestMetadata("classLiteralWithExpectedType.kt") + public void testClassLiteralWithExpectedType() throws Exception { + runTest("compiler/testData/codegen/box/reflection/annotations/onTypes/classLiteralWithExpectedType.kt"); + } + @TestMetadata("differentArgumentTypes.kt") public void testDifferentArgumentTypes() throws Exception { runTest("compiler/testData/codegen/box/reflection/annotations/onTypes/differentArgumentTypes.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java index 432d2e00f4c..1c8ae04c9d1 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java @@ -20008,6 +20008,11 @@ public class LightAnalysisModeTestGenerated extends AbstractLightAnalysisModeTes KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/codegen/box/reflection/annotations/onTypes"), Pattern.compile("^(.+)\\.kt$"), TargetBackend.JVM, true); } + @TestMetadata("classLiteralWithExpectedType.kt") + public void testClassLiteralWithExpectedType() throws Exception { + runTest("compiler/testData/codegen/box/reflection/annotations/onTypes/classLiteralWithExpectedType.kt"); + } + @TestMetadata("differentArgumentTypes.kt") public void testDifferentArgumentTypes() throws Exception { runTest("compiler/testData/codegen/box/reflection/annotations/onTypes/differentArgumentTypes.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java index f72f1e66592..54451b8298b 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java @@ -20013,6 +20013,11 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/codegen/box/reflection/annotations/onTypes"), Pattern.compile("^(.+)\\.kt$"), TargetBackend.JVM_IR, true); } + @TestMetadata("classLiteralWithExpectedType.kt") + public void testClassLiteralWithExpectedType() throws Exception { + runTest("compiler/testData/codegen/box/reflection/annotations/onTypes/classLiteralWithExpectedType.kt"); + } + @TestMetadata("differentArgumentTypes.kt") public void testDifferentArgumentTypes() throws Exception { runTest("compiler/testData/codegen/box/reflection/annotations/onTypes/differentArgumentTypes.kt"); 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 69afa7ef77e..e72873f80ca 100644 --- a/core/deserialization/src/org/jetbrains/kotlin/serialization/deserialization/AnnotationDeserializer.kt +++ b/core/deserialization/src/org/jetbrains/kotlin/serialization/deserialization/AnnotationDeserializer.kt @@ -20,7 +20,6 @@ import org.jetbrains.kotlin.builtins.KotlinBuiltIns import org.jetbrains.kotlin.descriptors.* import org.jetbrains.kotlin.descriptors.annotations.AnnotationDescriptor import org.jetbrains.kotlin.descriptors.annotations.AnnotationDescriptorImpl -import org.jetbrains.kotlin.descriptors.annotations.Annotations import org.jetbrains.kotlin.metadata.ProtoBuf.Annotation import org.jetbrains.kotlin.metadata.ProtoBuf.Annotation.Argument import org.jetbrains.kotlin.metadata.ProtoBuf.Annotation.Argument.Value @@ -31,8 +30,9 @@ import org.jetbrains.kotlin.name.ClassId import org.jetbrains.kotlin.name.Name import org.jetbrains.kotlin.resolve.DescriptorUtils import org.jetbrains.kotlin.resolve.constants.* -import org.jetbrains.kotlin.types.* -import org.jetbrains.kotlin.types.typeUtil.isSubtypeOf +import org.jetbrains.kotlin.resolve.descriptorUtil.classId +import org.jetbrains.kotlin.types.ErrorUtils +import org.jetbrains.kotlin.types.KotlinType class AnnotationDeserializer(private val module: ModuleDescriptor, private val notFoundClasses: NotFoundClasses) { private val builtIns: KotlinBuiltIns @@ -59,13 +59,19 @@ class AnnotationDeserializer(private val module: ModuleDescriptor, private val n nameResolver: NameResolver ): Pair>? { val parameter = parameterByName[nameResolver.getName(proto.nameId)] ?: return null - return Pair(nameResolver.getName(proto.nameId), resolveValue(parameter.type, proto.value, nameResolver)) + return Pair(nameResolver.getName(proto.nameId), resolveValueAndCheckExpectedType(parameter.type, proto.value, nameResolver)) + } + + private fun resolveValueAndCheckExpectedType(expectedType: KotlinType, value: Value, nameResolver: NameResolver): ConstantValue<*> { + return resolveValue(expectedType, value, nameResolver).takeIf { + doesValueConformToExpectedType(it, expectedType, value) + } ?: ErrorValue.create("Unexpected argument value: actual type ${value.type} != expected type $expectedType") } fun resolveValue(expectedType: KotlinType, value: Value, nameResolver: NameResolver): ConstantValue<*> { val isUnsigned = Flags.IS_UNSIGNED.get(value.flags) - val result: ConstantValue<*> = when (value.type) { + return when (value.type) { Type.BYTE -> value.intValue.toByte().letIf(isUnsigned, ::UByteValue, ::ByteValue) Type.CHAR -> CharValue(value.intValue.toChar()) Type.SHORT -> value.intValue.toShort().letIf(isUnsigned, ::UShortValue, ::ShortValue) @@ -74,81 +80,47 @@ class AnnotationDeserializer(private val module: ModuleDescriptor, private val n Type.FLOAT -> FloatValue(value.floatValue) Type.DOUBLE -> DoubleValue(value.doubleValue) Type.BOOLEAN -> BooleanValue(value.intValue != 0L) - Type.STRING -> { - StringValue(nameResolver.getString(value.stringValue)) - } - 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 -> { - val expectedIsArray = KotlinBuiltIns.isArray(expectedType) || KotlinBuiltIns.isPrimitiveArray(expectedType) - val arrayElements = value.arrayElementList - - val actualArrayType = - if (arrayElements.isNotEmpty()) { - val actualElementType = resolveArrayElementType(arrayElements.first(), nameResolver) - builtIns.getPrimitiveArrayKotlinTypeByPrimitiveKotlinType(actualElementType) - ?: builtIns.getArrayType(Variance.INVARIANT, actualElementType) - } else { - // In the case of empty array, no element has the element type, so we fall back to the expected type, if any. - // This is not very accurate when annotation class has been changed without recompiling clients, - // but should not in fact matter because the value is empty anyway - if (expectedIsArray) expectedType else builtIns.getArrayType(Variance.INVARIANT, builtIns.anyType) - } - - val expectedElementType = builtIns.getArrayElementType(if (expectedIsArray) expectedType else actualArrayType) - - ConstantValueFactory.createArrayValue( - arrayElements.map { - resolveValue(expectedElementType, it, nameResolver) - }, - actualArrayType - ) - } + Type.STRING -> StringValue(nameResolver.getString(value.stringValue)) + 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 -> ConstantValueFactory.createArrayValue( + value.arrayElementList.map { resolveValue(builtIns.anyType, it, nameResolver) }, + expectedType + ) else -> error("Unsupported annotation argument type: ${value.type} (expected $expectedType)") } + } - return if (result.getType(module).isSubtypeOf(expectedType)) { - result - } else { - // This usually means that an annotation class has been changed incompatibly without recompiling clients - ErrorValue.create( - "Unexpected argument value: type ${result.getType(module)} is not a subtype of $expectedType (value.type = ${value.type})" - ) + // This method returns false if the actual value loaded from an annotation argument does not conform to the expected type of the + // corresponding parameter in the annotation class. This usually means that the annotation class has been changed incompatibly + // without recompiling clients, in which case we prefer not to load the annotation argument value at all, to avoid constructing + // an incorrect model and breaking some assumptions in the compiler. + private fun doesValueConformToExpectedType(result: ConstantValue<*>, expectedType: KotlinType, value: Value): Boolean { + return when (value.type) { + Type.CLASS -> { + val expectedClass = expectedType.constructor.declarationDescriptor as? ClassDescriptor + // We could also check that the class value's type is a subtype of the expected type, but loading the definition of the + // referenced class here is undesirable and may even be incorrect (because the module might be different at the + // destination where these constant values are read). This can lead to slightly incorrect model in some edge cases. + expectedClass == null || KotlinBuiltIns.isKClass(expectedClass) + } + Type.ARRAY -> { + check(result is ArrayValue && result.value.size == value.arrayElementList.size) { + "Deserialized ArrayValue should have the same number of elements as the original array value: $result" + } + val expectedElementType = builtIns.getArrayElementType(expectedType) + result.value.indices.all { i -> + doesValueConformToExpectedType(result.value[i], expectedElementType, value.getArrayElement(i)) + } + } + else -> result.getType(module) == expectedType } } private inline fun T.letIf(predicate: Boolean, f: (T) -> R, g: (T) -> R): R = if (predicate) f(this) else g(this) - private fun resolveArrayElementType(value: Value, nameResolver: NameResolver): SimpleType = - with(builtIns) { - when (value.type) { - Type.BYTE -> byteType - Type.CHAR -> charType - Type.SHORT -> shortType - Type.INT -> intType - Type.LONG -> longType - Type.FLOAT -> floatType - Type.DOUBLE -> doubleType - Type.BOOLEAN -> booleanType - Type.STRING -> stringType - Type.CLASS -> KotlinTypeFactory.simpleNotNullType( - Annotations.EMPTY, kClass, listOf(StarProjectionImpl(kClass.declaredTypeParameters.single())) - ) - Type.ENUM -> resolveClass(nameResolver.getClassId(value.classId)).defaultType - Type.ANNOTATION -> resolveClass(nameResolver.getClassId(value.annotation.id)).defaultType - Type.ARRAY -> error("Array of arrays is impossible") - else -> error("Unknown type: ${value.type}") - } - } - private fun resolveClass(classId: ClassId): ClassDescriptor { return module.findNonGenericClassAcrossDependencies(classId, notFoundClasses) }