From cba8d83b2195a2c323e5fc430b8f68a0aff33a6f Mon Sep 17 00:00:00 2001 From: Chocobo1 Date: Thu, 10 Dec 2020 14:57:16 +0800 Subject: [PATCH 1/3] Migrate away from deprecated QVariant comparison operators Another idea would be manually define a custom comparison function for QVariant. However, having the function would be excessive due to its limited usage count, also note that we are already casting various QVariant to its underlying type in existing code. --- src/gui/transferlistsortmodel.cpp | 93 ++++++++++++++++++------------- 1 file changed, 53 insertions(+), 40 deletions(-) diff --git a/src/gui/transferlistsortmodel.cpp b/src/gui/transferlistsortmodel.cpp index 310dc0d78..e1d32b84f 100644 --- a/src/gui/transferlistsortmodel.cpp +++ b/src/gui/transferlistsortmodel.cpp @@ -41,7 +41,6 @@ TransferListSortModel::TransferListSortModel(QObject *parent) : QSortFilterProxyModel {parent} { setSortRole(TransferListModel::UnderlyingDataRole); - QMetaType::registerComparators(); } void TransferListSortModel::setStatusFilter(TorrentFilter::Type filter) @@ -122,21 +121,21 @@ bool TransferListSortModel::lessThan_impl(const QModelIndex &left, const QModelI return (Utils::String::naturalCompare(leftValue.toString(), rightValue.toString(), Qt::CaseInsensitive) < 0); case TransferListModel::TR_STATUS: - // QSortFilterProxyModel::lessThan() uses the < operator only for specific QVariant types - // so our custom type is outside that list. - // In this case QSortFilterProxyModel::lessThan() converts other types to QString and - // sorts them. - // Thus we can't use the code in the default label. - if (leftValue != rightValue) - return leftValue < rightValue; - return invokeLessThanForColumn(TransferListModel::TR_QUEUE_POSITION); + { + const auto stateL = leftValue.value(); + const auto stateR = rightValue.value(); + + if (stateL != stateR) + return stateL < stateR; + return invokeLessThanForColumn(TransferListModel::TR_QUEUE_POSITION); + } case TransferListModel::TR_ADD_DATE: case TransferListModel::TR_SEED_DATE: case TransferListModel::TR_SEEN_COMPLETE_DATE: - { - const QDateTime dateL = leftValue.toDateTime(); - const QDateTime dateR = rightValue.toDateTime(); + { + const auto dateL = leftValue.toDateTime(); + const auto dateR = rightValue.toDateTime(); if (dateL.isValid() && dateR.isValid()) { @@ -156,20 +155,21 @@ bool TransferListSortModel::lessThan_impl(const QModelIndex &left, const QModelI } case TransferListModel::TR_QUEUE_POSITION: - { - // QVariant has comparators for all basic types - if ((leftValue > 0) || (rightValue > 0)) - { - if ((leftValue > 0) && (rightValue > 0)) - return leftValue < rightValue; + { + const auto positionL = leftValue.toInt(); + const auto positionR = rightValue.toInt(); - return leftValue != 0; + if ((positionL > 0) || (positionR > 0)) + { + if ((positionL > 0) && (positionR > 0)) + return positionL < positionR; + return positionL != 0; } // Sort according to TR_SEED_DATE - const QDateTime dateL = left.sibling(left.row(), TransferListModel::TR_SEED_DATE) + const auto dateL = left.sibling(left.row(), TransferListModel::TR_SEED_DATE) .data(TransferListModel::UnderlyingDataRole).toDateTime(); - const QDateTime dateR = right.sibling(right.row(), TransferListModel::TR_SEED_DATE) + const auto dateR = right.sibling(right.row(), TransferListModel::TR_SEED_DATE) .data(TransferListModel::UnderlyingDataRole).toDateTime(); if (dateL.isValid() && dateR.isValid()) @@ -191,22 +191,23 @@ bool TransferListSortModel::lessThan_impl(const QModelIndex &left, const QModelI case TransferListModel::TR_SEEDS: case TransferListModel::TR_PEERS: - { - // QVariant has comparators for all basic types - // Active peers/seeds take precedence over total peers/seeds. - if (leftValue != rightValue) - return (leftValue < rightValue); + { + // Active peers/seeds take precedence over total peers/seeds + const auto activeL = leftValue.toInt(); + const auto activeR = rightValue.toInt(); + if (activeL != activeR) + return activeL < activeR; - const QVariant leftValueTotal = left.data(TransferListModel::AdditionalUnderlyingDataRole); - const QVariant rightValueTotal = right.data(TransferListModel::AdditionalUnderlyingDataRole); - if (leftValueTotal != rightValueTotal) - return (leftValueTotal < rightValueTotal); + const auto totalL = left.data(TransferListModel::AdditionalUnderlyingDataRole).toInt(); + const auto totalR = right.data(TransferListModel::AdditionalUnderlyingDataRole).toInt(); + if (totalL != totalR) + return totalL < totalR; return invokeLessThanForColumn(TransferListModel::TR_QUEUE_POSITION); } case TransferListModel::TR_ETA: - { + { // Sorting rules prioritized. // 1. Active torrents at the top // 2. Seeding torrents at the bottom @@ -221,9 +222,9 @@ bool TransferListSortModel::lessThan_impl(const QModelIndex &left, const QModelI if (isActiveL != isActiveR) return isActiveL; - const int queuePosL = left.sibling(left.row(), TransferListModel::TR_QUEUE_POSITION) + const auto queuePosL = left.sibling(left.row(), TransferListModel::TR_QUEUE_POSITION) .data(TransferListModel::UnderlyingDataRole).toInt(); - const int queuePosR = right.sibling(right.row(), TransferListModel::TR_QUEUE_POSITION) + const auto queuePosR = right.sibling(right.row(), TransferListModel::TR_QUEUE_POSITION) .data(TransferListModel::UnderlyingDataRole).toInt(); const bool isSeedingL = (queuePosL < 0); const bool isSeedingR = (queuePosR < 0); @@ -236,8 +237,8 @@ bool TransferListSortModel::lessThan_impl(const QModelIndex &left, const QModelI return isAscendingOrder; } - const qlonglong etaL = leftValue.toLongLong(); - const qlonglong etaR = rightValue.toLongLong(); + const auto etaL = leftValue.toLongLong(); + const auto etaR = rightValue.toLongLong(); const bool isInvalidL = ((etaL < 0) || (etaL >= MAX_ETA)); const bool isInvalidR = ((etaR < 0) || (etaR >= MAX_ETA)); if (isInvalidL && isInvalidR) @@ -255,12 +256,24 @@ bool TransferListSortModel::lessThan_impl(const QModelIndex &left, const QModelI } case TransferListModel::TR_LAST_ACTIVITY: - case TransferListModel::TR_RATIO_LIMIT: - // QVariant has comparators for all basic types - if (leftValue < 0) return false; - if (rightValue < 0) return true; + { + const auto lastActivityL = leftValue.toLongLong(); + const auto lastActivityR = rightValue.toLongLong(); - return (leftValue < rightValue); + if (lastActivityL < 0) return false; + if (lastActivityR < 0) return true; + return lastActivityL < lastActivityR; + } + + case TransferListModel::TR_RATIO_LIMIT: + { + const auto ratioL = leftValue.toReal(); + const auto ratioR = rightValue.toReal(); + + if (ratioL < 0) return false; + if (ratioR < 0) return true; + return ratioL < ratioR; + } } return (leftValue != rightValue) From 5ba6a5fca19ab7f143a1590f7e094aa8a30b862e Mon Sep 17 00:00:00 2001 From: Chocobo1 Date: Thu, 10 Dec 2020 15:58:54 +0800 Subject: [PATCH 2/3] Add operator< for InfoHash class --- src/base/bittorrent/infohash.cpp | 5 +++++ src/base/bittorrent/infohash.h | 1 + src/gui/transferlistsortmodel.cpp | 4 ++-- 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/base/bittorrent/infohash.cpp b/src/base/bittorrent/infohash.cpp index 495a4f47d..3a2c0371c 100644 --- a/src/base/bittorrent/infohash.cpp +++ b/src/base/bittorrent/infohash.cpp @@ -84,6 +84,11 @@ bool BitTorrent::operator!=(const InfoHash &left, const InfoHash &right) return !(left == right); } +bool BitTorrent::operator<(const InfoHash &left, const InfoHash &right) +{ + return static_cast(left) < static_cast(right); +} + uint BitTorrent::qHash(const InfoHash &key, const uint seed) { return ::qHash((std::hash {})(key), seed); diff --git a/src/base/bittorrent/infohash.h b/src/base/bittorrent/infohash.h index be0a0ef75..81c35f5dd 100644 --- a/src/base/bittorrent/infohash.h +++ b/src/base/bittorrent/infohash.h @@ -61,6 +61,7 @@ namespace BitTorrent bool operator==(const InfoHash &left, const InfoHash &right); bool operator!=(const InfoHash &left, const InfoHash &right); + bool operator<(const InfoHash &left, const InfoHash &right); uint qHash(const InfoHash &key, uint seed); } diff --git a/src/gui/transferlistsortmodel.cpp b/src/gui/transferlistsortmodel.cpp index e1d32b84f..1d37f4a02 100644 --- a/src/gui/transferlistsortmodel.cpp +++ b/src/gui/transferlistsortmodel.cpp @@ -102,8 +102,8 @@ bool TransferListSortModel::lessThan_impl(const QModelIndex &left, const QModelI const auto hashLessThan = [this, &left, &right]() -> bool { const TransferListModel *model = qobject_cast(sourceModel()); - const QString hashL = model->torrentHandle(left)->hash(); - const QString hashR = model->torrentHandle(right)->hash(); + const BitTorrent::InfoHash hashL = model->torrentHandle(left)->hash(); + const BitTorrent::InfoHash hashR = model->torrentHandle(right)->hash(); return hashL < hashR; }; From 4d1d5d6b209a6537c2d5441b813030eb852320fa Mon Sep 17 00:00:00 2001 From: Chocobo1 Date: Fri, 11 Dec 2020 12:30:13 +0800 Subject: [PATCH 3/3] Revise Utils::Version comparison operators --- src/base/utils/version.h | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/src/base/utils/version.h b/src/base/utils/version.h index 22952f7f6..d18df83e0 100644 --- a/src/base/utils/version.h +++ b/src/base/utils/version.h @@ -133,19 +133,15 @@ namespace Utils return (*this != ThisType {}); } - constexpr bool operator==(const ThisType &other) const + // TODO: remove manually defined operators and use compiler generated `operator<=>()` in C++20 + friend bool operator==(const ThisType &left, const ThisType &right) { - return (m_components == other.m_components); + return (left.m_components == right.m_components); } - constexpr bool operator<(const ThisType &other) const + friend bool operator<(const ThisType &left, const ThisType &right) { - return (m_components < other.m_components); - } - - constexpr bool operator>(const ThisType &other) const - { - return (m_components > other.m_components); + return (left.m_components < right.m_components); } template @@ -198,6 +194,12 @@ namespace Utils return !(left == right); } + template + constexpr bool operator>(const Version &left, const Version &right) + { + return (right < left); + } + template constexpr bool operator<=(const Version &left, const Version &right) {