From e37c0a33a40aa216e58e6b47b4cfc6e499e8369e Mon Sep 17 00:00:00 2001 From: Ivan Solovev Date: Thu, 23 May 2024 16:56:59 +0200 Subject: [PATCH] Deprecate Qt::compareThreeWay() overload for pointers compareThreeWay() was supposed to be an operator<=>(), but for C++17. The idea was that at some point when we unconditionally demand C++20, people could just replace all the usages of compareThreeWay() with operator<=>(). However, the Qt::compareThreeWay() overload for pointers is different from the operator<=>() for pointers, because it is actually using std::less{} or std::compare_three_way{} to do the comparison, thus avoiding an UB. This is not bad as such, but can potentially lead to UB when mass-replacing all compareThreeWay() calls with operator<=>(). To avoid this problem, deprecate the overload, and suggest to use the Qt::totally_ordered_wrapper together with the respective overload instead. Found in API Review. [ChangeLog][QtCore][QtCompare] Deprecate Qt::compareThreeWay() overload for pointers. Change-Id: I9c57871145dc3cb9656d6006db88b48a1553bef4 Reviewed-by: Marc Mutz (cherry picked from commit b1ae4334ea11f6942c7ce37a80a3aa98a3447809) Reviewed-by: Qt Cherry-pick Bot --- src/corelib/global/qcompare.cpp | 4 ++++ src/corelib/global/qcomparehelpers.h | 7 +++++++ .../corelib/global/qcompare/tst_qcompare.cpp | 16 +++++++++++++--- .../qcomparehelpers/tst_qcomparehelpers.cpp | 10 ++++++++++ 4 files changed, 34 insertions(+), 3 deletions(-) diff --git a/src/corelib/global/qcompare.cpp b/src/corelib/global/qcompare.cpp index d32f4ca251f..5a2ad0e2ee4 100644 --- a/src/corelib/global/qcompare.cpp +++ b/src/corelib/global/qcompare.cpp @@ -1274,9 +1274,12 @@ CHECK(strong, equivalent); \l Qt::partial_ordering::unordered is returned. */ +#if QT_DEPRECATED_SINCE(6, 8) /*! \fn template = true> Qt::compareThreeWay(const LeftType *lhs, const RightType *rhs) \since 6.7 + \deprecated [6.8] Wrap the pointers into Qt::totally_ordered_wrapper and + use the respective Qt::compareThreeWay() overload instead. \relates \overload @@ -1289,6 +1292,7 @@ CHECK(strong, equivalent); Returns an instance of \l Qt::strong_ordering that represents the relation between \a lhs and \a rhs. */ +#endif // QT_DEPRECATED_SINCE(6, 8) /*! \fn template = true> Qt::compareThreeWay(Enum lhs, Enum rhs) diff --git a/src/corelib/global/qcomparehelpers.h b/src/corelib/global/qcomparehelpers.h index cfdd395d6d5..48a440274b8 100644 --- a/src/corelib/global/qcomparehelpers.h +++ b/src/corelib/global/qcomparehelpers.h @@ -547,8 +547,11 @@ constexpr Qt::partial_ordering compareThreeWay(FloatType lhs, IntType rhs) noexc return compareThreeWay(lhs, FloatType(rhs)); } +#if QT_DEPRECATED_SINCE(6, 8) + template = true> +QT_DEPRECATED_VERSION_X_6_8("Wrap the pointers into Qt::totally_ordered_wrapper and use the respective overload instead.") constexpr Qt::strong_ordering compareThreeWay(const LeftType *lhs, const RightType *rhs) noexcept { #ifdef __cpp_lib_three_way_comparison @@ -564,17 +567,21 @@ constexpr Qt::strong_ordering compareThreeWay(const LeftType *lhs, const RightTy } template +QT_DEPRECATED_VERSION_X_6_8("Wrap the pointer into Qt::totally_ordered_wrapper and use the respective overload instead.") constexpr Qt::strong_ordering compareThreeWay(const T *lhs, std::nullptr_t rhs) noexcept { return compareThreeWay(lhs, static_cast(rhs)); } template +QT_DEPRECATED_VERSION_X_6_8("Wrap the pointer into Qt::totally_ordered_wrapper and use the respective overload instead.") constexpr Qt::strong_ordering compareThreeWay(std::nullptr_t lhs, const T *rhs) noexcept { return compareThreeWay(static_cast(lhs), rhs); } +#endif // QT_DEPRECATED_SINCE(6, 8) + template = true> constexpr Qt::strong_ordering compareThreeWay(Enum lhs, Enum rhs) noexcept { diff --git a/tests/auto/corelib/global/qcompare/tst_qcompare.cpp b/tests/auto/corelib/global/qcompare/tst_qcompare.cpp index e36429e62b8..556c8212b3b 100644 --- a/tests/auto/corelib/global/qcompare/tst_qcompare.cpp +++ b/tests/auto/corelib/global/qcompare/tst_qcompare.cpp @@ -785,9 +785,10 @@ void tst_QCompare::compareThreeWay() static_assert(noexcept(qCompareThreeWay(std::declval(), std::declval()))); // pointers - static_assert(noexcept(qCompareThreeWay(std::declval(), - std::declval()))); - static_assert(noexcept(qCompareThreeWay(std::declval(), nullptr))); + using StringWrapperPtr = Qt::totally_ordered_wrapper; + static_assert(noexcept(qCompareThreeWay(std::declval(), + std::declval()))); + static_assert(noexcept(qCompareThreeWay(std::declval(), nullptr))); // Test some actual comparison results @@ -832,8 +833,17 @@ void tst_QCompare::compareThreeWay() // pointers std::array arr{1, 0}; +#if QT_DEPRECATED_SINCE(6, 8) +QT_WARNING_PUSH +QT_WARNING_DISABLE_DEPRECATED QCOMPARE_EQ(qCompareThreeWay(&arr[1], &arr[0]), Qt::strong_ordering::greater); QCOMPARE_EQ(qCompareThreeWay(arr.data(), &arr[0]), Qt::strong_ordering::equivalent); +QT_WARNING_POP +#endif // QT_DEPRECATED_SINCE(6, 8) + const auto a0 = Qt::totally_ordered_wrapper(&arr[0]); + const auto a1 = Qt::totally_ordered_wrapper(&arr[1]); + QCOMPARE_EQ(qCompareThreeWay(a1, a0), Qt::strong_ordering::greater); + QCOMPARE_EQ(qCompareThreeWay(arr.data(), a0), Qt::strong_ordering::equivalent); } QTEST_MAIN(tst_QCompare) diff --git a/tests/auto/corelib/global/qcomparehelpers/tst_qcomparehelpers.cpp b/tests/auto/corelib/global/qcomparehelpers/tst_qcomparehelpers.cpp index c570151cb65..3a910e1839b 100644 --- a/tests/auto/corelib/global/qcomparehelpers/tst_qcomparehelpers.cpp +++ b/tests/auto/corelib/global/qcomparehelpers/tst_qcomparehelpers.cpp @@ -581,17 +581,27 @@ void tst_QCompareHelpers::builtinOrder() Qt::strong_ordering::equivalent); std::array arr{1, 0}; +#if QT_DEPRECATED_SINCE(6, 8) +QT_WARNING_PUSH +QT_WARNING_DISABLE_DEPRECATED QCOMPARE_EQ(Qt::compareThreeWay(&arr[0], &arr[1]), Qt::strong_ordering::less); QCOMPARE_EQ(Qt::compareThreeWay(arr.data(), &arr[0]), Qt::strong_ordering::equivalent); +QT_WARNING_POP +#endif // QT_DEPRECATED_SINCE(6, 8) class Base {}; class Derived : public Base {}; auto b = std::make_unique(); auto d = std::make_unique(); +#if QT_DEPRECATED_SINCE(6, 8) +QT_WARNING_PUSH +QT_WARNING_DISABLE_DEPRECATED QCOMPARE_NE(Qt::compareThreeWay(b.get(), d.get()), Qt::strong_ordering::equivalent); QCOMPARE_EQ(Qt::compareThreeWay(b.get(), nullptr), Qt::strong_ordering::greater); QCOMPARE_EQ(Qt::compareThreeWay(nullptr, d.get()), Qt::strong_ordering::less); +QT_WARNING_POP +#endif // QT_DEPRECATED_SINCE(6, 8) // Check Qt::totally_ordered_wrapper auto a0 = Qt::totally_ordered_wrapper(&arr[0]);