Cleanup QVariant::PrivateShared

Remove the additional indirection through ptr and replace
it with an offset calculation. Get rid of PrivateSharedEx
that was handling certain types differently.

This also fixes the support for overaligned types, by
using the alignment field from QMetaType to determine
the alignment requirements.

Change-Id: Icc6a78eb57f664c7747450578fe58dfee8f2863b
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>

3 files changed, 56 insertions, 45 deletions

diff --git a/src/corelib/kernel/qvariant.cpp b/src/corelib/kernel/qvariant.cpp
index a6b383d359..3c15287d12 100644
--- a/src/corelib/kernel/qvariant.cpp
+++ b/src/corelib/kernel/qvariant.cpp
@@ -229,13 +229,13 @@ static qlonglong qConvertToNumber(const QVariant::Private *d, bool *ok, bool all
         || d->type().id() == QMetaType::QCborSimpleType) {
         switch (typeInfo.sizeOf()) {
         case 1:
-            return d->is_shared ? *reinterpret_cast<signed char *>(d->data.shared->ptr) : d->data.sc;
+            return d->is_shared ? *reinterpret_cast<signed char *>(d->data.shared->data()) : d->data.sc;
         case 2:
-            return d->is_shared ? *reinterpret_cast<qint16 *>(d->data.shared->ptr) : d->data.s;
+            return d->is_shared ? *reinterpret_cast<qint16 *>(d->data.shared->data()) : d->data.s;
         case 4:
-            return d->is_shared ? *reinterpret_cast<qint32 *>(d->data.shared->ptr) : d->data.i;
+            return d->is_shared ? *reinterpret_cast<qint32 *>(d->data.shared->data()) : d->data.i;
         case 8:
-            return d->is_shared ? *reinterpret_cast<qint64 *>(d->data.shared->ptr) : d->data.ll;
+            return d->is_shared ? *reinterpret_cast<qint64 *>(d->data.shared->data()) : d->data.ll;
         }
     }
 
@@ -317,13 +317,13 @@ static qulonglong qConvertToUnsignedNumber(const QVariant::Private *d, bool *ok)
     if (typeInfo.flags() & QMetaType::IsEnumeration) {
         switch (typeInfo.sizeOf()) {
         case 1:
-            return d->is_shared ? *reinterpret_cast<uchar *>(d->data.shared->ptr) : d->data.uc;
+            return d->is_shared ? *reinterpret_cast<uchar *>(d->data.shared->data()) : d->data.uc;
         case 2:
-            return d->is_shared ? *reinterpret_cast<quint16 *>(d->data.shared->ptr) : d->data.us;
+            return d->is_shared ? *reinterpret_cast<quint16 *>(d->data.shared->data()) : d->data.us;
         case 4:
-            return d->is_shared ? *reinterpret_cast<quint32 *>(d->data.shared->ptr) : d->data.u;
+            return d->is_shared ? *reinterpret_cast<quint32 *>(d->data.shared->data()) : d->data.u;
         case 8:
-            return d->is_shared ? *reinterpret_cast<qint64 *>(d->data.shared->ptr) : d->data.ull;
+            return d->is_shared ? *reinterpret_cast<qint64 *>(d->data.shared->data()) : d->data.ull;
         }
     }
 
@@ -345,7 +345,7 @@ inline bool qt_convertToBool(const QVariant::Private *const d)
 
 static const void *constData(const QVariant::Private &d)
 {
-    return d.is_shared ? d.data.shared->ptr : reinterpret_cast<const void *>(&d.data.c);
+    return d.is_shared ? d.data.shared->data() : reinterpret_cast<const void *>(&d.data.c);
 }
 
 #ifndef QT_NO_QOBJECT
@@ -1402,16 +1402,9 @@ static void customConstruct(QVariant::Private *d, const void *copy)
         type.construct(&d->data, copy);
         d->is_shared = false;
     } else {
-        // Private::Data contains long long, and long double is the biggest standard type.
-        const size_t maxAlignment =
-            qMax(alignof(QVariant::Private::Data), alignof(long double));
-        const size_t s = sizeof(QVariant::PrivateShared);
-        const size_t offset = s + ((s * maxAlignment - s) % maxAlignment);
-        void *data = operator new(offset + size);
-        void *ptr = static_cast<char *>(data) + offset;
-        type.construct(ptr, copy);
+        d->data.shared = QVariant::PrivateShared::create(type);
+        type.construct(d->data.shared->data(), copy);
         d->is_shared = true;
-        d->data.shared = new (data) QVariant::PrivateShared(ptr);
     }
     // need to check for nullptr_t here, as this can get called by fromValue(nullptr). fromValue() uses
     // std::addressof(value) which in this case returns the address of the nullptr object.
@@ -1423,9 +1416,8 @@ static void customClear(QVariant::Private *d)
     if (!d->is_shared) {
         d->type().destruct(&d->data);
     } else {
-        d->type().destruct(d->data.shared->ptr);
-        d->data.shared->~PrivateShared();
-        operator delete(d->data.shared);
+        d->type().destruct(d->data.shared->data());
+        QVariant::PrivateShared::free(d->data.shared);
     }
 }
 
