Fix movablity of QVariant.

After 8fd64d22ac7892b061a09c42c72aacf033b80876 (Make usage of internal
QVariant space.) change QVariant started to "inherit" movablity from
interned type.

This change fix it by interning only movable type in QVariant and by
using external allocation for not movable ones.

Obviously, this change has negative impact on QVariant it self, but
after it, QVariant will behave a lot nicer with our containers.

Change-Id: Ibffc95833918f65be737f52d694ee81a2036c412
Reviewed-by: Friedemann Kleint <Friedemann.Kleint@nokia.com>
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>

2 files changed, 59 insertions, 26 deletions

diff --git a/src/corelib/kernel/qvariant_p.h b/src/corelib/kernel/qvariant_p.h
index 3c2176856d..f7f1399312 100644
--- a/src/corelib/kernel/qvariant_p.h
+++ b/src/corelib/kernel/qvariant_p.h
@@ -58,20 +58,33 @@
 
 #include <QtCore/qglobal.h>
 #include <QtCore/qvariant.h>
+#include <QtCore/private/qmetatype_p.h>
 
 #include "qmetatypeswitcher_p.h"
 
 QT_BEGIN_NAMESPACE
 
+namespace {
+template<typename T>
+struct QVariantIntegrator
+{
+    static const bool CanUseInternalSpace = sizeof(T) <= sizeof(QVariant::Private::Data)
+                                            && (!QTypeInfo<T>::isStatic);
+};
+Q_STATIC_ASSERT(QVariantIntegrator<double>::CanUseInternalSpace);
+Q_STATIC_ASSERT(QVariantIntegrator<long int>::CanUseInternalSpace);
+Q_STATIC_ASSERT(QVariantIntegrator<qulonglong>::CanUseInternalSpace);
+} // namespace
+
 #ifdef Q_CC_SUN // Sun CC picks the wrong overload, so introduce awful hack
 
 template <typename T>
 inline T *v_cast(const QVariant::Private *nd, T * = 0)
 {
     QVariant::Private *d = const_cast<QVariant::Private *>(nd);
-    return ((sizeof(T) > sizeof(QVariant::Private::Data))
+    return !QVariantIntegrator<T>::CanUseInternalSpace
             ? static_cast<T *>(d->data.shared->ptr)
-            : static_cast<T *>(static_cast<void *>(&d->data.c)));
+            : static_cast<T *>(static_cast<void *>(&d->data.c));
 }
 
 #else // every other compiler in this world
@@ -79,17 +92,17 @@ inline T *v_cast(const QVariant::Private *nd, T * = 0)
 template <typename T>
 inline const T *v_cast(const QVariant::Private *d, T * = 0)
 {
-    return ((sizeof(T) > sizeof(QVariant::Private::Data))
+    return !QVariantIntegrator<T>::CanUseInternalSpace
             ? static_cast<const T *>(d->data.shared->ptr)
-            : static_cast<const T *>(static_cast<const void *>(&d->data.c)));
+            : static_cast<const T *>(static_cast<const void *>(&d->data.c));
 }
 
 template <typename T>
 inline T *v_cast(QVariant::Private *d, T * = 0)
 {
-    return ((sizeof(T) > sizeof(QVariant::Private::Data))
+    return !QVariantIntegrator<T>::CanUseInternalSpace
             ? static_cast<T *>(d->data.shared->ptr)
-            : static_cast<T *>(static_cast<void *>(&d->data.c)));
+            : static_cast<T *>(static_cast<void *>(&d->data.c));
 }
 
 #endif
