From c58314fddfa75107b16f19fd7d384d7e6d1c9057 Mon Sep 17 00:00:00 2001 From: Alexander Udalov Date: Mon, 16 Jan 2023 22:41:47 +0100 Subject: [PATCH] Reflection: improve and optimize kotlinFunction/kotlinProperty - Make the implementations very similar, to fix KT-54833 where the companion object case was forgotten for kotlinProperty. - Optimize both functions to look up the function/property by name first, to cover the most probable case when the JVM name of a declaration is equal to its Kotlin name. This fixes KT-55937. #KT-54833 Fixed #KT-55937 Fixed --- .../FirBlackBoxCodegenTestGenerated.java | 18 ++++++ .../mapping/companionObjectProperty.kt | 16 +++++ .../mapping/nonTrivialFunctionNames.kt | 39 ++++++++++++ .../mapping/nonTrivialPropertyNames.kt | 40 +++++++++++++ .../codegen/BlackBoxCodegenTestGenerated.java | 18 ++++++ .../IrBlackBoxCodegenTestGenerated.java | 18 ++++++ .../LightAnalysisModeTestGenerated.java | 15 +++++ .../kotlin/reflect/jvm/ReflectJvmMapping.kt | 59 ++++++++++++++----- .../src/kotlin/reflect/jvm/internal/util.kt | 16 +++++ 9 files changed, 225 insertions(+), 14 deletions(-) create mode 100644 compiler/testData/codegen/box/reflection/mapping/companionObjectProperty.kt create mode 100644 compiler/testData/codegen/box/reflection/mapping/nonTrivialFunctionNames.kt create mode 100644 compiler/testData/codegen/box/reflection/mapping/nonTrivialPropertyNames.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 0fa2dbe8a35..13d7e9bd8c9 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 @@ -43806,6 +43806,12 @@ public class FirBlackBoxCodegenTestGenerated extends AbstractFirBlackBoxCodegenT KtTestUtil.assertAllTestsPresentByMetadataWithExcluded(this.getClass(), new File("compiler/testData/codegen/box/reflection/mapping"), Pattern.compile("^(.+)\\.kt$"), null, TargetBackend.JVM_IR, true); } + @Test + @TestMetadata("companionObjectProperty.kt") + public void testCompanionObjectProperty() throws Exception { + runTest("compiler/testData/codegen/box/reflection/mapping/companionObjectProperty.kt"); + } + @Test @TestMetadata("constructor.kt") public void testConstructor() throws Exception { @@ -43896,6 +43902,18 @@ public class FirBlackBoxCodegenTestGenerated extends AbstractFirBlackBoxCodegenT runTest("compiler/testData/codegen/box/reflection/mapping/methodsFromSuperInterface.kt"); } + @Test + @TestMetadata("nonTrivialFunctionNames.kt") + public void testNonTrivialFunctionNames() throws Exception { + runTest("compiler/testData/codegen/box/reflection/mapping/nonTrivialFunctionNames.kt"); + } + + @Test + @TestMetadata("nonTrivialPropertyNames.kt") + public void testNonTrivialPropertyNames() throws Exception { + runTest("compiler/testData/codegen/box/reflection/mapping/nonTrivialPropertyNames.kt"); + } + @Test @TestMetadata("openSuspendFun.kt") public void testOpenSuspendFun() throws Exception { diff --git a/compiler/testData/codegen/box/reflection/mapping/companionObjectProperty.kt b/compiler/testData/codegen/box/reflection/mapping/companionObjectProperty.kt new file mode 100644 index 00000000000..5453500fd4a --- /dev/null +++ b/compiler/testData/codegen/box/reflection/mapping/companionObjectProperty.kt @@ -0,0 +1,16 @@ +// WITH_REFLECT +// TARGET_BACKEND: JVM + +import kotlin.reflect.KProperty1 +import kotlin.reflect.jvm.kotlinProperty + +class C { + companion object { + val x = "OK" + } +} + +fun box(): String { + val f = C::class.java.getDeclaredField("x").kotlinProperty as KProperty1 + return f.get(C.Companion) +} diff --git a/compiler/testData/codegen/box/reflection/mapping/nonTrivialFunctionNames.kt b/compiler/testData/codegen/box/reflection/mapping/nonTrivialFunctionNames.kt new file mode 100644 index 00000000000..4532834983a --- /dev/null +++ b/compiler/testData/codegen/box/reflection/mapping/nonTrivialFunctionNames.kt @@ -0,0 +1,39 @@ +// WITH_REFLECT +// TARGET_BACKEND: JVM + +import kotlin.reflect.KFunction +import kotlin.reflect.jvm.javaMethod +import kotlin.reflect.jvm.kotlinFunction +import kotlin.test.assertEquals + +class A { + @JvmName("jvmFoo") + fun foo(s: String): Int = s.length + + fun mangled(z: Z): Number = z.value + + internal fun internal(s: String): Int = s.length + + + // Some different members with similar JVM signatures, to check that kotlinFunction doesn't incorrectly resolve into one of these. + private fun jvmFoo(s: String): String = s + private fun `mangled-IQRRRT4`(z: Number) {} + private fun `internal$main`(s: String): String = s +} + +@JvmInline +value class Z(val value: Number) + +fun test(f: KFunction<*>) { + val javaMethod = f.javaMethod + ?: error("javaMethod == null for $f") + assertEquals(f, javaMethod.kotlinFunction, "Incorrect kotlinFunction for $javaMethod") +} + +fun box(): String { + test(A::foo) + test(A::mangled) + test(A::internal) + + return "OK" +} diff --git a/compiler/testData/codegen/box/reflection/mapping/nonTrivialPropertyNames.kt b/compiler/testData/codegen/box/reflection/mapping/nonTrivialPropertyNames.kt new file mode 100644 index 00000000000..14dd4f114b9 --- /dev/null +++ b/compiler/testData/codegen/box/reflection/mapping/nonTrivialPropertyNames.kt @@ -0,0 +1,40 @@ +// WITH_REFLECT +// TARGET_BACKEND: JVM + +import kotlin.reflect.KProperty +import kotlin.reflect.full.declaredMemberProperties +import kotlin.reflect.jvm.javaField +import kotlin.reflect.jvm.kotlinProperty +import kotlin.test.assertEquals + +class A { + val x = "outer" // NB: backing field of this property has the name `x$1`, to avoid conflict with public field moved from companion. + val y = "outer" // Same here, `y$1`. + + companion object { + @JvmField + val x = "companion" + + const val y = "companion" + } +} + +fun test(f: KProperty<*>) { + val javaField = f.javaField + ?: error("javaField == null for $f") + + assertEquals(f, javaField.kotlinProperty, "Incorrect kotlinProperty for $javaField") +} + +fun box(): String { + test(A::x) + test(A::y) + + // We have to use reflection API to get companion object properties if we want to test the invariant `p.javaField.kotlinProperty == p`. + // If we used the callable reference syntax instead `A.Companion::x`, we'd get a bound property reference, which is an instance of + // `KProperty0`. Whereas `kotlinProperty` always return unbound reference, so a `KProperty1`. + test(A.Companion::class.declaredMemberProperties.single { it.name == "x" }) + test(A.Companion::class.declaredMemberProperties.single { it.name == "y" }) + + return "OK" +} diff --git a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/BlackBoxCodegenTestGenerated.java b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/BlackBoxCodegenTestGenerated.java index d1729ff3970..f6c68be7056 100644 --- a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/BlackBoxCodegenTestGenerated.java +++ b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/BlackBoxCodegenTestGenerated.java @@ -42012,6 +42012,12 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest { KtTestUtil.assertAllTestsPresentByMetadataWithExcluded(this.getClass(), new File("compiler/testData/codegen/box/reflection/mapping"), Pattern.compile("^(.+)\\.kt$"), null, TargetBackend.JVM, true); } + @Test + @TestMetadata("companionObjectProperty.kt") + public void testCompanionObjectProperty() throws Exception { + runTest("compiler/testData/codegen/box/reflection/mapping/companionObjectProperty.kt"); + } + @Test @TestMetadata("constructor.kt") public void testConstructor() throws Exception { @@ -42102,6 +42108,18 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest { runTest("compiler/testData/codegen/box/reflection/mapping/methodsFromSuperInterface.kt"); } + @Test + @TestMetadata("nonTrivialFunctionNames.kt") + public void testNonTrivialFunctionNames() throws Exception { + runTest("compiler/testData/codegen/box/reflection/mapping/nonTrivialFunctionNames.kt"); + } + + @Test + @TestMetadata("nonTrivialPropertyNames.kt") + public void testNonTrivialPropertyNames() throws Exception { + runTest("compiler/testData/codegen/box/reflection/mapping/nonTrivialPropertyNames.kt"); + } + @Test @TestMetadata("openSuspendFun.kt") public void testOpenSuspendFun() throws Exception { 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 0e4cb8d50af..b2f27506228 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 @@ -43806,6 +43806,12 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes KtTestUtil.assertAllTestsPresentByMetadataWithExcluded(this.getClass(), new File("compiler/testData/codegen/box/reflection/mapping"), Pattern.compile("^(.+)\\.kt$"), null, TargetBackend.JVM_IR, true); } + @Test + @TestMetadata("companionObjectProperty.kt") + public void testCompanionObjectProperty() throws Exception { + runTest("compiler/testData/codegen/box/reflection/mapping/companionObjectProperty.kt"); + } + @Test @TestMetadata("constructor.kt") public void testConstructor() throws Exception { @@ -43896,6 +43902,18 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes runTest("compiler/testData/codegen/box/reflection/mapping/methodsFromSuperInterface.kt"); } + @Test + @TestMetadata("nonTrivialFunctionNames.kt") + public void testNonTrivialFunctionNames() throws Exception { + runTest("compiler/testData/codegen/box/reflection/mapping/nonTrivialFunctionNames.kt"); + } + + @Test + @TestMetadata("nonTrivialPropertyNames.kt") + public void testNonTrivialPropertyNames() throws Exception { + runTest("compiler/testData/codegen/box/reflection/mapping/nonTrivialPropertyNames.kt"); + } + @Test @TestMetadata("openSuspendFun.kt") public void testOpenSuspendFun() throws Exception { diff --git a/compiler/tests-gen/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java b/compiler/tests-gen/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java index aff4a8dafd4..ca8781eac92 100644 --- a/compiler/tests-gen/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java +++ b/compiler/tests-gen/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java @@ -33661,6 +33661,11 @@ public class LightAnalysisModeTestGenerated extends AbstractLightAnalysisModeTes KtTestUtil.assertAllTestsPresentByMetadataWithExcluded(this.getClass(), new File("compiler/testData/codegen/box/reflection/mapping"), Pattern.compile("^(.+)\\.kt$"), null, TargetBackend.JVM, true); } + @TestMetadata("companionObjectProperty.kt") + public void testCompanionObjectProperty() throws Exception { + runTest("compiler/testData/codegen/box/reflection/mapping/companionObjectProperty.kt"); + } + @TestMetadata("constructor.kt") public void testConstructor() throws Exception { runTest("compiler/testData/codegen/box/reflection/mapping/constructor.kt"); @@ -33736,6 +33741,16 @@ public class LightAnalysisModeTestGenerated extends AbstractLightAnalysisModeTes runTest("compiler/testData/codegen/box/reflection/mapping/methodsFromSuperInterface.kt"); } + @TestMetadata("nonTrivialFunctionNames.kt") + public void testNonTrivialFunctionNames() throws Exception { + runTest("compiler/testData/codegen/box/reflection/mapping/nonTrivialFunctionNames.kt"); + } + + @TestMetadata("nonTrivialPropertyNames.kt") + public void testNonTrivialPropertyNames() throws Exception { + runTest("compiler/testData/codegen/box/reflection/mapping/nonTrivialPropertyNames.kt"); + } + @TestMetadata("openSuspendFun.kt") public void testOpenSuspendFun() throws Exception { runTest("compiler/testData/codegen/box/reflection/mapping/openSuspendFun.kt"); diff --git a/core/reflection.jvm/src/kotlin/reflect/jvm/ReflectJvmMapping.kt b/core/reflection.jvm/src/kotlin/reflect/jvm/ReflectJvmMapping.kt index 3beeabd1c51..9c3da04f3b3 100644 --- a/core/reflection.jvm/src/kotlin/reflect/jvm/ReflectJvmMapping.kt +++ b/core/reflection.jvm/src/kotlin/reflect/jvm/ReflectJvmMapping.kt @@ -25,6 +25,7 @@ import kotlin.reflect.* import kotlin.reflect.full.companionObject import kotlin.reflect.full.functions import kotlin.reflect.full.memberProperties +import kotlin.reflect.jvm.internal.* import kotlin.reflect.jvm.internal.KPackageImpl import kotlin.reflect.jvm.internal.KTypeImpl import kotlin.reflect.jvm.internal.asKCallableImpl @@ -92,14 +93,22 @@ val Field.kotlinProperty: KProperty<*>? get() { if (isSynthetic) return null - // TODO: optimize (search by name) + if (Modifier.isStatic(modifiers)) { + val kotlinPackage = getKPackage() + if (kotlinPackage != null) { + return kotlinPackage.members.findKProperty(this) + } - val kotlinPackage = getKPackage() - if (kotlinPackage != null) { - return kotlinPackage.members.filterIsInstance>().firstOrNull { it.javaField == this } + val companionKClass = declaringClass.kotlin.companionObject + if (companionKClass != null) { + val companionField = declaringClass.getDeclaredFieldOrNull(name) + if (companionField != null) { + companionKClass.memberProperties.findKProperty(companionField)?.let { return it } + } + } } - return declaringClass.kotlin.memberProperties.firstOrNull { it.javaField == this } + return declaringClass.kotlin.memberProperties.findKProperty(this) } @@ -119,23 +128,45 @@ val Method.kotlinFunction: KFunction<*>? if (Modifier.isStatic(modifiers)) { val kotlinPackage = getKPackage() if (kotlinPackage != null) { - return kotlinPackage.members.filterIsInstance>().firstOrNull { it.javaMethod == this } + return kotlinPackage.members.findKFunction(this) } // For static bridge method generated for a @JvmStatic function in the companion object, also try to find the latter - val companion = declaringClass.kotlin.companionObject - if (companion != null) { - companion.functions.firstOrNull { - val m = it.javaMethod - m != null && m.name == this.name && - m.parameterTypes.contentEquals(this.parameterTypes) && m.returnType == this.returnType - }?.let { return it } + val companionKClass = declaringClass.kotlin.companionObject + if (companionKClass != null) { + val companionMethod = companionKClass.java.getDeclaredMethodOrNull(name, *parameterTypes) + if (companionMethod != null) { + companionKClass.functions.findKFunction(companionMethod)?.let { return it } + } } } - return declaringClass.kotlin.functions.firstOrNull { it.javaMethod == this } + return declaringClass.kotlin.functions.findKFunction(this) } +private fun Collection>.findKFunction(method: Method): KFunction<*>? { + // As an optimization, try to search among functions with the same name first, and then among the rest of functions. + // This is needed because a function's JVM name might be different from its Kotlin name (because of `@JvmName`, inline class mangling, + // internal visibility, etc). + for (callable in this) { + if (callable is KFunction<*> && callable.name == method.name && callable.javaMethod == method) return callable + } + for (callable in this) { + if (callable is KFunction<*> && callable.name != method.name && callable.javaMethod == method) return callable + } + return null +} + +private fun Collection>.findKProperty(field: Field): KProperty<*>? { + for (callable in this) { + if (callable is KProperty<*> && callable.name == field.name && callable.javaField == field) return callable + } + for (callable in this) { + if (callable is KProperty<*> && callable.name != field.name && callable.javaField == field) return callable + } + return null +} + /** * Returns a [KFunction] instance corresponding to the given Java [Constructor] instance, * or `null` if this constructor cannot be represented by a Kotlin function 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 d7e37d16842..e09ed34ad60 100644 --- a/core/reflection.jvm/src/kotlin/reflect/jvm/internal/util.kt +++ b/core/reflection.jvm/src/kotlin/reflect/jvm/internal/util.kt @@ -46,6 +46,8 @@ 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.MemberDeserializer +import java.lang.reflect.Field +import java.lang.reflect.Method import java.lang.reflect.Type import kotlin.jvm.internal.FunctionReference import kotlin.jvm.internal.PropertyReference @@ -308,3 +310,17 @@ internal open class CreateKCallableVisitor(private val container: KDeclarationCo override fun visitFunctionDescriptor(descriptor: FunctionDescriptor, data: Unit): KCallableImpl<*> = KFunctionImpl(container, descriptor) } + +internal fun Class<*>.getDeclaredMethodOrNull(name: String, vararg parameterTypes: Class<*>): Method? = + try { + getDeclaredMethod(name, *parameterTypes) + } catch (e: NoSuchMethodException) { + null + } + +internal fun Class<*>.getDeclaredFieldOrNull(name: String): Field? = + try { + getDeclaredField(name) + } catch (e: NoSuchFieldException) { + null + }