From e55ee873e9f3e9f6047333434d5c9a4ddfe2bf6b Mon Sep 17 00:00:00 2001 From: Ivan Solovev Date: Fri, 1 Mar 2024 19:22:49 +0100 Subject: [PATCH] QTypeRevision: use comparison helper macros MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit QTypeRevision consists of two quint8 values: major and minor version. Each of the versions can be unknown. The rules for comparing with the unknown version are as follows: zero version < unknown version < non-zero version At the same time, two unknown versions are considered equal. This makes the comparison a bit tricky, but it still fits into the category of strong ordering. Replace the existing friend relational operators with helper functions and comparison helper macros, making this type strongly ordered. Update the unit-tests to use the new comparison helper test functions. As the test functions check the reversed arguments as well, we can reduce the number of rows for the data-driven comparison test. Fixes: QTBUG-120359 Change-Id: Ib6f1037fc7b5fed148e35ee48b56b05dcd36b3b4 Reviewed-by: Thiago Macieira Reviewed-by: Mårten Nordheim --- src/corelib/tools/qtyperevision.cpp | 1 + src/corelib/tools/qtyperevision.h | 78 ++++++++----------- .../tools/qtyperevision/CMakeLists.txt | 2 + .../tools/qtyperevision/tst_qtyperevision.cpp | 27 ++++--- 4 files changed, 50 insertions(+), 58 deletions(-) diff --git a/src/corelib/tools/qtyperevision.cpp b/src/corelib/tools/qtyperevision.cpp index c505e47f2f4..6426236288a 100644 --- a/src/corelib/tools/qtyperevision.cpp +++ b/src/corelib/tools/qtyperevision.cpp @@ -26,6 +26,7 @@ QT_IMPL_METATYPE_EXTERN(QTypeRevision) \brief The QTypeRevision class contains a lightweight representation of a version number with two 8-bit segments, major and minor, either of which can be unknown. + \compares strong Use this class to describe revisions of a type. Compatible revisions can be expressed as increments of the minor version. Breaking changes can be diff --git a/src/corelib/tools/qtyperevision.h b/src/corelib/tools/qtyperevision.h index 308d89b6aba..8f255a77e89 100644 --- a/src/corelib/tools/qtyperevision.h +++ b/src/corelib/tools/qtyperevision.h @@ -7,6 +7,7 @@ #define QTYPEREVISION_H #include +#include #include #include #include @@ -98,53 +99,38 @@ public: return Integer(m_majorVersion << 8) | Integer(m_minorVersion); } - [[nodiscard]] friend constexpr bool operator==(QTypeRevision lhs, QTypeRevision rhs) - { - return lhs.toEncodedVersion() == rhs.toEncodedVersion(); - } - - [[nodiscard]] friend constexpr bool operator!=(QTypeRevision lhs, QTypeRevision rhs) - { - return lhs.toEncodedVersion() != rhs.toEncodedVersion(); - } - - [[nodiscard]] friend constexpr bool operator<(QTypeRevision lhs, QTypeRevision rhs) - { - return (!lhs.hasMajorVersion() && rhs.hasMajorVersion()) - // non-0 major > unspecified major > major 0 - ? rhs.majorVersion() != 0 - : ((lhs.hasMajorVersion() && !rhs.hasMajorVersion()) - // major 0 < unspecified major < non-0 major - ? lhs.majorVersion() == 0 - : (lhs.majorVersion() != rhs.majorVersion() - // both majors specified and non-0 - ? lhs.majorVersion() < rhs.majorVersion() - : ((!lhs.hasMinorVersion() && rhs.hasMinorVersion()) - // non-0 minor > unspecified minor > minor 0 - ? rhs.minorVersion() != 0 - : ((lhs.hasMinorVersion() && !rhs.hasMinorVersion()) - // minor 0 < unspecified minor < non-0 minor - ? lhs.minorVersion() == 0 - // both minors specified and non-0 - : lhs.minorVersion() < rhs.minorVersion())))); - } - - [[nodiscard]] friend constexpr bool operator>(QTypeRevision lhs, QTypeRevision rhs) - { - return lhs != rhs && !(lhs < rhs); - } - - [[nodiscard]] friend constexpr bool operator<=(QTypeRevision lhs, QTypeRevision rhs) - { - return lhs == rhs || lhs < rhs; - } - - [[nodiscard]] friend constexpr bool operator>=(QTypeRevision lhs, QTypeRevision rhs) - { - return lhs == rhs || !(lhs < rhs); - } - private: + friend constexpr bool + comparesEqual(const QTypeRevision &lhs, const QTypeRevision &rhs) noexcept + { return lhs.toEncodedVersion() == rhs.toEncodedVersion(); } + friend constexpr Qt::strong_ordering + compareThreeWay(const QTypeRevision &lhs, const QTypeRevision &rhs) noexcept + { + // For both major and minor the following rule applies: + // non-0 ver > unspecified ver > 0 ver + auto cmpUnspecified = [](quint8 leftVer, quint8 rightVer) { + Q_ASSERT(leftVer != rightVer + && (leftVer == QTypeRevision::SegmentUnknown + || rightVer == QTypeRevision::SegmentUnknown)); + if (leftVer != QTypeRevision::SegmentUnknown) + return leftVer > 0 ? Qt::strong_ordering::greater : Qt::strong_ordering::less; + return rightVer > 0 ? Qt::strong_ordering::less : Qt::strong_ordering::greater; + }; + + if (lhs.hasMajorVersion() != rhs.hasMajorVersion()) { + return cmpUnspecified(lhs.majorVersion(), rhs.majorVersion()); + } else { + const auto majorRes = Qt::compareThreeWay(lhs.majorVersion(), rhs.majorVersion()); + if (is_eq(majorRes)) { + if (lhs.hasMinorVersion() != rhs.hasMinorVersion()) + return cmpUnspecified(lhs.minorVersion(), rhs.minorVersion()); + return Qt::compareThreeWay(lhs.minorVersion(), rhs.minorVersion()); + } + return majorRes; + } + } + Q_DECLARE_STRONGLY_ORDERED_LITERAL_TYPE(QTypeRevision) + enum { SegmentUnknown = 0xff }; #if Q_BYTE_ORDER == Q_LITTLE_ENDIAN diff --git a/tests/auto/corelib/tools/qtyperevision/CMakeLists.txt b/tests/auto/corelib/tools/qtyperevision/CMakeLists.txt index d39d051a513..527156e3c29 100644 --- a/tests/auto/corelib/tools/qtyperevision/CMakeLists.txt +++ b/tests/auto/corelib/tools/qtyperevision/CMakeLists.txt @@ -10,4 +10,6 @@ endif() qt_internal_add_test(tst_qtyperevision SOURCES tst_qtyperevision.cpp + LIBRARIES + Qt::TestPrivate ) diff --git a/tests/auto/corelib/tools/qtyperevision/tst_qtyperevision.cpp b/tests/auto/corelib/tools/qtyperevision/tst_qtyperevision.cpp index 7fc4754a648..66c746382af 100644 --- a/tests/auto/corelib/tools/qtyperevision/tst_qtyperevision.cpp +++ b/tests/auto/corelib/tools/qtyperevision/tst_qtyperevision.cpp @@ -4,6 +4,7 @@ #include #include +#include using namespace Qt::StringLiterals; @@ -15,6 +16,7 @@ private slots: void qTypeRevision_data(); void qTypeRevision(); void qTypeRevisionTypes(); + void qTypeRevisionComparisonCompiles(); void qTypeRevisionComparison_data(); void qTypeRevisionComparison(); }; @@ -131,11 +133,16 @@ void tst_QTypeRevision::qTypeRevisionTypes() QVERIFY(maxRevision.hasMinorVersion()); } +void tst_QTypeRevision::qTypeRevisionComparisonCompiles() +{ + QTestPrivate::testAllComparisonOperatorsCompile(); +} + void tst_QTypeRevision::qTypeRevisionComparison_data() { QTest::addColumn("lhs"); QTest::addColumn("rhs"); - QTest::addColumn("expectedResult"); + QTest::addColumn("expectedResult"); static auto versionStr = [](QTypeRevision r) { QByteArray res = r.hasMajorVersion() ? QByteArray::number(r.majorVersion()) @@ -167,10 +174,11 @@ void tst_QTypeRevision::qTypeRevisionComparison_data() const int length = sizeof(revisions) / sizeof(QTypeRevision); for (int i = 0; i < length; ++i) { - for (int j = 0; j < length; ++j) { - const int expectedRes = (i == j) - ? 0 - : (i < j) ? -1 : 1; + for (int j = i; j < length; ++j) { + const Qt::strong_ordering expectedRes = (i == j) + ? Qt::strong_ordering::equal + : (i < j) ? Qt::strong_ordering::less + : Qt::strong_ordering::greater; const auto lhs = revisions[i]; const auto rhs = revisions[j]; @@ -184,14 +192,9 @@ void tst_QTypeRevision::qTypeRevisionComparison() { QFETCH(const QTypeRevision, lhs); QFETCH(const QTypeRevision, rhs); - QFETCH(const int, expectedResult); + QFETCH(const Qt::strong_ordering, expectedResult); - QCOMPARE(lhs == rhs, expectedResult == 0); - QCOMPARE(lhs != rhs, expectedResult != 0); - QCOMPARE(lhs < rhs, expectedResult < 0); - QCOMPARE(lhs > rhs, expectedResult > 0); - QCOMPARE(lhs <= rhs, expectedResult <= 0); - QCOMPARE(lhs >= rhs, expectedResult >= 0); + QT_TEST_ALL_COMPARISON_OPS(lhs, rhs, expectedResult); } QTEST_APPLESS_MAIN(tst_QTypeRevision)