From 8669b8e60fc6a46baaeeea264a1ed3008dd52ea2 Mon Sep 17 00:00:00 2001
From: Fabian Kosmale <fabian.kosmale@qt.io>
Date: Wed, 11 Dec 2019 10:54:57 +0100
Subject: QVariant: Prefer direct conversion to QVariant{List,Map,Hash}

If a type has both a converter to QVariantList and to
QSequentialIterableImpl registered, we would have chosen the
QSequentialIterableImpl version. In the case of types like QJSValue,
this is more costly. With this change  we therefore uses the direct
conversion if it has been registered.

The same applies to QAssociativeIterableImpl and
QVariantHash/QVariantMap.

Change-Id: I9c0b5068efe4bfbc5e0598a200e6db59201e9974
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>
Reviewed-by: Olivier Goffart (Woboq GmbH) <ogoffart@woboq.com>
---
 src/corelib/kernel/qvariant.h                      | 11 +++---
 .../auto/corelib/kernel/qvariant/tst_qvariant.cpp  | 44 ++++++++++++++++++++++
 2 files changed, 49 insertions(+), 6 deletions(-)

diff --git a/src/corelib/kernel/qvariant.h b/src/corelib/kernel/qvariant.h
index a4957472ec..c95882d48f 100644
--- a/src/corelib/kernel/qvariant.h
+++ b/src/corelib/kernel/qvariant.h
@@ -801,7 +801,8 @@ namespace QtPrivate {
         static QVariantList invoke(const QVariant &v)
         {
             const int typeId = v.userType();
-            if (typeId == qMetaTypeId<QStringList>() || typeId == qMetaTypeId<QByteArrayList>() || QMetaType::hasRegisteredConverterFunction(typeId, qMetaTypeId<QtMetaTypePrivate::QSequentialIterableImpl>())) {
+            if (typeId == qMetaTypeId<QStringList>() || typeId == qMetaTypeId<QByteArrayList>() ||
+                (QMetaType::hasRegisteredConverterFunction(typeId, qMetaTypeId<QtMetaTypePrivate::QSequentialIterableImpl>()) && !QMetaType::hasRegisteredConverterFunction(typeId, qMetaTypeId<QVariantList>()))) {
                 QSequentialIterable iter = QVariantValueHelperInterface<QSequentialIterable>::invoke(v);
                 QVariantList l;
                 l.reserve(iter.size());
@@ -818,7 +819,7 @@ namespace QtPrivate {
         static QVariantHash invoke(const QVariant &v)
         {
             const int typeId = v.userType();
-            if (typeId == qMetaTypeId<QVariantMap>() || QMetaType::hasRegisteredConverterFunction(typeId, qMetaTypeId<QtMetaTypePrivate::QAssociativeIterableImpl>())) {
+            if (typeId == qMetaTypeId<QVariantMap>() || ((QMetaType::hasRegisteredConverterFunction(typeId, qMetaTypeId<QtMetaTypePrivate::QAssociativeIterableImpl>())) && !QMetaType::hasRegisteredConverterFunction(typeId, qMetaTypeId<QVariantHash>()))) {
                 QAssociativeIterable iter = QVariantValueHelperInterface<QAssociativeIterable>::invoke(v);
                 QVariantHash l;
                 l.reserve(iter.size());
@@ -835,7 +836,7 @@ namespace QtPrivate {
         static QVariantMap invoke(const QVariant &v)
         {
             const int typeId = v.userType();
-            if (typeId == qMetaTypeId<QVariantHash>() || QMetaType::hasRegisteredConverterFunction(typeId, qMetaTypeId<QtMetaTypePrivate::QAssociativeIterableImpl>())) {
+            if (typeId == qMetaTypeId<QVariantHash>() || (QMetaType::hasRegisteredConverterFunction(typeId, qMetaTypeId<QtMetaTypePrivate::QAssociativeIterableImpl>()) && !QMetaType::hasRegisteredConverterFunction(typeId, qMetaTypeId<QVariantMap>()))) {
                 QAssociativeIterable iter = QVariantValueHelperInterface<QAssociativeIterable>::invoke(v);
                 QVariantMap l;
                 for (QAssociativeIterable::const_iterator it = iter.begin(), end = iter.end(); it != end; ++it)
@@ -851,10 +852,8 @@ namespace QtPrivate {
         static QPair<QVariant, QVariant> invoke(const QVariant &v)
         {
             const int typeId = v.userType();
-            if (typeId == qMetaTypeId<QPair<QVariant, QVariant> >())
-                return QVariantValueHelper<QPair<QVariant, QVariant> >::invoke(v);
 
-            if (QMetaType::hasRegisteredConverterFunction(typeId, qMetaTypeId<QtMetaTypePrivate::QPairVariantInterfaceImpl>())) {
+            if (QMetaType::hasRegisteredConverterFunction(typeId, qMetaTypeId<QtMetaTypePrivate::QPairVariantInterfaceImpl>()) && !(typeId == qMetaTypeId<QPair<QVariant, QVariant> >())) {
                 QtMetaTypePrivate::QPairVariantInterfaceImpl pi = v.value<QtMetaTypePrivate::QPairVariantInterfaceImpl>();
 
                 const QtMetaTypePrivate::VariantData d1 = pi.first();
diff --git a/tests/auto/corelib/kernel/qvariant/tst_qvariant.cpp b/tests/auto/corelib/kernel/qvariant/tst_qvariant.cpp
index e719871128..45eb61f6e4 100644
--- a/tests/auto/corelib/kernel/qvariant/tst_qvariant.cpp
+++ b/tests/auto/corelib/kernel/qvariant/tst_qvariant.cpp
@@ -282,6 +282,8 @@ private slots:
     void fromStdVariant();
     void qt4UuidDataStream();
 
+    void preferDirectConversionOverInterfaces();
+
 private:
     void dataStream_data(QDataStream::Version version);
     void loadQVariantFromDataStream(QDataStream::Version version);
@@ -5140,5 +5142,47 @@ void tst_QVariant::qt4UuidDataStream()
     QCOMPARE(result.value<QUuid>(), source);
 }
 
+void tst_QVariant::preferDirectConversionOverInterfaces()
+{
+    using namespace QtMetaTypePrivate;
+    bool calledCorrectConverter = false;
+    QMetaType::registerConverter<MyType, QSequentialIterableImpl>([](const MyType &) {
+        return QSequentialIterableImpl {};
+    });
+    QMetaType::registerConverter<MyType, QVariantList>([&calledCorrectConverter](const MyType &) {
+        calledCorrectConverter = true;
+        return QVariantList {};
+    });
+    QMetaType::registerConverter<MyType, QAssociativeIterableImpl>([](const MyType &) {
+        return QAssociativeIterableImpl {};
+    });
+    QMetaType::registerConverter<MyType, QVariantHash>([&calledCorrectConverter](const MyType &) {
+        calledCorrectConverter = true;
+        return QVariantHash {};
+    });
+    QMetaType::registerConverter<MyType, QVariantMap>([&calledCorrectConverter](const MyType &) {
+        calledCorrectConverter = true;
+        return QVariantMap {};
+    });
+    auto holder = QVariant::fromValue(MyType {});
+
+    QVERIFY(holder.canConvert<QSequentialIterableImpl>());
+    QVERIFY(holder.canConvert<QVariantList>());
+    QVERIFY(holder.canConvert<QAssociativeIterableImpl>());
+    QVERIFY(holder.canConvert<QVariantHash>());
+    QVERIFY(holder.canConvert<QVariantMap>());
+
+    holder.value<QVariantList>();
+    QVERIFY(calledCorrectConverter);
+    calledCorrectConverter = false;
+
+    holder.value<QVariantHash>();
+    QVERIFY(calledCorrectConverter);
+    calledCorrectConverter = false;
+
+    holder.value<QVariantMap>();
+    QVERIFY(calledCorrectConverter);
+}
+
 QTEST_MAIN(tst_QVariant)
 #include "tst_qvariant.moc"
-- 
cgit v1.2.3