Enhance QPromise::finally implementation

Make sure that the chained value is not copied when `finally` is called for a fulfilled input promise. The value was copied 7 times in the previous version because it was captured in a lambda, which one copied multiple times.

src/qtpromise/qpromise.h

@@ -40,6 +40,9 @@ public:
     inline typename QtPromisePrivate::PromiseHandler<T, std::nullptr_t>::Promise
     fail(TRejected&& rejected) const;
 
+    template <typename THandler>
+    inline QPromise<T> finally(THandler handler) const;
+
     inline QPromise<T> wait() const;
 
     void swap(QPromiseBase<T>& other) { qSwap(m_d, other.m_d); }
@@ -63,9 +66,6 @@ public:
     template <typename F>
     QPromise(F&& resolver): QPromiseBase<T>(std::forward<F>(resolver)) { }
 
-    template <typename THandler>
-    inline QPromise<T> finally(THandler handler) const;
-
 public: // STATIC
     inline static QPromise<QVector<T> > all(const QVector<QPromise<T> >& promises);
     inline static QPromise<T> resolve(T&& value);
@@ -81,9 +81,6 @@ public:
     template <typename F>
     QPromise(F&& resolver): QPromiseBase<void>(std::forward<F>(resolver)) { }
 
-    template <typename THandler>
-    inline QPromise<void> finally(THandler handler) const;
-
 public: // STATIC
     inline static QPromise<void> all(const QVector<QPromise<void> >& promises);
     inline static QPromise<void> resolve();

src/qtpromise/qpromise.inl

@@ -129,6 +129,16 @@ QPromiseBase<T>::fail(TRejected&& rejected) const
     return then(nullptr, std::forward<TRejected>(rejected));
 }
 
+template <typename T>
+template <typename THandler>
+inline QPromise<T> QPromiseBase<T>::finally(THandler handler) const
+{
+    QPromise<T> p = *this;
+    return p.then(handler, handler).then([=]() {
+        return p;
+    });
+}
+
 template <typename T>
 inline QPromise<T> QPromiseBase<T>::wait() const
 {
@@ -150,23 +160,6 @@ inline QPromise<T> QPromiseBase<T>::reject(E&& error)
     });
 }
 
-template <typename T>
-template <typename THandler>
-inline QPromise<T> QPromise<T>::finally(THandler handler) const
-{
-    return this->then([=](const T& res) {
-        return QPromise<void>::resolve().then(handler).then([=](){
-            return res;
-        });
-    }, [=]() {
-        const auto exception = std::current_exception();
-        return QPromise<void>::resolve().then(handler).then([=](){
-            std::rethrow_exception(exception);
-            return T();
-        });
-    });
-}
-
 template <typename T>
 inline QPromise<QVector<T> > QPromise<T>::all(const QVector<QPromise<T> >& promises)
 {
@@ -206,19 +199,6 @@ inline QPromise<T> QPromise<T>::resolve(T&& value)
     });
 }
 
-template <typename THandler>
-inline QPromise<void> QPromise<void>::finally(THandler handler) const
-{
-    return this->then([=]() {
-        return QPromise<void>::resolve().then(handler).then([](){});
-    }, [=]() {
-        const auto exception = std::current_exception();
-        return QPromise<void>::resolve().then(handler).then([=](){
-            std::rethrow_exception(exception);
-        });
-    });
-}
-
 inline QPromise<void> QPromise<void>::all(const QVector<QPromise<void> >& promises)
 {
     const int count = promises.size();

tests/auto/benchmark/tst_benchmark.cpp

@@ -14,6 +14,7 @@ private Q_SLOTS:
     void valueResolve();
     void valueReject();
     void valueThen();
+    void valueFinally();
     void valueDelayed();
     void errorReject();
     void errorThen();
@@ -129,7 +130,7 @@ void tst_benchmark::valueThen()
 
         QCOMPARE(Data::logs().ctor, 0);
         QCOMPARE(Data::logs().copy, 0);
-        QCOMPARE(Data::logs().move, 0);     // move value to the promise data
+        QCOMPARE(Data::logs().move, 0);
         QCOMPARE(Data::logs().refs, 0);
         QCOMPARE(error, QString("foo"));
         QCOMPARE(value, -1);
@@ -193,6 +194,36 @@ void tst_benchmark::valueDelayed()
     }
 }
 
