diff --git a/src/corelib/kernel/qvariant.cpp b/src/corelib/kernel/qvariant.cpp index 22f2625b32..a8c5d756c7 100644 --- a/src/corelib/kernel/qvariant.cpp +++ b/src/corelib/kernel/qvariant.cpp @@ -2167,28 +2167,13 @@ static bool qIsFloatingPoint(uint tp) return tp == QMetaType::Double || tp == QMetaType::Float; } -static int normalizeLowerRanks(uint tp) -{ - static const qulonglong numericTypeBits = - Q_UINT64_C(1) << QMetaType::Bool | - Q_UINT64_C(1) << QMetaType::Char | - Q_UINT64_C(1) << QMetaType::SChar | - Q_UINT64_C(1) << QMetaType::UChar | - Q_UINT64_C(1) << QMetaType::Short | - Q_UINT64_C(1) << QMetaType::UShort; - return numericTypeBits & (Q_UINT64_C(1) << tp) ? uint(QMetaType::Int) : tp; -} - -static int normalizeLong(uint tp) -{ - const uint IntType = sizeof(long) == sizeof(int) ? QMetaType::Int : QMetaType::LongLong; - const uint UIntType = sizeof(ulong) == sizeof(uint) ? QMetaType::UInt : QMetaType::ULongLong; - return tp == QMetaType::Long ? IntType : - tp == QMetaType::ULong ? UIntType : tp; -} - -static int numericTypePromotion(uint t1, uint t2) +static int numericTypePromotion(const QtPrivate::QMetaTypeInterface *iface1, + const QtPrivate::QMetaTypeInterface *iface2) { + // We don't need QMetaType::id() here because the type Id is always stored + // directly for the types we're comparing against below. + uint t1 = iface1->typeId; + uint t2 = iface2->typeId; Q_ASSERT(qIsNumericType(t1)); Q_ASSERT(qIsNumericType(t2)); @@ -2214,26 +2199,30 @@ static int numericTypePromotion(uint t1, uint t2) if (qIsFloatingPoint(t1) || qIsFloatingPoint(t2)) return QMetaType::QReal; + auto isUnsigned = [](uint tp) { + // only types for which sizeof(T) >= sizeof(int); lesser ones promote to int + return tp == QMetaType::ULongLong || tp == QMetaType::ULong || + tp == QMetaType::UInt; + }; + bool isUnsigned1 = isUnsigned(t1); + bool isUnsigned2 = isUnsigned(t2); + // integral rules: - // for all platforms we support, int can always hold the values of lower-ranked types - t1 = normalizeLowerRanks(t1); - t2 = normalizeLowerRanks(t2); - - // normalize long / ulong: in all platforms we run, they're either the same as int or as long long - t1 = normalizeLong(t1); - t2 = normalizeLong(t2); - - // implement the other rules - // the four possibilities are Int, UInt, LongLong and ULongLong - // if any of the two is ULongLong, then it wins (highest rank, unsigned) - // otherwise, if one of the two is LongLong, then the other is either LongLong too or lower-ranked - // otherwise, if one of the two is UInt, then the other is either UInt too or Int - if (t1 == QMetaType::ULongLong || t2 == QMetaType::ULongLong) + // 1) if either type is a 64-bit unsigned, compare as 64-bit unsigned + if (isUnsigned1 && iface1->size > sizeof(int)) return QMetaType::ULongLong; - if (t1 == QMetaType::LongLong || t2 == QMetaType::LongLong) + if (isUnsigned2 && iface2->size > sizeof(int)) + return QMetaType::ULongLong; + + // 2) if either type is 64-bit, compare as 64-bit signed + if (iface1->size > sizeof(int) || iface2->size > sizeof(int)) return QMetaType::LongLong; - if (t1 == QMetaType::UInt || t2 == QMetaType::UInt) + + // 3) if either type is 32-bit unsigned, compare as 32-bit unsigned + if (isUnsigned1 || isUnsigned2) return QMetaType::UInt; + + // 4) otherwise, just do int promotion return QMetaType::Int; } @@ -2248,12 +2237,9 @@ static bool integralEquals(uint promotedType, const QVariant::Private *d1, const return int(*l1) == int(*l2); if (promotedType == QMetaType::UInt) return uint(*l1) == uint(*l2); - if (promotedType == QMetaType::LongLong) - return l1 == l2; if (promotedType == QMetaType::ULongLong) return qulonglong(*l1) == qulonglong(*l2); - - Q_UNREACHABLE_RETURN(0); + return l1 == l2; } namespace { @@ -2281,11 +2267,6 @@ static std::optional integralCompare(uint promotedType, const QVariant::Pri std::optional l2 = qConvertToNumber(d2, promotedType == QMetaType::Bool); if (!l1 || !l2) return std::nullopt; - - if (promotedType == QMetaType::Bool) - return spaceShip(*l1, *l2); - if (promotedType == QMetaType::Int) - return spaceShip(*l1, *l2); if (promotedType == QMetaType::UInt) return spaceShip(*l1, *l2); if (promotedType == QMetaType::LongLong) @@ -2293,12 +2274,12 @@ static std::optional integralCompare(uint promotedType, const QVariant::Pri if (promotedType == QMetaType::ULongLong) return spaceShip(*l1, *l2); - Q_UNREACHABLE_RETURN(0); + return spaceShip(*l1, *l2); } static std::optional numericCompare(const QVariant::Private *d1, const QVariant::Private *d2) { - uint promotedType = numericTypePromotion(d1->type().id(), d2->type().id()); + uint promotedType = numericTypePromotion(d1->typeInterface(), d2->typeInterface()); if (promotedType != QMetaType::QReal) return integralCompare(promotedType, d1, d2); @@ -2317,7 +2298,7 @@ static std::optional numericCompare(const QVariant::Private *d1, const QVar static bool numericEquals(const QVariant::Private *d1, const QVariant::Private *d2) { - uint promotedType = numericTypePromotion(d1->type().id(), d2->type().id()); + uint promotedType = numericTypePromotion(d1->typeInterface(), d2->typeInterface()); if (promotedType != QMetaType::QReal) return integralEquals(promotedType, d1, d2); diff --git a/src/testlib/qtest.h b/src/testlib/qtest.h index 7b845a61a9..f87995e0db 100644 --- a/src/testlib/qtest.h +++ b/src/testlib/qtest.h @@ -207,6 +207,19 @@ template<> inline char *toString(const QVariant &v) return qstrdup(vstring.constData()); } +template<> inline char *toString(const QPartialOrdering &o) +{ + if (o == QPartialOrdering::Less) + return qstrdup("Less"); + if (o == QPartialOrdering::Equivalent) + return qstrdup("Equivalent"); + if (o == QPartialOrdering::Greater) + return qstrdup("Greater"); + if (o == QPartialOrdering::Unordered) + return qstrdup("Unordered"); + return qstrdup(""); +} + namespace Internal { struct QCborValueFormatter { diff --git a/tests/auto/corelib/kernel/qvariant/tst_qvariant.cpp b/tests/auto/corelib/kernel/qvariant/tst_qvariant.cpp index 5f8f965973..e7579a7927 100644 --- a/tests/auto/corelib/kernel/qvariant/tst_qvariant.cpp +++ b/tests/auto/corelib/kernel/qvariant/tst_qvariant.cpp @@ -209,6 +209,8 @@ private slots: void variantHash(); void convertToQUint8() const; + void compareNumerics_data() const; + void compareNumerics() const; void comparePointers() const; void voidStar() const; void dataStar() const; @@ -2714,6 +2716,194 @@ void tst_QVariant::convertToQUint8() const } } +void tst_QVariant::compareNumerics_data() const +{ + QTest::addColumn("v1"); + QTest::addColumn("v2"); + QTest::addColumn("result"); + + QTest::addRow("invalid-invalid") + << QVariant() << QVariant() << QPartialOrdering::Unordered; + + static const auto asString = [](const QVariant &v) { + if (v.isNull()) + return QStringLiteral("null"); + switch (v.typeId()) { + case QMetaType::Char: + case QMetaType::Char16: + case QMetaType::Char32: + case QMetaType::UChar: + return QString::number(v.toUInt()); + case QMetaType::SChar: + return QString::number(v.toInt()); + } + return v.toString(); + }; + + auto addCompareToInvalid = [](auto value) { + QVariant v = QVariant::fromValue(value); + QTest::addRow("invalid-%s(%s)", v.typeName(), qPrintable(asString(v))) + << QVariant() << v << QPartialOrdering::Unordered; + QTest::addRow("%s(%s)-invalid", v.typeName(), qPrintable(asString(v))) + << v << QVariant() << QPartialOrdering::Unordered; + }; + addCompareToInvalid(false); + addCompareToInvalid(true); + addCompareToInvalid(char(0)); + addCompareToInvalid(qint8(0)); + addCompareToInvalid(quint8(0)); + addCompareToInvalid(short(0)); + addCompareToInvalid(ushort(0)); + addCompareToInvalid(int(0)); + addCompareToInvalid(uint(0)); + addCompareToInvalid(long(0)); + addCompareToInvalid(ulong(0)); + addCompareToInvalid(qint64(0)); + addCompareToInvalid(quint64(0)); + addCompareToInvalid(0.f); + addCompareToInvalid(0.0); + addCompareToInvalid(QCborSimpleType{}); + +QT_WARNING_PUSH +QT_WARNING_DISABLE_CLANG("-Wsign-compare") +QT_WARNING_DISABLE_GCC("-Wsign-compare") +QT_WARNING_DISABLE_MSVC(4018) // '<': signed/unsigned mismatch + static const auto addComparePair = [](auto value1, auto value2) { + QVariant v1 = QVariant::fromValue(value1); + QVariant v2 = QVariant::fromValue(value2); + QPartialOrdering order = QPartialOrdering::Unordered; + if (value1 == value2) + order = QPartialOrdering::Equivalent; + else if (value1 < value2) + order = QPartialOrdering::Less; + else if (value1 > value2) + order = QPartialOrdering::Greater; + QTest::addRow("%s(%s)-%s(%s)", v1.typeName(), qPrintable(asString(v1)), + v2.typeName(), qPrintable(asString(v2))) + << v1 << v2 << order; + }; +QT_WARNING_POP + + // homogeneous first + static const auto addList = [](auto list) { + for (auto v1 : list) + for (auto v2 : list) + addComparePair(v1, v2); + }; + + auto addSingleType = [](auto zero) { + using T = decltype(zero); + T one = T(zero + 1); + T min = std::numeric_limits::min(); + T max = std::numeric_limits::max(); + T mid = max / 2 + 1; + if (min != zero) + addList(std::array{zero, one, min, mid, max}); + else + addList(std::array{zero, one, mid, max}); + }; + addList(std::array{ false, true }); + addList(std::array{ QCborSimpleType{}, QCborSimpleType::False, QCborSimpleType(0xff) }); + addSingleType(char(0)); + addSingleType(qint8(0)); + addSingleType(quint8(0)); + addSingleType(qint16(0)); + addSingleType(quint16(0)); + addSingleType(qint32(0)); + addSingleType(quint32(0)); + addSingleType(qint64(0)); + addSingleType(quint64(0)); + addSingleType(0.f); + addSingleType(0.0); + + // heterogeneous + addComparePair(char(0), qint8(-127)); + addComparePair(char(127), qint8(127)); + addComparePair(char(127), quint8(127)); + addComparePair(qint8(-1), quint8(255)); + addComparePair(0U, -1); + addComparePair(~0U, -1); + addComparePair(Q_UINT64_C(0), -1); + addComparePair(~Q_UINT64_C(0), -1); + addComparePair(Q_UINT64_C(0), Q_INT64_C(-1)); + addComparePair(~Q_UINT64_C(0), Q_INT64_C(-1)); + addComparePair(INT_MAX, uint(INT_MAX)); + addComparePair(INT_MAX, qint64(INT_MAX) + 1); + addComparePair(INT_MAX, UINT_MAX); + addComparePair(INT_MAX, qint64(UINT_MAX)); + addComparePair(INT_MAX, qint64(UINT_MAX) + 1); + addComparePair(INT_MAX, quint64(UINT_MAX)); + addComparePair(INT_MAX, quint64(UINT_MAX) + 1); + addComparePair(INT_MAX, LONG_MIN); + addComparePair(INT_MAX, LONG_MAX); + addComparePair(INT_MAX, LLONG_MIN); + addComparePair(INT_MAX, LLONG_MAX); + addComparePair(INT_MIN, uint(INT_MIN)); + addComparePair(INT_MIN, uint(INT_MIN) + 1); + addComparePair(INT_MIN + 1, uint(INT_MIN)); + addComparePair(INT_MIN + 1, uint(INT_MIN) + 1); + addComparePair(INT_MIN, qint64(INT_MIN) - 1); + addComparePair(INT_MIN + 1, qint64(INT_MIN) + 1); + addComparePair(INT_MIN + 1, qint64(INT_MIN) - 1); + addComparePair(INT_MIN, UINT_MAX); + addComparePair(INT_MIN, qint64(UINT_MAX)); + addComparePair(INT_MIN, qint64(UINT_MAX) + 1); + addComparePair(INT_MIN, quint64(UINT_MAX)); + addComparePair(INT_MIN, quint64(UINT_MAX) + 1); + addComparePair(UINT_MAX, qint64(UINT_MAX) + 1); + addComparePair(UINT_MAX, quint64(UINT_MAX) + 1); + addComparePair(UINT_MAX, qint64(INT_MIN) - 1); + addComparePair(UINT_MAX, quint64(INT_MIN) + 1); + addComparePair(LLONG_MAX, quint64(LLONG_MAX)); + addComparePair(LLONG_MAX, quint64(LLONG_MAX) + 1); + addComparePair(LLONG_MIN, quint64(LLONG_MAX)); + addComparePair(LLONG_MIN, quint64(LLONG_MAX) + 1); + addComparePair(LLONG_MIN, quint64(LLONG_MIN) + 1); + addComparePair(LLONG_MIN + 1, quint64(LLONG_MIN) + 1); + addComparePair(LLONG_MIN, LLONG_MAX - 1); + addComparePair(LLONG_MIN, LLONG_MAX); + + // floating point + addComparePair(0.f, 0); + addComparePair(0.f, 0U); + addComparePair(0.f, Q_INT64_C(0)); + addComparePair(0.f, Q_UINT64_C(0)); + addComparePair(0.f, 0.); + addComparePair(0.f, 1.); + addComparePair(0.f, 1.); + addComparePair(float(1 << 24), 1 << 24); + addComparePair(float(1 << 24) - 1, (1 << 24) - 1); + addComparePair(-float(1 << 24), 1 << 24); + addComparePair(-float(1 << 24) + 1, -(1 << 24) + 1); + addComparePair(HUGE_VALF, qInf()); + addComparePair(HUGE_VALF, -qInf()); + addComparePair(qQNaN(), std::numeric_limits::quiet_NaN()); + if (sizeof(qreal) == sizeof(double)) { + addComparePair(std::numeric_limits::min(), std::numeric_limits::min()); + addComparePair(std::numeric_limits::min(), std::numeric_limits::min()); + addComparePair(std::numeric_limits::max(), std::numeric_limits::min()); + addComparePair(std::numeric_limits::max(), std::numeric_limits::max()); + addComparePair(double(Q_INT64_C(1) << 53), Q_INT64_C(1) << 53); + addComparePair(double(Q_INT64_C(1) << 53) - 1, (Q_INT64_C(1) << 53) - 1); + addComparePair(-double(Q_INT64_C(1) << 53), Q_INT64_C(1) << 53); + addComparePair(-double(Q_INT64_C(1) << 53) + 1, (Q_INT64_C(1) << 53) + 1); + } +} + +void tst_QVariant::compareNumerics() const +{ + QFETCH(QVariant, v1); + QFETCH(QVariant, v2); + QFETCH(QPartialOrdering, result); + QCOMPARE(QVariant::compare(v1, v2), result); + + QEXPECT_FAIL("invalid-invalid", "needs fixing", Continue); + if (result == QPartialOrdering::Equivalent) + QCOMPARE_EQ(v1, v2); + else + QCOMPARE_NE(v1, v2); +} + void tst_QVariant::comparePointers() const { class MyClass