From 74c6d2b9519a69818cc8b45ffbe11a6acc3f203e Mon Sep 17 00:00:00 2001 From: Alexander Udalov Date: Thu, 27 Aug 2020 17:30:32 +0200 Subject: [PATCH] Do not generate non-standard compareTo as primitive comparison in all backends Previous changes related to this in the old JVM backend were in 582b1c5e66acb167b36f3ac723364ba9ecf6c836 and 0482f7e9c5627ef076f66d9fd4a2c1832446a28f, but they did not affect the `ProperIeee754Comparisons` mode which became the default in 1.4.0. As a result, we had a regression here. Since the `PRIMITIVE_NUMERIC_COMPARISON_INFO` slice is used in psi2ir to determine how to generate the comparison, this fixes the regression both in the old JVM backend, and in all IR backends. #KT-41426 Fixed --- .../kotlin/codegen/ExpressionCodegen.java | 7 +-- .../ir/FirBlackBoxCodegenTestGenerated.java | 5 ++ .../PrimitiveNumericComparisonCallChecker.kt | 10 +++- .../compareTo/customCompareTo.kt | 58 +++++++++++++++++++ .../codegen/BlackBoxCodegenTestGenerated.java | 5 ++ .../LightAnalysisModeTestGenerated.java | 5 ++ .../ir/IrBlackBoxCodegenTestGenerated.java | 5 ++ .../IrJsCodegenBoxES6TestGenerated.java | 5 ++ .../IrJsCodegenBoxTestGenerated.java | 5 ++ .../semantics/JsCodegenBoxTestGenerated.java | 5 ++ 10 files changed, 105 insertions(+), 5 deletions(-) create mode 100644 compiler/testData/codegen/box/operatorConventions/compareTo/customCompareTo.kt diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/ExpressionCodegen.java b/compiler/backend/src/org/jetbrains/kotlin/codegen/ExpressionCodegen.java index 5b4b5db2aa9..7375dd17592 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/ExpressionCodegen.java +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/ExpressionCodegen.java @@ -4104,15 +4104,14 @@ public class ExpressionCodegen extends KtVisitor impleme TypeAndNullability right754Type = calcTypeForIeee754ArithmeticIfNeeded(right, getRightOperandType(primitiveNumericComparisonInfo)); boolean isSame754ArithmeticTypes = left754Type != null && right754Type != null && left754Type.type.equals(right754Type.type); boolean properIeee754Comparisons = shouldUseProperIeee754Comparisons(); + boolean isStandardCompareTo = primitiveNumericComparisonInfo != null; - if (properIeee754Comparisons && left754Type != null && right754Type != null) { + if (properIeee754Comparisons && isStandardCompareTo && left754Type != null && right754Type != null) { type = comparisonOperandType(left754Type.type, right754Type.type); - //type = comparisonOperandType(leftType, rightType); leftValue = gen(left); rightValue = gen(right); } - else if (!properIeee754Comparisons && - state.getIntrinsics().getIntrinsic((FunctionDescriptor) resolvedCall.getResultingDescriptor()) instanceof CompareTo && + else if (!properIeee754Comparisons && isStandardCompareTo && ((isPrimitive(leftType) && isPrimitive(rightType)) || isSame754ArithmeticTypes)) { type = isSame754ArithmeticTypes ? left754Type.type : comparisonOperandType(leftType, rightType); leftValue = gen(left); diff --git a/compiler/fir/fir2ir/tests/org/jetbrains/kotlin/codegen/ir/FirBlackBoxCodegenTestGenerated.java b/compiler/fir/fir2ir/tests/org/jetbrains/kotlin/codegen/ir/FirBlackBoxCodegenTestGenerated.java index 81f34f8f11a..42ae780e266 100644 --- a/compiler/fir/fir2ir/tests/org/jetbrains/kotlin/codegen/ir/FirBlackBoxCodegenTestGenerated.java +++ b/compiler/fir/fir2ir/tests/org/jetbrains/kotlin/codegen/ir/FirBlackBoxCodegenTestGenerated.java @@ -18925,6 +18925,11 @@ public class FirBlackBoxCodegenTestGenerated extends AbstractFirBlackBoxCodegenT runTest("compiler/testData/codegen/box/operatorConventions/compareTo/comparable.kt"); } + @TestMetadata("customCompareTo.kt") + public void testCustomCompareTo() throws Exception { + runTest("compiler/testData/codegen/box/operatorConventions/compareTo/customCompareTo.kt"); + } + @TestMetadata("doubleInt.kt") public void testDoubleInt() throws Exception { runTest("compiler/testData/codegen/box/operatorConventions/compareTo/doubleInt.kt"); diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/checkers/PrimitiveNumericComparisonCallChecker.kt b/compiler/frontend/src/org/jetbrains/kotlin/resolve/checkers/PrimitiveNumericComparisonCallChecker.kt index 463d7fbed80..41fb58ed048 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/checkers/PrimitiveNumericComparisonCallChecker.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/checkers/PrimitiveNumericComparisonCallChecker.kt @@ -6,6 +6,7 @@ package org.jetbrains.kotlin.resolve.checkers import com.intellij.psi.PsiElement +import org.jetbrains.kotlin.builtins.KotlinBuiltIns import org.jetbrains.kotlin.descriptors.TypeParameterDescriptor import org.jetbrains.kotlin.lexer.KtTokens import org.jetbrains.kotlin.psi.KtBinaryExpression @@ -36,6 +37,8 @@ object PrimitiveNumericComparisonCallChecker : CallChecker { val binaryExpression = resolvedCall.call.callElement as? KtBinaryExpression ?: return if (!comparisonOperatorTokens.contains(binaryExpression.operationReference.getReferencedNameElementType())) return + if (!resolvedCall.isStandardComparison()) return + val leftExpr = binaryExpression.left ?: return val rightExpr = binaryExpression.right ?: return @@ -68,6 +71,11 @@ object PrimitiveNumericComparisonCallChecker : CallChecker { ) } + private fun ResolvedCall<*>.isStandardComparison(): Boolean = + extensionReceiver == null && + dispatchReceiver != null && + KotlinBuiltIns.isUnderKotlinPackage(resultingDescriptor) + private fun leastCommonPrimitiveNumericType(t1: KotlinType, t2: KotlinType): KotlinType { val pt1 = t1.promoteIntegerTypeToIntIfRequired() val pt2 = t2.promoteIntegerTypeToIntIfRequired() @@ -112,4 +120,4 @@ object PrimitiveNumericComparisonCallChecker : CallChecker { else -> null } -} \ No newline at end of file +} diff --git a/compiler/testData/codegen/box/operatorConventions/compareTo/customCompareTo.kt b/compiler/testData/codegen/box/operatorConventions/compareTo/customCompareTo.kt new file mode 100644 index 00000000000..22a5b8733b2 --- /dev/null +++ b/compiler/testData/codegen/box/operatorConventions/compareTo/customCompareTo.kt @@ -0,0 +1,58 @@ +// IGNORE_BACKEND_FIR: JVM_IR + +var longCompareToInvocations = 0 +var doubleCompareToInvocations = 0 + +private operator fun Long?.compareTo(other: Long?): Int { + longCompareToInvocations++ + val diff = (this ?: 0L) - (other ?: 0L) + return when { + diff < 0L -> -1 + diff > 0L -> 1 + else -> 0 + } +} + +private operator fun Double?.compareTo(other: Double?): Int { + doubleCompareToInvocations++ + val diff = (this ?: 0.0) - (other ?: 0.0) + return when { + diff < 0.0 -> -1 + diff > 0.0 -> 1 + else -> 0 + } +} + +fun checkLong(): String? { + val a: Long? = null + val b: Long? = 42L + + if (a > b) return "Fail Long >" + if (a >= b) return "Fail Long >=" + if (!(a < b)) return "Fail Long <" + if (!(a <= b)) return "Fail Long <=" + + if (longCompareToInvocations != 4) return "Fail: expected 4 compareTo invocations, but was $longCompareToInvocations" + + return null +} + +fun checkDouble(): String? { + val a: Double? = null + val b: Double? = 3.14 + + if (a > b) return "Fail Double >" + if (a >= b) return "Fail Double >=" + if (!(a < b)) return "Fail Double <" + if (!(a <= b)) return "Fail Double <=" + + if (doubleCompareToInvocations != 4) return "Fail: expected 4 compareTo invocations, but was $doubleCompareToInvocations" + + return null +} + +fun box(): String { + checkLong()?.let { return it } + checkDouble()?.let { return it } + return "OK" +} diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java index 243aee6c85e..7260d404a0d 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java @@ -20516,6 +20516,11 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest { runTest("compiler/testData/codegen/box/operatorConventions/compareTo/comparable.kt"); } + @TestMetadata("customCompareTo.kt") + public void testCustomCompareTo() throws Exception { + runTest("compiler/testData/codegen/box/operatorConventions/compareTo/customCompareTo.kt"); + } + @TestMetadata("doubleInt.kt") public void testDoubleInt() throws Exception { runTest("compiler/testData/codegen/box/operatorConventions/compareTo/doubleInt.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java index cf2bc21f7c9..c6cda1bb1e7 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java @@ -20516,6 +20516,11 @@ public class LightAnalysisModeTestGenerated extends AbstractLightAnalysisModeTes runTest("compiler/testData/codegen/box/operatorConventions/compareTo/comparable.kt"); } + @TestMetadata("customCompareTo.kt") + public void testCustomCompareTo() throws Exception { + runTest("compiler/testData/codegen/box/operatorConventions/compareTo/customCompareTo.kt"); + } + @TestMetadata("doubleInt.kt") public void testDoubleInt() throws Exception { runTest("compiler/testData/codegen/box/operatorConventions/compareTo/doubleInt.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java index 583671af77d..31788a3955b 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java @@ -18925,6 +18925,11 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes runTest("compiler/testData/codegen/box/operatorConventions/compareTo/comparable.kt"); } + @TestMetadata("customCompareTo.kt") + public void testCustomCompareTo() throws Exception { + runTest("compiler/testData/codegen/box/operatorConventions/compareTo/customCompareTo.kt"); + } + @TestMetadata("doubleInt.kt") public void testDoubleInt() throws Exception { runTest("compiler/testData/codegen/box/operatorConventions/compareTo/doubleInt.kt"); diff --git a/js/js.tests/test/org/jetbrains/kotlin/js/test/es6/semantics/IrJsCodegenBoxES6TestGenerated.java b/js/js.tests/test/org/jetbrains/kotlin/js/test/es6/semantics/IrJsCodegenBoxES6TestGenerated.java index 0fd1352480f..3a1c2844c9c 100644 --- a/js/js.tests/test/org/jetbrains/kotlin/js/test/es6/semantics/IrJsCodegenBoxES6TestGenerated.java +++ b/js/js.tests/test/org/jetbrains/kotlin/js/test/es6/semantics/IrJsCodegenBoxES6TestGenerated.java @@ -15576,6 +15576,11 @@ public class IrJsCodegenBoxES6TestGenerated extends AbstractIrJsCodegenBoxES6Tes runTest("compiler/testData/codegen/box/operatorConventions/compareTo/comparable.kt"); } + @TestMetadata("customCompareTo.kt") + public void testCustomCompareTo() throws Exception { + runTest("compiler/testData/codegen/box/operatorConventions/compareTo/customCompareTo.kt"); + } + @TestMetadata("doubleInt.kt") public void testDoubleInt() throws Exception { runTest("compiler/testData/codegen/box/operatorConventions/compareTo/doubleInt.kt"); 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 70909b7ecc2..3e21d50e1cc 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 @@ -15576,6 +15576,11 @@ public class IrJsCodegenBoxTestGenerated extends AbstractIrJsCodegenBoxTest { runTest("compiler/testData/codegen/box/operatorConventions/compareTo/comparable.kt"); } + @TestMetadata("customCompareTo.kt") + public void testCustomCompareTo() throws Exception { + runTest("compiler/testData/codegen/box/operatorConventions/compareTo/customCompareTo.kt"); + } + @TestMetadata("doubleInt.kt") public void testDoubleInt() throws Exception { runTest("compiler/testData/codegen/box/operatorConventions/compareTo/doubleInt.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 719bdb603e4..08efd7d88ae 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 @@ -15681,6 +15681,11 @@ public class JsCodegenBoxTestGenerated extends AbstractJsCodegenBoxTest { runTest("compiler/testData/codegen/box/operatorConventions/compareTo/comparable.kt"); } + @TestMetadata("customCompareTo.kt") + public void testCustomCompareTo() throws Exception { + runTest("compiler/testData/codegen/box/operatorConventions/compareTo/customCompareTo.kt"); + } + @TestMetadata("doubleInt.kt") public void testDoubleInt() throws Exception { runTest("compiler/testData/codegen/box/operatorConventions/compareTo/doubleInt.kt");