+void tst_benchmark::valueFinally()
+{
+    {   // should not copy the value on continutation if fulfilled
+        int value = -1;
+        Data::logs().reset();
+        QPromise<Data>::resolve(Data(42)).finally([&]() {
+            value = 42;
+        }).wait();
+
+        QCOMPARE(Data::logs().ctor, 1);
+        QCOMPARE(Data::logs().copy, 0);
+        QCOMPARE(Data::logs().move, 1);     // move value to the input and output promise data
+        QCOMPARE(Data::logs().refs, 0);
+        QCOMPARE(value, 42);
+    }
+    {   // should not create value on continutation if rejected
+        int value = -1;
+        Data::logs().reset();
+        QPromise<Data>::reject(QString("foo")).finally([&]() {
+            value = 42;
+        }).wait();
+
+        QCOMPARE(Data::logs().ctor, 0);
+        QCOMPARE(Data::logs().copy, 0);
+        QCOMPARE(Data::logs().move, 0);
+        QCOMPARE(Data::logs().refs, 0);
+        QCOMPARE(value, 42);
+    }
+}
+
 void tst_benchmark::errorReject()
 {
     {   // should create one copy of the error when rejected by rvalue

tests/auto/qpromise/tst_qpromise.cpp

@@ -30,8 +30,10 @@ private Q_SLOTS:
     void failBaseClass();
     void failCatchAll();
 
-    void finallyReturns();
+    void finallyFulfilled();
-    void finallyReturns_void();
+    void finallyFulfilled_void();
+    void finallyRejected();
+    void finallyRejected_void();
     void finallyThrows();
     void finallyThrows_void();
     void finallyDelayedResolved();
@@ -355,9 +357,8 @@ void tst_qpromise::failCatchAll()
     QCOMPARE(error, QString("bar"));
 }
 
-void tst_qpromise::finallyReturns()
+void tst_qpromise::finallyFulfilled()
 {
-    {   // fulfilled
     int value = -1;
     auto p = QPromise<int>::resolve(42).finally([&]() {
         value = 8;
@@ -369,23 +370,9 @@ void tst_qpromise::finallyReturns()
     QCOMPARE(p.isFulfilled(), true);
     QCOMPARE(value, 8);
 }
-    {   // rejected
-        int value = -1;
-        auto p = QPromise<int>::reject(QString("foo")).finally([&]() {
-            value = 8;
-            return 16; // ignored!
-        });
 
-        Q_STATIC_ASSERT((std::is_same<decltype(p), QPromise<int> >::value));
+void tst_qpromise::finallyFulfilled_void()
-        QCOMPARE(waitForError(p, QString()), QString("foo"));
-        QCOMPARE(p.isRejected(), true);
-        QCOMPARE(value, 8);
-    }
-}
-
-void tst_qpromise::finallyReturns_void()
 {
-    {   // fulfilled
     int value = -1;
     auto p = QPromise<void>::resolve().finally([&]() {
         value = 8;
@@ -397,7 +384,23 @@ void tst_qpromise::finallyReturns_void()
     QCOMPARE(p.isFulfilled(), true);
     QCOMPARE(value, 8);
 }
-    {   // rejected
+
+void tst_qpromise::finallyRejected()
+{
+    int value = -1;
+    auto p = QPromise<int>::reject(QString("foo")).finally([&]() {
+        value = 8;
+        return 16; // ignored!
+    });
+
+    Q_STATIC_ASSERT((std::is_same<decltype(p), QPromise<int> >::value));
+    QCOMPARE(waitForError(p, QString()), QString("foo"));
+    QCOMPARE(p.isRejected(), true);
+    QCOMPARE(value, 8);
+}
+
+void tst_qpromise::finallyRejected_void()
+{
     int value = -1;
     auto p = QPromise<void>::reject(QString("foo")).finally([&]() {
         value = 8;
@@ -409,7 +412,6 @@ void tst_qpromise::finallyReturns_void()
     QCOMPARE(p.isRejected(), true);
     QCOMPARE(value, 8);
 }
-}
 
 void tst_qpromise::finallyThrows()
 {