IPC: Remove the "small" object optimization in QNativeIpcKey

The original design was supposed to be small, at a single pointer, but
that never made it through, with the "QString key" member. So the
anonymous union for typeAndFlags overlapping with the extension pointer
was just unnecessary headache.

So separate the two. This means QNativeIpcKey's size increases from 4
pointers to 40 bytes on 64-bit systems and to 24 bytes on 32-bit systems
(so we have 6 unused bytes on both architectures).

Fixes: QTBUG-116821
Change-Id: I512648fd617741199e67fffd1782b7d5ba5ddd12
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
(cherry picked from commit eb5c5a76cf369dd5e40289c1013f043c7e82ff2e)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
(cherry picked from commit ef5f2322a767d84d31dc67d0be9a751054cf8b19)

3 files changed, 47 insertions, 86 deletions

diff --git a/src/corelib/ipc/qtipccommon.cpp b/src/corelib/ipc/qtipccommon.cpp
index 42c8084f4c..f5b3561c06 100644
--- a/src/corelib/ipc/qtipccommon.cpp
+++ b/src/corelib/ipc/qtipccommon.cpp
@@ -367,8 +367,7 @@ QNativeIpcKey::Type QNativeIpcKey::defaultTypeForOs_internal() noexcept
 */
 void QNativeIpcKey::copy_internal(const QNativeIpcKey &other)
 {
-    auto copy = new QNativeIpcKeyPrivate(*other.d_func());
-    d = quintptr(copy) & 1;
+    d = new QNativeIpcKeyPrivate(*other.d);
 }
 
 void QNativeIpcKey::move_internal(QNativeIpcKey &&) noexcept
@@ -378,21 +377,13 @@ void QNativeIpcKey::move_internal(QNativeIpcKey &&) noexcept
 
 QNativeIpcKey &QNativeIpcKey::assign_internal(const QNativeIpcKey &other)
 {
-    QNativeIpcKeyPrivate *us = (d & 1) ? d_func() : nullptr;
-    const QNativeIpcKeyPrivate *them = (other.d & 1) ? other.d_func() : nullptr;
-    if (us && !them) {
-        // don't need the extra info, reset to skinny object
-        typeAndFlags = {};
-        typeAndFlags.type = us->type;
-        delete us;
-    } else {
-        // do need the extra info, so create if necessary
-        if (us)
-            *us = *them;
-        else
-            us = new QNativeIpcKeyPrivate(*them);
-        d = quintptr(us) | 1;
-    }
+    Q_ASSERT(d || other.d);     // only 3 cases to handle
+    if (d && !other.d)
+        *d = {};
+    else if (d)
+        *d = *other.d;
+    else
+        d = new QNativeIpcKeyPrivate(*other.d);
     return *this;
 }
 
@@ -403,7 +394,6 @@ QNativeIpcKey &QNativeIpcKey::assign_internal(const QNativeIpcKey &other)
 */
 void QNativeIpcKey::destroy_internal() noexcept
 {
-    Q_D(QNativeIpcKey);
     delete d;
 }
 
@@ -450,11 +440,6 @@ void QNativeIpcKey::destroy_internal() noexcept
 
     \sa nativeKey(), setType()
 */
