From 6fb83c2ba3a5fddacf8b1a39b083572531af3d44 Mon Sep 17 00:00:00 2001 From: Denis Zharkov Date: Thu, 2 Mar 2017 15:19:09 +0300 Subject: [PATCH] Force wrapping java classes from annotation methods into KClasses Before this change such wrapping happened only during coercion, i.e. when a call-site expected a KClass instance. But when call-site expects Any, for example, no wrapping happened, and raw j.l.Class instance was left on stack. The solution is to put wrapping code closer to generation of annotation's method call itself to guarantee that necessary wrapping will happen. #KT-9453 Fixed --- .../jetbrains/kotlin/codegen/StackValue.java | 30 +++++++------- .../kClassInAnnotation/forceWrapping.kt | 22 ++++++++++ .../wrappingForCallableReferences.kt | 41 +++++++++++++++++++ .../kClassInAnnotation/forceWrapping.txt | 13 ++++++ .../wrappingForCallableReferences.txt | 17 ++++++++ .../ir/IrBlackBoxCodegenTestGenerated.java | 12 ++++++ .../codegen/BlackBoxCodegenTestGenerated.java | 12 ++++++ .../semantics/JsCodegenBoxTestGenerated.java | 24 +++++++++++ 8 files changed, 157 insertions(+), 14 deletions(-) create mode 100644 compiler/testData/codegen/box/reflection/kClassInAnnotation/forceWrapping.kt create mode 100644 compiler/testData/codegen/box/reflection/kClassInAnnotation/wrappingForCallableReferences.kt create mode 100644 compiler/testData/codegen/light-analysis/reflection/kClassInAnnotation/forceWrapping.txt create mode 100644 compiler/testData/codegen/light-analysis/reflection/kClassInAnnotation/wrappingForCallableReferences.txt diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/StackValue.java b/compiler/backend/src/org/jetbrains/kotlin/codegen/StackValue.java index 3646148bade..a59f36fbd4e 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/StackValue.java +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/StackValue.java @@ -372,23 +372,12 @@ public abstract class StackValue { } } else if (toType.getSort() == Type.ARRAY) { - if (fromType.getSort() == Type.ARRAY && - fromType.getElementType().equals(AsmTypes.JAVA_CLASS_TYPE) && toType.equals(K_CLASS_ARRAY_TYPE)) { - wrapJavaClassesIntoKClasses(v); - } - else { - v.checkcast(toType); - } + v.checkcast(toType); } else if (toType.getSort() == Type.OBJECT) { if (fromType.getSort() == Type.OBJECT || fromType.getSort() == Type.ARRAY) { if (!toType.equals(OBJECT_TYPE)) { - if (fromType.equals(AsmTypes.JAVA_CLASS_TYPE) && toType.equals(AsmTypes.K_CLASS_TYPE)) { - wrapJavaClassIntoKClass(v); - } - else { - v.checkcast(toType); - } + v.checkcast(toType); } } else { @@ -1211,7 +1200,20 @@ public abstract class StackValue { else { getter.genInvokeInstruction(v); } - coerce(getter.getReturnType(), type, v); + + Type typeOfValueOnStack = getter.getReturnType(); + if (DescriptorUtils.isAnnotationClass(descriptor.getContainingDeclaration())) { + if (this.type.equals(K_CLASS_TYPE)) { + wrapJavaClassIntoKClass(v); + typeOfValueOnStack = K_CLASS_TYPE; + } + else if (this.type.equals(K_CLASS_ARRAY_TYPE)) { + wrapJavaClassesIntoKClasses(v); + typeOfValueOnStack = K_CLASS_ARRAY_TYPE; + } + } + + coerce(typeOfValueOnStack, type, v); KotlinType returnType = descriptor.getReturnType(); if (returnType != null && KotlinBuiltIns.isNothing(returnType)) { diff --git a/compiler/testData/codegen/box/reflection/kClassInAnnotation/forceWrapping.kt b/compiler/testData/codegen/box/reflection/kClassInAnnotation/forceWrapping.kt new file mode 100644 index 00000000000..256535980d1 --- /dev/null +++ b/compiler/testData/codegen/box/reflection/kClassInAnnotation/forceWrapping.kt @@ -0,0 +1,22 @@ +// IGNORE_BACKEND: JS +// WITH_REFLECT + +import kotlin.reflect.KClass +import kotlin.test.assertEquals + +annotation class Anno( + val klass: KClass<*>, + val kClasses: Array>, + vararg val kClassesVararg: KClass<*> +) + +@Anno(String::class, arrayOf(Int::class), Double::class) +fun foo() {} + +fun box(): String { + val k = ::foo.annotations.single() as Anno + assertEquals(String::class, k.klass) + assertEquals(Int::class, k.kClasses[0]) + assertEquals(Double::class, k.kClassesVararg[0]) + return "OK" +} diff --git a/compiler/testData/codegen/box/reflection/kClassInAnnotation/wrappingForCallableReferences.kt b/compiler/testData/codegen/box/reflection/kClassInAnnotation/wrappingForCallableReferences.kt new file mode 100644 index 00000000000..1c6a857d1c4 --- /dev/null +++ b/compiler/testData/codegen/box/reflection/kClassInAnnotation/wrappingForCallableReferences.kt @@ -0,0 +1,41 @@ +// IGNORE_BACKEND: JS +// WITH_REFLECT +import kotlin.reflect.KClass +import kotlin.test.assertEquals + +annotation class Anno( + val klass: KClass<*>, + val kClasses: Array>, + vararg val kClassesVararg: KClass<*> +) + +@Anno(String::class, arrayOf(Int::class), Double::class) +fun foo() {} + +fun Anno.checkReference(expected: Any?, x: Anno.() -> Any?) { + assertEquals(expected, x()) +} + +fun Anno.checkReferenceArray(expected: Any?, x: Anno.() -> Array) { + assertEquals(expected, x()[0]) +} + +fun checkBoundReference(expected: Any?, x: () -> Any?) { + assertEquals(expected, x()) +} + +fun checkBoundReferenceArray(expected: Any?, x: () -> Array) { + assertEquals(expected, x()[0]) +} + +fun box(): String { + val k = ::foo.annotations.single() as Anno + k.checkReference(String::class, Anno::klass) + k.checkReferenceArray(Int::class, Anno::kClasses) + k.checkReferenceArray(Double::class, Anno::kClassesVararg) + + checkBoundReference(String::class, k::klass) + checkBoundReferenceArray(Int::class, k::kClasses) + checkBoundReferenceArray(Double::class, k::kClassesVararg) + return "OK" +} diff --git a/compiler/testData/codegen/light-analysis/reflection/kClassInAnnotation/forceWrapping.txt b/compiler/testData/codegen/light-analysis/reflection/kClassInAnnotation/forceWrapping.txt new file mode 100644 index 00000000000..19f4584530b --- /dev/null +++ b/compiler/testData/codegen/light-analysis/reflection/kClassInAnnotation/forceWrapping.txt @@ -0,0 +1,13 @@ +@java.lang.annotation.Retention +@kotlin.Metadata +public annotation class Anno { + public abstract method kClasses(): java.lang.Class[] + public abstract method kClassesVararg(): java.lang.Class[] + public abstract method klass(): java.lang.Class +} + +@kotlin.Metadata +public final class ForceWrappingKt { + public final static @org.jetbrains.annotations.NotNull method box(): java.lang.String + public final static @Anno method foo(): void +} diff --git a/compiler/testData/codegen/light-analysis/reflection/kClassInAnnotation/wrappingForCallableReferences.txt b/compiler/testData/codegen/light-analysis/reflection/kClassInAnnotation/wrappingForCallableReferences.txt new file mode 100644 index 00000000000..cb20b67c616 --- /dev/null +++ b/compiler/testData/codegen/light-analysis/reflection/kClassInAnnotation/wrappingForCallableReferences.txt @@ -0,0 +1,17 @@ +@java.lang.annotation.Retention +@kotlin.Metadata +public annotation class Anno { + public abstract method kClasses(): java.lang.Class[] + public abstract method kClassesVararg(): java.lang.Class[] + public abstract method klass(): java.lang.Class +} + +@kotlin.Metadata +public final class WrappingForCallableReferencesKt { + public final static @org.jetbrains.annotations.NotNull method box(): java.lang.String + public final static method checkBoundReference(@org.jetbrains.annotations.Nullable p0: java.lang.Object, @org.jetbrains.annotations.NotNull p1: kotlin.jvm.functions.Function0): void + public final static method checkBoundReferenceArray(@org.jetbrains.annotations.Nullable p0: java.lang.Object, @org.jetbrains.annotations.NotNull p1: kotlin.jvm.functions.Function0): void + public final static method checkReference(@org.jetbrains.annotations.NotNull p0: Anno, @org.jetbrains.annotations.Nullable p1: java.lang.Object, @org.jetbrains.annotations.NotNull p2: kotlin.jvm.functions.Function1): void + public final static method checkReferenceArray(@org.jetbrains.annotations.NotNull p0: Anno, @org.jetbrains.annotations.Nullable p1: java.lang.Object, @org.jetbrains.annotations.NotNull p2: kotlin.jvm.functions.Function1): void + public final static @Anno method foo(): void +} diff --git a/compiler/tests-ir-jvm/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java b/compiler/tests-ir-jvm/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java index edb9a8c37ec..1449f1ee4cf 100644 --- a/compiler/tests-ir-jvm/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java +++ b/compiler/tests-ir-jvm/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java @@ -14209,6 +14209,12 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes doTest(fileName); } + @TestMetadata("forceWrapping.kt") + public void testForceWrapping() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/reflection/kClassInAnnotation/forceWrapping.kt"); + doTest(fileName); + } + @TestMetadata("vararg.kt") public void testVararg() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/reflection/kClassInAnnotation/vararg.kt"); @@ -14220,6 +14226,12 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/reflection/kClassInAnnotation/varargInJava.kt"); doTest(fileName); } + + @TestMetadata("wrappingForCallableReferences.kt") + public void testWrappingForCallableReferences() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/reflection/kClassInAnnotation/wrappingForCallableReferences.kt"); + doTest(fileName); + } } @TestMetadata("compiler/testData/codegen/box/reflection/lambdaClasses") diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java index dabaffefe63..6fdb7c5788f 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java @@ -14209,6 +14209,12 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest { doTest(fileName); } + @TestMetadata("forceWrapping.kt") + public void testForceWrapping() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/reflection/kClassInAnnotation/forceWrapping.kt"); + doTest(fileName); + } + @TestMetadata("vararg.kt") public void testVararg() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/reflection/kClassInAnnotation/vararg.kt"); @@ -14220,6 +14226,12 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/reflection/kClassInAnnotation/varargInJava.kt"); doTest(fileName); } + + @TestMetadata("wrappingForCallableReferences.kt") + public void testWrappingForCallableReferences() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/reflection/kClassInAnnotation/wrappingForCallableReferences.kt"); + doTest(fileName); + } } @TestMetadata("compiler/testData/codegen/box/reflection/lambdaClasses") diff --git a/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/JsCodegenBoxTestGenerated.java b/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/JsCodegenBoxTestGenerated.java index a3b95b376ed..42979b3ab6e 100644 --- a/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/JsCodegenBoxTestGenerated.java +++ b/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/JsCodegenBoxTestGenerated.java @@ -17276,6 +17276,18 @@ public class JsCodegenBoxTestGenerated extends AbstractJsCodegenBoxTest { throw new AssertionError("Looks like this test can be unmuted. Remove IGNORE_BACKEND directive for that."); } + @TestMetadata("forceWrapping.kt") + public void testForceWrapping() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/reflection/kClassInAnnotation/forceWrapping.kt"); + try { + doTest(fileName); + } + catch (Throwable ignore) { + return; + } + throw new AssertionError("Looks like this test can be unmuted. Remove IGNORE_BACKEND directive for that."); + } + @TestMetadata("vararg.kt") public void testVararg() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/reflection/kClassInAnnotation/vararg.kt"); @@ -17299,6 +17311,18 @@ public class JsCodegenBoxTestGenerated extends AbstractJsCodegenBoxTest { } throw new AssertionError("Looks like this test can be unmuted. Remove IGNORE_BACKEND directive for that."); } + + @TestMetadata("wrappingForCallableReferences.kt") + public void testWrappingForCallableReferences() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/reflection/kClassInAnnotation/wrappingForCallableReferences.kt"); + try { + doTest(fileName); + } + catch (Throwable ignore) { + return; + } + throw new AssertionError("Looks like this test can be unmuted. Remove IGNORE_BACKEND directive for that."); + } } @TestMetadata("compiler/testData/codegen/box/reflection/lambdaClasses")