From ee0a997bcd8d00f0515d037c964d77ecfdd3159f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C4=99drzej=20Nowacki?= Date: Fri, 2 Dec 2011 17:52:00 +0100 Subject: [PATCH] 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 Reviewed-by: Thiago Macieira --- src/corelib/kernel/qvariant_p.h | 59 +++++++++++-------- .../corelib/kernel/qvariant/tst_qvariant.cpp | 26 +++++++- 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 #include +#include #include "qmetatypeswitcher_p.h" QT_BEGIN_NAMESPACE +namespace { +template +struct QVariantIntegrator +{ + static const bool CanUseInternalSpace = sizeof(T) <= sizeof(QVariant::Private::Data) + && (!QTypeInfo::isStatic); +}; +Q_STATIC_ASSERT(QVariantIntegrator::CanUseInternalSpace); +Q_STATIC_ASSERT(QVariantIntegrator::CanUseInternalSpace); +Q_STATIC_ASSERT(QVariantIntegrator::CanUseInternalSpace); +} // namespace + #ifdef Q_CC_SUN // Sun CC picks the wrong overload, so introduce awful hack template inline T *v_cast(const QVariant::Private *nd, T * = 0) { QVariant::Private *d = const_cast(nd); - return ((sizeof(T) > sizeof(QVariant::Private::Data)) + return !QVariantIntegrator::CanUseInternalSpace ? static_cast(d->data.shared->ptr) - : static_cast(static_cast(&d->data.c))); + : static_cast(static_cast(&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 inline const T *v_cast(const QVariant::Private *d, T * = 0) { - return ((sizeof(T) > sizeof(QVariant::Private::Data)) + return !QVariantIntegrator::CanUseInternalSpace ? static_cast(d->data.shared->ptr) - : static_cast(static_cast(&d->data.c))); + : static_cast(static_cast(&d->data.c)); } template inline T *v_cast(QVariant::Private *d, T * = 0) { - return ((sizeof(T) > sizeof(QVariant::Private::Data)) + return !QVariantIntegrator::CanUseInternalSpace ? static_cast(d->data.shared->ptr) - : static_cast(static_cast(&d->data.c))); + : static_cast(static_cast(&d->data.c)); } #endif @@ -110,7 +123,7 @@ private: template inline void v_construct(QVariant::Private *x, const void *copy, T * = 0) { - if (sizeof(T) > sizeof(QVariant::Private::Data)) { + if (!QVariantIntegrator::CanUseInternalSpace) { x->data.shared = copy ? new QVariantPrivateSharedEx(*static_cast(copy)) : new QVariantPrivateSharedEx; x->is_shared = true; @@ -125,7 +138,7 @@ inline void v_construct(QVariant::Private *x, const void *copy, T * = 0) template inline void v_construct(QVariant::Private *x, const T &t) { - if (sizeof(T) > sizeof(QVariant::Private::Data)) { + if (!QVariantIntegrator::CanUseInternalSpace) { x->data.shared = new QVariantPrivateSharedEx(t); x->is_shared = true; } else { @@ -138,7 +151,7 @@ template inline void v_clear(QVariant::Private *d, T* = 0) { - if (sizeof(T) > sizeof(QVariant::Private::Data)) { + if (!QVariantIntegrator::CanUseInternalSpace) { //now we need to cast //because QVariant::PrivateShared doesn't have a virtual destructor delete static_cast*>(d->data.shared); @@ -293,11 +306,11 @@ protected: template class QVariantConstructor { - template + template::CanUseInternalSpace> struct CallConstructor {}; template - struct CallConstructor + struct CallConstructor { CallConstructor(const QVariantConstructor &tc) { @@ -310,7 +323,7 @@ class QVariantConstructor }; template - struct CallConstructor + struct CallConstructor { 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()); + + new (&variant) QVariant(); + } + QVERIFY(!MyNotMovable::count); +} void tst_QVariant::variantInVariant() {