Fix V4 calculations that vary from V8 results
authorMatthew Vogt <matthew.vogt@nokia.com>
Mon, 20 Aug 2012 06:42:11 +0000 (16:42 +1000)
committerQt by Nokia <qt-info@nokia.com>
Tue, 21 Aug 2012 22:20:59 +0000 (00:20 +0200)
Task-number: QTBUG-24706
Change-Id: I4475a772c5cd92776f2cb73dab9bc06ae4c019fa
Reviewed-by: Martin Jones <martin.jones@nokia.com>
src/qml/qml/v4/qv4bindings.cpp
src/qml/qml/v4/qv4program_p.h
tests/auto/qml/v4/data/mathMax.qml
tests/auto/qml/v4/data/mathMin.qml
tests/auto/qml/v4/tst_v4.cpp

index fb48454..6dce278 100644 (file)
@@ -84,13 +84,64 @@ QQmlAbstractBinding::VTable QV4Bindings_Binding_vtable = {
 };
 
 namespace {
+
+// The highest bit is the sign bit, in any endianness
+static const quint64 doubleSignMask = (quint64(0x1) << 63);
+
+inline bool signBitSet(const double &v)
+{
+    union { double d; quint64 u; } u;
+    u.d = v;
+    return (u.u & doubleSignMask);
+}
+
+inline double setSignBit(const double &v, bool b = true)
+{
+    union { double d; quint64 u; } u;
+
+    u.d = v;
+    if (b) {
+        u.u |= doubleSignMask;
+    } else {
+        u.u &= ~doubleSignMask;
+    }
+    return u.d;
+}
+
+inline double clearSignBit(const double &v, bool b = true)
+{
+    return setSignBit(v, !b);
+}
+
 struct Register {
     typedef QQmlRegisterType Type;
 
+    enum SpecialNumericValue {
+        NegativeZero = 1,
+        PositiveInfinity = 2,
+        NegativeInfinity = 3,
+        NotANumber = 4
+    };
+
     inline void setUndefined() { dataType = UndefinedType; }
+    inline bool isUndefined() const { return dataType == UndefinedType; }
+
     inline void setNull() { dataType = NullType; }
+
     inline void setNaN() { setnumber(qSNaN()); }
-    inline bool isUndefined() const { return dataType == UndefinedType; }
+    inline void setNaNType() { dataType = SpecialNumericType; intValue = NotANumber; } // non-numeric representation of NaN
+    inline bool isNaN() const { return (((dataType == SpecialNumericType) && (intValue == NotANumber)) ||
+                                        ((dataType == NumberType) && qIsNaN(numberValue))); }
+
+    inline void setInf(bool negative) { setnumber(setSignBit(qInf(), negative)); }
+    inline void setInfType(bool negative) { dataType = SpecialNumericType; intValue = (negative ? NegativeInfinity : PositiveInfinity); } // non-numeric representation of Inf
+    inline bool isInf() const { return (((dataType == SpecialNumericType) && ((intValue == NegativeInfinity) || (intValue == PositiveInfinity))) ||
+                                        ((dataType == NumberType) && qIsInf(numberValue))); }
+
+    inline void setNegativeZero() { setnumber(setSignBit(0)); }
+    inline void setNegativeZeroType() { dataType = SpecialNumericType; intValue = NegativeZero; } // non-numeric representation of -0
+    inline bool isNegativeZero() const { return (((dataType == SpecialNumericType) && (intValue == NegativeZero)) ||
+                                                 ((dataType == NumberType) && (numberValue == 0) && signBitSet(numberValue))); }
 
     inline void setQObject(QObject *o) { qobjectValue = o; dataType = QObjectStarType; }
     inline QObject *getQObject() const { return qobjectValue; }
@@ -1104,6 +1155,9 @@ void QV4Bindings::run(int instrIndex, quint32 &executedBlocks,
         const Register &src = registers[instr->unaryop.src];
         Register &output = registers[instr->unaryop.output];
         if (src.isUndefined()) output.setUndefined();
+        else if (src.isNaN()) output.setNaN();
+        else if (src.isInf()) output.setInf(src.getint() == Register::NegativeInfinity);
+        else if (src.isNegativeZero()) output.setNegativeZero();
         else output.setnumber(double(src.getint()));
     }
     QML_V4_END_INSTR(ConvertIntToNumber, unaryop)
@@ -1734,7 +1788,7 @@ void QV4Bindings::run(int instrIndex, quint32 &executedBlocks,
         const Register &src = registers[instr->unaryop.src];
         Register &output = registers[instr->unaryop.output];
         if (src.isUndefined()) output.setUndefined();
-        else output.setnumber(qAbs(src.getnumber()));
+        else output.setnumber(clearSignBit(qAbs(src.getnumber())));
     }
     QML_V4_END_INSTR(MathAbsNumber, unaryop)
 
@@ -1760,8 +1814,19 @@ void QV4Bindings::run(int instrIndex, quint32 &executedBlocks,
     {
         const Register &src = registers[instr->unaryop.src];
         Register &output = registers[instr->unaryop.output];
-        if (src.isUndefined()) output.setUndefined();
-        else output.setint(qCeil(src.getnumber()));
+        if (src.isUndefined())
+            output.setUndefined();
+        else if (src.isNaN())
+            // output should be an int, but still NaN
+            output.setNaNType();
+        else if (src.isInf())
+            // output should be an int, but still Inf
+            output.setInfType(signBitSet(src.getnumber()));
+        else if (src.isNegativeZero())
+            // output should be an int, but still -0
+            output.setNegativeZeroType();
+        else
+            output.setint(qCeil(src.getnumber()));
     }
     QML_V4_END_INSTR(MathCeilNumber, unaryop)
 
@@ -2101,8 +2166,21 @@ void QV4Bindings::run(int instrIndex, quint32 &executedBlocks,
         const Register &left = registers[instr->binaryop.left];
         const Register &right = registers[instr->binaryop.right];
         Register &output = registers[instr->binaryop.output];
-        if (left.isUndefined() || right.isUndefined()) output.setUndefined();
-        else output.setnumber(qMax(left.getnumber(), right.getnumber()));
+        if (left.isUndefined() || right.isUndefined()) {
+            output.setUndefined();
+        } else {
+            const double lhs = left.getnumber();
+            const double rhs = right.getnumber();
+            double result(lhs);
+            if (lhs == rhs) {
+                // If these are both zero, +0 is greater than -0
+                if (signBitSet(lhs) && !signBitSet(rhs))
+                    result = rhs;
+            } else {
+                result = qMax(lhs, rhs);
+            }
+            output.setnumber(result);
+        }
     }
     QML_V4_END_INSTR(MathMaxNumber, binaryop)
 
@@ -2111,8 +2189,21 @@ void QV4Bindings::run(int instrIndex, quint32 &executedBlocks,
         const Register &left = registers[instr->binaryop.left];
         const Register &right = registers[instr->binaryop.right];
         Register &output = registers[instr->binaryop.output];
-        if (left.isUndefined() || right.isUndefined()) output.setUndefined();
-        else output.setnumber(qMin(left.getnumber(), right.getnumber()));
+        if (left.isUndefined() || right.isUndefined()) {
+            output.setUndefined();
+        } else {
+            const double lhs = left.getnumber();
+            const double rhs = right.getnumber();
+            double result(lhs);
+            if (lhs == rhs) {
+                // If these are both zero, -0 is lesser than +0
+                if (!signBitSet(lhs) && signBitSet(rhs))
+                    result = rhs;
+            } else {
+                result = qMin(lhs, rhs);
+            }
+            output.setnumber(result);
+        }
     }
     QML_V4_END_INSTR(MathMinNumber, binaryop)
 
index d859a83..6d932d9 100644 (file)
@@ -95,6 +95,7 @@ enum QQmlRegisterType {
     FloatType,
     IntType,
     BoolType,
+    SpecialNumericType,
 
     PODValueType,
 
index 543b499..cd8a88a 100644 (file)
@@ -20,9 +20,13 @@ Item {
     property real subtest9: Math.max(i1.p10, i1.p9)
     property bool test9: subtest9 === 0 && (1/subtest9) === Infinity
 
-    property real test10: Math.max(i1.p11, i1.p1)
-    property real test11: Math.max(i1.p11, i1.p2)
-    property real test12: Math.max(i1.p1, i1.p2, i1.p3)
+    // Reverse the inputs to Math.max
+    property real subtest10: Math.max(i1.p9, i1.p10)
+    property bool test10: subtest10 === 0 && (1/subtest10) === Infinity
+
+    property real test11: Math.max(i1.p11, i1.p1)
+    property real test12: Math.max(i1.p11, i1.p2)
+    property real test13: Math.max(i1.p1, i1.p2, i1.p3)
 
     QtObject {
         id: i1
@@ -38,4 +42,4 @@ Item {
         property real p10: -0
         property var p11: null
     }
- }
+}
index 7d2a561..4ae5408 100644 (file)
@@ -20,9 +20,13 @@ Item {
     property real subtest9: Math.min(i1.p10, i1.p9)
     property bool test9: subtest9 === 0 && (1/subtest9) === -Infinity
 
-    property real test10: Math.min(i1.p11, i1.p1)
-    property real test11: Math.min(i1.p11, i1.p2)
-    property real test12: Math.min(i1.p1, i1.p2, i1.p3)
+    // Reverse the inputs to Math.min
+    property real subtest10: Math.min(i1.p9, i1.p10)
+    property bool test10: subtest10 === 0 && (1/subtest10) === -Infinity
+
+    property real test11: Math.min(i1.p11, i1.p1)
+    property real test12: Math.min(i1.p11, i1.p2)
+    property real test13: Math.min(i1.p1, i1.p2, i1.p3)
 
     QtObject {
         id: i1
index 90551e1..78d6b86 100644 (file)
@@ -142,13 +142,13 @@ void tst_v4::qtscript_data()
     QTest::newRow("unary minus") << "unaryMinus.qml";
     QTest::newRow("null qobject") << "nullQObject.qml";
     QTest::newRow("qobject -> bool") << "objectToBool.qml";
-    QTest::newRow("conversion from bool") << "conversions.1.qml"; // QTBUG-24706
-    QTest::newRow("conversion from int") << "conversions.2.qml"; // QTBUG-24706
+    QTest::newRow("conversion from bool") << "conversions.1.qml";
+    QTest::newRow("conversion from int") << "conversions.2.qml";
     QTest::newRow("conversion from float") << "conversions.3.qml";
-    QTest::newRow("conversion from double") << "conversions.4.qml"; // QTBUG-24706
-    QTest::newRow("conversion from real") << "conversions.5.qml"; // QTBUG-24706
-    QTest::newRow("conversion from string") << "conversions.6.qml"; // QTBUG-24706
-    QTest::newRow("conversion from url") << "conversions.7.qml"; // QTBUG-24706
+    QTest::newRow("conversion from double") << "conversions.4.qml";
+    QTest::newRow("conversion from real") << "conversions.5.qml";
+    QTest::newRow("conversion from string") << "conversions.6.qml";
+    QTest::newRow("conversion from url") << "conversions.7.qml";
     QTest::newRow("conversion from vec3") << "conversions.8.qml";
     QTest::newRow("variantHandling") << "variantHandling.qml";
     QTest::newRow("varHandling") << "varHandling.qml";
@@ -536,7 +536,7 @@ void tst_v4::mathAbs()
     QCOMPARE(o->property("test9").toBool(), true);
     QCOMPARE(o->property("test10").toBool(), true);
     QCOMPARE(o->property("test11").toInt(), 0);
-    //QCOMPARE(o->property("test12").toBool(), true);   //QTBUG-24706
+    QCOMPARE(o->property("test12").toBool(), true);
 
     delete o;
 }
@@ -551,13 +551,13 @@ void tst_v4::mathCeil()
     QCOMPARE(o->property("test1").toReal(), qreal(-3));
     QCOMPARE(o->property("test2").toReal(), qreal(5));
     QCOMPARE(o->property("test3").toBool(), true);
-    //QCOMPARE(o->property("test4").toBool(), true);    //QTBUG-24706
+    QCOMPARE(o->property("test4").toBool(), true);
     QCOMPARE(o->property("test5").toBool(), true);
     QCOMPARE(o->property("test6").toReal(), qreal(83));
-    //QCOMPARE(o->property("test7").toBool(), true);    //QTBUG-24706
-    //QCOMPARE(o->property("test8").toBool(), true);    //QTBUG-24706
+    QCOMPARE(o->property("test7").toBool(), true);
+    QCOMPARE(o->property("test8").toBool(), true);
     QCOMPARE(o->property("test9").toInt(), 0);
-    //QCOMPARE(o->property("test10").toBool(), true);   //QTBUG-24706
+    QCOMPARE(o->property("test10").toBool(), true);
 
     delete o;
 }
@@ -577,10 +577,11 @@ void tst_v4::mathMax()
     QCOMPARE(o->property("test6").toReal(), qreal(82.6));
     QCOMPARE(o->property("test7").toReal(), qreal(4.4));
     QCOMPARE(o->property("test8").toBool(), true);
-    //QCOMPARE(o->property("test9").toBool(), true);    //QTBUG-24706
-    QCOMPARE(o->property("test10").toReal(), qreal(0));
-    QCOMPARE(o->property("test11").toReal(), qreal(4.4));
-    QCOMPARE(o->property("test12").toReal(), qreal(7));
+    QCOMPARE(o->property("test9").toBool(), true);
+    QCOMPARE(o->property("test10").toBool(), true);
+    QCOMPARE(o->property("test11").toReal(), qreal(0));
+    QCOMPARE(o->property("test12").toReal(), qreal(4.4));
+    QCOMPARE(o->property("test13").toReal(), qreal(7));
 
     delete o;
 }
@@ -600,10 +601,11 @@ void tst_v4::mathMin()
     QCOMPARE(o->property("test6").toReal(), qreal(82.6));
     QCOMPARE(o->property("test7").toBool(), true);
     QCOMPARE(o->property("test8").toReal(), qreal(4.4));
-    //QCOMPARE(o->property("test9").toBool(), true);    //QTBUG-24706
-    QCOMPARE(o->property("test10").toReal(), qreal(-3.7));
-    QCOMPARE(o->property("test11").toReal(), qreal(0));
-    QCOMPARE(o->property("test12").toReal(), qreal(-3.7));
+    QCOMPARE(o->property("test9").toBool(), true);
+    QCOMPARE(o->property("test10").toBool(), true);
+    QCOMPARE(o->property("test11").toReal(), qreal(-3.7));
+    QCOMPARE(o->property("test12").toReal(), qreal(0));
+    QCOMPARE(o->property("test13").toReal(), qreal(-3.7));
     delete o;
 }
 
@@ -726,7 +728,6 @@ void tst_v4::conversions_data()
             << QUrl(testFileUrl("").toString() + QString(QLatin1String("4")))
             << QVector3D(4, 4, 4);
 
-    // QTBUG-24706
     QTest::newRow("from url") << testFileUrl("conversions.7.qml")
             << (QStringList() << (testFileUrl("conversions.7.qml").toString() + QLatin1String(":6:14: Unable to assign QUrl to int"))
                               << (testFileUrl("conversions.7.qml").toString() + QLatin1String(":7:16: Unable to assign QUrl to number"))