From 5ea434b09f6a760b84fb14a69cc578063cad99a1 Mon Sep 17 00:00:00 2001 From: Ivan Solovev Date: Mon, 4 Mar 2024 16:12:11 +0100 Subject: [PATCH] QOperatingSystemVersion: use partial ordering for relational operators MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit QOperatingSystemVersion intentionally does not define operator==() and operator!=() since ae072cd9c4a575e0ed3f073c6ab395ccdf7c2b37. It means that we cannot use comparison helper macros. Still, we can manually define four relational operators or operator<=>() in C++20 mode, and give the class a partial ordering. We choose partial ordering, because versions of different OS types are incomparable. Implement the operators in terms of helper function compareThreeWay(), which potentially allows to use this class in some templated code. As a drive-by: make the static compare() function noexcept, because it really is. Fixes: QTBUG-120360 Change-Id: Id4c9ce740e42baa719ca0ee84146d087b21675c6 Reviewed-by: Mårten Nordheim Reviewed-by: Thiago Macieira --- .../global/qoperatingsystemversion.cpp | 4 +- src/corelib/global/qoperatingsystemversion.h | 46 +++++--- .../tst_qoperatingsystemversion.cpp | 107 +++++++++--------- 3 files changed, 86 insertions(+), 71 deletions(-) diff --git a/src/corelib/global/qoperatingsystemversion.cpp b/src/corelib/global/qoperatingsystemversion.cpp index 780ee6a5305..d25a626b8d3 100644 --- a/src/corelib/global/qoperatingsystemversion.cpp +++ b/src/corelib/global/qoperatingsystemversion.cpp @@ -199,13 +199,13 @@ QOperatingSystemVersionBase QOperatingSystemVersionBase::current_impl() } #endif -static inline int compareVersionComponents(int lhs, int rhs) +static inline int compareVersionComponents(int lhs, int rhs) noexcept { return lhs >= 0 && rhs >= 0 ? lhs - rhs : 0; } int QOperatingSystemVersionBase::compare(QOperatingSystemVersionBase v1, - QOperatingSystemVersionBase v2) + QOperatingSystemVersionBase v2) noexcept { if (v1.m_major == v2.m_major) { if (v1.m_minor == v2.m_minor) { diff --git a/src/corelib/global/qoperatingsystemversion.h b/src/corelib/global/qoperatingsystemversion.h index 3128c5a4e64..6aecdef3411 100644 --- a/src/corelib/global/qoperatingsystemversion.h +++ b/src/corelib/global/qoperatingsystemversion.h @@ -2,6 +2,7 @@ // SPDX-License-Identifier: LicenseRef-Qt-Commercial OR LGPL-3.0-only OR GPL-2.0-only OR GPL-3.0-only #include +#include #include #ifndef QOPERATINGSYSTEMVERSION_H @@ -79,22 +80,39 @@ public: constexpr OSType type() const { return m_os; } inline QString name() const { return name(*this); } - friend bool operator>(QOperatingSystemVersionBase lhs, QOperatingSystemVersionBase rhs) - { return lhs.type() == rhs.type() && QOperatingSystemVersionBase::compare(lhs, rhs) > 0; } - - friend bool operator>=(QOperatingSystemVersionBase lhs, QOperatingSystemVersionBase rhs) - { return lhs.type() == rhs.type() && QOperatingSystemVersionBase::compare(lhs, rhs) >= 0; } - - friend bool operator<(QOperatingSystemVersionBase lhs, QOperatingSystemVersionBase rhs) - { return lhs.type() == rhs.type() && QOperatingSystemVersionBase::compare(lhs, rhs) < 0; } - - friend bool operator<=(QOperatingSystemVersionBase lhs, QOperatingSystemVersionBase rhs) - { return lhs.type() == rhs.type() && QOperatingSystemVersionBase::compare(lhs, rhs) <= 0; } - - protected: static Q_CORE_EXPORT int compare(QOperatingSystemVersionBase v1, - QOperatingSystemVersionBase v2); + QOperatingSystemVersionBase v2) noexcept; + + friend Qt::partial_ordering compareThreeWay(const QOperatingSystemVersionBase &lhs, + const QOperatingSystemVersionBase &rhs) noexcept + { + if (lhs.type() != rhs.type()) + return Qt::partial_ordering::unordered; + const int res = QOperatingSystemVersionBase::compare(lhs, rhs); + return Qt::compareThreeWay(res, 0); + } +#ifdef __cpp_lib_three_way_comparison + friend std::partial_ordering + operator<=>(QOperatingSystemVersionBase lhs, QOperatingSystemVersionBase rhs) noexcept + { return compareThreeWay(lhs, rhs); } +#else + friend bool + operator>(QOperatingSystemVersionBase lhs, QOperatingSystemVersionBase rhs) noexcept + { return is_gt(compareThreeWay(lhs, rhs)); } + + friend bool + operator>=(QOperatingSystemVersionBase lhs, QOperatingSystemVersionBase rhs) noexcept + { return is_gteq(compareThreeWay(lhs, rhs)); } + + friend bool + operator<(QOperatingSystemVersionBase lhs, QOperatingSystemVersionBase rhs) noexcept + { return is_lt(compareThreeWay(lhs, rhs)); } + + friend bool + operator<=(QOperatingSystemVersionBase lhs, QOperatingSystemVersionBase rhs) noexcept + { return is_lteq(compareThreeWay(lhs, rhs)); } +#endif QOperatingSystemVersionBase() = default; private: diff --git a/tests/auto/corelib/global/qoperatingsystemversion/tst_qoperatingsystemversion.cpp b/tests/auto/corelib/global/qoperatingsystemversion/tst_qoperatingsystemversion.cpp index 8ea624c3ee8..d59598ecf0c 100644 --- a/tests/auto/corelib/global/qoperatingsystemversion/tst_qoperatingsystemversion.cpp +++ b/tests/auto/corelib/global/qoperatingsystemversion/tst_qoperatingsystemversion.cpp @@ -114,47 +114,44 @@ void tst_QOperatingSystemVersion::comparison_data() QTest::addColumn("rhsMinor"); QTest::addColumn("rhsMicro"); - QTest::addColumn("lessResult"); - QTest::addColumn("lessEqualResult"); - QTest::addColumn("moreResult"); - QTest::addColumn("moreEqualResult"); + QTest::addColumn("expectedResult"); QTest::addRow("mismatching types") << QOperatingSystemVersion::OSType::Windows << 1 << 2 << 3 << QOperatingSystemVersion::OSType::MacOS << 1 << 2 << 3 - << false << false << false << false; + << Qt::partial_ordering::unordered; QTest::addRow("equal versions") << QOperatingSystemVersion::OSType::Windows << 1 << 2 << 3 << QOperatingSystemVersion::OSType::Windows << 1 << 2 << 3 - << false << true << false << true; + << Qt::partial_ordering::equivalent; QTest::addRow("lhs micro less") << QOperatingSystemVersion::OSType::Windows << 1 << 2 << 2 << QOperatingSystemVersion::OSType::Windows << 1 << 2 << 3 - << true << true << false << false; + << Qt::partial_ordering::less; QTest::addRow("rhs micro less") << QOperatingSystemVersion::OSType::Windows << 1 << 2 << 2 << QOperatingSystemVersion::OSType::Windows << 1 << 2 << 1 - << false << false << true << true; + << Qt::partial_ordering::greater; QTest::addRow("lhs minor less") << QOperatingSystemVersion::OSType::Windows << 1 << 2 << 3 << QOperatingSystemVersion::OSType::Windows << 1 << 3 << 3 - << true << true << false << false; + << Qt::partial_ordering::less; QTest::addRow("rhs minor less") << QOperatingSystemVersion::OSType::Windows << 1 << 2 << 2 << QOperatingSystemVersion::OSType::Windows << 1 << 1 << 3 - << false << false << true << true; + << Qt::partial_ordering::greater; QTest::addRow("lhs major less") << QOperatingSystemVersion::OSType::Windows << 0 << 5 << 6 << QOperatingSystemVersion::OSType::Windows << 1 << 2 << 3 - << true << true << false << false; + << Qt::partial_ordering::less; QTest::addRow("rhs major less") << QOperatingSystemVersion::OSType::Windows << 1 << 2 << 3 << QOperatingSystemVersion::OSType::Windows << 0 << 2 << 3 - << false << false << true << true; + << Qt::partial_ordering::greater; QTest::addRow("different segmentCount") << QOperatingSystemVersion::OSType::Windows << 1 << 2 << 3 << QOperatingSystemVersion::OSType::Windows << 1 << 2 << -1 - << false << true << false << true; + << Qt::partial_ordering::equivalent; } void tst_QOperatingSystemVersion::comparison() @@ -173,56 +170,54 @@ void tst_QOperatingSystemVersion::comparison() const QOperatingSystemVersion rhsSystemInfo(rhsType, rhsMajor, rhsMinor, rhsMicro); - QFETCH(bool, lessResult); - QCOMPARE(lhsSystemInfo < rhsSystemInfo, lessResult); + QFETCH(const Qt::partial_ordering, expectedResult); - QFETCH(bool, lessEqualResult); - QCOMPARE(lhsSystemInfo <= rhsSystemInfo, lessEqualResult); - - QFETCH(bool, moreResult); - QCOMPARE(lhsSystemInfo > rhsSystemInfo, moreResult); - - QFETCH(bool, moreEqualResult); - QCOMPARE(lhsSystemInfo >= rhsSystemInfo, moreEqualResult); + QCOMPARE_EQ(lhsSystemInfo < rhsSystemInfo, is_lt(expectedResult)); + QCOMPARE_EQ(lhsSystemInfo <= rhsSystemInfo, is_lteq(expectedResult)); + QCOMPARE_EQ(lhsSystemInfo > rhsSystemInfo, is_gt(expectedResult)); + QCOMPARE_EQ(lhsSystemInfo >= rhsSystemInfo, is_gteq(expectedResult)); +#ifdef __cpp_lib_three_way_comparison + Q_COMPARE_EQ(lhsSystemInfo <=> rhsSystemInfo, expectedResult); +#endif } void tst_QOperatingSystemVersion::comparison2_data() { QTest::addColumn("lhs"); QTest::addColumn("rhs"); - QTest::addColumn("result"); + QTest::addColumn("result"); #define ADDROW(os1, os2) \ QTest::newRow(#os1 "-vs-" #os2) << QOperatingSystemVersion(QOperatingSystemVersion::os1) \ << QOperatingSystemVersion(QOperatingSystemVersion::os2) // Cross-OS testing: not comparables. - ADDROW(Windows10, MacOSMonterey) << -128; - ADDROW(Windows11, MacOSMonterey) << -128; - ADDROW(MacOSMonterey, Windows10) << -128; - ADDROW(MacOSMonterey, Windows11) << -128; - ADDROW(Windows10, MacOSVentura) << -128; - ADDROW(Windows11, MacOSVentura) << -128; - ADDROW(MacOSVentura, Windows10) << -128; - ADDROW(MacOSVentura, Windows11) << -128; - ADDROW(Windows10, Android10) << -128; - ADDROW(Windows11, Android11) << -128; + ADDROW(Windows10, MacOSMonterey) << Qt::partial_ordering::unordered; + ADDROW(Windows11, MacOSMonterey) << Qt::partial_ordering::unordered; + ADDROW(MacOSMonterey, Windows10) << Qt::partial_ordering::unordered; + ADDROW(MacOSMonterey, Windows11) << Qt::partial_ordering::unordered; + ADDROW(Windows10, MacOSVentura) << Qt::partial_ordering::unordered; + ADDROW(Windows11, MacOSVentura) << Qt::partial_ordering::unordered; + ADDROW(MacOSVentura, Windows10) << Qt::partial_ordering::unordered; + ADDROW(MacOSVentura, Windows11) << Qt::partial_ordering::unordered; + ADDROW(Windows10, Android10) << Qt::partial_ordering::unordered; + ADDROW(Windows11, Android11) << Qt::partial_ordering::unordered; // Same-OS tests. This list does not have to be exhaustive. - ADDROW(Windows7, Windows7) << 0; - ADDROW(Windows7, Windows8) << -1; - ADDROW(Windows8, Windows7) << 1; - ADDROW(Windows8, Windows10) << -1; - ADDROW(Windows10, Windows8) << 1; - ADDROW(Windows10, Windows10_21H1) << -1; - ADDROW(Windows10_21H1, Windows10) << 1; - ADDROW(Windows10, Windows11) << -1; - ADDROW(MacOSCatalina, MacOSCatalina) << 0; - ADDROW(MacOSCatalina, MacOSBigSur) << -1; - ADDROW(MacOSBigSur, MacOSCatalina) << 1; - ADDROW(MacOSMonterey, MacOSVentura) << -1; - ADDROW(MacOSVentura, MacOSVentura) << 0; - ADDROW(MacOSVentura, MacOSMonterey) << 1; + ADDROW(Windows7, Windows7) << Qt::partial_ordering::equivalent; + ADDROW(Windows7, Windows8) << Qt::partial_ordering::less; + ADDROW(Windows8, Windows7) << Qt::partial_ordering::greater; + ADDROW(Windows8, Windows10) << Qt::partial_ordering::less; + ADDROW(Windows10, Windows8) << Qt::partial_ordering::greater; + ADDROW(Windows10, Windows10_21H1) << Qt::partial_ordering::less; + ADDROW(Windows10_21H1, Windows10) << Qt::partial_ordering::greater; + ADDROW(Windows10, Windows11) << Qt::partial_ordering::less; + ADDROW(MacOSCatalina, MacOSCatalina) << Qt::partial_ordering::equivalent; + ADDROW(MacOSCatalina, MacOSBigSur) << Qt::partial_ordering::less; + ADDROW(MacOSBigSur, MacOSCatalina) << Qt::partial_ordering::greater; + ADDROW(MacOSMonterey, MacOSVentura) << Qt::partial_ordering::less; + ADDROW(MacOSVentura, MacOSVentura) << Qt::partial_ordering::equivalent; + ADDROW(MacOSVentura, MacOSMonterey) << Qt::partial_ordering::greater; #undef ADDROW } @@ -230,18 +225,20 @@ void tst_QOperatingSystemVersion::comparison2() { QFETCH(QOperatingSystemVersion, lhs); QFETCH(QOperatingSystemVersion, rhs); - QFETCH(int, result); + QFETCH(const Qt::partial_ordering, result); QEXPECT_FAIL("Windows10-vs-Windows10_21H1", "QTBUG-107907: Unexpected behavior", Abort); QEXPECT_FAIL("Windows10-vs-Windows11", "QTBUG-107907: Unexpected behavior", Abort); - // value -128 indicates "not comparable" - bool comparable = (result != -128); - QCOMPARE(lhs < rhs, result < 0 && comparable); + const bool comparable = (result != Qt::partial_ordering::unordered); + QCOMPARE_EQ(lhs < rhs, is_lt(result) && comparable); QEXPECT_FAIL("Windows10_21H1-vs-Windows10", "QTBUG-107907: Unexpected behavior", Abort); - QCOMPARE(lhs <= rhs, result <= 0 && comparable); - QCOMPARE(lhs > rhs, result > 0 && comparable); - QCOMPARE(lhs >= rhs, result >= 0 && comparable); + QCOMPARE_EQ(lhs <= rhs, is_lteq(result) && comparable); + QCOMPARE_EQ(lhs > rhs, is_gt(result) && comparable); + QCOMPARE_EQ(lhs >= rhs, is_gteq(result) && comparable); +#ifdef __cpp_lib_three_way_comparison + Q_COMPARE_EQ(lhs <=> rhs, result); +#endif } void tst_QOperatingSystemVersion::mixedComparison()