QPromise/QFuture: fix value semantics

In random order:

* QFutureInterfaceBase has a d-pointer, and copy semantics. So,
"naturally" extend it to support move semantics as well. (These get
used by QPromise/QFuture, as they both hold a QFutureInterface*
as a data member). The only addition needed is a check for a null
d-pointer in the destructor.

Drive by, reorganize the code for the copies, and use copy-and-swap
instead of the hand-rolled solution. Also, add a free swap()
overload, and mark the existing one as candidate for inlining
in Qt 7 (doesn't need to be an exported function).

* QFutureInterface inherits QFutureInterfaceBase, again with value
semantics. To be honest, I'm not sure why QFutureInterfaceBase is
polymorphic -- could be a design mistake, as polymorphic classes
don't mix with value semantics. Anyways, reorganize the code for
copies, apply copy-and-swap, and add move semantics. This requires
adding a check into derefT().

* Finally, QPromise was already move-only, but had broken move
semantics: the move constructor was not noexcept (!) and it actually
allocated memory (!!!). Fix that one (can be defaulted now), and
streamline the move assignment via the proper macro.

Drive by, fix the signature of the constructor from QFutureInterface
(take const-ref, not plain ref -- it's eventually copied, so it
can keep the const), and add another internal constructor from
rvalue QFutureInterface that moves from it.

Change-Id: I9d61a9dd4d45f34942d8f34416baa118c0307390
Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io>
Reviewed-by: Andrei Golubev <andrei.golubev@qt.io>
Reviewed-by: Sona Kurazyan <sona.kurazyan@qt.io>

1 files changed, 9 insertions, 6 deletions

diff --git a/src/corelib/thread/qfutureinterface.cpp b/src/corelib/thread/qfutureinterface.cpp
index 76af95e3a3..22c8a48411 100644
--- a/src/corelib/thread/qfutureinterface.cpp
+++ b/src/corelib/thread/qfutureinterface.cpp
@@ -84,7 +84,7 @@ QFutureInterfaceBase::QFutureInterfaceBase(const QFutureInterfaceBase &other)
 
 QFutureInterfaceBase::~QFutureInterfaceBase()
 {
-    if (!d->refCount.deref())
+    if (d && !d->refCount.deref())
         delete d;
 }
 
@@ -394,6 +394,9 @@ bool QFutureInterfaceBase::queryState(State state) const
 
 int QFutureInterfaceBase::loadState() const
 {
+    // Used from ~QPromise, so this check is needed
+    if (!d)
+        return QFutureInterfaceBase::State::NoState;
     return d->state.loadRelaxed();
 }
 
@@ -568,13 +571,12 @@ const QtPrivate::ResultStoreBase &QFutureInterfaceBase::resultStoreBase() const
 
 QFutureInterfaceBase &QFutureInterfaceBase::operator=(const QFutureInterfaceBase &other)
 {
-    other.d->refCount.ref();
-    if (!d->refCount.deref())
-        delete d;
-    d = other.d;
+    QFutureInterfaceBase copy(other);
+    swap(copy);
     return *this;
 }
 
+// ### Qt 7: inline
 void QFutureInterfaceBase::swap(QFutureInterfaceBase &other) noexcept
 {
     qSwap(d, other.d);
@@ -587,7 +589,8 @@ bool QFutureInterfaceBase::refT() const
 
 bool QFutureInterfaceBase::derefT() const
 {
-    return d->refCount.derefT();
+    // Called from ~QFutureInterface
+    return !d || d->refCount.derefT();
 }
 
 void QFutureInterfaceBase::reset()