From 1f1790d60e837347d99921dd1fb4f00e6ec868d2 Mon Sep 17 00:00:00 2001 From: Alexander Udalov Date: Wed, 27 May 2020 16:50:31 +0200 Subject: [PATCH] Do not rely on descriptors in KParameterImpl.equals/hashCode For the same reason as in the previous commit: descriptors are cached via weak references in moduleByClassLoader.kt and can be garbage-collected at any point. So different instances of KParameterImpl representing the same parameter may store different instances of descriptors. --- .../ir/FirBlackBoxCodegenTestGenerated.java | 5 ++ .../parametersEqualsWithClearCaches.kt | 46 +++++++++++++++++++ .../codegen/BlackBoxCodegenTestGenerated.java | 5 ++ .../LightAnalysisModeTestGenerated.java | 5 ++ .../ir/IrBlackBoxCodegenTestGenerated.java | 5 ++ .../reflect/jvm/internal/KParameterImpl.kt | 4 +- 6 files changed, 68 insertions(+), 2 deletions(-) create mode 100644 compiler/testData/codegen/box/reflection/methodsFromAny/parametersEqualsWithClearCaches.kt diff --git a/compiler/fir/fir2ir/tests/org/jetbrains/kotlin/codegen/ir/FirBlackBoxCodegenTestGenerated.java b/compiler/fir/fir2ir/tests/org/jetbrains/kotlin/codegen/ir/FirBlackBoxCodegenTestGenerated.java index e03f9bf0fd3..5773d65ece4 100644 --- a/compiler/fir/fir2ir/tests/org/jetbrains/kotlin/codegen/ir/FirBlackBoxCodegenTestGenerated.java +++ b/compiler/fir/fir2ir/tests/org/jetbrains/kotlin/codegen/ir/FirBlackBoxCodegenTestGenerated.java @@ -26167,6 +26167,11 @@ public class FirBlackBoxCodegenTestGenerated extends AbstractFirBlackBoxCodegenT runTest("compiler/testData/codegen/box/reflection/methodsFromAny/parametersEqualsHashCode.kt"); } + @TestMetadata("parametersEqualsWithClearCaches.kt") + public void testParametersEqualsWithClearCaches() throws Exception { + runTest("compiler/testData/codegen/box/reflection/methodsFromAny/parametersEqualsWithClearCaches.kt"); + } + @TestMetadata("parametersToString.kt") public void testParametersToString() throws Exception { runTest("compiler/testData/codegen/box/reflection/methodsFromAny/parametersToString.kt"); diff --git a/compiler/testData/codegen/box/reflection/methodsFromAny/parametersEqualsWithClearCaches.kt b/compiler/testData/codegen/box/reflection/methodsFromAny/parametersEqualsWithClearCaches.kt new file mode 100644 index 00000000000..647256599a9 --- /dev/null +++ b/compiler/testData/codegen/box/reflection/methodsFromAny/parametersEqualsWithClearCaches.kt @@ -0,0 +1,46 @@ +// TARGET_BACKEND: JVM +// WITH_REFLECT +// FILE: box.kt + +import kotlin.reflect.KMutableProperty +import kotlin.test.assertEquals + +inline fun check(message: String, generate: () -> Any?) { + val x1: Any? + val x2: Any? + try { + x1 = generate() + + // Force clear the internal maps, as if the weak values in them are garbage-collected. + kotlin.reflect.jvm.internal.ReflectionFactoryImpl.clearCaches() + + x2 = generate() + } catch (e: Throwable) { + throw AssertionError("Fail $message", e) + } + + assertEquals(x1, x2, "Fail equals $message") + assertEquals(x2, x1, "Fail equals $message") + assertEquals(x1.hashCode(), x2.hashCode(), "Fail hashCode $message") +} + +class C(c: Any) { + fun Any.a(a: Any): Any = a + + var X.x: X + get() = this + set(value) {} +} + +fun box(): String { + check("constructor parameter") { C::class.constructors.single().parameters.single() } + check("instance parameter") { C::class.members.single { it.name == "a" }.parameters[0] } + check("value parameter") { C::class.members.single { it.name == "a" }.parameters[1] } + + check("extension receiver parameter") { (C::class.members.single { it.name == "x" } as KMutableProperty<*>).parameters[1] } + + // TODO: depends on KT-13490 + // check("property setter parameter") { (C::class.members.single { it.name == "x" } as KMutableProperty<*>).setter.parameters[2] } + + return "OK" +} diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java index 2f8a1f68660..f60648aa48f 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java @@ -27753,6 +27753,11 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest { runTest("compiler/testData/codegen/box/reflection/methodsFromAny/parametersEqualsHashCode.kt"); } + @TestMetadata("parametersEqualsWithClearCaches.kt") + public void testParametersEqualsWithClearCaches() throws Exception { + runTest("compiler/testData/codegen/box/reflection/methodsFromAny/parametersEqualsWithClearCaches.kt"); + } + @TestMetadata("parametersToString.kt") public void testParametersToString() throws Exception { runTest("compiler/testData/codegen/box/reflection/methodsFromAny/parametersToString.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java index d871ad6de8a..06603b859a9 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java @@ -25387,6 +25387,11 @@ public class LightAnalysisModeTestGenerated extends AbstractLightAnalysisModeTes runTest("compiler/testData/codegen/box/reflection/methodsFromAny/parametersEqualsHashCode.kt"); } + @TestMetadata("parametersEqualsWithClearCaches.kt") + public void testParametersEqualsWithClearCaches() throws Exception { + runTest("compiler/testData/codegen/box/reflection/methodsFromAny/parametersEqualsWithClearCaches.kt"); + } + @TestMetadata("parametersToString.kt") public void testParametersToString() throws Exception { runTest("compiler/testData/codegen/box/reflection/methodsFromAny/parametersToString.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java index 0fc4eba18b5..0ede77e0b2f 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java @@ -26167,6 +26167,11 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes runTest("compiler/testData/codegen/box/reflection/methodsFromAny/parametersEqualsHashCode.kt"); } + @TestMetadata("parametersEqualsWithClearCaches.kt") + public void testParametersEqualsWithClearCaches() throws Exception { + runTest("compiler/testData/codegen/box/reflection/methodsFromAny/parametersEqualsWithClearCaches.kt"); + } + @TestMetadata("parametersToString.kt") public void testParametersToString() throws Exception { runTest("compiler/testData/codegen/box/reflection/methodsFromAny/parametersToString.kt"); diff --git a/core/reflection.jvm/src/kotlin/reflect/jvm/internal/KParameterImpl.kt b/core/reflection.jvm/src/kotlin/reflect/jvm/internal/KParameterImpl.kt index 337c8018e02..79061727fef 100644 --- a/core/reflection.jvm/src/kotlin/reflect/jvm/internal/KParameterImpl.kt +++ b/core/reflection.jvm/src/kotlin/reflect/jvm/internal/KParameterImpl.kt @@ -64,10 +64,10 @@ internal class KParameterImpl( get() = descriptor.let { it is ValueParameterDescriptor && it.varargElementType != null } override fun equals(other: Any?) = - other is KParameterImpl && callable == other.callable && descriptor == other.descriptor + other is KParameterImpl && callable == other.callable && index == other.index override fun hashCode() = - (callable.hashCode() * 31) + descriptor.hashCode() + (callable.hashCode() * 31) + index.hashCode() override fun toString() = ReflectionObjectRenderer.renderParameter(this)