Generate compareTo() properly

Take into account: 1) resolved call to compareTo (instead of always calling Comparable's method) 2) types of both caller and callee, when primitive, to avoid wrong casting (instead of always using caller's type) #KT-3078 Fixed
2012-11-27 22:09:57 +04:00
parent 0c5960922a
commit c9bdfd2f07
16 changed files with 261 additions and 22 deletions
@@ -311,15 +311,6 @@ public class AsmUtil {
        mv.visitInsn(L2I);
    }

-    static StackValue compareExpressionsOnStack(InstructionAdapter v, IElementType opToken, Type operandType) {
-        if (operandType.getSort() == Type.OBJECT) {
-            v.invokeinterface("java/lang/Comparable", "compareTo", "(Ljava/lang/Object;)I");
-            v.iconst(0);
-            operandType = Type.INT_TYPE;
-        }
-        return StackValue.cmp(opToken, operandType);
-    }
-
    static StackValue genNullSafeEquals(
            InstructionAdapter v,
            IElementType opToken,
@@ -394,7 +385,7 @@ public class AsmUtil {
            boolean rightNullable
    ) {
        if ((isNumberPrimitive(leftType) || leftType.getSort() == Type.BOOLEAN) && leftType == rightType) {
-            return compareExpressionsOnStack(v, opToken, leftType);
+            return StackValue.cmp(opToken, leftType);
        }
        else {
            if (opToken == JetTokens.EQEQEQ || opToken == JetTokens.EXCLEQEQEQ) {
@@ -2311,7 +2311,7 @@ public class ExpressionCodegen extends JetVisitor<StackValue, StackValue> implem
        }
        else if (opToken == JetTokens.LT || opToken == JetTokens.LTEQ ||
                 opToken == JetTokens.GT || opToken == JetTokens.GTEQ) {
-            return generateCompareOp(expression.getLeft(), expression.getRight(), opToken, expressionType(expression.getLeft()));
+            return generateCompareTo(expression);
        }
        else if (opToken == JetTokens.ELVIS) {
            return generateElvis(expression);
@@ -2519,10 +2519,25 @@ public class ExpressionCodegen extends JetVisitor<StackValue, StackValue> implem
        return StackValue.onStack(exprType);
    }

-    private StackValue generateCompareOp(JetExpression left, JetExpression right, IElementType opToken, Type operandType) {
-        gen(left, operandType);
-        gen(right, operandType);
-        return compareExpressionsOnStack(v, opToken, operandType);
+    private StackValue generateCompareTo(JetBinaryExpression expression) {
+        DeclarationDescriptor target = bindingContext.get(BindingContext.REFERENCE_TARGET, expression.getOperationReference());
+        assert target instanceof FunctionDescriptor : "compareTo target should be a function: " + target;
+        FunctionDescriptor descriptor = (FunctionDescriptor) target;
+
+        StackValue receiver = gen(expression.getLeft());
+        Callable callable = resolveToCallable(descriptor, false);
+
+        StackValue result;
+        if (callable instanceof IntrinsicMethod) {
+            result = ((IntrinsicMethod) callable).generate(
+                    this, v, Type.INT_TYPE, expression, Collections.singletonList(expression.getRight()), receiver, state);
+        } else {
+            result = invokeOperation(expression, descriptor, (CallableMethod) callable);
+        }
+
+        result.put(Type.INT_TYPE, v);
+        v.iconst(0);
+        return StackValue.cmp(expression.getOperationToken(), Type.INT_TYPE);
    }

    private StackValue generateAssignmentExpression(JetBinaryExpression expression) {
@@ -28,6 +28,8 @@ import org.jetbrains.jet.lang.psi.JetExpression;

 import java.util.List;

+import static org.jetbrains.jet.codegen.AsmUtil.isPrimitive;
+
 /**
 * @author alex.tkachman
 */
@@ -43,21 +45,25 @@ public class CompareTo implements IntrinsicMethod {
            @NotNull GenerationState state
    ) {
        assert arguments != null;
-        receiver.put(receiver.type, v);
-        codegen.gen(arguments.get(0), receiver.type);
-        if (receiver.type == Type.BYTE_TYPE || receiver.type == Type.SHORT_TYPE || receiver.type == Type.CHAR_TYPE) {
+        JetExpression argument = arguments.get(0);
+        Type type = comparisonOperandType(receiver.type, codegen.expressionType(argument));
+
+        receiver.put(type, v);
+        codegen.gen(argument, type);
+
+        if (type == Type.BYTE_TYPE || type == Type.SHORT_TYPE || type == Type.CHAR_TYPE) {
            v.sub(Type.INT_TYPE);
        }
-        else if (receiver.type == Type.INT_TYPE) {
+        else if (type == Type.INT_TYPE) {
            v.invokestatic("jet/runtime/Intrinsics", "compare", "(II)I");
        }
-        else if (receiver.type == Type.LONG_TYPE) {
+        else if (type == Type.LONG_TYPE) {
            v.invokestatic("jet/runtime/Intrinsics", "compare", "(JJ)I");
        }
-        else if (receiver.type == Type.FLOAT_TYPE) {
+        else if (type == Type.FLOAT_TYPE) {
            v.invokestatic("java/lang/Float", "compare", "(FF)I");
        }
-        else if (receiver.type == Type.DOUBLE_TYPE) {
+        else if (type == Type.DOUBLE_TYPE) {
            v.invokestatic("java/lang/Double", "compare", "(DD)I");
        }
        else {
@@ -65,4 +71,11 @@ public class CompareTo implements IntrinsicMethod {
        }
        return StackValue.onStack(Type.INT_TYPE);
    }
+
+    private static Type comparisonOperandType(Type left, Type right) {
+        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;
+        return Type.INT_TYPE;
+    }
 }
@@ -0,0 +1,16 @@
+trait A : Comparable<A>
+
+class B(val x: Int) : A {
+    override fun compareTo(other: A) = x.compareTo((other as B).x)
+}
+
+fun checkLess(x: A, y: A) = when {
+    x >= y    -> "Fail $x >= $y"
+    !(x < y)  -> "Fail !($x < $y)"
+    !(x <= y) -> "Fail !($x <= $y)"
+    x > y     -> "Fail $x > $y"
+    x.compareTo(y) >= 0 -> "Fail $x.compareTo($y) >= 0"
+    else -> "OK"
+}
+
+fun box() = checkLess(B(0), B(1))
@@ -0,0 +1,10 @@
+fun checkLess(x: Double, y: Int) = when {
+    x >= y    -> "Fail $x >= $y"
+    !(x < y)  -> "Fail !($x < $y)"
+    !(x <= y) -> "Fail !($x <= $y)"
+    x > y     -> "Fail $x > $y"
+    x.compareTo(y) >= 0 -> "Fail $x.compareTo($y) >= 0"
+    else -> "OK"
+}
+
+fun box() = checkLess(0.5, 1)
@@ -0,0 +1,10 @@
+fun checkLess(x: Double, y: Long) = when {
+    x >= y    -> "Fail $x >= $y"
+    !(x < y)  -> "Fail !($x < $y)"
+    !(x <= y) -> "Fail !($x <= $y)"
+    x > y     -> "Fail $x > $y"
+    x.compareTo(y) >= 0 -> "Fail $x.compareTo($y) >= 0"
+    else -> "OK"
+}
+
+fun box() = checkLess(0.5, 1.toLong())
@@ -0,0 +1,16 @@
+fun checkLess(x: Array<Int>, y: Array<Int>) = when {
+    x >= y    -> "Fail $x >= $y"
+    !(x < y)  -> "Fail !($x < $y)"
+    !(x <= y) -> "Fail !($x <= $y)"
+    x > y     -> "Fail $x > $y"
+    x.compareTo(y) >= 0 -> "Fail $x.compareTo($y) >= 0"
+    else -> "OK"
+}
+
+fun Array<Int>.compareTo(other: Array<Int>) = size - other.size
+
+fun box(): String {
+    val a = Array<Int>(0, {0})
+    val b = Array<Int>(1, {0})
+    return checkLess(a, b)
+}
@@ -0,0 +1,14 @@
+class A(val x: Int)
+
+fun A.compareTo(other: A) = x.compareTo(other.x)
+
+fun checkLess(x: A, y: A) = when {
+    x >= y    -> "Fail $x >= $y"
+    !(x < y)  -> "Fail !($x < $y)"
+    !(x <= y) -> "Fail !($x <= $y)"
+    x > y     -> "Fail $x > $y"
+    x.compareTo(y) >= 0 -> "Fail $x.compareTo($y) >= 0"
+    else -> "OK"
+}
+
+fun box() = checkLess(A(0), A(1))
@@ -0,0 +1,10 @@
+fun checkLess(x: Int, y: Double) = when {
+    x >= y    -> "Fail $x >= $y"
+    !(x < y)  -> "Fail !($x < $y)"
+    !(x <= y) -> "Fail !($x <= $y)"
+    x > y     -> "Fail $x > $y"
+    x.compareTo(y) >= 0 -> "Fail $x.compareTo($y) >= 0"
+    else -> "OK"
+}
+
+fun box() = checkLess(0, 0.5)
@@ -0,0 +1,10 @@
+fun checkLess(x: Int, y: Long) = when {
+    x >= y    -> "Fail $x >= $y"
+    !(x < y)  -> "Fail !($x < $y)"
+    !(x <= y) -> "Fail !($x <= $y)"
+    x > y     -> "Fail $x > $y"
+    x.compareTo(y) >= 0 -> "Fail $x.compareTo($y) >= 0"
+    else -> "OK"
+}
+
+fun box() = checkLess(0, 123456789123.toLong())
@@ -0,0 +1,10 @@
+fun checkLess(x: Long, y: Double) = when {
+    x >= y    -> "Fail $x >= $y"
+    !(x < y)  -> "Fail !($x < $y)"
+    !(x <= y) -> "Fail !($x <= $y)"
+    x > y     -> "Fail $x > $y"
+    x.compareTo(y) >= 0 -> "Fail $x.compareTo($y) >= 0"
+    else -> "OK"
+}
+
+fun box() = checkLess(0.toLong(), 0.5)
@@ -0,0 +1,10 @@
+fun checkLess(x: Long, y: Int) = when {
+    x >= y    -> "Fail $x >= $y"
+    !(x < y)  -> "Fail !($x < $y)"
+    !(x <= y) -> "Fail !($x <= $y)"
+    x > y     -> "Fail $x > $y"
+    x.compareTo(y) >= 0 -> "Fail $x.compareTo($y) >= 0"
+    else -> "OK"
+}
+
+fun box() = checkLess(-123456789123.toLong(), 0)
@@ -0,0 +1,7 @@
+fun box(): String {
+    if (1 >= 1.9) return "Fail #1"
+    if (1.compareTo(1.1) >= 0) return "Fail #2"
+    if (1.9 <= 1) return "Fail #3"
+    if (1.1.compareTo(1) <= 0) return "Fail #4"
+    return "OK"
+}
@@ -460,4 +460,8 @@ public class PrimitiveTypesTest extends CodegenTestCase {
    public void testUnboxComparable() {
        blackBoxFile("primitiveTypes/unboxComparable.kt");
    }
+
+    public void testKt3078() {
+        blackBoxFile("regressions/kt3078.kt");
+    }
 }
@@ -0,0 +1,96 @@
+/*
+ * Copyright 2010-2012 JetBrains s.r.o.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.jetbrains.jet.codegen.generated;
+
+import junit.framework.Assert;
+import junit.framework.Test;
+import junit.framework.TestSuite;
+
+import java.io.File;
+import org.jetbrains.jet.JetTestUtils;
+import org.jetbrains.jet.test.InnerTestClasses;
+import org.jetbrains.jet.test.TestMetadata;
+
+import org.jetbrains.jet.codegen.generated.AbstractCodegenTest;
+
+/** This class is generated by {@link org.jetbrains.jet.generators.tests.GenerateTests}. DO NOT MODIFY MANUALLY */
+@TestMetadata("compiler/testData/codegen/operatorConventions")
+@InnerTestClasses({OperatorConventionsTestGenerated.CompareTo.class})
+public class OperatorConventionsTestGenerated extends AbstractCodegenTest {
+    public void testAllFilesPresentInOperatorConventions() throws Exception {
+        JetTestUtils.assertAllTestsPresentByMetadata(this.getClass(), "org.jetbrains.jet.generators.tests.GenerateTests", new File("compiler/testData/codegen/operatorConventions"), "kt", true);
+    }
+    
+    @TestMetadata("compiler/testData/codegen/operatorConventions/compareTo")
+    public static class CompareTo extends AbstractCodegenTest {
+        public void testAllFilesPresentInCompareTo() throws Exception {
+            JetTestUtils.assertAllTestsPresentByMetadata(this.getClass(), "org.jetbrains.jet.generators.tests.GenerateTests", new File("compiler/testData/codegen/operatorConventions/compareTo"), "kt", true);
+        }
+
+        @TestMetadata("comparable.kt")
+        public void testComparable() throws Exception {
+            doTest("compiler/testData/codegen/operatorConventions/compareTo/comparable.kt");
+        }
+        
+        @TestMetadata("doubleInt.kt")
+        public void testDoubleInt() throws Exception {
+            doTest("compiler/testData/codegen/operatorConventions/compareTo/doubleInt.kt");
+        }
+        
+        @TestMetadata("doubleLong.kt")
+        public void testDoubleLong() throws Exception {
+            doTest("compiler/testData/codegen/operatorConventions/compareTo/doubleLong.kt");
+        }
+        
+        @TestMetadata("extensionArray.kt")
+        public void testExtensionArray() throws Exception {
+            doTest("compiler/testData/codegen/operatorConventions/compareTo/extensionArray.kt");
+        }
+        
+        @TestMetadata("extensionObject.kt")
+        public void testExtensionObject() throws Exception {
+            doTest("compiler/testData/codegen/operatorConventions/compareTo/extensionObject.kt");
+        }
+        
+        @TestMetadata("intDouble.kt")
+        public void testIntDouble() throws Exception {
+            doTest("compiler/testData/codegen/operatorConventions/compareTo/intDouble.kt");
+        }
+        
+        @TestMetadata("intLong.kt")
+        public void testIntLong() throws Exception {
+            doTest("compiler/testData/codegen/operatorConventions/compareTo/intLong.kt");
+        }
+        
+        @TestMetadata("longDouble.kt")
+        public void testLongDouble() throws Exception {
+            doTest("compiler/testData/codegen/operatorConventions/compareTo/longDouble.kt");
+        }
+        
+        @TestMetadata("longInt.kt")
+        public void testLongInt() throws Exception {
+            doTest("compiler/testData/codegen/operatorConventions/compareTo/longInt.kt");
+        }
+        
+    }
+    
+    public static Test suite() {
+        TestSuite suite = new TestSuite("OperatorConventionsTestGenerated");
+        suite.addTestSuite(OperatorConventionsTestGenerated.class);
+        suite.addTestSuite(CompareTo.class);
+        return suite;
+    }
+}
@@ -114,6 +114,13 @@ public class GenerateTests {
                testModel("compiler/testData/codegen/instructions")
        );

+        generateTest(
+                "compiler/tests/",
+                "OperatorConventionsTestGenerated",
+                AbstractCodegenTest.class,
+                testModel("compiler/testData/codegen/operatorConventions")
+        );
+
        generateTest(
                "compiler/tests/",
                "ExtensionPropertiesTestGenerated",