-QNativeIpcKey::Type QNativeIpcKey::type_internal() const noexcept
-{
-    Q_D(const QNativeIpcKey);
-    return d->type;
-}
 
 /*!
     \fn QNativeIpcKey::setType(Type type)
@@ -465,8 +450,7 @@ QNativeIpcKey::Type QNativeIpcKey::type_internal() const noexcept
 */
 void QNativeIpcKey::setType_internal(Type type)
 {
-    Q_D(QNativeIpcKey);
-    d->type = type;
+    Q_UNUSED(type);
 }
 
 /*!
@@ -512,7 +496,8 @@ size_t qHash(const QNativeIpcKey &ipcKey, size_t seed) noexcept
 */
 int QNativeIpcKey::compare_internal(const QNativeIpcKey &lhs, const QNativeIpcKey &rhs) noexcept
 {
-    return *lhs.d_func() == *rhs.d_func() ? 0 : 1;
+    Q_UNUSED(lhs); Q_UNUSED(rhs);
+    return 0;
 }
 
 /*!
diff --git a/src/corelib/ipc/qtipccommon.h b/src/corelib/ipc/qtipccommon.h
index ba26681787..bf0936cb42 100644
--- a/src/corelib/ipc/qtipccommon.h
+++ b/src/corelib/ipc/qtipccommon.h
@@ -18,7 +18,7 @@ class QNativeIpcKey
 {
     Q_GADGET_EXPORT(Q_CORE_EXPORT)
 public:
-    enum class Type : quintptr {
+    enum class Type : quint16 {
         // 0 is reserved for the invalid type
         // keep 1 through 0xff free, except for SystemV
         SystemV = 0x51,         // 'Q'
@@ -37,31 +37,28 @@ public:
             ;
     static Type legacyDefaultTypeForOs() noexcept;
 
-    constexpr QNativeIpcKey() noexcept
-        : QNativeIpcKey(DefaultTypeForOs)
-    {}
+    constexpr QNativeIpcKey() noexcept = default;
 
     explicit constexpr QNativeIpcKey(Type type) noexcept
-        : d()
+        : typeAndFlags{type}
     {
-        typeAndFlags.type = type;
     }
 
     Q_IMPLICIT QNativeIpcKey(const QString &k, Type type = DefaultTypeForOs)
-        : d(), key(k)
+        : key(k), typeAndFlags{type}
     {
-        typeAndFlags.type = type;
     }
 
     QNativeIpcKey(const QNativeIpcKey &other)
-        : d(other.d), key(other.key)
+        : d(other.d), key(other.key), typeAndFlags(other.typeAndFlags)
     {
         if (isSlowPath())
             copy_internal(other);
     }
 
     QNativeIpcKey(QNativeIpcKey &&other) noexcept
-        : d(std::exchange(other.d, 0)), key(std::move(other.key))
+        : d(std::exchange(other.d, nullptr)), key(std::move(other.key)),
+          typeAndFlags(std::move(other.typeAndFlags))
     {
         if (isSlowPath())
             move_internal(std::move(other));
@@ -75,10 +72,11 @@ public:
 
     QNativeIpcKey &operator=(const QNativeIpcKey &other)
     {
+        typeAndFlags = other.typeAndFlags;
         key = other.key;
         if (isSlowPath() || other.isSlowPath())
             return assign_internal(other);
-        d = other.d;
+        Q_ASSERT(!d);
         return *this;
     }
 
@@ -87,6 +85,7 @@ public:
     {
         std::swap(d, other.d);
         key.swap(other.key);
+        typeAndFlags.swap(other.typeAndFlags);
     }
 
     bool isEmpty() const noexcept
@@ -101,8 +100,6 @@ public:
 
     constexpr Type type() const noexcept
     {
-        if (isSlowPath())
-            return type_internal();
         return typeAndFlags.type;
     }
 
@@ -114,7 +111,9 @@ public:
     }
 
     QString nativeKey() const noexcept
-    { return key; }
+    {
+        return key;
+    }
     void setNativeKey(const QString &newKey)
     {
         key = newKey;
@@ -127,36 +126,32 @@ public:
 
 private:
     struct TypeAndFlags {
-#if Q_BYTE_ORDER == Q_LITTLE_ENDIAN
-        // this is the LSB
-        quintptr isExtended : 1;
-        Type type : 15;
-#endif
-
-        quintptr reserved : sizeof(quintptr) * 8 - 16;
-
-#if Q_BYTE_ORDER == Q_BIG_ENDIAN
-        Type type : 15;
-        quint16 isExtended : 1;
-        // this was the LSB
-#endif
-    };
-
-    // Bit 0: if set, holds a pointer (with the LSB set); if clear, holds the
-    // the TypeAndFlags structure.
-    union {
-        quintptr d = 0;
-        TypeAndFlags typeAndFlags;
-        static_assert(sizeof(typeAndFlags) == sizeof(d));
+        Type type = DefaultTypeForOs;
+        quint16 reserved1 = {};
+        quint32 reserved2 = {};
+
+        void swap(TypeAndFlags &other) noexcept
+        {
+            std::swap(type, other.type);
+            std::swap(reserved1, other.reserved1);
+            std::swap(reserved2, other.reserved2);
+        }
+
+        friend constexpr bool operator==(const TypeAndFlags &lhs, const TypeAndFlags &rhs) noexcept
+        {
+            return lhs.type == rhs.type &&
+                    lhs.reserved1 == rhs.reserved1 &&
+                    lhs.reserved2 == rhs.reserved2;
+        }
     };
 
+    QNativeIpcKeyPrivate *d = nullptr;
     QString key;
+    TypeAndFlags typeAndFlags;
 
     friend class QNativeIpcKeyPrivate;
-    QNativeIpcKeyPrivate *d_func();
-    const QNativeIpcKeyPrivate *d_func() const;
     constexpr bool isSlowPath() const noexcept
-    { return Q_UNLIKELY(typeAndFlags.isExtended); }
+    { return Q_UNLIKELY(d); }
 
     friend Q_CORE_EXPORT size_t qHash(const QNativeIpcKey &ipcKey, size_t seed) noexcept;
     friend size_t qHash(const QNativeIpcKey &ipcKey) noexcept
@@ -164,13 +159,13 @@ private:
 
     friend bool operator==(const QNativeIpcKey &lhs, const QNativeIpcKey &rhs) noexcept
     {
+        if (!(lhs.typeAndFlags == rhs.typeAndFlags))
+            return false;
         if (lhs.key != rhs.key)
             return false;
         if (lhs.d == rhs.d)
             return true;
-        if (lhs.isSlowPath() && rhs.isSlowPath())
-            return compare_internal(lhs, rhs) == 0;
-        return false;
+        return compare_internal(lhs, rhs) == 0;
     }
     friend bool operator!=(const QNativeIpcKey &lhs, const QNativeIpcKey &rhs) noexcept
     {
@@ -181,7 +176,6 @@ private:
     Q_CORE_EXPORT void move_internal(QNativeIpcKey &&other) noexcept;
     Q_CORE_EXPORT QNativeIpcKey &assign_internal(const QNativeIpcKey &other);
     Q_CORE_EXPORT void destroy_internal() noexcept;
-    Q_DECL_PURE_FUNCTION Q_CORE_EXPORT Type type_internal() const noexcept;
     Q_CORE_EXPORT void setType_internal(Type);
     Q_CORE_EXPORT void setNativeKey_internal(const QString &);
     Q_DECL_PURE_FUNCTION Q_CORE_EXPORT static int
diff --git a/src/corelib/ipc/qtipccommon_p.h b/src/corelib/ipc/qtipccommon_p.h
index 13c76671f6..4416359820 100644
--- a/src/corelib/ipc/qtipccommon_p.h
+++ b/src/corelib/ipc/qtipccommon_p.h
@@ -31,26 +31,8 @@ QT_BEGIN_NAMESPACE
 class QNativeIpcKeyPrivate
 {
 public:
-    QNativeIpcKey::Type type = {};
-
-    friend bool operator==(const QNativeIpcKeyPrivate &lhs, const QNativeIpcKeyPrivate &rhs)
-    {
-        return lhs.type == rhs.type;
-    }
 };
 
-inline QNativeIpcKeyPrivate *QNativeIpcKey::d_func()
-{
-    Q_ASSERT(d & 1);    // Q_ASSERT(isSlowPath) but without the unlikely
-    return reinterpret_cast<QNativeIpcKeyPrivate *>(d & ~1);
-}
-
-inline const QNativeIpcKeyPrivate *QNativeIpcKey::d_func() const
-{
-    Q_ASSERT(d & 1);    // Q_ASSERT(isSlowPath) but without the unlikely
-    return reinterpret_cast<QNativeIpcKeyPrivate *>(d & ~1);
-}
-
 namespace QtIpcCommon {
 enum class IpcType {
     SharedMemory,