@@ -3885,7 +3877,7 @@ bool QVariant::equals(const QVariant &v) const
 
 const void *QVariant::constData() const
 {
-    return d.is_shared ? d.data.shared->ptr : reinterpret_cast<const void *>(&d.data);
+    return d.is_shared ? d.data.shared->data() : reinterpret_cast<const void *>(&d.data);
 }
 
 /*!
@@ -3922,7 +3914,7 @@ bool QVariant::isNull() const
     if (d.is_null || !metaType().isValid())
         return true;
     if (metaType().flags() & QMetaType::IsPointer) {
-        const void *d_ptr = d.is_shared ? d.data.shared->ptr : &(d.data);
+        const void *d_ptr = d.is_shared ? d.data.shared->data() : &(d.data);
         return *static_cast<void *const *>(d_ptr) == nullptr;
     }
     return false;
diff --git a/src/corelib/kernel/qvariant.h b/src/corelib/kernel/qvariant.h
index b738b80d38..96756fdda7 100644
--- a/src/corelib/kernel/qvariant.h
+++ b/src/corelib/kernel/qvariant.h
@@ -52,7 +52,6 @@
 #ifndef QT_BOOTSTRAPPED
 #include <QtCore/qbytearraylist.h>
 #endif
-
 #include <memory>
 
 #if __has_include(<variant>) && __cplusplus >= 201703L
@@ -405,9 +404,40 @@ class Q_CORE_EXPORT QVariant
  public:
     struct PrivateShared
     {
-        inline PrivateShared(void *v) : ptr(v), ref(1) { }
-        void *ptr;
-        QAtomicInt ref;
+    private:
+        inline PrivateShared() : ref(1) { }
+    public:
+        static PrivateShared *create(QMetaType type)
+        {
+            size_t size = type.sizeOf();
+            size_t align = type.alignOf();
+
+            size += sizeof(PrivateShared);
+            if (align > sizeof(PrivateShared)) {
+                // The alignment is larger than the alignment we can guarantee for the pointer
+                // directly following PrivateShared, so we need to allocate some additional
+                // memory to be able to fit the object into the available memory with suitable
+                // alignment.
+                size += align - sizeof(PrivateShared);
+            }
+            void *data = operator new(size);
+            auto *ps = new (data) QVariant::PrivateShared();
+            ps->offset = int(((quintptr(ps) + sizeof(PrivateShared) + align - 1) & ~(align - 1)) - quintptr(ps));
+            return ps;
+        }
+        static void free(PrivateShared *p)
+        {
+            p->~PrivateShared();
+            operator delete(p);
+        }
+
+        alignas(8) QAtomicInt ref;
+        int offset;
+
+        const void *data() const
+        { return reinterpret_cast<const unsigned char *>(this) + offset; }
+        void *data()
+        { return reinterpret_cast<unsigned char *>(this) + offset; }
     };
     struct Private
     {
diff --git a/src/corelib/kernel/qvariant_p.h b/src/corelib/kernel/qvariant_p.h
index f92f97d836..926e95386f 100644
--- a/src/corelib/kernel/qvariant_p.h
+++ b/src/corelib/kernel/qvariant_p.h
@@ -89,7 +89,7 @@ template <typename T>
 inline const T *v_cast(const QVariant::Private *d, T * = nullptr)
 {
     return !QVariantIntegrator<T>::CanUseInternalSpace
-            ? static_cast<const T *>(d->data.shared->ptr)
+            ? static_cast<const T *>(d->data.shared->data())
             : static_cast<const T *>(static_cast<const void *>(&d->data.c));
 }
 
@@ -97,7 +97,7 @@ template <typename T>
 inline T *v_cast(QVariant::Private *d, T * = nullptr)
 {
     return !QVariantIntegrator<T>::CanUseInternalSpace
-            ? static_cast<T *>(d->data.shared->ptr)
+            ? static_cast<T *>(d->data.shared->data())
             : static_cast<T *>(static_cast<void *>(&d->data.c));
 }
 
@@ -108,17 +108,6 @@ enum QVariantConstructionFlags : uint {
     PointerType = 0x1
 };
 
-//a simple template that avoids to allocate 2 memory chunks when creating a QVariant
-template <class T> class QVariantPrivateSharedEx : public QVariant::PrivateShared
-{
-public:
-    QVariantPrivateSharedEx() : QVariant::PrivateShared(&m_t), m_t() { }
-    QVariantPrivateSharedEx(const T&t) : QVariant::PrivateShared(&m_t), m_t(t) { }
-
-private:
-    T m_t;
-};
-
 template <class T>
 inline void v_construct_helper(QVariant::Private *x, const T &t, std::true_type)
 {
@@ -129,7 +118,8 @@ inline void v_construct_helper(QVariant::Private *x, const T &t, std::true_type)
 template <class T>
 inline void v_construct_helper(QVariant::Private *x, const T &t, std::false_type)
 {
-    x->data.shared = new QVariantPrivateSharedEx<T>(t);
+    x->data.shared = QVariant::PrivateShared::create(QMetaType::fromType<T>());
+    new (x->data.shared->data()) T(t);
     x->is_shared = true;
 }
 
@@ -143,7 +133,8 @@ inline void v_construct_helper(QVariant::Private *x, std::true_type)
 template <class T>
 inline void v_construct_helper(QVariant::Private *x, std::false_type)
 {
-    x->data.shared = new QVariantPrivateSharedEx<T>;
+    x->data.shared = QVariant::PrivateShared::create(QMetaType::fromType<T>());
+    new (x->data.shared->data()) T();
     x->is_shared = true;
 }
 
@@ -170,9 +161,7 @@ inline void v_clear(QVariant::Private *d, T* = nullptr)
 {
 
     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);
+        delete static_cast<T *>(d->data.shared->data());
     } else {
         v_cast<T>(d)->~T();
     }