From 14f09643441cc2938dc2791e790f6309ab12cb1d Mon Sep 17 00:00:00 2001 From: Thiago Macieira Date: Mon, 19 Sep 2016 13:13:44 -0700 Subject: [PATCH] QVariant: don't do fuzzy comparisons with NaN and infinities There was a test that tested this, but was wrong. [ChangeLog][QtCore][QVariant] Fixed a bug that caused wrong results for comparisons of QVariants containing either NaN or infinite numbers. Task-number: QTBUG-56073 Change-Id: I33dc971f005a4848bb8ffffd1475d29d00dd1b7f Reviewed-by: Edward Welbourne Reviewed-by: Thiago Macieira --- src/corelib/kernel/qvariant.cpp | 12 +++++++++++- .../auto/corelib/kernel/qvariant/tst_qvariant.cpp | 15 +++++++++++++-- 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/src/corelib/kernel/qvariant.cpp b/src/corelib/kernel/qvariant.cpp index 9476ab3f5b..7c9df5b46f 100644 --- a/src/corelib/kernel/qvariant.cpp +++ b/src/corelib/kernel/qvariant.cpp @@ -74,6 +74,7 @@ #include "qline.h" #endif +#include #include #include @@ -3472,8 +3473,17 @@ static int numericCompare(const QVariant::Private *d1, const QVariant::Private * Q_ASSERT(ok); qreal r2 = qConvertToRealNumber(d2, &ok); Q_ASSERT(ok); - if (r1 == r2 || qFuzzyCompare(r1, r2)) + if (r1 == r2) return 0; + + // only do fuzzy comparisons for finite, non-zero numbers + int c1 = std::fpclassify(r1); + int c2 = std::fpclassify(r2); + if ((c1 == FP_NORMAL || c1 == FP_SUBNORMAL) && (c2 == FP_NORMAL || c2 == FP_SUBNORMAL)) { + if (qFuzzyCompare(r1, r2)) + return 0; + } + return r1 < r2 ? -1 : 1; } diff --git a/tests/auto/corelib/kernel/qvariant/tst_qvariant.cpp b/tests/auto/corelib/kernel/qvariant/tst_qvariant.cpp index 3a51e67768..442f7e80d1 100644 --- a/tests/auto/corelib/kernel/qvariant/tst_qvariant.cpp +++ b/tests/auto/corelib/kernel/qvariant/tst_qvariant.cpp @@ -1755,6 +1755,10 @@ void tst_QVariant::compareNumbers_data() const QTest::newRow("double5") << qVariantFromValue(0.) << qVariantFromValue(-qInf()) << +1; QTest::newRow("double6") << qVariantFromValue(-double(qInf())) << qVariantFromValue(-qInf()) << 0; QTest::newRow("double7") << qVariantFromValue(qInf()) << qVariantFromValue(qInf()) << 0; + QTest::newRow("double8") << qVariantFromValue(-qInf()) << qVariantFromValue(qInf()) << -1; + QTest::newRow("double9") << qVariantFromValue(qQNaN()) << qVariantFromValue(0.) << INT_MAX; + QTest::newRow("double10") << qVariantFromValue(0.) << qVariantFromValue(qQNaN()) << INT_MAX; + QTest::newRow("double11") << qVariantFromValue(qQNaN()) << qVariantFromValue(qQNaN()) << INT_MAX; // mixed comparisons // fp + fp @@ -1763,8 +1767,12 @@ void tst_QVariant::compareNumbers_data() const QTest::newRow("float+double3") << qVariantFromValue(0.f) << qVariantFromValue(-1.) << +1; QTest::newRow("float+double4") << qVariantFromValue(-float(qInf())) << qVariantFromValue(0.) << -1; QTest::newRow("float+double5") << qVariantFromValue(0.f) << qVariantFromValue(-qInf()) << +1; - QTest::newRow("float+double6") << qVariantFromValue(-float(qInf())) << qVariantFromValue(qInf()) << 0; + QTest::newRow("float+double6") << qVariantFromValue(-float(qInf())) << qVariantFromValue(-qInf()) << 0; QTest::newRow("float+double7") << qVariantFromValue(float(qInf())) << qVariantFromValue(qInf()) << 0; + QTest::newRow("float+double8") << qVariantFromValue(-float(qInf())) << qVariantFromValue(qInf()) << -1; + QTest::newRow("float+double9") << qVariantFromValue(qQNaN()) << qVariantFromValue(0.) << INT_MAX; + QTest::newRow("float+double10") << qVariantFromValue(0.) << qVariantFromValue(qQNaN()) << INT_MAX; + QTest::newRow("float+double11") << qVariantFromValue(qQNaN()) << qVariantFromValue(qQNaN()) << INT_MAX; // fp + int QTest::newRow("float+int1") << qVariantFromValue(0.f) << qVariantFromValue(0) << 0; @@ -1978,7 +1986,7 @@ void tst_QVariant::compareNumbers() const QCOMPARE(v2, v1); QVERIFY(v2 >= v1); QVERIFY(!(v2 > v1)); - } else { + } else if (expected == +1) { QVERIFY(!(v1 < v2)); QVERIFY(!(v1 <= v2)); QVERIFY(!(v1 == v2)); @@ -1990,6 +1998,9 @@ void tst_QVariant::compareNumbers() const QVERIFY(!(v2 == v1)); QVERIFY(!(v2 >= v1)); QVERIFY(!(v2 > v1)); + } else { + // unorderable (NaN) + QVERIFY(!(v1 == v2)); } }