Cleanup and minor fixes

Quotient/connection.cpp

@@ -243,45 +243,46 @@ bool Connection::capabilitiesReady() const
 
 QStringList Connection::supportedMatrixSpecVersions() const { return d->data->homeserverData().supportedSpecVersions; }
 
+namespace {
+QFuture<QKeychain::Job*> runKeychainJob(QKeychain::Job* j, const QString& keychainId)
+{
+    j->setAutoDelete(true);
+    j->setKey(keychainId);
+    auto ft = QtFuture::connect(j, &QKeychain::Job::finished);
+    j->start();
+    return ft;
+}
+}
+
 void Connection::Private::saveAccessTokenToKeychain() const
 {
     qCDebug(MAIN) << "Saving access token to keychain for" << q->userId();
-    auto job = new QKeychain::WritePasswordJob(qAppName());
-    job->setKey(q->userId());
+    using namespace QKeychain;
+    auto job = new WritePasswordJob(qAppName());
     job->setBinaryData(data->accessToken());
-    job->start();
-    QObject::connect(job, &QKeychain::Job::finished, q, [job] {
-        if (job->error() == QKeychain::Error::NoError)
+    runKeychainJob(job, q->userId()).then([](const Job* j) {
+        if (j->error() == Error::NoError)
             return;
-        qWarning(MAIN).noquote()
-            << "Could not save access token to the keychain:"
-            << qUtf8Printable(job->errorString());
+        qWarning(MAIN).noquote() << "Could not save access token to the keychain:"
+                                 << qUtf8Printable(j->errorString());
         // TODO: emit a signal
     });
-
 }
 
 void Connection::Private::dropAccessToken()
 {
     // TODO: emit a signal on important (i.e. access denied) keychain errors
     using namespace QKeychain;
-    qCDebug(MAIN) << "Removing access token from keychain for" << q->userId();
-    auto job = new DeletePasswordJob(qAppName());
-    job->setKey(q->userId());
-    job->start();
-    QObject::connect(job, &Job::finished, q, [job] {
-        if (job->error() == Error::NoError
-            || job->error() == Error::EntryNotFound)
+    qCDebug(MAIN) << "Removing access token and pickle from keychain for" << q->userId();
+    runKeychainJob(new DeletePasswordJob(qAppName()), q->userId()).then([](const Job* job) {
+        if (job->error() == Error::NoError || job->error() == Error::EntryNotFound)
             return;
-        qWarning(MAIN).noquote()
-            << "Could not delete access token from the keychain:"
-            << qUtf8Printable(job->errorString());
+        qWarning(MAIN).noquote() << "Could not delete access token from the keychain:"
+                                 << qUtf8Printable(job->errorString());
     });
 
-    auto pickleJob = new DeletePasswordJob(qAppName());
-    pickleJob->setKey(q->userId() + "-Pickle"_L1);
-    pickleJob->start();
-    QObject::connect(job, &Job::finished, q, [job] {
+    runKeychainJob(new DeletePasswordJob(qAppName()), q->userId() + "-Pickle"_L1)
+        .then([](const Job* job) {
         if (job->error() == Error::NoError
             || job->error() == Error::EntryNotFound)
             return;

Quotient/converters.h

@@ -46,8 +46,8 @@ struct JsonObjectConverter;
 //static void dumpTo(QJsonObject&, const T&); // For toJson() and fillJson() to work
 //static void fillFrom(const QJsonObject&, T&); // For fromJson() and fillFromJson() to work
 
-template <typename PodT, typename JsonT>
-PodT fromJson(const JsonT&);
+template <typename PodT>
+PodT fromJson(const auto&);
 
 template <typename T>
 struct JsonObjectUnpacker {
@@ -121,8 +121,8 @@ inline void fillJson(QJsonObject& json, const T& data)
     JsonObjectConverter<T>::dumpTo(json, data);
 }
 
-template <typename PodT, typename JsonT>
-inline PodT fromJson(const JsonT& json)
+template <typename PodT>
+inline PodT fromJson(const auto& json)
 {
     // JsonT here can be whatever the respective JsonConverter specialisation
     // accepts but by default it's QJsonValue, QJsonDocument, or QJsonObject
@@ -133,8 +133,8 @@ inline PodT fromJson(const JsonT& json)
 // the coder to explicitly type it. It still enforces the
 // overwrite-everything semantics of fromJson(), unlike fillFromJson()
 
-template <typename JsonT, typename PodT>
-inline void fromJson(const JsonT& json, PodT& pod)
+template <typename PodT>
+inline void fromJson(const auto& json, PodT& pod)
 {
     pod = fromJson<PodT>(json);
 }
@@ -161,8 +161,8 @@ namespace _impl {
 //! iterable container of string'y values (const char*, QLatin1String, etc.)
 //! matching respective enum values, 0-based.
 //! \sa enumToJsonString
-template <typename EnumT, typename EnumStringValuesT>
-inline std::optional<EnumT> enumFromJsonString(const QString& s, const EnumStringValuesT& enumValues)
+template <typename EnumT>
+inline std::optional<EnumT> enumFromJsonString(const QString& s, const auto& enumValues)
 {
     static_assert(std::is_unsigned_v<std::underlying_type_t<EnumT>>);
     if (const auto it = std::ranges::find(enumValues, s); it != cend(enumValues))
@@ -180,8 +180,8 @@ inline std::optional<EnumT> enumFromJsonString(const QString& s, const EnumStrin
 //!       should be defined as <tt>{ "", "Value1", "", "Value2", "", "Value3"
 //!       }</tt> (mind the gap at value 0, in particular).
 //! \sa enumFromJsonString
-template <typename EnumT, typename EnumStringValuesT>
-inline QString enumToJsonString(EnumT v, const EnumStringValuesT& enumValues)
+template <typename EnumT>
+inline QString enumToJsonString(EnumT v, const auto& enumValues)
 {
     static_assert(std::is_unsigned_v<std::underlying_type_t<EnumT>>);
     if (v < size(enumValues))
@@ -202,8 +202,8 @@ inline QString enumToJsonString(EnumT v, const EnumStringValuesT& enumValues)
 //! \note Unlike enumFromJsonString, the values start from 1 and not from 0.
 //! \note This function does not support flag combinations.
 //! \sa QUO_DECLARE_FLAGS, QUO_DECLARE_FLAGS_NS
-template <typename FlagT, typename FlagStringValuesT>
-inline std::optional<FlagT> flagFromJsonString(const QString& s, const FlagStringValuesT& flagValues)
+template <typename FlagT>
+inline std::optional<FlagT> flagFromJsonString(const QString& s, const auto& flagValues)
 {
     // Enums based on signed integers don't make much sense for flag types
     static_assert(std::is_unsigned_v<std::underlying_type_t<FlagT>>);
@@ -213,8 +213,8 @@ inline std::optional<FlagT> flagFromJsonString(const QString& s, const FlagStrin
     return std::nullopt;
 }
 
-template <typename FlagT, typename FlagStringValuesT>
-inline QString flagToJsonString(FlagT v, const FlagStringValuesT& flagValues)
+template <typename FlagT>
+inline QString flagToJsonString(FlagT v, const auto& flagValues)
 {
     static_assert(std::is_unsigned_v<std::underlying_type_t<FlagT>>);
     if (const auto offset = std::countr_zero(std::to_underlying(v)); offset < ssize(flagValues))
@@ -551,8 +551,8 @@ namespace _impl {
  *               by default; passing IfNotEmpty or false for this parameter
  *               enables emptiness checks as described above
  */
-template <bool Force = true, typename ContT, typename KeyT, typename ValT>
-inline void addParam(ContT& container, KeyT&& key, ValT&& value)
+template <bool Force = true, typename KeyT, typename ValT>
+inline void addParam(auto& container, KeyT&& key, ValT&& value)
 {
     _impl::AddNode<std::decay_t<ValT>, Force>::impl(container, std::forward<KeyT>(key),
                                                     std::forward<ValT>(value));

Quotient/events/event.h

@@ -514,9 +514,8 @@ inline bool is(const Event& e)
 //! can be either "dumb" (BaseEventT*) or "smart" (`event_ptr_tt<>`). This
 //! overload doesn't affect the event ownership - if the original pointer owns
 //! the event it must outlive the downcast pointer to keep it from dangling.
-template <EventClass EventT, typename BasePtrT>
-inline auto eventCast(const BasePtrT& eptr)
-    -> decltype(static_cast<EventT*>(std::to_address(eptr)))
+template <EventClass EventT>
+inline auto eventCast(const auto& eptr) -> decltype(static_cast<EventT*>(std::to_address(eptr)))
 {
     return eptr && is<std::decay_t<EventT>>(*eptr)
                ? static_cast<EventT*>(std::to_address(eptr))

Quotient/events/stateevent.h

@@ -90,8 +90,7 @@ class EventTemplate<EventT, StateEvent, ContentT>
 
     const ContentT& content() const { return _content; }
 
-    template <typename VisitorT>
-    void editContent(VisitorT&& visitor)
+    void editContent(auto&& visitor)
     {
         visitor(_content);
         editJson()[ContentKey] = toJson(_content);

Quotient/jobs/requestdata.cpp

@@ -12,7 +12,7 @@
 
 using namespace Quotient;
 
-auto fromData(const QByteArray& data)
+auto bufferFromData(const QByteArray& data)
 {
     auto source = makeImpl<QBuffer, QIODevice>();
     source->setData(data);
@@ -21,16 +21,17 @@ auto fromData(const QByteArray& data)
 }
 
 template <typename JsonDataT>
-inline auto fromJson(const JsonDataT& jdata)
+    requires std::constructible_from<QJsonDocument, JsonDataT>
+inline auto bufferFromJson(const JsonDataT& jdata)
 {
-    return fromData(QJsonDocument(jdata).toJson(QJsonDocument::Compact));
+    return bufferFromData(QJsonDocument(jdata).toJson(QJsonDocument::Compact));
 }
 
-RequestData::RequestData(const QByteArray& a) : _source(fromData(a)) {}
+RequestData::RequestData(const QByteArray& a) : _source(bufferFromData(a)) {}
 
-RequestData::RequestData(const QJsonObject& jo) : _source(fromJson(jo)) {}
+RequestData::RequestData(const QJsonObject& jo) : _source(bufferFromJson(jo)) {}
 
-RequestData::RequestData(const QJsonArray& ja) : _source(fromJson(ja)) {}
+RequestData::RequestData(const QJsonArray& ja) : _source(bufferFromJson(ja)) {}
 
 RequestData::RequestData(QIODevice* source)
     : _source(acquireImpl(source))

Quotient/ranges_extras.h

@@ -13,7 +13,8 @@ namespace Quotient {
 template <typename RangeT, typename ValT, typename ProjT = std::identity>
     requires std::indirectly_comparable<std::ranges::iterator_t<RangeT>, const ValT*,
                                         std::ranges::equal_to, ProjT>
-inline auto findIndex(const RangeT& range, const ValT& value, ProjT proj = {})
+[[nodiscard]] constexpr inline auto findIndex(const RangeT& range, const ValT& value,
+                                              ProjT proj = {})
 {
     using namespace std::ranges;
     return distance(begin(range), find(range, value, std::move(proj)));
@@ -27,37 +28,67 @@ inline auto findIndex(const RangeT& range, const ValT& value, ProjT proj = {})
 //! available; otherwise, returns the result of calling
 //! `TargetT(ranges::begin(sourceRange), ranges::end(sourceRange))`.
 template <class TargetT, typename SourceT>
-[[nodiscard]] constexpr auto rangeTo(SourceT&& sourceRange)
+[[nodiscard]] constexpr inline auto rangeTo(SourceT&& sourceRange)
 {
 #if defined(__cpp_lib_ranges_to_container)
     return std::ranges::to<TargetT>(std::forward<SourceT>(sourceRange));
 #else
-    using std::begin, std::end;
-    return TargetT(begin(sourceRange), end(sourceRange));
+    // Provide the minimal necessary subset of what std::ranges::to() can do
+    using namespace std::ranges;
+    if constexpr (std::constructible_from<TargetT, SourceT>)
+        return TargetT(std::forward<SourceT>(sourceRange));
+    else {
+        using iter_t = iterator_t<SourceT>;
+        using iter_category_t = typename std::iterator_traits<iter_t>::iterator_category;
+        if constexpr (requires {
+                          requires common_range<SourceT>;
+                          typename iter_category_t;
+                          requires std::derived_from<iter_category_t, std::input_iterator_tag>;
+                          requires std::constructible_from<TargetT, iter_t, sentinel_t<SourceT>>;
+                      })
+            return TargetT(begin(sourceRange), end(sourceRange));
+        else {
+            TargetT c{};
+            if constexpr (sized_range<SourceT>
+                          && requires(range_size_t<TargetT> n) { c.reserve(n); })
+                c.reserve(static_cast<range_size_t<TargetT>>(size(sourceRange)));
+            using ValT = std::iter_value_t<iter_t>;
+            for (auto&& e : sourceRange) {
+                if constexpr (requires { c.emplace_back(std::forward<ValT>(e)); })
+                    c.emplace_back(std::forward<ValT>(e));
+                else if constexpr (requires { c.push_back(std::forward<ValT>(e)); })
+                    c.push_back(std::forward<ValT>(e));
+                else if constexpr (requires { c.emplace(c.end(), std::forward<ValT>(e)); })
+                    c.emplace(c.end(), std::forward<ValT>(e));
+                else
+                    c.insert(c.end(), std::forward<ValT>(e));
+            }
+            return c;
+        }
+    }
 #endif
 }
 
 //! An overload that accepts unspecialised container template
 template <template <typename> class TargetT, typename SourceT>
-[[nodiscard]] constexpr auto rangeTo(SourceT&& sourceRange)
+[[nodiscard]] constexpr inline auto rangeTo(SourceT&& sourceRange)
 {
     // Avoid template argument deduction because Xcode still can't do it when TargetT is an alias
 #if defined(__cpp_lib_ranges_to_container)
     return std::ranges::to<TargetT<std::ranges::range_value_t<SourceT>>>(
         std::forward<SourceT>(sourceRange));
 #else
-    using std::begin, std::end;
-    return TargetT<std::ranges::range_value_t<SourceT>>(begin(sourceRange), end(sourceRange));
+    return rangeTo<TargetT<std::ranges::range_value_t<SourceT>>>(std::forward<SourceT>(sourceRange));
 #endif
 }
 
 #ifdef __cpp_lib_ranges_contains
-constexpr auto rangeContains = std::ranges::contains;
+constexpr inline auto rangeContains = std::ranges::contains;
 #else
-[[nodiscard]] constexpr auto rangeContains(const auto& c, const auto& v, auto proj)
+[[nodiscard]] constexpr inline auto rangeContains(const auto& c, const auto& v, auto proj)
 {
     return std::ranges::find(c, v, std::move(proj)) != std::ranges::end(c);
 }
 #endif
 
-}
+} // namespace Quotient

Quotient/syncdata.cpp

@@ -51,8 +51,8 @@ void JsonObjectConverter<RoomSummary>::fillFrom(const QJsonObject& jo,
     fromJson(jo["m.heroes"_L1], rs.heroes);
 }
 
-template <typename EventsArrayT, typename StrT>
-inline EventsArrayT load(const QJsonObject& batches, StrT keyName)
+template <typename EventsArrayT>
+inline EventsArrayT load(const QJsonObject& batches, auto keyName)
 {
     return fromJson<EventsArrayT>(batches[keyName].toObject().value("events"_L1));
 }

Quotient/util.h

@@ -62,14 +62,14 @@ namespace _impl {
 //! based on QUO_CSTR() contents if you need to store it).
 #define QUO_CSTR(StringConvertible_) std::data(::Quotient::_impl::toUtf8(StringConvertible_))
 
-inline bool alarmX(bool alarmCondition, const auto& msg,
+inline bool alarmX(bool alarmCondition, const auto& msg, bool invertReturnValue = false,
                    [[maybe_unused]] std::source_location loc = std::source_location::current())
 {
     if (alarmCondition) [[unlikely]] {
         qt_assert_x(loc.function_name(), QUO_CSTR(msg), loc.file_name(), loc.line());
         qCritical() << msg;
     }
-    return alarmCondition;
+    return alarmCondition != invertReturnValue;
 }
 
 //! \brief A negative assertion facility that can be put in an if statement
@@ -88,7 +88,7 @@ inline bool alarmX(bool alarmCondition, const auto& msg,
 
 //! Evaluate the boolean expression and, in Debug mode, assert it to be true
 #define QUO_CHECK(...) \
-    !::Quotient::alarmX(!(__VA_ARGS__) ? true : false, "Failing expression: " #__VA_ARGS__)
+    ::Quotient::alarmX((__VA_ARGS__) ? false : true, "Failing expression: " #__VA_ARGS__, true)
 
 //! A substitute for QtFuture::makeReadyVoidFuture() for compatibility with Qt pre-6.6
 inline auto makeReadyVoidFuture()
@@ -210,10 +210,10 @@ asKeyValueRange(U&&) -> asKeyValueRange<U>;
  * Convenient for cases when you need to know which particular "first of"
  * [sFirst, sLast) has been found in [first, last).
  */
-template <typename InputIt, typename ForwardIt, typename Pred>
+template <typename InputIt, typename ForwardIt>
 inline std::pair<InputIt, ForwardIt> findFirstOf(InputIt first, InputIt last,
                                                  ForwardIt sFirst,
-                                                 ForwardIt sLast, Pred pred)
+                                                 ForwardIt sLast, auto pred)
 {
     for (; first != last; ++first)
         for (auto it = sFirst; it != sLast; ++it)