@@ -110,7 +123,7 @@ private:
 template <class T>
 inline void v_construct(QVariant::Private *x, const void *copy, T * = 0)
 {
-    if (sizeof(T) > sizeof(QVariant::Private::Data)) {
+    if (!QVariantIntegrator<T>::CanUseInternalSpace) {
         x->data.shared = copy ? new QVariantPrivateSharedEx<T>(*static_cast<const T *>(copy))
                               : new QVariantPrivateSharedEx<T>;
         x->is_shared = true;
@@ -125,7 +138,7 @@ inline void v_construct(QVariant::Private *x, const void *copy, T * = 0)
 template <class T>
 inline void v_construct(QVariant::Private *x, const T &t)
 {
-    if (sizeof(T) > sizeof(QVariant::Private::Data)) {
+    if (!QVariantIntegrator<T>::CanUseInternalSpace) {
         x->data.shared = new QVariantPrivateSharedEx<T>(t);
         x->is_shared = true;
     } else {
@@ -138,7 +151,7 @@ template <class T>
 inline void v_clear(QVariant::Private *d, T* = 0)
 {
     
-    if (sizeof(T) > sizeof(QVariant::Private::Data)) {
+    if (!QVariantIntegrator<T>::CanUseInternalSpace) {
         //now we need to cast
         //because QVariant::PrivateShared doesn't have a virtual destructor
         delete static_cast<QVariantPrivateSharedEx<T>*>(d->data.shared);
@@ -293,11 +306,11 @@ protected:
 template<class Filter>
 class QVariantConstructor
 {
-    template<typename T, bool IsSmall = (sizeof(T) <= sizeof(QVariant::Private::Data))>
+    template<typename T, bool CanUseInternalSpace = QVariantIntegrator<T>::CanUseInternalSpace>
     struct CallConstructor {};
 
     template<typename T>
-    struct CallConstructor<T, /* IsSmall = */ true>
+    struct CallConstructor<T, /* CanUseInternalSpace = */ true>
     {
         CallConstructor(const QVariantConstructor &tc)
         {
@@ -310,7 +323,7 @@ class QVariantConstructor
     };
 
     template<typename T>
-    struct CallConstructor<T, /* IsSmall = */ false>
+    struct CallConstructor<T, /* CanUseInternalSpace = */ false>
     {
         CallConstructor(const QVariantConstructor &tc)
         {
@@ -359,23 +372,21 @@ public:
             m_x->is_shared = false;
             return;
         }
-        // it is not a static known type, lets ask QMetaType if it can be constructed for us.
         const uint size = QMetaType::sizeOf(m_x->type);
+        if (!size) {
+            m_x->type = QVariant::Invalid;
+            return;
+        }
 
-        if (size && size <= sizeof(QVariant::Private::Data)) {
-            void *ptr = QMetaType::construct(m_x->type, &m_x->data.ptr, m_copy);
-            if (!ptr) {
-                m_x->type = QVariant::Invalid;
-            }
+        // this logic should match with QVariantIntegrator::CanUseInternalSpace
+        if (size <= sizeof(QVariant::Private::Data)
+                && (QMetaType::typeFlags(m_x->type) & QMetaType::MovableType)) {
+            QMetaType::construct(m_x->type, &m_x->data.ptr, m_copy);
             m_x->is_shared = false;
         } else {
             void *ptr = QMetaType::create(m_x->type, m_copy);
-            if (!ptr) {
-                m_x->type = QVariant::Invalid;
-            } else {
-                m_x->is_shared = true;
-                m_x->data.shared = new QVariant::PrivateShared(ptr);
-            }
+            m_x->is_shared = true;
+            m_x->data.shared = new QVariant::PrivateShared(ptr);
         }
     }
 
diff --git a/tests/auto/corelib/kernel/qvariant/tst_qvariant.cpp b/tests/auto/corelib/kernel/qvariant/tst_qvariant.cpp
index e540474b3e..9404130d35 100644
--- a/tests/auto/corelib/kernel/qvariant/tst_qvariant.cpp
+++ b/tests/auto/corelib/kernel/qvariant/tst_qvariant.cpp
@@ -253,6 +253,7 @@ private slots:
 
     void numericalConvert();
     void moreCustomTypes();
+    void movabilityTest();
     void variantInVariant();
 
     void colorInteger();
@@ -2006,7 +2007,7 @@ void tst_QVariant::userType()
 
             QVariant userVar3;
             qVariantSetValue(userVar3, data2);
-            QVERIFY(userVar2 == userVar3);
+
             userVar3 = userVar2;
             QVERIFY(userVar2 == userVar3);
         }
@@ -2049,7 +2050,7 @@ void tst_QVariant::userType()
         QCOMPARE(instanceCount, 3);
         {
             QVariant second = myCarrier;
-            QCOMPARE(instanceCount, 4);
+            QCOMPARE(instanceCount, 3);
             second.detach();
             QCOMPARE(instanceCount, 4);
         }
@@ -3220,6 +3221,27 @@ void tst_QVariant::moreCustomTypes()
     QCOMPARE(MyMovable::count, 0);
 }
 
+void tst_QVariant::movabilityTest()
+{
+    // This test checks if QVariant is movable even if an internal data is not movable.
+    QVERIFY(!MyNotMovable::count);
+    {
+        QVariant variant = QVariant::fromValue(MyNotMovable());
+        QVERIFY(MyNotMovable::count);
+
+        // prepare destination memory space to which variant will be moved
+        QVariant buffer[1];
+        QCOMPARE(buffer[0].type(), QVariant::Invalid);
+        buffer[0].~QVariant();
+
+        memcpy(buffer, &variant, sizeof(QVariant));
+        QCOMPARE(buffer[0].type(), QVariant::UserType);
+        MyNotMovable tmp(buffer[0].value<MyNotMovable>());
+
+        new (&variant) QVariant();
+    }
+    QVERIFY(!MyNotMovable::count);
+}
 
 void tst_QVariant::variantInVariant()
 {