From 8171192df54547bd5080addbbc63e1d21623cfac Mon Sep 17 00:00:00 2001 From: John Maguire Date: Thu, 22 Nov 2012 15:15:07 +0100 Subject: [PATCH] Port Closure to variadic templates. --- ext/libclementine-common/core/closure.cpp | 79 ++----- ext/libclementine-common/core/closure.h | 270 +++++++++------------- src/internet/googledriveclient.cpp | 14 +- src/internet/oauthenticator.cpp | 1 + src/moodbar/moodbarloader.cpp | 14 +- src/podcasts/gpoddersync.cpp | 2 +- src/podcasts/podcasturlloader.cpp | 16 +- src/songinfo/songkickconcerts.cpp | 1 + src/songinfo/songkickconcertwidget.cpp | 12 +- tests/CMakeLists.txt | 2 +- tests/closure_test.cpp | 23 +- 11 files changed, 185 insertions(+), 249 deletions(-) diff --git a/ext/libclementine-common/core/closure.cpp b/ext/libclementine-common/core/closure.cpp index 247750197..d751f8b1d 100644 --- a/ext/libclementine-common/core/closure.cpp +++ b/ext/libclementine-common/core/closure.cpp @@ -19,80 +19,47 @@ #include -#include "core/logging.h" #include "core/timeconstants.h" namespace _detail { -Closure::Closure(QObject* sender, - const char* signal, - QObject* receiver, - const char* slot, - const ClosureArgumentWrapper* val0, - const ClosureArgumentWrapper* val1, - const ClosureArgumentWrapper* val2, - const ClosureArgumentWrapper* val3) - : QObject(receiver), - callback_(NULL), - val0_(val0), - val1_(val1), - val2_(val2), - val3_(val3) { - const QMetaObject* meta_receiver = receiver->metaObject(); - - QByteArray normalised_slot = QMetaObject::normalizedSignature(slot + 1); - const int index = meta_receiver->indexOfSlot(normalised_slot.constData()); - Q_ASSERT(index != -1); - slot_ = meta_receiver->method(index); - - Connect(sender, signal); +ClosureBase::ClosureBase(ObjectHelper* helper) + : helper_(helper) { } -Closure::Closure(QObject* sender, - const char* signal, - std::tr1::function callback) - : callback_(callback) { - Connect(sender, signal); +ClosureBase::~ClosureBase() { } -Closure::~Closure() { +ObjectHelper* ClosureBase::helper() const { + return helper_; } -void Closure::Connect(QObject* sender, const char* signal) { - bool success = connect(sender, signal, SLOT(Invoked())); - Q_ASSERT(success); - success = connect(sender, SIGNAL(destroyed()), SLOT(Cleanup())); - Q_ASSERT(success); - Q_UNUSED(success); +ObjectHelper::ObjectHelper( + QObject* sender, + const char* signal, + ClosureBase* closure) + : closure_(closure) { + connect(sender, signal, SLOT(Invoked())); + connect(sender, SIGNAL(destroyed()), SLOT(deleteLater())); } -void Closure::Invoked() { - if (callback_) { - callback_(); - } else { - slot_.invoke( - parent(), - val0_ ? val0_->arg() : QGenericArgument(), - val1_ ? val1_->arg() : QGenericArgument(), - val2_ ? val2_->arg() : QGenericArgument(), - val3_ ? val3_->arg() : QGenericArgument()); - } +void ObjectHelper::Invoked() { + closure_->Invoke(); deleteLater(); } -void Closure::Cleanup() { - disconnect(); - deleteLater(); -} +template<> +void Arg>( + const boost::tuples::tuple<>&, + QList* list) {} + +template<> +void Arg( + const boost::tuples::null_type&, + QList* list) {} } // namespace _detail -_detail::Closure* NewClosure( - QObject* sender, const char* signal, - QObject* receiver, const char* slot) { - return new _detail::Closure(sender, signal, receiver, slot); -} - void DoAfter(QObject* receiver, const char* slot, int msec) { QTimer::singleShot(msec, receiver, slot); } diff --git a/ext/libclementine-common/core/closure.h b/ext/libclementine-common/core/closure.h index 4a3b3bee2..351d926ab 100644 --- a/ext/libclementine-common/core/closure.h +++ b/ext/libclementine-common/core/closure.h @@ -18,211 +18,169 @@ #ifndef CLOSURE_H #define CLOSURE_H -#include - #include #include #include +#include +#include #include #include - -#include "core/logging.h" +#include namespace _detail { -class ClosureArgumentWrapper { - public: - virtual ~ClosureArgumentWrapper() {} +class ObjectHelper; - virtual QGenericArgument arg() const = 0; +// Interface for ObjectHelper to call on signal emission. +class ClosureBase : boost::noncopyable { + public: + virtual ~ClosureBase(); + virtual void Invoke() = 0; + + // Tests only. + ObjectHelper* helper() const; + + protected: + explicit ClosureBase(ObjectHelper*); + ObjectHelper* helper_; }; -template -class ClosureArgument : public ClosureArgumentWrapper { - public: - explicit ClosureArgument(const T& data) : data_(data) {} - - virtual QGenericArgument arg() const { - return Q_ARG(T, data_); - } - - private: - T data_; -}; - -class Closure : public QObject, boost::noncopyable { +// QObject helper as templated QObjects do not work. +// Connects to the given signal and invokes the closure when called. +// Deletes itself and the Closure after being invoked. +class ObjectHelper : public QObject { Q_OBJECT - public: - Closure(QObject* sender, const char* signal, - QObject* receiver, const char* slot, - const ClosureArgumentWrapper* val0 = 0, - const ClosureArgumentWrapper* val1 = 0, - const ClosureArgumentWrapper* val2 = 0, - const ClosureArgumentWrapper* val3 = 0); - - Closure(QObject* sender, const char* signal, - std::tr1::function callback); - - virtual ~Closure(); + ObjectHelper( + QObject* parent, + const char* signal, + ClosureBase* closure); private slots: void Invoked(); - void Cleanup(); private: - void Connect(QObject* sender, const char* signal); - - QMetaMethod slot_; - std::tr1::function callback_; - - boost::scoped_ptr val0_; - boost::scoped_ptr val1_; - boost::scoped_ptr val2_; - boost::scoped_ptr val3_; -}; - -class SharedPointerWrapper { - public: - virtual ~SharedPointerWrapper() {} - virtual QObject* data() const = 0; + boost::scoped_ptr closure_; + Q_DISABLE_COPY(ObjectHelper); }; +// Helper to unpack a tuple into a QList. template -class SharedPointer : public SharedPointerWrapper { +void Arg(const T& tail, QList* list) { + typedef decltype(tail.get_head()) HeadType; + list->append(Q_ARG(HeadType, tail.get_head())); + Arg(tail.get_tail(), list); +} + +// Specialisation for starting with an empty tuple, ie. no arguments. +template<> +void Arg>( + const boost::tuples::tuple<>&, + QList* list); + +// Specialisation for the end of a tuple, where get_tail() returns null_type. +template<> +void Arg( + const boost::tuples::null_type&, + QList* list); + +template +class Closure : public ClosureBase { public: - explicit SharedPointer(QSharedPointer ptr) - : ptr_(ptr) { + Closure( + QObject* sender, + const char* signal, + QObject* receiver, + const char* slot, + const Args&... args) + : ClosureBase(new ObjectHelper(sender, signal, this)), + // boost::bind is the easiest way to store an argument list. + function_(boost::bind(&Closure::Call, this, args...)), + receiver_(receiver) { + const QMetaObject* meta_receiver = receiver->metaObject(); + QByteArray normalised_slot = QMetaObject::normalizedSignature(slot + 1); + const int index = meta_receiver->indexOfSlot(normalised_slot.constData()); + Q_ASSERT(index != -1); + slot_ = meta_receiver->method(index); } - QObject* data() const { - return ptr_.data(); + virtual void Invoke() { + function_(); } private: - QSharedPointer ptr_; + void Call(const Args&... args) { + auto t = boost::make_tuple(args...); + QList arg_list; + Arg(t, &arg_list); + + slot_.invoke( + receiver_, + arg_list.size() > 0 ? arg_list[0] : QGenericArgument(), + arg_list.size() > 1 ? arg_list[1] : QGenericArgument(), + arg_list.size() > 2 ? arg_list[2] : QGenericArgument(), + arg_list.size() > 3 ? arg_list[3] : QGenericArgument(), + arg_list.size() > 4 ? arg_list[4] : QGenericArgument(), + arg_list.size() > 5 ? arg_list[5] : QGenericArgument(), + arg_list.size() > 6 ? arg_list[6] : QGenericArgument(), + arg_list.size() > 7 ? arg_list[7] : QGenericArgument(), + arg_list.size() > 8 ? arg_list[8] : QGenericArgument(), + arg_list.size() > 9 ? arg_list[9] : QGenericArgument()); + } + + boost::function function_; + QObject* receiver_; + QMetaMethod slot_; }; -// For use with a QSharedPointer as a sender. -class SharedClosure : public Closure { - Q_OBJECT - +template +class SharedClosure : public Closure { public: - SharedClosure(SharedPointerWrapper* sender, const char* signal, - QObject* receiver, const char* slot, - const ClosureArgumentWrapper* val0 = 0, - const ClosureArgumentWrapper* val1 = 0, - const ClosureArgumentWrapper* val2 = 0, - const ClosureArgumentWrapper* val3 = 0) - : Closure(sender->data(), signal, - receiver, slot, - val0, val1, val2, val3), - shared_sender_(sender) { + SharedClosure( + QSharedPointer sender, + const char* signal, + QObject* receiver, + const char* slot, + const Args&... args) + : Closure( + sender.data(), + signal, + receiver, + slot, + args...), + data_(sender) { } private: - boost::scoped_ptr shared_sender_; + QSharedPointer data_; }; } // namespace _detail -#define C_ARG(type, data) new _detail::ClosureArgument(data) - -_detail::Closure* NewClosure( - QObject* sender, - const char* signal, - QObject* receiver, - const char* slot); - -template -_detail::Closure* NewClosure( +template +_detail::ClosureBase* NewClosure( QObject* sender, const char* signal, QObject* receiver, const char* slot, - const T& val0) { - return new _detail::Closure( - sender, signal, receiver, slot, - C_ARG(T, val0)); + const Args&... args) { + return new _detail::Closure( + sender, signal, receiver, slot, args...); } -template -_detail::Closure* NewClosure( - QObject* sender, +// QSharedPointer variant +template +_detail::ClosureBase* NewClosure( + QSharedPointer sender, const char* signal, QObject* receiver, const char* slot, - const T0& val0, - const T1& val1) { - return new _detail::Closure( - sender, signal, receiver, slot, - C_ARG(T0, val0), C_ARG(T1, val1)); + const Args&... args) { + return new _detail::SharedClosure( + sender, signal, receiver, slot, args...); } -template -_detail::Closure* NewClosure( - QObject* sender, - const char* signal, - QObject* receiver, - const char* slot, - const T0& val0, - const T1& val1, - const T2& val2) { - return new _detail::Closure( - sender, signal, receiver, slot, - C_ARG(T0, val0), C_ARG(T1, val1), C_ARG(T2, val2)); -} - -template -_detail::Closure* NewClosure( - QObject* sender, - const char* signal, - QObject* receiver, - const char* slot, - const T0& val0, - const T1& val1, - const T2& val2, - const T3& val3) { - return new _detail::Closure( - sender, signal, receiver, slot, - C_ARG(T0, val0), C_ARG(T1, val1), C_ARG(T2, val2), C_ARG(T3, val3)); -} - -template -_detail::Closure* NewClosure( - QSharedPointer sender, - const char* signal, - QObject* receiver, - const char* slot) { - return new _detail::SharedClosure( - new _detail::SharedPointer(sender), signal, receiver, slot); -} - -template -_detail::Closure* NewClosure( - QSharedPointer sender, - const char* signal, - QObject* receiver, - const char* slot, - const T0& val0) { - return new _detail::SharedClosure( - new _detail::SharedPointer(sender), signal, receiver, slot, - C_ARG(T0, val0)); -} - -template -_detail::Closure* NewClosure( - QSharedPointer sender, - const char* signal, - QObject* receiver, - const char* slot, - const T0& val0, - const T1& val1) { - return new _detail::SharedClosure( - new _detail::SharedPointer(sender), signal, receiver, slot, - C_ARG(T0, val0), C_ARG(T1, val1)); -} void DoAfter(QObject* receiver, const char* slot, int msec); void DoInAMinuteOrSo(QObject* receiver, const char* slot); diff --git a/src/internet/googledriveclient.cpp b/src/internet/googledriveclient.cpp index e7e2d4c72..6f25ec8b2 100644 --- a/src/internet/googledriveclient.cpp +++ b/src/internet/googledriveclient.cpp @@ -1,27 +1,29 @@ /* This file is part of Clementine. Copyright 2012, David Sansome - + Clementine is free software: you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software Foundation, either version 3 of the License, or (at your option) any later version. - + Clementine is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for more details. - + You should have received a copy of the GNU General Public License along with Clementine. If not, see . */ #include "googledriveclient.h" -#include "oauthenticator.h" -#include "core/closure.h" -#include "core/network.h" #include +#include "oauthenticator.h" +#include "core/closure.h" +#include "core/logging.h" +#include "core/network.h" + using namespace google_drive; const char* File::kFolderMimeType = "application/vnd.google-apps.folder"; diff --git a/src/internet/oauthenticator.cpp b/src/internet/oauthenticator.cpp index 6a8227b5c..399418ced 100644 --- a/src/internet/oauthenticator.cpp +++ b/src/internet/oauthenticator.cpp @@ -12,6 +12,7 @@ #include #include "core/closure.h" +#include "core/logging.h" namespace { diff --git a/src/moodbar/moodbarloader.cpp b/src/moodbar/moodbarloader.cpp index 0811fd95e..acbceee77 100644 --- a/src/moodbar/moodbarloader.cpp +++ b/src/moodbar/moodbarloader.cpp @@ -16,11 +16,8 @@ */ #include "moodbarloader.h" -#include "moodbarpipeline.h" -#include "core/application.h" -#include "core/closure.h" -#include "core/qhash_qurl.h" -#include "core/utilities.h" + +#include #include #include @@ -30,7 +27,12 @@ #include #include -#include +#include "moodbarpipeline.h" +#include "core/application.h" +#include "core/closure.h" +#include "core/logging.h" +#include "core/qhash_qurl.h" +#include "core/utilities.h" MoodbarLoader::MoodbarLoader(Application* app, QObject* parent) : QObject(parent), diff --git a/src/podcasts/gpoddersync.cpp b/src/podcasts/gpoddersync.cpp index e638c4976..4110f56cb 100644 --- a/src/podcasts/gpoddersync.cpp +++ b/src/podcasts/gpoddersync.cpp @@ -189,7 +189,7 @@ void GPodderSync::DeviceUpdatesFinished(mygpo::DeviceUpdatesPtr reply) { // have a list of the episodes. PodcastUrlLoaderReply* loader_reply = loader_->Load(url); NewClosure(loader_reply, SIGNAL(Finished(bool)), - this, SLOT(NewPodcastLoaded(PodcastUrlLoaderReply*,QUrl,QList >)), + this, SLOT(NewPodcastLoaded(PodcastUrlLoaderReply*,QUrl,QList)), loader_reply, url, episodes_by_podcast[url]); } diff --git a/src/podcasts/podcasturlloader.cpp b/src/podcasts/podcasturlloader.cpp index def0314f3..e342d5350 100644 --- a/src/podcasts/podcasturlloader.cpp +++ b/src/podcasts/podcasturlloader.cpp @@ -1,28 +1,30 @@ /* This file is part of Clementine. Copyright 2012, David Sansome - + Clementine is free software: you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software Foundation, either version 3 of the License, or (at your option) any later version. - + Clementine is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for more details. - + You should have received a copy of the GNU General Public License along with Clementine. If not, see . */ -#include "podcastparser.h" #include "podcasturlloader.h" -#include "core/closure.h" -#include "core/network.h" -#include "core/utilities.h" #include +#include "podcastparser.h" +#include "core/closure.h" +#include "core/logging.h" +#include "core/network.h" +#include "core/utilities.h" + const int PodcastUrlLoader::kMaxRedirects = 5; diff --git a/src/songinfo/songkickconcerts.cpp b/src/songinfo/songkickconcerts.cpp index bf0bf3023..456371b69 100644 --- a/src/songinfo/songkickconcerts.cpp +++ b/src/songinfo/songkickconcerts.cpp @@ -26,6 +26,7 @@ #include #include "core/closure.h" +#include "core/logging.h" #include "songkickconcertwidget.h" const char* SongkickConcerts::kSongkickArtistBucket = "id:songkick"; diff --git a/src/songinfo/songkickconcertwidget.cpp b/src/songinfo/songkickconcertwidget.cpp index 2ba6c4e8a..584a26c44 100644 --- a/src/songinfo/songkickconcertwidget.cpp +++ b/src/songinfo/songkickconcertwidget.cpp @@ -15,18 +15,20 @@ along with Clementine. If not, see . */ -#include "songinfotextview.h" #include "songkickconcertwidget.h" -#include "ui_songkickconcertwidget.h" -#include "core/closure.h" -#include "core/network.h" -#include "core/utilities.h" #include #include #include #include +#include "songinfotextview.h" +#include "ui_songkickconcertwidget.h" +#include "core/closure.h" +#include "core/logging.h" +#include "core/network.h" +#include "core/utilities.h" + const int SongKickConcertWidget::kStaticMapWidth = 100; const int SongKickConcertWidget::kStaticMapHeight = 100; diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index acfe67d7b..51dc461ee 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -1,6 +1,6 @@ cmake_minimum_required(VERSION 2.6) -set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++0x -U__STRICT_ANSI__ -fpermissive") +set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++0x -fpermissive -U__STRICT_ANSI__") if(USE_SYSTEM_GMOCK) include_directories(${GMOCK_INCLUDE_DIRS} ${GTEST_INCLUDE_DIRS}) diff --git a/tests/closure_test.cpp b/tests/closure_test.cpp index b22bf6629..8d26ccfad 100644 --- a/tests/closure_test.cpp +++ b/tests/closure_test.cpp @@ -11,7 +11,7 @@ TEST(ClosureTest, ClosureInvokesReceiver) { TestQObject sender; TestQObject receiver; - _detail::Closure* closure = NewClosure( + _detail::ClosureBase* closure = NewClosure( &sender, SIGNAL(Emitted()), &receiver, SLOT(Invoke())); EXPECT_EQ(0, receiver.invoked()); @@ -22,17 +22,18 @@ TEST(ClosureTest, ClosureInvokesReceiver) { TEST(ClosureTest, ClosureDeletesSelf) { TestQObject sender; TestQObject receiver; - _detail::Closure* closure = NewClosure( + _detail::ClosureBase* closure = NewClosure( &sender, SIGNAL(Emitted()), &receiver, SLOT(Invoke())); - QSignalSpy spy(closure, SIGNAL(destroyed())); + _detail::ObjectHelper* helper = closure->helper(); + QSignalSpy spy(helper, SIGNAL(destroyed())); EXPECT_EQ(0, receiver.invoked()); sender.Emit(); EXPECT_EQ(1, receiver.invoked()); EXPECT_EQ(0, spy.count()); QEventLoop loop; - QObject::connect(closure, SIGNAL(destroyed()), &loop, SLOT(quit())); + QObject::connect(helper, SIGNAL(destroyed()), &loop, SLOT(quit())); loop.exec(); EXPECT_EQ(1, spy.count()); } @@ -41,23 +42,23 @@ TEST(ClosureTest, ClosureDoesNotCrashWithSharedPointerSender) { TestQObject receiver; TestQObject* sender; boost::scoped_ptr spy; - QPointer<_detail::Closure> closure; + QPointer<_detail::ObjectHelper> closure; { QSharedPointer sender_shared(new TestQObject); sender = sender_shared.data(); - closure = QPointer<_detail::Closure>(NewClosure( + closure = QPointer<_detail::ObjectHelper>(NewClosure( sender_shared, SIGNAL(Emitted()), - &receiver, SLOT(Invoke()))); + &receiver, SLOT(Invoke()))->helper()); spy.reset(new QSignalSpy(sender, SIGNAL(destroyed()))); } - EXPECT_EQ(0, receiver.invoked()); + ASSERT_EQ(0, receiver.invoked()); sender->Emit(); - EXPECT_EQ(1, receiver.invoked()); + ASSERT_EQ(1, receiver.invoked()); - EXPECT_EQ(0, spy->count()); + ASSERT_EQ(0, spy->count()); QEventLoop loop; QObject::connect(sender, SIGNAL(destroyed()), &loop, SLOT(quit())); loop.exec(); - EXPECT_EQ(1, spy->count()); + ASSERT_EQ(1, spy->count()); EXPECT_TRUE(closure.isNull()); }