Allow QSharedPointer as rejection reason

Embed the promise fulfillment value and rejection reason in respectively PromiseValue and PromiseError private wrappers, both storing the data in a shared pointer (QPromiseError is now deprecated).

src/qtpromise/qpromise.h

@@ -3,6 +3,7 @@
 
 // QtPromise
 #include "qpromise_p.h"
+#include "qpromiseerror.h"
 #include "qpromiseglobal.h"
 
 // Qt

src/qtpromise/qpromise_p.h

@@ -2,7 +2,6 @@
 #define QTPROMISE_QPROMISE_P_H
 
 // QtPromise
-#include "qpromiseerror.h"
 #include "qpromiseglobal.h"
 
 // Qt
@@ -72,6 +71,43 @@ static void qtpromise_defer(F&& f)
     qtpromise_defer(std::forward<F>(f), QThread::currentThread());
 }
 
+template <typename T>
+class PromiseValue
+{
+public:
+    PromiseValue() { }
+    PromiseValue(const T& data) : m_data(new T(data)) { }
+    PromiseValue(T&& data) : m_data(new T(std::move(data))) { }
+    bool isNull() const { return m_data.isNull(); }
+    const T& data() const { return *m_data; }
+
+private:
+    QSharedPointer<T> m_data;
+};
+
+class PromiseError
+{
+public:
+    template <typename T>
+    PromiseError(const T& value)
+    {
+        try {
+            throw value;
+        } catch (...) {
+            m_data = std::current_exception();
+        }
+    }
+
+    PromiseError() { }
+    PromiseError(const std::exception_ptr& exception) : m_data(exception) { }
+    void rethrow() const { std::rethrow_exception(m_data); }
+    bool isNull() const { return m_data == nullptr; }
+
+private:
+    // NOTE(SB) std::exception_ptr is already a shared pointer
+    std::exception_ptr m_data;
+};
+
 template <typename T>
 struct PromiseDeduce
 {
@@ -306,12 +342,12 @@ struct PromiseCatcher
     using ResType = typename std::result_of<THandler(TArg)>::type;
 
     template <typename TResolve, typename TReject>
-    static std::function<void(const QtPromise::QPromiseError&)> create(
+    static std::function<void(const PromiseError&)> create(
         const THandler& handler,
         const TResolve& resolve,
         const TReject& reject)
     {
-        return [=](const QtPromise::QPromiseError& error) {
+        return [=](const PromiseError& error) {
             try {
                 error.rethrow();
             } catch (const TArg& error) {
@@ -329,12 +365,12 @@ struct PromiseCatcher<T, THandler, void>
     using ResType = typename std::result_of<THandler()>::type;
 
     template <typename TResolve, typename TReject>
-    static std::function<void(const QtPromise::QPromiseError&)> create(
+    static std::function<void(const PromiseError&)> create(
         const THandler& handler,
         const TResolve& resolve,
         const TReject& reject)
     {
-        return [=](const QtPromise::QPromiseError& error) {
+        return [=](const PromiseError& error) {
             try {
                 error.rethrow();
             } catch (...) {
@@ -348,12 +384,12 @@ template <typename T>
 struct PromiseCatcher<T, std::nullptr_t, void>
 {
     template <typename TResolve, typename TReject>
-    static std::function<void(const QtPromise::QPromiseError&)> create(
+    static std::function<void(const PromiseError&)> create(
         std::nullptr_t,
         const TResolve&,
         const TReject& reject)
     {
-        return [=](const QtPromise::QPromiseError& error) {
+        return [=](const PromiseError& error) {
             // 2.2.7.4. If onRejected is not a function and promise1 is rejected,
             // promise2 must be rejected with the same reason as promise1
             reject(error);
@@ -367,9 +403,8 @@ template <typename T, typename F>
 class PromiseDataBase : public QSharedData
 {
 public:
-    using Error = QtPromise::QPromiseError;
     using Handler = std::pair<QPointer<QThread>, std::function<F>>;
-    using Catcher = std::pair<QPointer<QThread>, std::function<void(const Error&)>>;
+    using Catcher = std::pair<QPointer<QThread>, std::function<void(const PromiseError&)>>;
 
     virtual ~PromiseDataBase() {}
 
@@ -395,29 +430,22 @@ public:
         m_handlers.append({QThread::currentThread(), std::move(handler)});
     }
 
-    void addCatcher(std::function<void(const Error&)> catcher)
+    void addCatcher(std::function<void(const PromiseError&)> catcher)
     {
         QWriteLocker lock(&m_lock);
         m_catchers.append({QThread::currentThread(), std::move(catcher)});
     }
 
-    void reject(Error error)
+    template <typename E>
+    void reject(E&& error)
     {
         Q_ASSERT(isPending());
         Q_ASSERT(m_error.isNull());
-        m_error.reset(new Error(std::move(error)));
+        m_error = PromiseError(std::forward<E>(error));
         setSettled();
     }
 
-    void reject(const QSharedPointer<Error>& error)
+    const PromiseError& error() const
-    {
-        Q_ASSERT(isPending());
-        Q_ASSERT(m_error.isNull());
-        m_error = error;
-        this->setSettled();
-    }
-
-    const QSharedPointer<Error>& error() const
     {
         Q_ASSERT(isRejected());
         return m_error;
@@ -446,13 +474,13 @@ public:
             return;
         }
 
-        QSharedPointer<Error> error = m_error;
+        PromiseError error(m_error);
         Q_ASSERT(!error.isNull());
 
         for (const auto& catcher: catchers) {
             const auto& fn = catcher.second;
             qtpromise_defer([=]() {
-                fn(*error);
+                fn(error);
             }, catcher.first);
         }
     }
@@ -473,7 +501,7 @@ private:
     bool m_settled = false;
     QVector<Handler> m_handlers;
     QVector<Catcher> m_catchers;
-    QSharedPointer<Error> m_error;
+    PromiseError m_error;
 };
 
 template <typename T>
@@ -482,31 +510,16 @@ class PromiseData : public PromiseDataBase<T, void(const T&)>
     using Handler = typename PromiseDataBase<T, void(const T&)>::Handler;
 
 public:
-    void resolve(T&& value)
+    template <typename V>
+    void resolve(V&& value)
     {
         Q_ASSERT(this->isPending());
         Q_ASSERT(m_value.isNull());
-        m_value.reset(new T(std::move(value)));
+        m_value = PromiseValue<T>(std::forward<V>(value));
         this->setSettled();
     }
 
-    void resolve(const T& value)
+    const PromiseValue<T>& value() const
-    {
-        Q_ASSERT(this->isPending());
-        Q_ASSERT(m_value.isNull());
-        m_value.reset(new T(value));
-        this->setSettled();
-    }
-
-    void resolve(const QSharedPointer<T>& value)
-    {
-        Q_ASSERT(this->isPending());
-        Q_ASSERT(m_value.isNull());
-        m_value = value;
-        this->setSettled();
-    }
-
-    const QSharedPointer<T>& value() const
     {
         Q_ASSERT(this->isFulfilled());
         return m_value;
@@ -514,19 +527,19 @@ public:
 
     void notify(const QVector<Handler>& handlers) Q_DECL_OVERRIDE
     {
-        QSharedPointer<T> value(m_value);
+        PromiseValue<T> value(m_value);
         Q_ASSERT(!value.isNull());
 
         for (const auto& handler: handlers) {
             const auto& fn = handler.second;
             qtpromise_defer([=]() {
-                fn(*value);
+                fn(value.data());
             }, handler.first);
         }
     }
 
 private:
-    QSharedPointer<T> m_value;
+    PromiseValue<T> m_value;
 };
 
 template <>

src/qtpromise/qpromiseerror.h

@@ -2,6 +2,7 @@
 #define QTPROMISE_QPROMISEERROR_H
 
 // QtPromise
+#include "qpromise_p.h"
 #include "qpromiseglobal.h"
 
 // Qt
@@ -9,53 +10,6 @@
 
 namespace QtPromise {
 
-class QPromiseError
-{
-public:
-    template <typename T>
-    QPromiseError(const T& value)
-    {
-        try {
-            throw value;
-        } catch (...) {
-            m_exception = std::current_exception();
-        }
-    }
-
-    QPromiseError(const std::exception_ptr& exception)
-        : m_exception(exception)
-    { }
-
-    QPromiseError(const QPromiseError& error)
-        : m_exception(error.m_exception)
-    { }
-
-    QPromiseError(QPromiseError&& other)
-        : m_exception(nullptr)
-    {
-        swap(other);
-    }
-
-    QPromiseError& operator=(QPromiseError other)
-    {
-        swap(other);
-        return *this;
-    }
-
-    void swap(QPromiseError& other)
-    {
-        qSwap(m_exception, other.m_exception);
-    }
-
-    void rethrow() const
-    {
-        std::rethrow_exception(m_exception);
-    }
-
-private:
-    std::exception_ptr m_exception;
-};
-
 class QPromiseTimeoutException : public QException
 {
 public:
@@ -66,6 +20,12 @@ public:
     }
 };
 
+// QPromiseError is provided for backward compatibility and will be
+// removed in the next major version: it wasn't intended to be used
+// directly and thus should not be part of the public API.
+// TODO Remove QPromiseError at version 1.0
+using QPromiseError = QtPromisePrivate::PromiseError;
+
 } // namespace QtPromise
 
 #endif // QTPROMISE_QPROMISEERROR_H

tests/auto/qtpromise/qpromise/qpromise.pro

@@ -6,6 +6,7 @@ SUBDIRS += \
     fail \
     finally \
     operators \
+    reject \
     resolve \
     tap \
     tapfail \

tests/auto/qtpromise/qpromise/reject/reject.pro

@@ -0,0 +1,4 @@
+TARGET = tst_qpromise_reject
+SOURCES += $$PWD/tst_reject.cpp
+
+include(../../qtpromise.pri)

tests/auto/qtpromise/qpromise/reject/tst_reject.cpp

@@ -0,0 +1,74 @@
+// Tests
+#include "../../shared/utils.h"
+
+// QtPromise
+#include <QtPromise>
+
+// Qt
+#include <QtTest>
+
+// STL
+#include <memory>
+
+using namespace QtPromise;
+
+class tst_qpromise_reject : public QObject
+{
+    Q_OBJECT
+
+private Q_SLOTS:
+    void rejectWithValue();
+    void rejectWithQSharedPtr();
+    void rejectWithStdSharedPtr();
+};
+
+QTEST_MAIN(tst_qpromise_reject)
+#include "tst_reject.moc"
+
+void tst_qpromise_reject::rejectWithValue()
+{
+    auto p = QPromise<int>::reject(42);
+
+    QCOMPARE(p.isRejected(), true);
+    QCOMPARE(waitForError(p, -1), 42);
+}
+
+// https://github.com/simonbrunel/qtpromise/issues/6
+void tst_qpromise_reject::rejectWithQSharedPtr()
+{
+    QWeakPointer<int> wptr;
+
+    {
+        QSharedPointer<int> sptr(new int(42));
+        auto p = QPromise<int>::reject(sptr);
+
+        QCOMPARE(waitForError(p, QSharedPointer<int>()), sptr);
+
+        wptr = sptr;
+        sptr.reset();
+
+        QCOMPARE(wptr.isNull(), false); // "p" still holds a reference
+    }
+
+    QCOMPARE(wptr.isNull(), true);
+}
+
+// https://github.com/simonbrunel/qtpromise/issues/6
+void tst_qpromise_reject::rejectWithStdSharedPtr()
+{
+    std::weak_ptr<int> wptr;
+
+    {
+        std::shared_ptr<int> sptr(new int(42));
+        auto p = QPromise<int>::reject(sptr);
+
+        QCOMPARE(waitForError(p, std::shared_ptr<int>()), sptr);
+
+        wptr = sptr;
+        sptr.reset();
+
+        QCOMPARE(wptr.use_count(), 1l); // "p" still holds a reference
+    }
+
+    QCOMPARE(wptr.use_count(), 0l);
+}

tests/auto/qtpromise/qpromise/resolve/tst_resolve.cpp

@@ -7,6 +7,9 @@
 // Qt
 #include <QtTest>
 
+// STL
+#include <memory>
+
 using namespace QtPromise;
 
 class tst_qpromise_resolve : public QObject
@@ -14,14 +17,16 @@ class tst_qpromise_resolve : public QObject
     Q_OBJECT
 
 private Q_SLOTS:
-    void value();
+    void resolveWithValue();
-    void empty_void();
+    void resolveWithNoValue();
+    void resolveWithQSharedPtr();
+    void resolveWithStdSharedPtr();
 };
 
 QTEST_MAIN(tst_qpromise_resolve)
 #include "tst_resolve.moc"
 
-void tst_qpromise_resolve::value()
+void tst_qpromise_resolve::resolveWithValue()
 {
     const int value = 42;
     auto p0 = QPromise<int>::resolve(value);
@@ -33,9 +38,49 @@ void tst_qpromise_resolve::value()
     QCOMPARE(waitForValue(p1, -1), 43);
 }
 
-void tst_qpromise_resolve::empty_void()
+void tst_qpromise_resolve::resolveWithNoValue()
 {
     auto p = QPromise<void>::resolve();
 
     QCOMPARE(p.isFulfilled(), true);
 }
+
+// https://github.com/simonbrunel/qtpromise/issues/6
+void tst_qpromise_resolve::resolveWithQSharedPtr()
+{
+    QWeakPointer<int> wptr;
+
+    {
+        QSharedPointer<int> sptr(new int(42));
+        auto p = QPromise<QSharedPointer<int>>::resolve(sptr);
+
+        QCOMPARE(waitForValue(p, QSharedPointer<int>()), sptr);
+
+        wptr = sptr;
+        sptr.reset();
+
+        QCOMPARE(wptr.isNull(), false); // "p" still holds a reference
+    }
+
+    QCOMPARE(wptr.isNull(), true);
+}
+
+// https://github.com/simonbrunel/qtpromise/issues/6
+void tst_qpromise_resolve::resolveWithStdSharedPtr()
+{
+    std::weak_ptr<int> wptr;
+
+    {
+        std::shared_ptr<int> sptr(new int(42));
+        auto p = QPromise<std::shared_ptr<int>>::resolve(sptr);
+
+        QCOMPARE(waitForValue(p, std::shared_ptr<int>()), sptr);
+
+        wptr = sptr;
+        sptr.reset();
+
+        QCOMPARE(wptr.use_count(), 1l); // "p" still holds a reference
+    }
+
+    QCOMPARE(wptr.use_count(), 0l);
+}