From ed3d60cfca56ce21a3b05a2837284aa101c31f51 Mon Sep 17 00:00:00 2001
From: Giuseppe D'Angelo <giuseppe.dangelo@kdab.com>
Date: Tue, 30 Aug 2022 16:02:22 +0200
Subject: [PATCH] QTaggedPointer: disable operator= with an empty initializer
 list

Given a QTaggedPointer, users may write

  taggedPtr = {};

to mean "reset it". This is error-prone: due to overload resolution,
this actually ends up calling QTaggedPointer<T>::operator=(T *),
which changes the pointer but *not* the tag, and not the implicitly
declared QTaggedPointer<T>:operator=(const QTaggedPointer<T> &)
which would reset both pointer and tag.

Given the idiomatic usage of {} is indeed to perform a full reset (cf.
std::exchange(obj, {}), std::take, etc.), work around this by disabling
the operator= overload for pointers in case an initializer list is
passed. In other words, make `={}` fall back to the implicitly
declared overload.

Note, this breaks some usages, such as

  taggedPtr = {rawPtr};

but at least we get a compile error for these, and they don't look
common at all.

[ChangeLog][QtCore][QTaggedPointer] The operator assignment
taking a raw pointer has been reimplemented in order to avoid
subtle issues when assigning `{}` to a QTaggedPointer. This will
cause code that assigns a braced-init-list to a QTaggedPointer object
to stop compiling (for instance, `tagPtr = {ptr}` is now ill-formed).

Change-Id: I5e572a9b0f119ddb2df17f1797934933dff2ba7b
Task-number: QTBUG-106070
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
(cherry picked from commit 42b66bd809f0cadf232cd8527746b55cd00d9f10)
---
 src/corelib/tools/qtaggedpointer.h            | 23 +++++++++-
 .../qtaggedpointer/tst_qtaggedpointer.cpp     | 42 +++++++++++++++++++
 2 files changed, 63 insertions(+), 2 deletions(-)

diff --git a/src/corelib/tools/qtaggedpointer.h b/src/corelib/tools/qtaggedpointer.h
index 504645993a2..a08912c4a57 100644
--- a/src/corelib/tools/qtaggedpointer.h
+++ b/src/corelib/tools/qtaggedpointer.h
@@ -72,12 +72,31 @@ public:
         return !isNull();
     }
 
-    QTaggedPointer &operator=(T *other) noexcept
+#ifdef Q_QDOC
+    QTaggedPointer &operator=(T *other) noexcept;
+#else
+    // Disables the usage of `ptr = {}`, which would go through this operator
+    // (rather than using the implicitly-generated assignment operator).
+    // The operators have different semantics: the ones here leave the tag intact,
+    // the implicitly-generated one overwrites it.
+    template <typename U,
+              std::enable_if_t<std::is_convertible_v<U *, T *>, bool> = false>
+    QTaggedPointer &operator=(U *other) noexcept
     {
-        d = reinterpret_cast<quintptr>(other) | (d & tagMask());
+        T *otherT = other;
+        d = reinterpret_cast<quintptr>(otherT) | (d & tagMask());
         return *this;
     }
 
+    template <typename U,
+              std::enable_if_t<std::is_null_pointer_v<U>, bool> = false>
+    QTaggedPointer &operator=(U) noexcept
+    {
+        d = reinterpret_cast<quintptr>(static_cast<T *>(nullptr)) | (d & tagMask());
+        return *this;
+    }
+#endif
+
     static constexpr Tag maximumTag() noexcept
     {
         return TagType(typename QtPrivate::TagInfo<T>::TagType(tagMask()));
diff --git a/tests/auto/corelib/tools/qtaggedpointer/tst_qtaggedpointer.cpp b/tests/auto/corelib/tools/qtaggedpointer/tst_qtaggedpointer.cpp
index 752bf48d93a..5cb82329d0c 100644
--- a/tests/auto/corelib/tools/qtaggedpointer/tst_qtaggedpointer.cpp
+++ b/tests/auto/corelib/tools/qtaggedpointer/tst_qtaggedpointer.cpp
@@ -11,6 +11,7 @@ class tst_QTaggedPointer : public QObject
 private Q_SLOTS:
     void constExpr();
     void construction();
+    void assignment();
     void dereferenceOperator();
     void pointerOperator();
     void negationOperator();
@@ -80,6 +81,47 @@ void tst_QTaggedPointer::construction()
     }
 }
 
+void tst_QTaggedPointer::assignment()
+{
+    QScopedPointer<int> rawPointer(new int(5));
+    QTaggedPointer<int> p(rawPointer.data(), 0x1);
+    QTaggedPointer<int> p2(rawPointer.data(), 0x2);
+
+    QCOMPARE(p.data(), rawPointer.data());
+    QCOMPARE(p.tag(), quintptr(0x1));
+
+    QCOMPARE(p2.data(), rawPointer.data());
+    QCOMPARE(p2.tag(), quintptr(0x2));
+
+    p = nullptr;
+    QCOMPARE(p.data(), nullptr);
+    QCOMPARE(p.tag(), quintptr(0x1));
+
+    p = rawPointer.data();
+    QCOMPARE(p.data(), rawPointer.data());
+    QCOMPARE(p.tag(), quintptr(0x1));
+
+    p = {};
+    QCOMPARE(p.data(), nullptr);
+    QCOMPARE(p.tag(), quintptr(0x0));
+
+    p = p2;
+    QCOMPARE(p.data(), rawPointer.data());
+    QCOMPARE(p.tag(), quintptr(0x2));
+
+    p = nullptr;
+    QCOMPARE(p.data(), nullptr);
+    QCOMPARE(p.tag(), quintptr(0x2));
+
+    p = {};
+    QCOMPARE(p.data(), nullptr);
+    QCOMPARE(p.tag(), quintptr(0x0));
+
+    p = rawPointer.data();
+    QCOMPARE(p.data(), rawPointer.data());
+    QCOMPARE(p.tag(), quintptr(0x0));
+}
+
 class AbstractClass
 {
 public: