Add ImplPtr and makeImpl

The original (more complex and comprehensive) solution belongs to
https://oliora.github.io/2015/12/29/pimpl-and-rule-of-zero.html - this
commit only provides a small wrapper for non-copyable Private class
implementations common throughout libQuotient. Unlike the original,
default initialisation is made explicit - you have to pass
ZeroImpl<Private>() instead (and I firmly believe it's a good thing:
normally pointers to Private should not remain nullptr). The reason
ZeroImpl<> is not a template variable is quite simple: unique_ptr is
non-copyable and so cannot be initialised from; while a template
function will initialise the value in-place thanks to copy elision.

19 files changed, 75 insertions, 54 deletions

diff --git a/lib/avatar.cpp b/lib/avatar.cpp
index 77648562..9304a3de 100644
--- a/lib/avatar.cpp
+++ b/lib/avatar.cpp
@@ -47,15 +47,11 @@ public:
     mutable std::vector<get_callback_t> callbacks;
 };
 
-Avatar::Avatar() : d(std::make_unique<Private>()) {}
+Avatar::Avatar()
+    : d(makeImpl<Private>())
+{}
 
-Avatar::Avatar(QUrl url) : d(std::make_unique<Private>(std::move(url))) {}
-
-Avatar::Avatar(Avatar&&) = default;
-
-Avatar::~Avatar() = default;
-
-Avatar& Avatar::operator=(Avatar&&) = default;
+Avatar::Avatar(QUrl url) : d(makeImpl<Private>(std::move(url))) {}
 
 QImage Avatar::get(Connection* connection, int dimension,
                    get_callback_t callback) const
diff --git a/lib/avatar.h b/lib/avatar.h
index 93f43948..c94dc369 100644
--- a/lib/avatar.h
+++ b/lib/avatar.h
@@ -3,13 +3,12 @@
 
 #pragma once
 
-#include "quotient_export.h"
+#include "util.h"
 
 #include <QtCore/QUrl>
 #include <QtGui/QIcon>
 
 #include <functional>
-#include <memory>
 
 namespace Quotient {
 class Connection;
@@ -18,9 +17,6 @@ class QUOTIENT_API Avatar {
 public:
     explicit Avatar();
     explicit Avatar(QUrl url);
-    Avatar(Avatar&&);
-    ~Avatar();
-    Avatar& operator=(Avatar&&);
 
     using get_callback_t = std::function<void()>;
     using upload_callback_t = std::function<void(QUrl)>;
@@ -41,6 +37,6 @@ public:
 
 private:
     class Private;
-    std::unique_ptr<Private> d;
+    ImplPtr<Private> d;
 };
 } // namespace Quotient
diff --git a/lib/connection.cpp b/lib/connection.cpp
index 8d1c80f1..1915c2a9 100644
--- a/lib/connection.cpp
+++ b/lib/connection.cpp
@@ -246,7 +246,8 @@ public:
 };
 
 Connection::Connection(const QUrl& server, QObject* parent)
-    : QObject(parent), d(new Private(std::make_unique<ConnectionData>(server)))
+    : QObject(parent)
+    , d(makeImpl<Private>(std::make_unique<ConnectionData>(server)))
 {
     d->q = this; // All d initialization should occur before this line
 }
diff --git a/lib/connection.h b/lib/connection.h
index 23062664..b4476347 100644
--- a/lib/connection.h
+++ b/lib/connection.h
@@ -844,7 +844,7 @@ protected Q_SLOTS:
 
 private:
     class Private;
-    QScopedPointer<Private> d;
+    ImplPtr<Private> d;
 
     static room_factory_t _roomFactory;
     static user_factory_t _userFactory;
diff --git a/lib/connectiondata.cpp b/lib/connectiondata.cpp
index 87ad4577..aca218be 100644
--- a/lib/connectiondata.cpp
+++ b/lib/connectiondata.cpp
@@ -41,7 +41,7 @@ public:
 };
 
 ConnectionData::ConnectionData(QUrl baseUrl)
-    : d(std::make_unique<Private>(std::move(baseUrl)))
+    : d(makeImpl<Private>(std::move(baseUrl)))
 {
     // Each lambda invocation below takes no more than one job from the
     // queues (first foreground, then background) and resumes it; then
diff --git a/lib/connectiondata.h b/lib/connectiondata.h
index e16a2dac..75fc332f 100644
--- a/lib/connectiondata.h
+++ b/lib/connectiondata.h
@@ -4,9 +4,10 @@
 
 #pragma once
 
+#include "util.h"
+
 #include <QtCore/QUrl>
 
-#include <memory>
 #include <chrono>
 
 class QNetworkAccessManager;
@@ -42,6 +43,6 @@ public:
 
 private:
     class Private;
-    std::unique_ptr<Private> d;
+    ImplPtr<Private> d;
 };
 } // namespace Quotient
diff --git a/lib/jobs/basejob.cpp b/lib/jobs/basejob.cpp
index 971fea7b..f518a1b0 100644
--- a/lib/jobs/basejob.cpp
+++ b/lib/jobs/basejob.cpp
@@ -194,8 +194,8 @@ BaseJob::BaseJob(HttpVerb verb, const QString& name, QByteArray endpoint,
 
 BaseJob::BaseJob(HttpVerb verb, const QString& name, QByteArray endpoint,
                  const QUrlQuery& query, RequestData&& data, bool needsToken)
-    : d(new Private(verb, std::move(endpoint), query, std::move(data),
-                    needsToken))
+    : d(makeImpl<Private>(verb, std::move(endpoint), query, std::move(data),
+                          needsToken))
 {
     setObjectName(name);
     connect(&d->timer, &QTimer::timeout, this, &BaseJob::timeout);
diff --git a/lib/jobs/basejob.h b/lib/jobs/basejob.h
index f41fc63c..c899170d 100644
--- a/lib/jobs/basejob.h
+++ b/lib/jobs/basejob.h
@@ -467,7 +467,7 @@ private:
     void finishJob();
 
     class Private;
-    QScopedPointer<Private> d;
+    ImplPtr<Private> d;
 };
 
 inline bool QUOTIENT_API isJobPending(BaseJob* job)
diff --git a/lib/jobs/downloadfilejob.cpp b/lib/jobs/downloadfilejob.cpp
index 6bf221cb..4a507ebd 100644
--- a/lib/jobs/downloadfilejob.cpp
+++ b/lib/jobs/downloadfilejob.cpp
@@ -32,13 +32,12 @@ DownloadFileJob::DownloadFileJob(const QString& serverName,
                                  const QString& mediaId,
                                  const QString& localFilename)
     : GetContentJob(serverName, mediaId)
-    , d(localFilename.isEmpty() ? new Private : new Private(localFilename))
+    , d(localFilename.isEmpty() ? makeImpl<Private>()
+                                : makeImpl<Private>(localFilename))
 {
     setObjectName(QStringLiteral("DownloadFileJob"));
 }
 
-DownloadFileJob::~DownloadFileJob() = default;
-
 QString DownloadFileJob::targetFileName() const
 {
     return (d->targetFile ? d->targetFile : d->tempFile)->fileName();
diff --git a/lib/jobs/downloadfilejob.h b/lib/jobs/downloadfilejob.h
index 9e807fe7..f8c62e4b 100644
--- a/lib/jobs/downloadfilejob.h
+++ b/lib/jobs/downloadfilejob.h
@@ -13,13 +13,12 @@ public:
 
     DownloadFileJob(const QString& serverName, const QString& mediaId,
                     const QString& localFilename = {});
-    ~DownloadFileJob() override;
 
     QString targetFileName() const;
 
 private:
     class Private;
-    QScopedPointer<Private> d;
+    ImplPtr<Private> d;
 
     void doPrepare() override;
     void onSentRequest(QNetworkReply* reply) override;
diff --git a/lib/mxcreply.cpp b/lib/mxcreply.cpp
index fb16c79f..d3cc3c37 100644
--- a/lib/mxcreply.cpp
+++ b/lib/mxcreply.cpp
@@ -17,7 +17,7 @@ public:
 };
 
 MxcReply::MxcReply(QNetworkReply* reply)
-    : d(std::make_unique<Private>(reply))
+    : d(makeImpl<Private>(reply))
 {
     reply->setParent(this);
     connect(d->m_reply, &QNetworkReply::finished, this, [this]() {
@@ -28,7 +28,7 @@ MxcReply::MxcReply(QNetworkReply* reply)
 }
 
 MxcReply::MxcReply(QNetworkReply* reply, Room* room, const QString &eventId)
-    : d(std::make_unique<Private>(reply))
+    : d(makeImpl<Private>(reply))
 {
     reply->setParent(this);
     connect(d->m_reply, &QNetworkReply::finished, this, [this, room, eventId]() {
@@ -38,8 +38,6 @@ MxcReply::MxcReply(QNetworkReply* reply, Room* room, const QString &eventId)
     });
 }
 
-MxcReply::~MxcReply() = default;
-
 #if QT_VERSION >= QT_VERSION_CHECK(5, 15, 0)
 #define ERROR_SIGNAL errorOccurred
 #else
@@ -47,6 +45,7 @@ MxcReply::~MxcReply() = default;
 #endif
 
 MxcReply::MxcReply()
+    : d(ZeroImpl<Private>())
 {
     static const auto BadRequestPhrase = tr("Bad Request");
     QMetaObject::invokeMethod(this, [this]() {
diff --git a/lib/mxcreply.h b/lib/mxcreply.h
index 1d31d608..f6c4a34d 100644
--- a/lib/mxcreply.h
+++ b/lib/mxcreply.h
@@ -3,10 +3,9 @@
 
 #pragma once
 
-#include "quotient_export.h"
+#include "util.h"
 
 #include <QtNetwork/QNetworkReply>
-#include <memory>
 
 namespace Quotient {
 class Room;
@@ -18,7 +17,6 @@ public:
     explicit MxcReply();
     explicit MxcReply(QNetworkReply *reply);
     MxcReply(QNetworkReply* reply, Room* room, const QString& eventId);
-    ~MxcReply() override;
 
 public Q_SLOTS:
     void abort() override;
@@ -28,6 +26,6 @@ protected:
 
 private:
     class Private;
-    std::unique_ptr<Private> d;
+    ImplPtr<Private> d;
 };
 }
diff --git a/lib/networkaccessmanager.cpp b/lib/networkaccessmanager.cpp
index 57618329..2c0f716b 100644
--- a/lib/networkaccessmanager.cpp
+++ b/lib/networkaccessmanager.cpp
@@ -39,7 +39,7 @@ public:
 };
 
 NetworkAccessManager::NetworkAccessManager(QObject* parent)
-    : QNetworkAccessManager(parent), d(std::make_unique<Private>(this))
+    : QNetworkAccessManager(parent), d(makeImpl<Private>(this))
 {}
 
 QList<QSslError> NetworkAccessManager::ignoredSslErrors() const
@@ -79,8 +79,6 @@ NetworkAccessManager* NetworkAccessManager::instance()
     return storage.localData();
 }
 
-NetworkAccessManager::~NetworkAccessManager() = default;
-
 QNetworkReply* NetworkAccessManager::createRequest(
     Operation op, const QNetworkRequest& request, QIODevice* outgoingData)
 {
diff --git a/lib/networkaccessmanager.h b/lib/networkaccessmanager.h
index d06f9736..5a9c134c 100644
--- a/lib/networkaccessmanager.h
+++ b/lib/networkaccessmanager.h
@@ -3,20 +3,18 @@
 
 #pragma once
 
-#include "quotient_export.h"
+#include "util.h"
 
 #include <QtNetwork/QNetworkAccessManager>
 
-#include <memory>
-
 namespace Quotient {
 class Room;
 class Connection;
+
 class QUOTIENT_API NetworkAccessManager : public QNetworkAccessManager {
     Q_OBJECT
 public:
     NetworkAccessManager(QObject* parent = nullptr);
-    ~NetworkAccessManager() override;
 
     QList<QSslError> ignoredSslErrors() const;
     void addIgnoredSslError(const QSslError& error);
@@ -33,6 +31,6 @@ private:
                                  QIODevice* outgoingData = Q_NULLPTR) override;
 
     class Private;
-    std::unique_ptr<Private> d;
+    ImplPtr<Private> d;
 };
 } // namespace Quotient
diff --git a/lib/ssosession.cpp b/lib/ssosession.cpp
index a1d27504..5f3479b8 100644
--- a/lib/ssosession.cpp
+++ b/lib/ssosession.cpp
@@ -61,7 +61,7 @@ public:
 SsoSession::SsoSession(Connection* connection, const QString& initialDeviceName,
                        const QString& deviceId)
     : QObject(connection)
-    , d(std::make_unique<Private>(this, initialDeviceName, deviceId, connection))
+    , d(makeImpl<Private>(this, initialDeviceName, deviceId, connection))
 {
     qCDebug(MAIN) << "SSO session constructed";
 }
diff --git a/lib/ssosession.h b/lib/ssosession.h
index a658c043..0f3fc3b8 100644
--- a/lib/ssosession.h
+++ b/lib/ssosession.h
@@ -3,13 +3,11 @@
 
 #pragma once
 
-#include "quotient_export.h"
+#include "util.h"
 
 #include <QtCore/QUrl>
 #include <QtCore/QObject>
 
-#include <memory>
-
 class QTcpServer;
 class QTcpSocket;
 
@@ -44,6 +42,6 @@ public:
 
 private:
     class Private;
-    std::unique_ptr<Private> d;
+    ImplPtr<Private> d;
 };
 } // namespace Quotient
diff --git a/lib/user.cpp b/lib/user.cpp
index 7da71dba..0dbc444a 100644
--- a/lib/user.cpp
+++ b/lib/user.cpp
@@ -46,7 +46,7 @@ public:
 decltype(User::Private::otherAvatars) User::Private::otherAvatars {};
 
 User::User(QString userId, Connection* connection)
-    : QObject(connection), d(new Private(move(userId)))
+    : QObject(connection), d(makeImpl<Private>(move(userId)))
 {
     setObjectName(id());
     if (connection->userId() == id()) {
@@ -61,8 +61,6 @@ Connection* User::connection() const
     return static_cast<Connection*>(parent());
 }
 
-User::~User() = default;
-
 void User::load()
 {
     auto* profileJob =
diff --git a/lib/user.h b/lib/user.h
index 435304ce..8412b7fd 100644
--- a/lib/user.h
+++ b/lib/user.h
@@ -5,7 +5,7 @@
 #pragma once
 
 #include "avatar.h"
-#include "quotient_export.h"
+#include "util.h"
 
 #include <QtCore/QObject>
 
@@ -27,7 +27,6 @@ class QUOTIENT_API User : public QObject {
     Q_PROPERTY(QUrl avatarUrl READ avatarUrl NOTIFY defaultAvatarChanged)
 public:
     User(QString userId, Connection* connection);
-    ~User() override;
 
     Connection* connection() const;
 
@@ -126,7 +125,7 @@ Q_SIGNALS:
 
 private:
     class Private;
-    QScopedPointer<Private> d;
+    ImplPtr<Private> d;
 
     template <typename SourceT>
     bool doSetAvatar(SourceT&& source);
diff --git a/lib/util.h b/lib/util.h
index 399f93c2..f5650ee2 100644
--- a/lib/util.h
+++ b/lib/util.h
@@ -245,6 +245,47 @@ inline std::pair<InputIt, ForwardIt> findFirstOf(InputIt first, InputIt last,
     return std::make_pair(last, sLast);
 }
 
+//! \brief An owning implementation pointer
+//!
+//! This is basically std::unique_ptr<> to hold your pimpl's but without having
+//! to define default constructors/operator=() out of line.
+//! Thanks to https://oliora.github.io/2015/12/29/pimpl-and-rule-of-zero.html
+//! for inspiration
+template <typename ImplType>
+using ImplPtr = std::unique_ptr<ImplType, void (*)(ImplType*)>;
+
+// Why this works (see also the link above): because this defers the moment
+// of requiring sizeof of ImplType to the place where makeImpl is invoked
+// (which is located, necessarily, in the .cpp file after ImplType definition).
+// The stock unique_ptr deleter (std::default_delete) normally needs sizeof
+// at the same spot - as long as you defer definition of the owning type
+// constructors and operator='s to the .cpp file as well. Which means you
+// have to explicitly declare and define them (even if with = default),
+// formally breaking the rule of zero; informally, just adding boilerplate code.
+// The custom deleter itself is instantiated at makeImpl invocation - there's
+// no way earlier to even know how ImplType will be deleted and whether that
+// will need sizeof(ImplType) earlier. In theory it's a tad slower because
+// the deleter is called by the pointer; however, the difference will not
+// be noticeable (if exist at all) for any class with non-trivial contents.
+
+//! \brief make_unique for ImplPtr
+//!
+//! Since std::make_unique is not compatible with ImplPtr, this should be used
+//! in constructors of frontend classes to create implementation instances.
+template <typename ImplType, typename DeleterType = void (*)(ImplType*),
+          typename... ArgTs>
+inline ImplPtr<ImplType> makeImpl(ArgTs&&... args)
+{
+    return ImplPtr<ImplType> { new ImplType(std::forward<ArgTs>(args)...),
+                               [](ImplType* impl) { delete impl; } };
+}
+
+template <typename ImplType>
+const inline ImplPtr<ImplType> ZeroImpl()
+{
+    return { nullptr, [](ImplType*) {} };
+}
+
 /** Convert what looks like a URL or a Matrix ID to an HTML hyperlink */
 QUOTIENT_API void linkifyUrls(QString& htmlEscapedText);