From d59f2bcc800f537657b56ac9a5d61721b1c489e1 Mon Sep 17 00:00:00 2001 From: Alexander Udalov Date: Tue, 20 Aug 2019 12:08:41 +0200 Subject: [PATCH] Fix KotlinReflectionInternalError on invoking callBy with defaults in supertypes There are two parts in this change: 1) Previously, we looked up $default methods with the incorrect signature in supertypes. For example in defaultInSuperClass.kt, we'd try to find a method foo$default with the signature `(B, String, String, int, Object)` in the class A. Now we're modifying the array of parameter types on each step if we're looking for a static $default method, by assigning its first element to be the containing class. This fixes cases when defaults come from a superclass. 2) For interfaces, $default methods are actually located in the corresponding DefaultImpls class. Now we look up that class and search for the $default method there. Note that this is needed because of KT-33430. This fixes cases when defaults come from a superinterface. #KT-13936 Fixed --- .../reflection/callBy/defaultInSuperClass.kt | 21 ++++++++ .../callBy/defaultInSuperInterface.kt | 29 +++++++++++ .../codegen/BlackBoxCodegenTestGenerated.java | 10 ++++ .../LightAnalysisModeTestGenerated.java | 10 ++++ .../ir/IrBlackBoxCodegenTestGenerated.java | 10 ++++ .../jvm/internal/KDeclarationContainerImpl.kt | 48 +++++++++++++------ .../IrJsCodegenBoxTestGenerated.java | 10 ++++ .../semantics/JsCodegenBoxTestGenerated.java | 10 ++++ 8 files changed, 133 insertions(+), 15 deletions(-) create mode 100644 compiler/testData/codegen/box/reflection/callBy/defaultInSuperClass.kt create mode 100644 compiler/testData/codegen/box/reflection/callBy/defaultInSuperInterface.kt diff --git a/compiler/testData/codegen/box/reflection/callBy/defaultInSuperClass.kt b/compiler/testData/codegen/box/reflection/callBy/defaultInSuperClass.kt new file mode 100644 index 00000000000..00921823d45 --- /dev/null +++ b/compiler/testData/codegen/box/reflection/callBy/defaultInSuperClass.kt @@ -0,0 +1,21 @@ +// IGNORE_BACKEND: JS_IR, JS, NATIVE +// WITH_REFLECT + +open class A { + open fun foo(a: String, b: String = "b") = b + a +} + +class B : A() { + override fun foo(a: String, b: String) = a + b +} + +fun box(): String { + val f = B::foo + + assert("ab" == f.callBy(mapOf( + f.parameters.first() to B(), + f.parameters.single { it.name == "a" } to "a" + ))) + + return "OK" +} diff --git a/compiler/testData/codegen/box/reflection/callBy/defaultInSuperInterface.kt b/compiler/testData/codegen/box/reflection/callBy/defaultInSuperInterface.kt new file mode 100644 index 00000000000..70bad880ed3 --- /dev/null +++ b/compiler/testData/codegen/box/reflection/callBy/defaultInSuperInterface.kt @@ -0,0 +1,29 @@ +// IGNORE_BACKEND: JS_IR, JS, NATIVE +// WITH_REFLECT + +interface A1 { + fun test(o: String, k: String): String +} +interface A2 : A1 + +interface B1 { + fun test(o: String = "O", k: String = "K"): String +} +interface B2 : B1 + +interface C1 +interface C2 : C1 + +interface D +interface E : A2, B2, C2 { + override fun test(o: String, k: String): String { + return o + k + } +} + +class Z : D, E + +fun box(): String { + val f = Z::test + return f.callBy(mapOf(f.parameters.first() to Z())) +} diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java index e3f8d106c15..c23b7eb3b69 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java @@ -20614,6 +20614,16 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest { runTest("compiler/testData/codegen/box/reflection/callBy/defaultAndNonDefaultIntertwined.kt"); } + @TestMetadata("defaultInSuperClass.kt") + public void testDefaultInSuperClass() throws Exception { + runTest("compiler/testData/codegen/box/reflection/callBy/defaultInSuperClass.kt"); + } + + @TestMetadata("defaultInSuperInterface.kt") + public void testDefaultInSuperInterface() throws Exception { + runTest("compiler/testData/codegen/box/reflection/callBy/defaultInSuperInterface.kt"); + } + @TestMetadata("extensionFunction.kt") public void testExtensionFunction() throws Exception { runTest("compiler/testData/codegen/box/reflection/callBy/extensionFunction.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java index 942803e1de6..ec3ee5fca19 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java @@ -20614,6 +20614,16 @@ public class LightAnalysisModeTestGenerated extends AbstractLightAnalysisModeTes runTest("compiler/testData/codegen/box/reflection/callBy/defaultAndNonDefaultIntertwined.kt"); } + @TestMetadata("defaultInSuperClass.kt") + public void testDefaultInSuperClass() throws Exception { + runTest("compiler/testData/codegen/box/reflection/callBy/defaultInSuperClass.kt"); + } + + @TestMetadata("defaultInSuperInterface.kt") + public void testDefaultInSuperInterface() throws Exception { + runTest("compiler/testData/codegen/box/reflection/callBy/defaultInSuperInterface.kt"); + } + @TestMetadata("extensionFunction.kt") public void testExtensionFunction() throws Exception { runTest("compiler/testData/codegen/box/reflection/callBy/extensionFunction.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java index d7e0fa7f1af..5cbc20e89fd 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java @@ -19499,6 +19499,16 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes runTest("compiler/testData/codegen/box/reflection/callBy/defaultAndNonDefaultIntertwined.kt"); } + @TestMetadata("defaultInSuperClass.kt") + public void testDefaultInSuperClass() throws Exception { + runTest("compiler/testData/codegen/box/reflection/callBy/defaultInSuperClass.kt"); + } + + @TestMetadata("defaultInSuperInterface.kt") + public void testDefaultInSuperInterface() throws Exception { + runTest("compiler/testData/codegen/box/reflection/callBy/defaultInSuperInterface.kt"); + } + @TestMetadata("extensionFunction.kt") public void testExtensionFunction() throws Exception { runTest("compiler/testData/codegen/box/reflection/callBy/extensionFunction.kt"); diff --git a/core/reflection.jvm/src/kotlin/reflect/jvm/internal/KDeclarationContainerImpl.kt b/core/reflection.jvm/src/kotlin/reflect/jvm/internal/KDeclarationContainerImpl.kt index dfb3922f91f..c52c0ede770 100644 --- a/core/reflection.jvm/src/kotlin/reflect/jvm/internal/KDeclarationContainerImpl.kt +++ b/core/reflection.jvm/src/kotlin/reflect/jvm/internal/KDeclarationContainerImpl.kt @@ -26,6 +26,7 @@ import java.lang.reflect.Constructor import java.lang.reflect.Method import kotlin.jvm.internal.ClassBasedDeclarationContainer import kotlin.reflect.jvm.internal.components.RuntimeModuleData +import kotlin.reflect.jvm.internal.components.tryLoadClass import kotlin.reflect.jvm.internal.structure.createArrayType import kotlin.reflect.jvm.internal.structure.safeClassLoader import kotlin.reflect.jvm.internal.structure.wrapperByPrimitive @@ -165,26 +166,30 @@ internal abstract class KDeclarationContainerImpl : ClassBasedDeclarationContain return functions.single() } - private fun Class<*>.lookupMethod(name: String, parameterTypes: List>, returnType: Class<*>): Method? { - lookupMethod(name, parameterTypes.toTypedArray(), returnType)?.let { return it } - - // Methods from java.lang.Object (equals, hashCode, toString) cannot be found in the interface via - // Class.getMethod/getDeclaredMethod, so for interfaces, we also look in java.lang.Object. - if (isInterface) { - Any::class.java.lookupMethod(name, parameterTypes.toTypedArray(), returnType)?.let { return it } + private fun Class<*>.lookupMethod( + name: String, parameterTypes: Array>, returnType: Class<*>, isStaticDefault: Boolean + ): Method? { + // Static "$default" method in any class takes an instance of that class as the first parameter. + if (isStaticDefault) { + parameterTypes[0] = this } - return null - } - - private fun Class<*>.lookupMethod(name: String, parameterTypes: Array>, returnType: Class<*>): Method? { tryGetMethod(name, parameterTypes, returnType)?.let { return it } - superclass?.lookupMethod(name, parameterTypes, returnType)?.let { return it } + superclass?.lookupMethod(name, parameterTypes, returnType, isStaticDefault)?.let { return it } // TODO: avoid exponential complexity here for (superInterface in interfaces) { - superInterface.lookupMethod(name, parameterTypes, returnType)?.let { return it } + superInterface.lookupMethod(name, parameterTypes, returnType, isStaticDefault)?.let { return it } + + // Static "$default" methods should be looked up in each DefaultImpls class, see KT-33430 + if (isStaticDefault) { + val defaultImpls = superInterface.classLoader.tryLoadClass(superInterface.name + JvmAbi.DEFAULT_IMPLS_SUFFIX) + if (defaultImpls != null) { + parameterTypes[0] = superInterface + defaultImpls.tryGetMethod(name, parameterTypes, returnType)?.let { return it } + } + } } return null @@ -220,7 +225,17 @@ internal abstract class KDeclarationContainerImpl : ClassBasedDeclarationContain fun findMethodBySignature(name: String, desc: String): Method? { if (name == "") return null - return methodOwner.lookupMethod(name, loadParameterTypes(desc), loadReturnType(desc)) + val parameterTypes = loadParameterTypes(desc).toTypedArray() + val returnType = loadReturnType(desc) + methodOwner.lookupMethod(name, parameterTypes, returnType, false)?.let { return it } + + // Methods from java.lang.Object (equals, hashCode, toString) cannot be found in the interface via + // Class.getMethod/getDeclaredMethod, so for interfaces, we also look in java.lang.Object. + if (methodOwner.isInterface) { + Any::class.java.lookupMethod(name, parameterTypes, returnType, false)?.let { return it } + } + + return null } fun findDefaultMethod(name: String, desc: String, isMember: Boolean): Method? { @@ -228,11 +243,14 @@ internal abstract class KDeclarationContainerImpl : ClassBasedDeclarationContain val parameterTypes = arrayListOf>() if (isMember) { + // Note that this value is replaced inside the lookupMethod call below, for each class/interface in the hierarchy. parameterTypes.add(jClass) } addParametersAndMasks(parameterTypes, desc, false) - return methodOwner.lookupMethod(name + JvmAbi.DEFAULT_PARAMS_IMPL_SUFFIX, parameterTypes, loadReturnType(desc)) + return methodOwner.lookupMethod( + name + JvmAbi.DEFAULT_PARAMS_IMPL_SUFFIX, parameterTypes.toTypedArray(), loadReturnType(desc), isStaticDefault = isMember + ) } fun findConstructorBySignature(desc: String): Constructor<*>? = diff --git a/js/js.tests/test/org/jetbrains/kotlin/js/test/ir/semantics/IrJsCodegenBoxTestGenerated.java b/js/js.tests/test/org/jetbrains/kotlin/js/test/ir/semantics/IrJsCodegenBoxTestGenerated.java index 098bec3d0c7..48781cd8845 100644 --- a/js/js.tests/test/org/jetbrains/kotlin/js/test/ir/semantics/IrJsCodegenBoxTestGenerated.java +++ b/js/js.tests/test/org/jetbrains/kotlin/js/test/ir/semantics/IrJsCodegenBoxTestGenerated.java @@ -16304,6 +16304,16 @@ public class IrJsCodegenBoxTestGenerated extends AbstractIrJsCodegenBoxTest { runTest("compiler/testData/codegen/box/reflection/callBy/defaultAndNonDefaultIntertwined.kt"); } + @TestMetadata("defaultInSuperClass.kt") + public void testDefaultInSuperClass() throws Exception { + runTest("compiler/testData/codegen/box/reflection/callBy/defaultInSuperClass.kt"); + } + + @TestMetadata("defaultInSuperInterface.kt") + public void testDefaultInSuperInterface() throws Exception { + runTest("compiler/testData/codegen/box/reflection/callBy/defaultInSuperInterface.kt"); + } + @TestMetadata("extensionFunction.kt") public void testExtensionFunction() throws Exception { runTest("compiler/testData/codegen/box/reflection/callBy/extensionFunction.kt"); 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 eee46852b9a..d6b2778c4d6 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 @@ -17459,6 +17459,16 @@ public class JsCodegenBoxTestGenerated extends AbstractJsCodegenBoxTest { runTest("compiler/testData/codegen/box/reflection/callBy/defaultAndNonDefaultIntertwined.kt"); } + @TestMetadata("defaultInSuperClass.kt") + public void testDefaultInSuperClass() throws Exception { + runTest("compiler/testData/codegen/box/reflection/callBy/defaultInSuperClass.kt"); + } + + @TestMetadata("defaultInSuperInterface.kt") + public void testDefaultInSuperInterface() throws Exception { + runTest("compiler/testData/codegen/box/reflection/callBy/defaultInSuperInterface.kt"); + } + @TestMetadata("extensionFunction.kt") public void testExtensionFunction() throws Exception { runTest("compiler/testData/codegen/box/reflection/callBy/extensionFunction.kt");