From e6b78f68cd8932d786a325193d9cebad47d7a4a2 Mon Sep 17 00:00:00 2001 From: Alexey Andreev Date: Fri, 23 Dec 2016 19:34:16 +0300 Subject: [PATCH] JS: support case when class inherits method from superclass and both implements same method (but with optional parameter) from superinterface. --- .../box/defaultArguments/implementedByFake.kt | 42 +++++++++++++++++ .../defaultArguments/implementedByFake.txt | 33 +++++++++++++ .../ir/IrBlackBoxCodegenTestGenerated.java | 12 +++++ .../codegen/BlackBoxCodegenTestGenerated.java | 12 +++++ ...LightAnalysisModeCodegenTestGenerated.java | 12 +++++ .../semantics/JsCodegenBoxTestGenerated.java | 6 +++ .../declaration/InterfaceFunctionCopier.kt | 46 ++++++++++++++++--- 7 files changed, 156 insertions(+), 7 deletions(-) create mode 100644 compiler/testData/codegen/box/defaultArguments/implementedByFake.kt create mode 100644 compiler/testData/codegen/light-analysis/defaultArguments/implementedByFake.txt diff --git a/compiler/testData/codegen/box/defaultArguments/implementedByFake.kt b/compiler/testData/codegen/box/defaultArguments/implementedByFake.kt new file mode 100644 index 00000000000..d5c4564cbac --- /dev/null +++ b/compiler/testData/codegen/box/defaultArguments/implementedByFake.kt @@ -0,0 +1,42 @@ +// TARGET_BACKEND: JS +// This test causes JVM to generate incorrect bytecode +// Could not use IGNORE_BACKEND directive, since it makes LightAnalysisModeCodegenTestsGenerated fail on this test + +interface I { + val prop: T + + fun f(x: String = "1"): String + + fun g(x: String = "2"): String + + fun h(x: T = prop): T +} + +open class A { + open fun f(x: String) = x + + open fun g(x: T) = x + + open fun h(x: String) = x +} + +class B : A(), I { + override val prop + get() = "3" +} + +fun box(): String { + val i: I = B() + var result = i.f() + i.g() + i.h() + if (result != "123") return "fail1: $result" + + val b = B() + result = b.f() + b.g() + b.h() + if (result != "123") return "fail2: $result" + + val a: A = B() + result = a.f("q") + a.g("w") + a.h("e") + if (result != "qwe") return "fail3: $result" + + return "OK" +} \ No newline at end of file diff --git a/compiler/testData/codegen/light-analysis/defaultArguments/implementedByFake.txt b/compiler/testData/codegen/light-analysis/defaultArguments/implementedByFake.txt new file mode 100644 index 00000000000..5559efe8625 --- /dev/null +++ b/compiler/testData/codegen/light-analysis/defaultArguments/implementedByFake.txt @@ -0,0 +1,33 @@ +public class A { + public method (): void + public @org.jetbrains.annotations.NotNull method f(@org.jetbrains.annotations.NotNull p0: java.lang.String): java.lang.String + public method g(p0: java.lang.Object): java.lang.Object + public @org.jetbrains.annotations.NotNull method h(@org.jetbrains.annotations.NotNull p0: java.lang.String): java.lang.String +} + +public final class B { + public method (): void + public synthetic method g(p0: java.lang.String): java.lang.String + public synthetic method getProp(): java.lang.Object + public @org.jetbrains.annotations.NotNull method getProp(): java.lang.String + public synthetic method h(p0: java.lang.Object): java.lang.Object +} + +public interface I { + inner class I/DefaultImpls + public abstract @org.jetbrains.annotations.NotNull method f(@org.jetbrains.annotations.NotNull p0: java.lang.String): java.lang.String + public abstract @org.jetbrains.annotations.NotNull method g(@org.jetbrains.annotations.NotNull p0: java.lang.String): java.lang.String + public abstract method getProp(): java.lang.Object + public abstract method h(p0: java.lang.Object): java.lang.Object +} + +public final class I/DefaultImpls { + inner class I/DefaultImpls + public synthetic static method f$default(p0: I, p1: java.lang.String, p2: int, p3: java.lang.Object): java.lang.String + public synthetic static method g$default(p0: I, p1: java.lang.String, p2: int, p3: java.lang.Object): java.lang.String + public synthetic static method h$default(p0: I, p1: java.lang.Object, p2: int, p3: java.lang.Object): java.lang.Object +} + +public final class ImplementedByFakeKt { + public final static @org.jetbrains.annotations.NotNull method box(): java.lang.String +} 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 55f740cb4e4..c5c7f7e008e 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 @@ -5474,6 +5474,18 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/codegen/box/defaultArguments"), Pattern.compile("^(.+)\\.kt$"), TargetBackend.JVM, true); } + @TestMetadata("implementedByFake.kt") + public void testImplementedByFake() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/defaultArguments/implementedByFake.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("kt6382.kt") public void testKt6382() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/defaultArguments/kt6382.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java index b26a036dd01..3b81b9256c0 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java @@ -5474,6 +5474,18 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest { KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/codegen/box/defaultArguments"), Pattern.compile("^(.+)\\.kt$"), TargetBackend.JVM, true); } + @TestMetadata("implementedByFake.kt") + public void testImplementedByFake() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/defaultArguments/implementedByFake.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("kt6382.kt") public void testKt6382() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/defaultArguments/kt6382.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeCodegenTestGenerated.java index b6287a5507e..51899487095 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeCodegenTestGenerated.java @@ -5474,6 +5474,18 @@ public class LightAnalysisModeCodegenTestGenerated extends AbstractLightAnalysis KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/codegen/box/defaultArguments"), Pattern.compile("^(.+)\\.kt$"), TargetBackend.JVM, true); } + @TestMetadata("implementedByFake.kt") + public void testImplementedByFake() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/defaultArguments/implementedByFake.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("kt6382.kt") public void testKt6382() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/defaultArguments/kt6382.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 d81bf539c71..6f53a609726 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 @@ -6321,6 +6321,12 @@ public class JsCodegenBoxTestGenerated extends AbstractJsCodegenBoxTest { KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/codegen/box/defaultArguments"), Pattern.compile("^(.+)\\.kt$"), TargetBackend.JS, true); } + @TestMetadata("implementedByFake.kt") + public void testImplementedByFake() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/defaultArguments/implementedByFake.kt"); + doTest(fileName); + } + @TestMetadata("kt6382.kt") public void testKt6382() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/defaultArguments/kt6382.kt"); diff --git a/js/js.translator/src/org/jetbrains/kotlin/js/translate/declaration/InterfaceFunctionCopier.kt b/js/js.translator/src/org/jetbrains/kotlin/js/translate/declaration/InterfaceFunctionCopier.kt index 42bcdee61d3..75ba46791c3 100644 --- a/js/js.translator/src/org/jetbrains/kotlin/js/translate/declaration/InterfaceFunctionCopier.kt +++ b/js/js.translator/src/org/jetbrains/kotlin/js/translate/declaration/InterfaceFunctionCopier.kt @@ -28,9 +28,7 @@ import org.jetbrains.kotlin.js.translate.utils.JsAstUtils import org.jetbrains.kotlin.js.translate.utils.JsAstUtils.prototypeOf import org.jetbrains.kotlin.js.translate.utils.JsAstUtils.pureFqn import org.jetbrains.kotlin.resolve.DescriptorUtils -import org.jetbrains.kotlin.resolve.descriptorUtil.hasOwnParametersWithDefaultValue -import org.jetbrains.kotlin.resolve.descriptorUtil.hasOrInheritsParametersWithDefaultValue -import org.jetbrains.kotlin.resolve.descriptorUtil.module +import org.jetbrains.kotlin.resolve.descriptorUtil.* import org.jetbrains.kotlin.resolve.scopes.DescriptorKindFilter import org.jetbrains.kotlin.utils.DFS import org.jetbrains.kotlin.utils.identity @@ -82,6 +80,10 @@ class InterfaceFunctionCopier(val context: StaticContext) { } } + for (member in members.filterIsInstance()) { + moveDefaultFunction(member, context) + } + for (member in members.filterIsInstance() .filter { it.kind.isReal && it.hasOwnParametersWithDefaultValue() }) { val names = generateAllNames(member, context) @@ -92,10 +94,11 @@ class InterfaceFunctionCopier(val context: StaticContext) { val superModels = DescriptorUtils.getSuperclassDescriptors(descriptor) .filter { it.kind == ClassKind.INTERFACE && !isNativeObject(it) } .map { classModels[it]!! } + for (superModel in superModels) { for (name in superModel.copiedFunctions) { if (classModel.copiedFunctions.add(name)) { - addDefaultMethodFromInterface(name, superModel.descriptor, descriptor, context) + addDefaultMethodFromInterface(name, name, superModel.descriptor, descriptor, context) } } for (name in superModel.copiedProperties) { @@ -106,8 +109,37 @@ class InterfaceFunctionCopier(val context: StaticContext) { } } + private fun moveDefaultFunction(function: FunctionDescriptor, context: StaticContext) { + if (function.kind.isReal || function.modality == Modality.ABSTRACT) return + + val overriddenWithDefaultArg = function.overriddenDescriptors + .firstOrNull { it.hasOwnParametersWithDefaultValue() } ?: return + + if (!DescriptorUtils.isInterface(overriddenWithDefaultArg.containingDeclaration)) return + val fakeImplementation = function.overriddenDescriptors.first { it.modality != Modality.ABSTRACT } + + val interfaceFunctionName = context.getNameForDescriptor(overriddenWithDefaultArg).ident + val targetName = interfaceFunctionName + Namer.DEFAULT_PARAMETER_IMPLEMENTOR_SUFFIX + val sourceName = context.getNameForDescriptor(fakeImplementation).ident + + addDefaultMethodFromInterface(sourceName, targetName, fakeImplementation.containingDeclaration as ClassDescriptor, + function.containingDeclaration as ClassDescriptor, context) + + val overrideNames = generateAllNames(function, context).map { it.ident } + val namesFromBaseClass = function.overriddenDescriptors + .filter { !DescriptorUtils.isInterface(it.containingDeclaration) } + .flatMap { generateAllNames(it, context).map { it.ident }.asIterable() } + .distinct() + + for (name in overrideNames.filter { it in namesFromBaseClass }) { + addDefaultMethodFromInterface(interfaceFunctionName, name, overriddenWithDefaultArg.containingDeclaration as ClassDescriptor, + function.containingDeclaration as ClassDescriptor, context) + } + } + private fun addDefaultMethodFromInterface( - name: String, + sourceName: String, + targetName: String, sourceDescriptor: ClassDescriptor, targetDescriptor: ClassDescriptor, context: StaticContext @@ -116,8 +148,8 @@ class InterfaceFunctionCopier(val context: StaticContext) { val targetPrototype = prototypeOf(pureFqn(context.getInnerNameForDescriptor(targetDescriptor), null)) val sourcePrototype = prototypeOf(pureFqn(context.getInnerNameForDescriptor(sourceDescriptor), null)) - val targetFunction = JsNameRef(name, targetPrototype) - val sourceFunction = JsNameRef(name, sourcePrototype) + val targetFunction = JsNameRef(targetName, targetPrototype) + val sourceFunction = JsNameRef(sourceName, sourcePrototype) context.declarationStatements += JsAstUtils.assignment(targetFunction, sourceFunction).makeStmt() }