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)
This commit is contained in:
Giuseppe D'Angelo 2022-08-30 16:02:22 +02:00 committed by Marc Mutz
parent f90107341c
commit ed3d60cfca
2 changed files with 63 additions and 2 deletions

View File

@ -72,12 +72,31 @@ public:
return !isNull(); 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; 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 static constexpr Tag maximumTag() noexcept
{ {
return TagType(typename QtPrivate::TagInfo<T>::TagType(tagMask())); return TagType(typename QtPrivate::TagInfo<T>::TagType(tagMask()));

View File

@ -11,6 +11,7 @@ class tst_QTaggedPointer : public QObject
private Q_SLOTS: private Q_SLOTS:
void constExpr(); void constExpr();
void construction(); void construction();
void assignment();
void dereferenceOperator(); void dereferenceOperator();
void pointerOperator(); void pointerOperator();
void negationOperator(); 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 class AbstractClass
{ {
public: public: