Restrict comparison of variants

Comparing two variants will not try to convert the types
of the variant anymore. Exceptions are when both types are
numeric types or one type is numeric and the other one a
QString. The exceptions are there to keep compatibility with
C++ and to not completely break QSettings (which needs automatic
conversions from QString to numeric types).

[ChangeLog][Important Behavior Changes] Comparing two
variants in Qt 6 will not try attempt any type conversions before
comparing the variants anymore. Instead variants of different type
will not compare equal, with two exceptions: If both types are numeric
types they will get compared according to C++ type promotion rules. If
one type is a QString and the other type a numeric type, a conversion
from the string to the numeric tpye will be attempted.

Fixes: QTBUG-84636
Change-Id: I0cdd0b7259a525a41679fb6761f1e37e1d5b257f
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>

src/corelib/kernel/qvariant.cpp

@@ -174,13 +174,28 @@ static qulonglong qMetaTypeUNumber(const QVariant::Private *d)
     return 0;
 }
 
-static qlonglong qConvertToNumber(const QVariant::Private *d, bool *ok)
+static qlonglong qConvertToNumber(const QVariant::Private *d, bool *ok, bool allowStringToBool = false)
 {
     *ok = true;
 
     switch (uint(d->type().id())) {
-    case QMetaType::QString:
-        return v_cast<QString>(d)->toLongLong(ok);
+    case QMetaType::QString: {
+        const QString *s = v_cast<QString>(d);
+        qlonglong l = s->toLongLong(ok);
+        if (*ok)
+            return l;
+        if (allowStringToBool) {
+            if (*s == QLatin1String("false") || *s == QLatin1String("0")) {
+                *ok = true;
+                return 0;
+            }
+            if (*s == QLatin1String("true") || *s == QLatin1String("1")) {
+                *ok = true;
+                return 1;
+            }
+        }
+        return 0;
+    }
     case QMetaType::QChar:
         return v_cast<QChar>(d)->unicode();
     case QMetaType::QByteArray:
@@ -238,6 +253,8 @@ static qreal qConvertToRealNumber(const QVariant::Private *d, bool *ok)
 {
     *ok = true;
     switch (uint(d->type().id())) {
+    case QMetaType::QString:
+        return v_cast<QString>(d)->toDouble(ok);
     case QMetaType::Double:
         return qreal(d->data.d);
     case QMetaType::Float:
@@ -3727,21 +3744,45 @@ bool QVariant::convert(const int type, void *ptr) const
     equal; otherwise returns \c false.
 
     QVariant uses the equality operator of the type() it contains to
-    check for equality. QVariant will try to convert() \a v if its
-    type is not the same as this variant's type. See canConvert() for
-    a list of possible conversions.
+    check for equality.
+
+    Variants of different types will always compare as not equal with a few
+    exceptions:
+
+    \list
+    \i If both types are numeric types (integers and floatins point numbers)
+    Qt will compare those types using standard C++ type promotion rules.
+    \i If one type is numeric and the other one a QString, Qt will try to
+    convert the QString to a matching numeric type and if successful compare
+    those.
+    \endlist
 */
 
 /*!
     \fn bool QVariant::operator!=(const QVariant &v) const
 
     Compares this QVariant with \a v and returns \c true if they are not
     equal; otherwise returns \c false.
+
+    QVariant uses the equality operator of the type() it contains to
+    check for equality.
+
+    Variants of different types will always compare as not equal with a few
+    exceptions:
+
+    \list
+    \i If both types are numeric types (integers and floatins point numbers)
+    Qt will compare those types using standard C++ type promotion rules.
+    \i If one type is numeric and the other one a QString, Qt will try to
+    convert the QString to a matching numeric type and if successful compare
+    those.
+    \endlist
 */
 
 static bool qIsNumericType(uint tp)
 {
     static const qulonglong numericTypeBits =
+            Q_UINT64_C(1) << QMetaType::QString |
             Q_UINT64_C(1) << QMetaType::Bool |
             Q_UINT64_C(1) << QMetaType::Double |
             Q_UINT64_C(1) << QMetaType::Float |
@@ -3789,6 +3830,10 @@ static int numericTypePromotion(uint t1, uint t2)
     Q_ASSERT(qIsNumericType(t1));
     Q_ASSERT(qIsNumericType(t2));
 
+    if ((t1 == QMetaType::Bool && t2 == QMetaType::QString) ||
+        (t2 == QMetaType::Bool && t1 == QMetaType::QString))
+        return QMetaType::Bool;
+
     // C++ integral ranks: (4.13 Integer conversion rank [conv.rank])
     //   bool < signed char < short < int < long < long long
     //   unsigneds have the same rank as their signed counterparts
@@ -3830,53 +3875,59 @@ static int numericTypePromotion(uint t1, uint t2)
     return QMetaType::Int;
 }
 
-static int integralCompare(uint promotedType, const QVariant::Private *d1, const QVariant::Private *d2)
+static bool integralEquals(uint promotedType, const QVariant::Private *d1, const QVariant::Private *d2)
 {
     // use toLongLong to retrieve the data, it gets us all the bits
     bool ok;
-    qlonglong l1 = qConvertToNumber(d1, &ok);
-    Q_ASSERT(ok);
+    qlonglong l1 = qConvertToNumber(d1, &ok, promotedType == QMetaType::Bool);
+    if (!ok)
+        return false;
 
-    qlonglong l2 = qConvertToNumber(d2, &ok);
-    Q_ASSERT(ok);
+    qlonglong l2 = qConvertToNumber(d2, &ok, promotedType == QMetaType::Bool);
+    if (!ok)
+        return false;
 
+    if (promotedType == QMetaType::Bool)
+        return bool(l1) == bool(l2);
     if (promotedType == QMetaType::Int)
-        return int(l1) < int(l2) ? -1 : int(l1) == int(l2) ? 0 : 1;
+        return int(l1) == int(l2);
     if (promotedType == QMetaType::UInt)
-        return uint(l1) < uint(l2) ? -1 : uint(l1) == uint(l2) ? 0 : 1;
+        return uint(l1) == uint(l2);
     if (promotedType == QMetaType::LongLong)
-        return l1 < l2 ? -1 : l1 == l2 ? 0 : 1;
+        return l1 == l2;
     if (promotedType == QMetaType::ULongLong)
-        return qulonglong(l1) < qulonglong(l2) ? -1 : qulonglong(l1) == qulonglong(l2) ? 0 : 1;
+        return qulonglong(l1) == qulonglong(l2);
 
     Q_UNREACHABLE();
     return 0;
 }
 
-static int numericCompare(const QVariant::Private *d1, const QVariant::Private *d2)
+static bool numericEquals(const QVariant::Private *d1, const QVariant::Private *d2)
 {
     uint promotedType = numericTypePromotion(d1->type().id(), d2->type().id());
     if (promotedType != QMetaType::QReal)
-        return integralCompare(promotedType, d1, d2);
+        return integralEquals(promotedType, d1, d2);
 
     // qreal comparisons
     bool ok;
     qreal r1 = qConvertToRealNumber(d1, &ok);
-    Q_ASSERT(ok);
+    if (!ok)
+        return false;
     qreal r2 = qConvertToRealNumber(d2, &ok);
-    Q_ASSERT(ok);
+    if (!ok)
+        return false;
     if (r1 == r2)
-        return 0;
+        return true;
 
     // only do fuzzy comparisons for finite, non-zero numbers
     int c1 = qFpClassify(r1);
     int c2 = qFpClassify(r2);
     if ((c1 == FP_NORMAL || c1 == FP_SUBNORMAL) && (c2 == FP_NORMAL || c2 == FP_SUBNORMAL)) {
         if (qFuzzyCompare(r1, r2))
-            return 0;
+            return true;
     }
 
-    return r1 < r2 ? -1 : 1;
+    return false;
 }
 
 /*!
@@ -3885,26 +3936,19 @@ static int numericCompare(const QVariant::Private *d1, const QVariant::Private *
 bool QVariant::equals(const QVariant &v) const
 {
     auto metatype = d.type();
-    // try numerics first, with C++ type promotion rules (no conversion)
-    if (qIsNumericType(metatype.id()) && qIsNumericType(v.d.type().id()))
-        return numericCompare(&d, &v.d) == 0;
-
-    QVariant v1 = *this;
-    QVariant v2 = v;
-    if (v2.canConvert(v1.d.type().id())) {
-        if (!v2.convert(v1.d.type().id()))
-            return false;
-    } else {
-        // try the opposite conversion, it might work
-        qSwap(v1, v2);
-        if (!v2.convert(v1.d.type().id()))
-            return false;
+
+    if (metatype != v.metaType()) {
+        // try numeric comparisons, with C++ type promotion rules (no conversion)
+        if (qIsNumericType(metatype.id()) && qIsNumericType(v.d.type().id()))
+            return numericEquals(&d, &v.d);
+        return false;
     }
-    metatype = v1.metaType();
+
     // For historical reasons: QVariant() == QVariant()
     if (!metatype.isValid())
         return true;
-    return metatype.equals(QT_PREPEND_NAMESPACE(constData(v1.d)), QT_PREPEND_NAMESPACE(constData(v2.d)));
+
+    return metatype.equals(QT_PREPEND_NAMESPACE(constData(d)), QT_PREPEND_NAMESPACE(constData(v.d)));
 }
 
 /*!

tests/auto/corelib/kernel/qvariant/tst_qvariant.cpp

@@ -1491,38 +1491,38 @@ void tst_QVariant::operator_eq_eq_data()
 
     QTest::newRow( "double_int" ) << QVariant(42.0) << QVariant(42) << true;
     QTest::newRow( "float_int" ) << QVariant(42.f) << QVariant(42) << true;
-    QTest::newRow( "mInt_mIntString" ) << mInt << mIntString << true;
-    QTest::newRow( "mIntString_mInt" ) << mIntString << mInt << true;
+    QTest::newRow( "mInt_mIntString" ) << mInt << mIntString << false;
+    QTest::newRow( "mIntString_mInt" ) << mIntString << mInt << false;
     QTest::newRow( "mInt_mIntQString" ) << mInt << mIntQString << true;
     QTest::newRow( "mIntQString_mInt" ) << mIntQString << mInt << true;
 
-    QTest::newRow( "mUInt_mUIntString" ) << mUInt << mUIntString << true;
-    QTest::newRow( "mUIntString_mUInt" ) << mUIntString << mUInt << true;
+    QTest::newRow( "mUInt_mUIntString" ) << mUInt << mUIntString << false;
+    QTest::newRow( "mUIntString_mUInt" ) << mUIntString << mUInt << false;
     QTest::newRow( "mUInt_mUIntQString" ) << mUInt << mUIntQString << true;
     QTest::newRow( "mUIntQString_mUInt" ) << mUIntQString << mUInt << true;
 
-    QTest::newRow( "mDouble_mDoubleString" ) << mDouble << mDoubleString << true;
-    QTest::newRow( "mDoubleString_mDouble" ) << mDoubleString << mDouble << true;
+    QTest::newRow( "mDouble_mDoubleString" ) << mDouble << mDoubleString << false;
+    QTest::newRow( "mDoubleString_mDouble" ) << mDoubleString << mDouble << false;
     QTest::newRow( "mDouble_mDoubleQString" ) << mDouble << mDoubleQString << true;
     QTest::newRow( "mDoubleQString_mDouble" ) << mDoubleQString << mDouble << true;
 
-    QTest::newRow( "mFloat_mFloatString" ) << mFloat << mFloatString << true;
-    QTest::newRow( "mFloatString_mFloat" ) << mFloatString << mFloat << true;
+    QTest::newRow( "mFloat_mFloatString" ) << mFloat << mFloatString << false;
+    QTest::newRow( "mFloatString_mFloat" ) << mFloatString << mFloat << false;
     QTest::newRow( "mFloat_mFloatQString" ) << mFloat << mFloatQString << true;
     QTest::newRow( "mFloatQString_mFloat" ) << mFloatQString << mFloat << true;
 
-    QTest::newRow( "mLongLong_mLongLongString" ) << mLongLong << mLongLongString << true;
-    QTest::newRow( "mLongLongString_mLongLong" ) << mLongLongString << mLongLong << true;
+    QTest::newRow( "mLongLong_mLongLongString" ) << mLongLong << mLongLongString << false;
+    QTest::newRow( "mLongLongString_mLongLong" ) << mLongLongString << mLongLong << false;
     QTest::newRow( "mLongLong_mLongLongQString" ) << mLongLong << mLongLongQString << true;
     QTest::newRow( "mLongLongQString_mLongLong" ) << mLongLongQString << mLongLong << true;
 
-    QTest::newRow( "mULongLong_mULongLongString" ) << mULongLong << mULongLongString << true;
-    QTest::newRow( "mULongLongString_mULongLong" ) << mULongLongString << mULongLong << true;
+    QTest::newRow( "mULongLong_mULongLongString" ) << mULongLong << mULongLongString << false;
+    QTest::newRow( "mULongLongString_mULongLong" ) << mULongLongString << mULongLong << false;
     QTest::newRow( "mULongLong_mULongLongQString" ) << mULongLong << mULongLongQString << true;
     QTest::newRow( "mULongLongQString_mULongLong" ) << mULongLongQString << mULongLong << true;
 
-    QTest::newRow( "mBool_mBoolString" ) << mBool << mBoolString << true;
-    QTest::newRow( "mBoolString_mBool" ) << mBoolString << mBool << true;
+    QTest::newRow( "mBool_mBoolString" ) << mBool << mBoolString << false;
+    QTest::newRow( "mBoolString_mBool" ) << mBoolString << mBool << false;
     QTest::newRow( "mBool_mBoolQString" ) << mBool << mBoolQString << true;
     QTest::newRow( "mBoolQString_mBool" ) << mBoolQString << mBool << true;
 
@@ -1531,16 +1531,16 @@ void tst_QVariant::operator_eq_eq_data()
     QTest::newRow("char_char") << QVariant(QChar('a')) << QVariant(QChar('a')) << true;
     QTest::newRow("char_char2") << QVariant(QChar('a')) << QVariant(QChar('b')) << false;
 
-    QTest::newRow("invalidConversion") << QVariant(QString("bubu")) << QVariant(0) << false;
-    QTest::newRow("invalidConversionR") << QVariant(0) << QVariant(QString("bubu")) << false;
+    QTest::newRow("invalidConversion") << QVariant(QString("bubu")) << QVariant() << false;
+    QTest::newRow("invalidConversionR") << QVariant() << QVariant(QString("bubu")) << false;
     // ### many other combinations missing
 
     {
         QUuid uuid(0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10);
-        QTest::newRow("uuidstring") << QVariant(uuid) << QVariant(uuid.toString()) << true;
-        QTest::newRow("stringuuid") << QVariant(uuid.toString()) << QVariant(uuid) << true;
-        QTest::newRow("uuidbytearray") << QVariant(uuid) << QVariant(uuid.toByteArray()) << true;
-        QTest::newRow("bytearrayuuid") << QVariant(uuid.toByteArray()) << QVariant(uuid) << true;
+        QTest::newRow("uuidstring") << QVariant(uuid) << QVariant(uuid.toString()) << false;
+        QTest::newRow("stringuuid") << QVariant(uuid.toString()) << QVariant(uuid) << false;
+        QTest::newRow("uuidbytearray") << QVariant(uuid) << QVariant(uuid.toByteArray()) << false;
+        QTest::newRow("bytearrayuuid") << QVariant(uuid.toByteArray()) << QVariant(uuid) << false;
     }
 
     {
@@ -2234,7 +2234,9 @@ void tst_QVariant::variantMap()
     QVariant v3 = QVariant(QMetaType::type("QMap<QString, QVariant>"), &map);
     QCOMPARE(qvariant_cast<QVariantMap>(v3).value("test").toInt(), 42);
 
-    QCOMPARE(v, QVariant(v.toHash()));
+    QHash<QString, QVariant> hash;
+    hash["test"] = 42;
+    QCOMPARE(hash, v.toHash());
 }
 
 void tst_QVariant::variantHash()
@@ -2257,7 +2259,9 @@ void tst_QVariant::variantHash()
     QVariant v3 = QVariant(QMetaType::type("QHash<QString, QVariant>"), &hash);
     QCOMPARE(qvariant_cast<QVariantHash>(v3).value("test").toInt(), 42);
 
-    QCOMPARE(v, QVariant(v.toMap()));
+    QMap<QString, QVariant> map;
+    map["test"] = 42;
+    QCOMPARE(map, v.toMap());
 }
 
 class CustomQObject : public QObject {

tests/auto/corelib/serialization/json/tst_qtjson.cpp

@@ -1361,7 +1361,7 @@ void tst_QtJson::fromVariant_data()
     jsonObject["null"] = QJsonValue::Null;
     jsonObject["default"] = QJsonValue();
 
-    QTest::newRow("default") << QVariant() <<  QJsonValue(QJsonValue::Null);
+    QTest::newRow("default") << QVariant() <<  QJsonValue();
     QTest::newRow("nullptr") << QVariant::fromValue(nullptr) <<  QJsonValue(QJsonValue::Null);
     QTest::newRow("bool") << QVariant(boolValue) <<  QJsonValue(boolValue);
     QTest::newRow("int") << QVariant(intValue) <<  QJsonValue(intValue);
@@ -1391,6 +1391,14 @@ static QVariant normalizedVariant(const QVariant &v)
             out << normalizedVariant(v);
         return out;
     }
+    case QMetaType::QStringList: {
+        const QStringList in = v.toStringList();
+        QVariantList out;
+        out.reserve(in.size());
+        for (const QString &v : in)
+            out << v;
+        return out;
+    }
     case QMetaType::QVariantMap: {
         const QVariantMap in = v.toMap();
         QVariantMap out;
@@ -1400,7 +1408,7 @@ static QVariant normalizedVariant(const QVariant &v)
     }
     case QMetaType::QVariantHash: {
         const QVariantHash in = v.toHash();
-        QVariantHash out;
+        QVariantMap out;
         for (auto it = in.begin(); it != in.end(); ++it)
             out.insert(it.key(), normalizedVariant(it.value()));
         return out;