From 6d664bcd10a4850558481c57077f6ad765aa4d71 Mon Sep 17 00:00:00 2001 From: Alexander Udalov Date: Thu, 28 Apr 2022 23:01:59 +0200 Subject: [PATCH] JVM IR: fix operand type for CompareTo intrinsic It's incorrect to take the first parameter type from the expression itself because it can be nullable if smart casts are used. And if it's nullable, it's mapped to the wrapper type and calling `comparisonOperandType` for it makes no sense. Instead, take the type from the callee function, as it's guaranteed to be mapped to a JVM primitive type. E.g. in `test1` function in the added test, the problem was that the dispatch receiver type of the call expression is `Double?`, which is mapped to `java/lang/Double`, whereas we clearly wanted to obtain the primitive `D` (double) type. #KT-52163 Fixed --- .../org/jetbrains/kotlin/codegen/AsmUtil.java | 9 +++++++ .../kotlin/codegen/intrinsics/CompareTo.kt | 9 ++++++- .../codegen/intrinsics/IntrinsicMethods.java | 2 +- .../FirBlackBoxCodegenTestGenerated.java | 12 +++++++++ .../backend/jvm/intrinsics/CompareTo.kt | 26 ++++++++++++------- .../codegen/box/intrinsics/kt52163_boolean.kt | 8 ++++++ .../intrinsics/kt52163_doubleCompareToInt.kt | 17 ++++++++++++ .../codegen/BlackBoxCodegenTestGenerated.java | 12 +++++++++ .../IrBlackBoxCodegenTestGenerated.java | 12 +++++++++ .../LightAnalysisModeTestGenerated.java | 10 +++++++ .../js/test/JsCodegenBoxTestGenerated.java | 12 +++++++++ .../test/ir/IrJsCodegenBoxTestGenerated.java | 12 +++++++++ .../IrCodegenBoxWasmTestGenerated.java | 10 +++++++ .../NativeCodegenBoxTestGenerated.java | 12 +++++++++ 14 files changed, 151 insertions(+), 12 deletions(-) create mode 100644 compiler/testData/codegen/box/intrinsics/kt52163_boolean.kt create mode 100644 compiler/testData/codegen/box/intrinsics/kt52163_doubleCompareToInt.kt diff --git a/compiler/backend.common.jvm/src/org/jetbrains/kotlin/codegen/AsmUtil.java b/compiler/backend.common.jvm/src/org/jetbrains/kotlin/codegen/AsmUtil.java index 525ac90acbb..497728dc41a 100644 --- a/compiler/backend.common.jvm/src/org/jetbrains/kotlin/codegen/AsmUtil.java +++ b/compiler/backend.common.jvm/src/org/jetbrains/kotlin/codegen/AsmUtil.java @@ -275,6 +275,15 @@ public class AsmUtil { } public static Type comparisonOperandType(Type left, Type right) { + if (!isPrimitive(left) || !isPrimitive(right)) { + throw new IllegalArgumentException("Cannot compute comparison operand type: " + left + ", " + right); + } + if (left == Type.BOOLEAN_TYPE || right == Type.BOOLEAN_TYPE) { + if (left != Type.BOOLEAN_TYPE || right != Type.BOOLEAN_TYPE) { + throw new IllegalArgumentException("Cannot compare boolean with non-boolean: " + left + ", " + right); + } + return Type.BOOLEAN_TYPE; + } if (left == Type.DOUBLE_TYPE || right == Type.DOUBLE_TYPE) return Type.DOUBLE_TYPE; if (left == Type.FLOAT_TYPE || right == Type.FLOAT_TYPE) return Type.FLOAT_TYPE; if (left == Type.LONG_TYPE || right == Type.LONG_TYPE) return Type.LONG_TYPE; diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/intrinsics/CompareTo.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/intrinsics/CompareTo.kt index 0401cd8b4f2..d8c740ded3b 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/intrinsics/CompareTo.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/intrinsics/CompareTo.kt @@ -19,16 +19,23 @@ package org.jetbrains.kotlin.codegen.intrinsics import org.jetbrains.kotlin.codegen.AsmUtil.comparisonOperandType import org.jetbrains.kotlin.codegen.Callable import org.jetbrains.kotlin.codegen.CallableMethod +import org.jetbrains.kotlin.config.JvmTarget import org.jetbrains.org.objectweb.asm.Type import org.jetbrains.org.objectweb.asm.commons.InstructionAdapter -class CompareTo : IntrinsicMethod() { +class CompareTo(private val jvmTarget: JvmTarget) : IntrinsicMethod() { private fun genInvoke(type: Type?, v: InstructionAdapter) { when (type) { Type.INT_TYPE, Type.CHAR_TYPE -> v.invokestatic(IntrinsicMethods.INTRINSICS_CLASS_NAME, "compare", "(II)I", false) Type.LONG_TYPE -> v.lcmp() Type.FLOAT_TYPE -> v.invokestatic("java/lang/Float", "compare", "(FF)I", false) Type.DOUBLE_TYPE -> v.invokestatic("java/lang/Double", "compare", "(DD)I", false) + Type.BOOLEAN_TYPE -> { + check(jvmTarget >= JvmTarget.JVM_1_8) { + "Cannot generate boolean comparison for JVM target 1.6" + } + v.invokestatic("java/lang/Boolean", "compare", "(ZZ)I", false) + } else -> throw UnsupportedOperationException() } } 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 fa6979aaa9a..fa5a8101e82 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/intrinsics/IntrinsicMethods.java +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/intrinsics/IntrinsicMethods.java @@ -149,7 +149,7 @@ public class IntrinsicMethods { intrinsicsMap.registerIntrinsic(BUILT_INS_PACKAGE_FQ_NAME, null, "arrayOfNulls", 1, new NewArray()); for (PrimitiveType type : PrimitiveType.values()) { - declareIntrinsicFunction(type.getTypeFqName(), "compareTo", 1, new CompareTo()); + declareIntrinsicFunction(type.getTypeFqName(), "compareTo", 1, new CompareTo(jvmTarget)); declareIntrinsicFunction(COLLECTIONS_PACKAGE_FQ_NAME.child(Name.identifier(type.getTypeName().asString() + "Iterator")), "next", 0, ITERATOR_NEXT); } diff --git a/compiler/fir/fir2ir/tests-gen/org/jetbrains/kotlin/test/runners/codegen/FirBlackBoxCodegenTestGenerated.java b/compiler/fir/fir2ir/tests-gen/org/jetbrains/kotlin/test/runners/codegen/FirBlackBoxCodegenTestGenerated.java index a69ab7ff007..caefff1ab7b 100644 --- a/compiler/fir/fir2ir/tests-gen/org/jetbrains/kotlin/test/runners/codegen/FirBlackBoxCodegenTestGenerated.java +++ b/compiler/fir/fir2ir/tests-gen/org/jetbrains/kotlin/test/runners/codegen/FirBlackBoxCodegenTestGenerated.java @@ -25615,6 +25615,18 @@ public class FirBlackBoxCodegenTestGenerated extends AbstractFirBlackBoxCodegenT runTest("compiler/testData/codegen/box/intrinsics/kt12125_inc_2.kt"); } + @Test + @TestMetadata("kt52163_boolean.kt") + public void testKt52163_boolean() throws Exception { + runTest("compiler/testData/codegen/box/intrinsics/kt52163_boolean.kt"); + } + + @Test + @TestMetadata("kt52163_doubleCompareToInt.kt") + public void testKt52163_doubleCompareToInt() throws Exception { + runTest("compiler/testData/codegen/box/intrinsics/kt52163_doubleCompareToInt.kt"); + } + @Test @TestMetadata("kt5937.kt") public void testKt5937() throws Exception { diff --git a/compiler/ir/backend.jvm/codegen/src/org/jetbrains/kotlin/backend/jvm/intrinsics/CompareTo.kt b/compiler/ir/backend.jvm/codegen/src/org/jetbrains/kotlin/backend/jvm/intrinsics/CompareTo.kt index 8f651932339..92f7a5f2995 100644 --- a/compiler/ir/backend.jvm/codegen/src/org/jetbrains/kotlin/backend/jvm/intrinsics/CompareTo.kt +++ b/compiler/ir/backend.jvm/codegen/src/org/jetbrains/kotlin/backend/jvm/intrinsics/CompareTo.kt @@ -28,6 +28,7 @@ import org.jetbrains.kotlin.codegen.AsmUtil.comparisonOperandType import org.jetbrains.kotlin.codegen.BranchedValue import org.jetbrains.kotlin.codegen.NumberCompare import org.jetbrains.kotlin.codegen.ObjectCompare +import org.jetbrains.kotlin.config.JvmTarget import org.jetbrains.kotlin.config.LanguageFeature import org.jetbrains.kotlin.ir.expressions.IrFunctionAccessExpression import org.jetbrains.kotlin.lexer.KtSingleValueToken @@ -39,18 +40,21 @@ import org.jetbrains.org.objectweb.asm.Type import org.jetbrains.org.objectweb.asm.commons.InstructionAdapter object CompareTo : IntrinsicMethod() { - private fun genInvoke(type: Type?, v: InstructionAdapter) { + private fun genInvoke(type: Type?, v: InstructionAdapter, context: JvmBackendContext) { when (type) { Type.CHAR_TYPE, Type.BYTE_TYPE, Type.SHORT_TYPE, Type.INT_TYPE -> - v.invokestatic( - JvmSymbols.INTRINSICS_CLASS_NAME, - "compare", - "(II)I", - false - ) + v.invokestatic(JvmSymbols.INTRINSICS_CLASS_NAME, "compare", "(II)I", false) Type.LONG_TYPE -> v.invokestatic(JvmSymbols.INTRINSICS_CLASS_NAME, "compare", "(JJ)I", false) Type.FLOAT_TYPE -> v.invokestatic("java/lang/Float", "compare", "(FF)I", false) Type.DOUBLE_TYPE -> v.invokestatic("java/lang/Double", "compare", "(DD)I", false) + Type.BOOLEAN_TYPE -> { + // We could support it for JVM target 1.6, but it's prohibited now anyway (except for stdlib, which doesn't have such code), + // so throwing an exception instead. + check(context.state.target >= JvmTarget.JVM_1_8) { + "Cannot generate boolean comparison for JVM target 1.6" + } + v.invokestatic("java/lang/Boolean", "compare", "(ZZ)I", false) + } else -> throw UnsupportedOperationException() } } @@ -60,12 +64,14 @@ object CompareTo : IntrinsicMethod() { signature: JvmMethodSignature, context: JvmBackendContext ): IrIntrinsicFunction { + val callee = expression.symbol.owner + val calleeParameter = callee.dispatchReceiverParameter ?: callee.extensionReceiverParameter!! val parameterType = comparisonOperandType( - expressionType(expression.dispatchReceiver ?: expression.extensionReceiver!!, context), - signature.valueParameters.single().asmType + context.typeMapper.mapType(calleeParameter.type), + signature.valueParameters.single().asmType, ) return IrIntrinsicFunction.create(expression, signature, context, listOf(parameterType, parameterType)) { - genInvoke(parameterType, it) + genInvoke(parameterType, it, context) } } } diff --git a/compiler/testData/codegen/box/intrinsics/kt52163_boolean.kt b/compiler/testData/codegen/box/intrinsics/kt52163_boolean.kt new file mode 100644 index 00000000000..93696c55df0 --- /dev/null +++ b/compiler/testData/codegen/box/intrinsics/kt52163_boolean.kt @@ -0,0 +1,8 @@ +fun test(): Int { + val d: Any? + d = true + return d.compareTo(false) +} + +fun box(): String = + if (test() == 1) "OK" else "Fail" diff --git a/compiler/testData/codegen/box/intrinsics/kt52163_doubleCompareToInt.kt b/compiler/testData/codegen/box/intrinsics/kt52163_doubleCompareToInt.kt new file mode 100644 index 00000000000..b90ba51ecf6 --- /dev/null +++ b/compiler/testData/codegen/box/intrinsics/kt52163_doubleCompareToInt.kt @@ -0,0 +1,17 @@ +fun test1(): Int { + val d: Double? + d = 8.3 + return d.compareTo(8) +} + +fun test2(): Int { + val d: Double + d = 8.3 + return d.compareTo(8) +} + +fun box(): String { + if (test1() != 1) return "Fail test1" + if (test2() != 1) return "Fail test2" + return "OK" +} diff --git a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/BlackBoxCodegenTestGenerated.java b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/BlackBoxCodegenTestGenerated.java index 32d6398d5a8..e7c7caa81aa 100644 --- a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/BlackBoxCodegenTestGenerated.java +++ b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/BlackBoxCodegenTestGenerated.java @@ -25165,6 +25165,18 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest { runTest("compiler/testData/codegen/box/intrinsics/kt12125_inc_2.kt"); } + @Test + @TestMetadata("kt52163_boolean.kt") + public void testKt52163_boolean() throws Exception { + runTest("compiler/testData/codegen/box/intrinsics/kt52163_boolean.kt"); + } + + @Test + @TestMetadata("kt52163_doubleCompareToInt.kt") + public void testKt52163_doubleCompareToInt() throws Exception { + runTest("compiler/testData/codegen/box/intrinsics/kt52163_doubleCompareToInt.kt"); + } + @Test @TestMetadata("kt5937.kt") public void testKt5937() throws Exception { diff --git a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrBlackBoxCodegenTestGenerated.java b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrBlackBoxCodegenTestGenerated.java index 1c8d333a4bf..c199b37930e 100644 --- a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrBlackBoxCodegenTestGenerated.java +++ b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrBlackBoxCodegenTestGenerated.java @@ -25615,6 +25615,18 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes runTest("compiler/testData/codegen/box/intrinsics/kt12125_inc_2.kt"); } + @Test + @TestMetadata("kt52163_boolean.kt") + public void testKt52163_boolean() throws Exception { + runTest("compiler/testData/codegen/box/intrinsics/kt52163_boolean.kt"); + } + + @Test + @TestMetadata("kt52163_doubleCompareToInt.kt") + public void testKt52163_doubleCompareToInt() throws Exception { + runTest("compiler/testData/codegen/box/intrinsics/kt52163_doubleCompareToInt.kt"); + } + @Test @TestMetadata("kt5937.kt") public void testKt5937() throws Exception { diff --git a/compiler/tests-gen/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java b/compiler/tests-gen/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java index 37afdd40688..43a4c8b26f5 100644 --- a/compiler/tests-gen/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java +++ b/compiler/tests-gen/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java @@ -21170,6 +21170,16 @@ public class LightAnalysisModeTestGenerated extends AbstractLightAnalysisModeTes runTest("compiler/testData/codegen/box/intrinsics/kt12125_inc_2.kt"); } + @TestMetadata("kt52163_boolean.kt") + public void testKt52163_boolean() throws Exception { + runTest("compiler/testData/codegen/box/intrinsics/kt52163_boolean.kt"); + } + + @TestMetadata("kt52163_doubleCompareToInt.kt") + public void testKt52163_doubleCompareToInt() throws Exception { + runTest("compiler/testData/codegen/box/intrinsics/kt52163_doubleCompareToInt.kt"); + } + @TestMetadata("kt5937.kt") public void testKt5937() throws Exception { runTest("compiler/testData/codegen/box/intrinsics/kt5937.kt"); diff --git a/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/JsCodegenBoxTestGenerated.java b/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/JsCodegenBoxTestGenerated.java index f9854732e30..c8d1e138700 100644 --- a/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/JsCodegenBoxTestGenerated.java +++ b/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/JsCodegenBoxTestGenerated.java @@ -20253,6 +20253,18 @@ public class JsCodegenBoxTestGenerated extends AbstractJsCodegenBoxTest { runTest("compiler/testData/codegen/box/intrinsics/kt12125_inc_2.kt"); } + @Test + @TestMetadata("kt52163_boolean.kt") + public void testKt52163_boolean() throws Exception { + runTest("compiler/testData/codegen/box/intrinsics/kt52163_boolean.kt"); + } + + @Test + @TestMetadata("kt52163_doubleCompareToInt.kt") + public void testKt52163_doubleCompareToInt() throws Exception { + runTest("compiler/testData/codegen/box/intrinsics/kt52163_doubleCompareToInt.kt"); + } + @Test @TestMetadata("kt8666.kt") public void testKt8666() throws Exception { diff --git a/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/ir/IrJsCodegenBoxTestGenerated.java b/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/ir/IrJsCodegenBoxTestGenerated.java index 25b5a3a3e6d..5249e2ff776 100644 --- a/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/ir/IrJsCodegenBoxTestGenerated.java +++ b/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/ir/IrJsCodegenBoxTestGenerated.java @@ -20217,6 +20217,18 @@ public class IrJsCodegenBoxTestGenerated extends AbstractIrJsCodegenBoxTest { runTest("compiler/testData/codegen/box/intrinsics/kt12125_inc_2.kt"); } + @Test + @TestMetadata("kt52163_boolean.kt") + public void testKt52163_boolean() throws Exception { + runTest("compiler/testData/codegen/box/intrinsics/kt52163_boolean.kt"); + } + + @Test + @TestMetadata("kt52163_doubleCompareToInt.kt") + public void testKt52163_doubleCompareToInt() throws Exception { + runTest("compiler/testData/codegen/box/intrinsics/kt52163_doubleCompareToInt.kt"); + } + @Test @TestMetadata("kt8666.kt") public void testKt8666() throws Exception { diff --git a/js/js.tests/tests-gen/org/jetbrains/kotlin/js/testOld/wasm/semantics/IrCodegenBoxWasmTestGenerated.java b/js/js.tests/tests-gen/org/jetbrains/kotlin/js/testOld/wasm/semantics/IrCodegenBoxWasmTestGenerated.java index dae2c858546..796894ed1f8 100644 --- a/js/js.tests/tests-gen/org/jetbrains/kotlin/js/testOld/wasm/semantics/IrCodegenBoxWasmTestGenerated.java +++ b/js/js.tests/tests-gen/org/jetbrains/kotlin/js/testOld/wasm/semantics/IrCodegenBoxWasmTestGenerated.java @@ -17898,6 +17898,16 @@ public class IrCodegenBoxWasmTestGenerated extends AbstractIrCodegenBoxWasmTest runTest("compiler/testData/codegen/box/intrinsics/kt12125_inc_2.kt"); } + @TestMetadata("kt52163_boolean.kt") + public void testKt52163_boolean() throws Exception { + runTest("compiler/testData/codegen/box/intrinsics/kt52163_boolean.kt"); + } + + @TestMetadata("kt52163_doubleCompareToInt.kt") + public void testKt52163_doubleCompareToInt() throws Exception { + runTest("compiler/testData/codegen/box/intrinsics/kt52163_doubleCompareToInt.kt"); + } + @TestMetadata("kt8666.kt") public void testKt8666() throws Exception { runTest("compiler/testData/codegen/box/intrinsics/kt8666.kt"); diff --git a/native/native.tests/tests-gen/org/jetbrains/kotlin/konan/blackboxtest/NativeCodegenBoxTestGenerated.java b/native/native.tests/tests-gen/org/jetbrains/kotlin/konan/blackboxtest/NativeCodegenBoxTestGenerated.java index d6fe2ed5d08..831b9499701 100644 --- a/native/native.tests/tests-gen/org/jetbrains/kotlin/konan/blackboxtest/NativeCodegenBoxTestGenerated.java +++ b/native/native.tests/tests-gen/org/jetbrains/kotlin/konan/blackboxtest/NativeCodegenBoxTestGenerated.java @@ -22864,6 +22864,18 @@ public class NativeCodegenBoxTestGenerated extends AbstractNativeCodegenBoxTest runTest("compiler/testData/codegen/box/intrinsics/kt12125_inc_2.kt"); } + @Test + @TestMetadata("kt52163_boolean.kt") + public void testKt52163_boolean() throws Exception { + runTest("compiler/testData/codegen/box/intrinsics/kt52163_boolean.kt"); + } + + @Test + @TestMetadata("kt52163_doubleCompareToInt.kt") + public void testKt52163_doubleCompareToInt() throws Exception { + runTest("compiler/testData/codegen/box/intrinsics/kt52163_doubleCompareToInt.kt"); + } + @Test @TestMetadata("kt8666.kt") public void testKt8666() throws Exception {