From 08d1c47ac368ad023a76a168df411ab4d1d75bdc Mon Sep 17 00:00:00 2001 From: Dmitry Petrov Date: Mon, 10 Dec 2018 17:29:28 +0300 Subject: [PATCH] KT-14227 Intrinsify MutableMap.set This fixes the most common (and rather annoying) bug in augmented assignment desugaring with collection element receiver. Fix is somewhat hackish: introduce an intrinsic for MutableMap.set, thus bypassing discrepancies in 'get' and 'set' call generation. Fixing it properly requires design decisions for corner cases where ad hoc augmented assignment desugaring with collection element receiver "accidentally" works, producing identical objects and vararg arrays for arguments of 'get' and 'set'. --- .../codegen/intrinsics/IntrinsicMethods.java | 2 ++ .../codegen/intrinsics/MutableMapSet.kt | 33 +++++++++++++++++++ .../box/operatorConventions/kt14227.kt | 13 ++++++++ .../codegen/BlackBoxCodegenTestGenerated.java | 5 +++ .../LightAnalysisModeTestGenerated.java | 5 +++ .../ir/IrBlackBoxCodegenTestGenerated.java | 5 +++ .../IrJsCodegenBoxTestGenerated.java | 5 +++ .../semantics/JsCodegenBoxTestGenerated.java | 5 +++ 8 files changed, 73 insertions(+) create mode 100644 compiler/backend/src/org/jetbrains/kotlin/codegen/intrinsics/MutableMapSet.kt create mode 100644 compiler/testData/codegen/box/operatorConventions/kt14227.kt diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/intrinsics/IntrinsicMethods.java b/compiler/backend/src/org/jetbrains/kotlin/codegen/intrinsics/IntrinsicMethods.java index 83d4a214ac2..7644fe2c6ae 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/intrinsics/IntrinsicMethods.java +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/intrinsics/IntrinsicMethods.java @@ -79,6 +79,8 @@ public class IntrinsicMethods { intrinsicsMap.registerIntrinsic(BUILT_INS_PACKAGE_FQ_NAME, null, "arrayOf", 1, new ArrayOf()); + intrinsicsMap.registerIntrinsic(new FqName("kotlin.collections"), new FqNameUnsafe("kotlin.collections.MutableMap"), "set", 2, new MutableMapSet()); + ImmutableList primitiveCastMethods = OperatorConventions.NUMBER_CONVERSIONS.asList(); for (Name method : primitiveCastMethods) { String methodName = method.asString(); diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/intrinsics/MutableMapSet.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/intrinsics/MutableMapSet.kt new file mode 100644 index 00000000000..6a371bcc603 --- /dev/null +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/intrinsics/MutableMapSet.kt @@ -0,0 +1,33 @@ +/* + * Copyright 2010-2018 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license + * that can be found in the license/LICENSE.txt file. + */ + +package org.jetbrains.kotlin.codegen.intrinsics + +import org.jetbrains.kotlin.codegen.Callable +import org.jetbrains.kotlin.codegen.CallableMethod +import org.jetbrains.org.objectweb.asm.Type + +/** + * This intrinsic mitigates issue KT-14227. + * + * `MutableMap.get` is an instance method, while `MutableMap.set` is an inline-only extension function from kotlin stdlib. + * This confuses very adhoc-ish code written in "old" JVM back-end for augmented assignment and increment/decrement operations desugaring, + * which produces incorrect bytecode for such rather trivial constructs. + * + * Fixing it in general case requires some design decisions for corner cases with arbitrary argument shapes and types for `get` and `set`. + */ +class MutableMapSet : IntrinsicMethod() { + override fun toCallable(method: CallableMethod): Callable = + object : IntrinsicCallable( + method, + { v -> + v.invokeinterface("java/util/Map", "put", "(Ljava/lang/Object;Ljava/lang/Object;)Ljava/lang/Object;") + v.pop() + } + ) { + override val parameterTypes: Array + get() = method.valueParameterTypes.toTypedArray() + } +} \ No newline at end of file diff --git a/compiler/testData/codegen/box/operatorConventions/kt14227.kt b/compiler/testData/codegen/box/operatorConventions/kt14227.kt new file mode 100644 index 00000000000..5a26f95ad07 --- /dev/null +++ b/compiler/testData/codegen/box/operatorConventions/kt14227.kt @@ -0,0 +1,13 @@ +// IGNORE_BACKEND: JS_IR +// WITH_RUNTIME +import kotlin.test.* + +fun box(): String { + val m = HashMap() + m["a"] = "A" + m["a"] += "B" + + assertEquals("AB", m["a"]) + + return "OK" +} diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java index e0c6026d27a..3ab7fd4ef69 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java @@ -16394,6 +16394,11 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest { runTest("compiler/testData/codegen/box/operatorConventions/kt14201_2.kt"); } + @TestMetadata("kt14227.kt") + public void testKt14227() throws Exception { + runTest("compiler/testData/codegen/box/operatorConventions/kt14227.kt"); + } + @TestMetadata("kt20387.kt") public void testKt20387() throws Exception { runTest("compiler/testData/codegen/box/operatorConventions/kt20387.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java index 3f7c45f9dbe..eb6e477b4c0 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java @@ -16394,6 +16394,11 @@ public class LightAnalysisModeTestGenerated extends AbstractLightAnalysisModeTes runTest("compiler/testData/codegen/box/operatorConventions/kt14201_2.kt"); } + @TestMetadata("kt14227.kt") + public void testKt14227() throws Exception { + runTest("compiler/testData/codegen/box/operatorConventions/kt14227.kt"); + } + @TestMetadata("kt20387.kt") public void testKt20387() throws Exception { runTest("compiler/testData/codegen/box/operatorConventions/kt20387.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java index a31d9c97c39..63eb43fcb32 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java @@ -16399,6 +16399,11 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes runTest("compiler/testData/codegen/box/operatorConventions/kt14201_2.kt"); } + @TestMetadata("kt14227.kt") + public void testKt14227() throws Exception { + runTest("compiler/testData/codegen/box/operatorConventions/kt14227.kt"); + } + @TestMetadata("kt20387.kt") public void testKt20387() throws Exception { runTest("compiler/testData/codegen/box/operatorConventions/kt20387.kt"); diff --git a/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/IrJsCodegenBoxTestGenerated.java b/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/IrJsCodegenBoxTestGenerated.java index e2434bc7734..6a3a8b7b8cf 100644 --- a/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/IrJsCodegenBoxTestGenerated.java +++ b/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/IrJsCodegenBoxTestGenerated.java @@ -13989,6 +13989,11 @@ public class IrJsCodegenBoxTestGenerated extends AbstractIrJsCodegenBoxTest { runTest("compiler/testData/codegen/box/operatorConventions/kt14201_2.kt"); } + @TestMetadata("kt14227.kt") + public void testKt14227() throws Exception { + runTest("compiler/testData/codegen/box/operatorConventions/kt14227.kt"); + } + @TestMetadata("kt20387.kt") public void testKt20387() throws Exception { runTest("compiler/testData/codegen/box/operatorConventions/kt20387.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 6ed4f02f0a9..d3e3840b59f 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 @@ -15044,6 +15044,11 @@ public class JsCodegenBoxTestGenerated extends AbstractJsCodegenBoxTest { runTest("compiler/testData/codegen/box/operatorConventions/kt14201_2.kt"); } + @TestMetadata("kt14227.kt") + public void testKt14227() throws Exception { + runTest("compiler/testData/codegen/box/operatorConventions/kt14227.kt"); + } + @TestMetadata("kt20387.kt") public void testKt20387() throws Exception { runTest("compiler/testData/codegen/box/operatorConventions/kt20387.kt");