From bc8e01df4286f5f8ff9103fbebad801f355db689 Mon Sep 17 00:00:00 2001 From: Alexey Rusakov Date: Sun, 3 Jul 2022 22:14:13 +0200 Subject: Shorten switchOnType, function_traits and connect* ...thanks to C++20 awesomeness. A notable change is that wrap_in_function() (and respectively function_traits<>::function_type) and fn_return_t alias are gone. The former are no more needed because connectUntil/connectSingleShot no more use std::function. The latter has been relatively underused and with the optimisation of switchOnType hereby, could be completely replaced with std::invoke_result_t. Rewriting connect* functions using constexpr and auto parameters made the implementation 30% more compact and much easier to understand (though still with a couple of - now thoroughly commented - tricky places). Dropping std::function<> from it may also bring some (quite modest, likely) performance benefits. --- lib/events/event.h | 64 +++++++------------ lib/function_traits.cpp | 3 + lib/function_traits.h | 36 +++++------ lib/qt_connection_util.h | 160 +++++++++++++++++------------------------------ 4 files changed, 101 insertions(+), 162 deletions(-) diff --git a/lib/events/event.h b/lib/events/event.h index ec21c6aa..a966e613 100644 --- a/lib/events/event.h +++ b/lib/events/event.h @@ -341,56 +341,34 @@ inline auto eventCast(const BasePtrT& eptr) : nullptr; } -// A trivial generic catch-all "switch" -template -inline auto switchOnType(const BaseEventT& event, FnT&& fn) - -> decltype(fn(event)) -{ - return fn(event); -} - namespace _impl { - // Using bool instead of auto below because auto apparently upsets MSVC - template - constexpr bool needs_downcast = - std::is_base_of_v>> - && !std::is_same_v>>; -} - -// A trivial type-specific "switch" for a void function -template -inline auto switchOnType(const BaseT& event, FnT&& fn) - -> std::enable_if_t<_impl::needs_downcast - && std::is_void_v>> -{ - using event_type = fn_arg_t; - if (is>(event)) - fn(static_cast(event)); + template + concept Invocable_With_Downcast = + std::derived_from>, BaseT>; } -// A trivial type-specific "switch" for non-void functions with an optional -// default value; non-voidness is guarded by defaultValue type -template -inline auto switchOnType(const BaseT& event, FnT&& fn, - fn_return_t&& defaultValue = {}) - -> std::enable_if_t<_impl::needs_downcast, fn_return_t> +template +inline auto switchOnType(const BaseT& event, TailT&& tail) { - using event_type = fn_arg_t; - if (is>(event)) - return fn(static_cast(event)); - return std::move(defaultValue); + if constexpr (std::is_invocable_v) { + return tail(event); + } else if constexpr (_impl::Invocable_With_Downcast) { + using event_type = fn_arg_t; + if (is>(event)) + return tail(static_cast(event)); + return std::invoke_result_t(); // Default-constructed + } else { // Treat it as a value to return + return std::forward(tail); + } } -// A switch for a chain of 2 or more functions -template -inline std::common_type_t, fn_return_t> -switchOnType(const BaseT& event, FnT1&& fn1, FnT2&& fn2, FnTs&&... fns) +template +inline auto switchOnType(const BaseT& event, FnT1&& fn1, FnTs&&... fns) { using event_type1 = fn_arg_t; if (is>(event)) - return fn1(static_cast(event)); - return switchOnType(event, std::forward(fn2), - std::forward(fns)...); + return fn1(static_cast(event)); + return switchOnType(event, std::forward(fns)...); } template @@ -405,8 +383,8 @@ inline auto visit(const BaseT& event, FnTs&&... fns) // TODO: replace with ranges::for_each once all standard libraries have it template inline auto visitEach(RangeT&& events, FnTs&&... fns) - -> std::enable_if_t(fns)...))>> + requires std::is_void_v< + decltype(switchOnType(**begin(events), std::forward(fns)...))> { for (auto&& evtPtr: events) switchOnType(*evtPtr, std::forward(fns)...); diff --git a/lib/function_traits.cpp b/lib/function_traits.cpp index 6542101a..e3d27122 100644 --- a/lib/function_traits.cpp +++ b/lib/function_traits.cpp @@ -7,6 +7,9 @@ using namespace Quotient; +template +using fn_return_t = typename function_traits::return_type; + int f_(); static_assert(std::is_same_v, int>, "Test fn_return_t<>"); diff --git a/lib/function_traits.h b/lib/function_traits.h index 83b8e425..143ed162 100644 --- a/lib/function_traits.h +++ b/lib/function_traits.h @@ -8,7 +8,7 @@ namespace Quotient { namespace _impl { - template + template struct fn_traits {}; } @@ -21,73 +21,73 @@ namespace _impl { */ template struct function_traits - : public _impl::fn_traits> {}; + : public _impl::fn_traits> {}; // Specialisation for a function template struct function_traits { using return_type = ReturnT; using arg_types = std::tuple; - // See also the comment for wrap_in_function() in qt_connection_util.h - using function_type = std::function; }; namespace _impl { - template + template struct fn_object_traits; // Specialisation for a lambda function template - struct fn_object_traits + struct fn_object_traits : function_traits {}; // Specialisation for a const lambda function template - struct fn_object_traits + struct fn_object_traits : function_traits {}; // Specialisation for function objects with (non-overloaded) operator() // (this includes non-generic lambdas) template - struct fn_traits - : public fn_object_traits {}; + requires requires { &T::operator(); } + struct fn_traits + : public fn_object_traits {}; // Specialisation for a member function in a non-functor class template - struct fn_traits + struct fn_traits : function_traits {}; // Specialisation for a const member function template - struct fn_traits + struct fn_traits : function_traits {}; // Specialisation for a constref member function template - struct fn_traits + struct fn_traits : function_traits {}; // Specialisation for a prvalue member function template - struct fn_traits + struct fn_traits : function_traits {}; // Specialisation for a pointer-to-member template - struct fn_traits + struct fn_traits : function_traits {}; // Specialisation for a const pointer-to-member template - struct fn_traits + struct fn_traits : function_traits {}; } // namespace _impl -template -using fn_return_t = typename function_traits::return_type; - template using fn_arg_t = std::tuple_element_t::arg_types>; +template +constexpr auto fn_arg_count_v = + std::tuple_size_v::arg_types>; + } // namespace Quotient diff --git a/lib/qt_connection_util.h b/lib/qt_connection_util.h index 86593cc8..7477273f 100644 --- a/lib/qt_connection_util.h +++ b/lib/qt_connection_util.h @@ -9,101 +9,68 @@ namespace Quotient { namespace _impl { - template - using decorated_slot_tt = - std::function; + enum ConnectionType { SingleShot, Until }; - template - inline QMetaObject::Connection - connectDecorated(SenderT* sender, SignalT signal, ContextT* context, - decorated_slot_tt decoratedSlot, - Qt::ConnectionType connType) + template + inline auto connect(auto* sender, auto signal, auto* context, auto slotLike, + Qt::ConnectionType connType) { - auto pc = std::make_unique(); - auto& c = *pc; // Resolve a reference before pc is moved to lambda - - // Perfect forwarding doesn't work through signal-slot connections - - // arguments are always copied (at best - COWed) to the context of - // the slot. Therefore the slot decorator receives const ArgTs&... - // rather than ArgTs&&... - // TODO (C++20): std::bind_front() instead of lambda. - c = QObject::connect(sender, signal, context, - [pc = std::move(pc), - decoratedSlot = std::move(decoratedSlot)](const ArgTs&... args) { - Q_ASSERT(*pc); // If it's been triggered, it should exist - decoratedSlot(*pc, args...); + std::unique_ptr pConn = + std::make_unique(); + auto& c = *pConn; // Save the reference before pConn is moved from + c = QObject::connect( + sender, signal, context, + [slotLike, pConn = std::move(pConn)](const auto&... args) + // The requires-expression below is necessary to prevent Qt + // from eagerly trying to fill the lambda with more arguments + // than slotLike() (i.e., the original slot) can handle + requires requires { slotLike(args...); } { + static_assert(CType == Until || CType == SingleShot, + "Unsupported disconnection type"); + if constexpr (CType == SingleShot) { + // Disconnect early to avoid re-triggers during slotLike() + QObject::disconnect(*pConn); + // Qt kindly keeps slot objects until they do their job, + // even if they disconnect themselves in the process (see + // how doActivate() in qobject.cpp handles c->slotObj). + slotLike(args...); + } else if constexpr (CType == Until) { + if (slotLike(args...)) + QObject::disconnect(*pConn); + } }, connType); return c; } - template - inline QMetaObject::Connection - connectUntil(SenderT* sender, SignalT signal, ContextT* context, - std::function functor, - Qt::ConnectionType connType) - { - return connectDecorated(sender, signal, context, - decorated_slot_tt( - [functor = std::move(functor)](QMetaObject::Connection& c, - const ArgTs&... args) { - if (functor(args...)) - QObject::disconnect(c); - }), - connType); - } - template - inline QMetaObject::Connection - connectSingleShot(SenderT* sender, SignalT signal, ContextT* context, - std::function slot, - Qt::ConnectionType connType) - { - return connectDecorated(sender, signal, context, - decorated_slot_tt( - [slot = std::move(slot)](QMetaObject::Connection& c, - const ArgTs&... args) { - QObject::disconnect(c); - slot(args...); - }), - connType); - } - // TODO: get rid of it as soon as Apple Clang gets proper deduction guides - // for std::function<> - // ...or consider using QtPrivate magic used by QObject::connect() - // ...for inspiration, also check a possible std::not_fn implementation - // at https://en.cppreference.com/w/cpp/utility/functional/not_fn - template - inline auto wrap_in_function(FnT&& f) - { - return typename function_traits::function_type(std::forward(f)); - } + template + concept PmfSlot = + (fn_arg_count_v > 0 + && std::derived_from>>); } // namespace _impl -/*! \brief Create a connection that self-disconnects when its "slot" returns true - * - * A slot accepted by connectUntil() is different from classic Qt slots - * in that its return value must be bool, not void. The slot's return value - * controls whether the connection should be kept; if the slot returns false, - * the connection remains; upon returning true, the slot is disconnected from - * the signal. Because of a different slot signature connectUntil() doesn't - * accept member functions as QObject::connect or Quotient::connectSingleShot - * do; you should pass a lambda or a pre-bound member function to it. - */ -template -inline auto connectUntil(SenderT* sender, SignalT signal, ContextT* context, - const FunctorT& slot, +//! \brief Create a connection that self-disconnects when its slot returns true +//! +//! A slot accepted by connectUntil() is different from classic Qt slots +//! in that its return value must be bool, not void. Because of that different +//! signature connectUntil() doesn't accept member functions in the way +//! QObject::connect or Quotient::connectSingleShot do; you should pass a lambda +//! or a pre-bound member function to it. +//! \return whether the connection should be dropped; false means that the +//! connection remains; upon returning true, the slot is disconnected +//! from the signal. +inline auto connectUntil(auto* sender, auto signal, auto* context, + auto smartSlot, Qt::ConnectionType connType = Qt::AutoConnection) { - return _impl::connectUntil(sender, signal, context, _impl::wrap_in_function(slot), - connType); + return _impl::connect<_impl::Until>(sender, signal, context, smartSlot, + connType); } -/// Create a connection that self-disconnects after triggering on the signal -template -inline auto connectSingleShot(SenderT* sender, SignalT signal, - ContextT* context, const FunctorT& slot, +//! Create a connection that self-disconnects after triggering on the signal +template +inline auto connectSingleShot(auto* sender, auto signal, ContextT* context, + SlotT slot, Qt::ConnectionType connType = Qt::AutoConnection) { #if QT_VERSION_MAJOR >= 6 @@ -111,25 +78,16 @@ inline auto connectSingleShot(SenderT* sender, SignalT signal, Qt::ConnectionType(connType | Qt::SingleShotConnection)); #else - return _impl::connectSingleShot( - sender, signal, context, _impl::wrap_in_function(slot), connType); -} - -// Specialisation for usual Qt slots passed as pointers-to-members. -template -inline auto connectSingleShot(SenderT* sender, SignalT signal, - ReceiverT* receiver, - void (SlotObjectT::*slot)(ArgTs...), - Qt::ConnectionType connType = Qt::AutoConnection) -{ - // TODO: when switching to C++20, use std::bind_front() instead - return _impl::connectSingleShot(sender, signal, receiver, - _impl::wrap_in_function( - [receiver, slot](const ArgTs&... args) { - (receiver->*slot)(args...); - }), - connType); + // In case for usual Qt slots passed as pointers-to-members the receiver + // object has to be pre-bound to the slot to make it self-contained + if constexpr (_impl::PmfSlot) { + return _impl::connect<_impl::SingleShot>(sender, signal, context, + std::bind_front(slot, context), + connType); + } else { + return _impl::connect<_impl::SingleShot>(sender, signal, context, slot, + connType); + } #endif } -- cgit v1.2.3