AuroraMiddleware/qt5base-lts
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

QVariant: make a major simplification in the numeric comparison

Browse Source 
The code implementing the C++ rules of type promotion and conversion
was too pedantic. There's no need to follow the letter of the standard,
not when we can now assume that everything is two's complement (this was
true for all architectures we supported when I wrote this code in 2014,
but wasn't required by the standard).

So we can reduce this to fewer comparisons and fewer rules, using the
size of the type, not just the type ID.

Change-Id: I3d74c753055744deb8acfffd172446b02444c0c0
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>
This commit is contained in:
Thiago Macieira 2022-11-04 15:15:26 -07:00
parent 250ca8d5f8
commit 393d5efda3
3 changed files with 233 additions and 49 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

79
src/corelib/kernel/qvariant.cpp
View File

@ -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<int> integralCompare(uint promotedType, const QVariant::Pri
std::optional<qlonglong> l2 = qConvertToNumber(d2, promotedType == QMetaType::Bool);
if (!l1 || !l2)
return std::nullopt;
if (promotedType == QMetaType::Bool)
return spaceShip<bool>(*l1, *l2);
if (promotedType == QMetaType::Int)
return spaceShip<int>(*l1, *l2);
if (promotedType == QMetaType::UInt)
return spaceShip<uint>(*l1, *l2);
if (promotedType == QMetaType::LongLong)
@ -2293,12 +2274,12 @@ static std::optional<int> integralCompare(uint promotedType, const QVariant::Pri
if (promotedType == QMetaType::ULongLong)
return spaceShip<qulonglong>(*l1, *l2);
Q_UNREACHABLE_RETURN(0);
return spaceShip<int>(*l1, *l2);
}
static std::optional<int> 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<int> 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);

13
src/testlib/qtest.h
View File

@ -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("<invalid>");
}
namespace Internal {
struct QCborValueFormatter
{

190
tests/auto/corelib/kernel/qvariant/tst_qvariant.cpp
View File

@ -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<QVariant>("v1");
QTest::addColumn<QVariant>("v2");
QTest::addColumn<QPartialOrdering>("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<T>::min();
T max = std::numeric_limits<T>::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<float>::quiet_NaN());
if (sizeof(qreal) == sizeof(double)) {
addComparePair(std::numeric_limits<float>::min(), std::numeric_limits<double>::min());
addComparePair(std::numeric_limits<float>::min(), std::numeric_limits<double>::min());
addComparePair(std::numeric_limits<float>::max(), std::numeric_limits<double>::min());
addComparePair(std::numeric_limits<float>::max(), std::numeric_limits<double>::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