From b48d7f4ba727266bcf4e718c83672047aa9589d2 Mon Sep 17 00:00:00 2001 From: Alexander Udalov Date: Fri, 27 Dec 2019 23:24:04 +0100 Subject: [PATCH] JVM IR: fix InterfaceLowering for $default methods from base interfaces The first change here is swapping the isCompatibility and hasJvmDefault checks. Otherwise behavior could be different depending on -Xjvm-default mode even for non-JvmDefault declarations, which makes little sense. Another change is avoiding generating $default stubs for fake overrides in interfaces, which replicates the behavior of the current backend. (Note that this change also fixes the first problem on the newly added tests, but the first change seems useful anyway.) --- .../backend/jvm/lower/InterfaceLowering.kt | 23 +++++++++++-------- .../inheritedFunctionWithDefaultParameters.kt | 12 ++++++++++ .../inheritedInterfaceFunction.kt | 15 ++++++++++++ .../codegen/BlackBoxCodegenTestGenerated.java | 5 ++++ .../codegen/BytecodeTextTestGenerated.java | 5 ++++ .../LightAnalysisModeTestGenerated.java | 5 ++++ .../ir/FirBlackBoxCodegenTestGenerated.java | 5 ++++ .../ir/IrBlackBoxCodegenTestGenerated.java | 5 ++++ .../ir/IrBytecodeTextTestGenerated.java | 5 ++++ 9 files changed, 70 insertions(+), 10 deletions(-) create mode 100644 compiler/testData/codegen/box/jvm8/defaults/compatibility/inheritedFunctionWithDefaultParameters.kt create mode 100644 compiler/testData/codegen/bytecodeText/defaultArguments/inheritedInterfaceFunction.kt diff --git a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/lower/InterfaceLowering.kt b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/lower/InterfaceLowering.kt index 801476bc7ec..86301fc51cb 100644 --- a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/lower/InterfaceLowering.kt +++ b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/lower/InterfaceLowering.kt @@ -71,12 +71,13 @@ internal class InterfaceLowering(val context: JvmBackendContext) : IrElementTran continue@loop /** - * 2) They inherit a default implementation from an interface this interface - * extends: create a bridge from companion to companion, unless - * - the implementation is private or belongs to java.lang.Object - * - we're in JVM Compatibility Default mode, in which case we go via + * 2) They inherit a default implementation from an interface this interface extends: + * create a bridge from DefaultImpls of derived to DefaultImpls of base, unless + * - the implementation is private, or belongs to java.lang.Object, + * or is a stub for function with default parameters ($default) + * - we're in -Xjvm-default=compatibility mode, in which case we go via * accessors on the parent class rather than the DefaultImpls - * - we're in JVM Default mode, and we have that default implementation, + * - we're in -Xjvm-default=enable mode, and we have that default implementation, * in which case we simply leave it. * * ``` @@ -95,18 +96,20 @@ internal class InterfaceLowering(val context: JvmBackendContext) : IrElementTran val implementation = function.resolveFakeOverride()!! when { - Visibilities.isPrivate(implementation.visibility) || implementation.isMethodOfAny() -> + Visibilities.isPrivate(implementation.visibility) || + implementation.isMethodOfAny() || + implementation.origin == IrDeclarationOrigin.FUNCTION_FOR_DEFAULT_PARAMETER -> continue@loop - context.state.jvmDefaultMode.isCompatibility -> { - val defaultImpl = createDefaultImpl(function) - defaultImpl.bridgeViaAccessorTo(function) - } !implementation.hasJvmDefault() -> { val defaultImpl = createDefaultImpl(function) context.declarationFactory.getDefaultImplsFunction(implementation).also { defaultImpl.bridgeToStatic(it) } } + context.state.jvmDefaultMode.isCompatibility -> { + val defaultImpl = createDefaultImpl(function) + defaultImpl.bridgeViaAccessorTo(function) + } // else -> Do nothing. } } diff --git a/compiler/testData/codegen/box/jvm8/defaults/compatibility/inheritedFunctionWithDefaultParameters.kt b/compiler/testData/codegen/box/jvm8/defaults/compatibility/inheritedFunctionWithDefaultParameters.kt new file mode 100644 index 00000000000..0bc3583db64 --- /dev/null +++ b/compiler/testData/codegen/box/jvm8/defaults/compatibility/inheritedFunctionWithDefaultParameters.kt @@ -0,0 +1,12 @@ +// !JVM_DEFAULT_MODE: compatibility +// TARGET_BACKEND: JVM + +interface I { + fun foo(x: String = "OK"): String = x +} + +interface J : I + +object O : J + +fun box(): String = O.foo() diff --git a/compiler/testData/codegen/bytecodeText/defaultArguments/inheritedInterfaceFunction.kt b/compiler/testData/codegen/bytecodeText/defaultArguments/inheritedInterfaceFunction.kt new file mode 100644 index 00000000000..f4fceda5505 --- /dev/null +++ b/compiler/testData/codegen/bytecodeText/defaultArguments/inheritedInterfaceFunction.kt @@ -0,0 +1,15 @@ +// FILE: I.kt + +interface I { + fun foo(x: String = "OK"): String = x +} + +// FILE: J.kt + +interface J : I + +// @I$DefaultImpls.class: +// 1 foo\$default + +// @J$DefaultImpls.class: +// 0 foo\$default diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java index 2c721575850..9e2872d1f61 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java @@ -15104,6 +15104,11 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest { runTest("compiler/testData/codegen/box/jvm8/defaults/compatibility/defaultArgsViaAnonymousObject.kt"); } + @TestMetadata("inheritedFunctionWithDefaultParameters.kt") + public void testInheritedFunctionWithDefaultParameters() throws Exception { + runTest("compiler/testData/codegen/box/jvm8/defaults/compatibility/inheritedFunctionWithDefaultParameters.kt"); + } + @TestMetadata("inheritedJvmDefault.kt") public void testInheritedJvmDefault() throws Exception { runTest("compiler/testData/codegen/box/jvm8/defaults/compatibility/inheritedJvmDefault.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeTextTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeTextTestGenerated.java index a8846248061..7296a520c42 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeTextTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeTextTestGenerated.java @@ -1560,6 +1560,11 @@ public class BytecodeTextTestGenerated extends AbstractBytecodeTextTest { KotlinTestUtils.assertAllTestsPresentByMetadataWithExcluded(this.getClass(), new File("compiler/testData/codegen/bytecodeText/defaultArguments"), Pattern.compile("^(.+)\\.kt$"), null, TargetBackend.JVM, true); } + @TestMetadata("inheritedInterfaceFunction.kt") + public void testInheritedInterfaceFunction() throws Exception { + runTest("compiler/testData/codegen/bytecodeText/defaultArguments/inheritedInterfaceFunction.kt"); + } + @TestMetadata("kt11962.kt") public void testKt11962() throws Exception { runTest("compiler/testData/codegen/bytecodeText/defaultArguments/kt11962.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java index 22a0fc73c29..52672a43b4a 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java @@ -15104,6 +15104,11 @@ public class LightAnalysisModeTestGenerated extends AbstractLightAnalysisModeTes runTest("compiler/testData/codegen/box/jvm8/defaults/compatibility/defaultArgsViaAnonymousObject.kt"); } + @TestMetadata("inheritedFunctionWithDefaultParameters.kt") + public void testInheritedFunctionWithDefaultParameters() throws Exception { + runTest("compiler/testData/codegen/box/jvm8/defaults/compatibility/inheritedFunctionWithDefaultParameters.kt"); + } + @TestMetadata("inheritedJvmDefault.kt") public void testInheritedJvmDefault() throws Exception { runTest("compiler/testData/codegen/box/jvm8/defaults/compatibility/inheritedJvmDefault.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/ir/FirBlackBoxCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/ir/FirBlackBoxCodegenTestGenerated.java index 2447addb236..1641eadfa9f 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/ir/FirBlackBoxCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/ir/FirBlackBoxCodegenTestGenerated.java @@ -13954,6 +13954,11 @@ public class FirBlackBoxCodegenTestGenerated extends AbstractFirBlackBoxCodegenT runTest("compiler/testData/codegen/box/jvm8/defaults/compatibility/defaultArgsViaAnonymousObject.kt"); } + @TestMetadata("inheritedFunctionWithDefaultParameters.kt") + public void testInheritedFunctionWithDefaultParameters() throws Exception { + runTest("compiler/testData/codegen/box/jvm8/defaults/compatibility/inheritedFunctionWithDefaultParameters.kt"); + } + @TestMetadata("inheritedJvmDefault.kt") public void testInheritedJvmDefault() throws Exception { runTest("compiler/testData/codegen/box/jvm8/defaults/compatibility/inheritedJvmDefault.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java index 0d572f0471b..b0a19582cbb 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java @@ -13954,6 +13954,11 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes runTest("compiler/testData/codegen/box/jvm8/defaults/compatibility/defaultArgsViaAnonymousObject.kt"); } + @TestMetadata("inheritedFunctionWithDefaultParameters.kt") + public void testInheritedFunctionWithDefaultParameters() throws Exception { + runTest("compiler/testData/codegen/box/jvm8/defaults/compatibility/inheritedFunctionWithDefaultParameters.kt"); + } + @TestMetadata("inheritedJvmDefault.kt") public void testInheritedJvmDefault() throws Exception { runTest("compiler/testData/codegen/box/jvm8/defaults/compatibility/inheritedJvmDefault.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBytecodeTextTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBytecodeTextTestGenerated.java index 84ea87ec6fa..b6bf34f258e 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBytecodeTextTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBytecodeTextTestGenerated.java @@ -1515,6 +1515,11 @@ public class IrBytecodeTextTestGenerated extends AbstractIrBytecodeTextTest { KotlinTestUtils.assertAllTestsPresentByMetadataWithExcluded(this.getClass(), new File("compiler/testData/codegen/bytecodeText/defaultArguments"), Pattern.compile("^(.+)\\.kt$"), null, TargetBackend.JVM_IR, true); } + @TestMetadata("inheritedInterfaceFunction.kt") + public void testInheritedInterfaceFunction() throws Exception { + runTest("compiler/testData/codegen/bytecodeText/defaultArguments/inheritedInterfaceFunction.kt"); + } + @TestMetadata("kt11962.kt") public void testKt11962() throws Exception { runTest("compiler/testData/codegen/bytecodeText/defaultArguments/kt11962.kt");