QList: rework comparison operators

Now that we have lexicographicalCompareThreeWay(), we can implement
the compareThreeWay() helper method in terms of this function. This
method is only available if the contained type provides its own
compareThreeWay() helper method.

However, we decided to not change the existing relational operators
for C++17 mode, and only provide operator<=>() if the contained type
also implements operator<=>() or operator<(). The reason for that is
that we do not want to introduce any potential regressions in container
comparison code.

This required extending QTypeTraits with the traits to detect support
for operator<=>(). We use the std::three_way_comparable concept for
that, because the check is only needed for C++20 code.

The implementation uses std::lexicographical_compare_three_way with
a custom QtOrderingPrivate::synthThreeWay() comparator, which is
modelled after the exposition-only synth-three-way function, as defined
by the standard [0].

[0]: https://wg21.link/expos.only.entity#2

The tests show that in C++17 mode the relational operators do not
handle the partially_ordered::unordered case correctly. That is because
operator<=() and operator>=() are implemented in terms of operator<()
and operator==(). The same problem exists for std containers.

Added the unit-tests to tst_containerapisymmetry.cpp to later reuse
them for QVarLengthArray.

[ChangeLog][QtCore][QList] QList now implements operator<=>() in
C++20 mode.

Task-number: QTBUG-120305
Change-Id: I7f6fd9c1b060449a265447ec0922088dcf521cd2
Reviewed-by: Marc Mutz <marc.mutz@qt.io>

src/corelib/global/qcomparehelpers.h

@@ -1233,6 +1233,25 @@ constexpr bool compareThreeWayNoexcept() noexcept
 
 } // namespace CompareThreeWayTester
 
+#ifdef __cpp_lib_three_way_comparison
+[[maybe_unused]] inline constexpr struct { /* Niebloid */
+    template <typename LT, typename RT = LT>
+    [[maybe_unused]] constexpr auto operator()(const LT &lhs, const RT &rhs) const
+    {
+        // like [expos.only.entity]/2
+        if constexpr (QTypeTraits::has_operator_compare_three_way_with_v<LT, RT>) {
+            return lhs <=> rhs;
+        } else {
+            if (lhs < rhs)
+                return std::weak_ordering::less;
+            if (rhs < lhs)
+                return std::weak_ordering::greater;
+            return std::weak_ordering::equivalent;
+        }
+    }
+} synthThreeWay;
+#endif // __cpp_lib_three_way_comparison
+
 // These checks do not use Qt::compareThreeWay(), so only work for user-defined
 // compareThreeWay() helper functions.
 // We cannot use the same condition as in CompareThreeWayTester::hasCompareThreeWay,

src/corelib/global/qttypetraits.h

@@ -13,6 +13,11 @@
 #include <utility>
 #include <variant>
 
+#if defined(__cpp_lib_three_way_comparison) && defined(__cpp_lib_concepts)
+#include <compare>
+#include <concepts>
+#endif
+
 #if 0
 #pragma qt_class(QtTypeTraits)
 #pragma qt_sync_stop_processing
@@ -215,7 +220,7 @@ struct expand_operator_less_than_tuple<std::tuple<T...>> : expand_operator_less_
 template<typename ...T>
 struct expand_operator_less_than_tuple<std::variant<T...>> : expand_operator_less_than_recursive<T...> {};
 
-}
+} // namespace detail
 
 template<typename T, typename = void>
 struct is_dereferenceable : std::false_type {};
@@ -255,6 +260,28 @@ using compare_lt_result = std::enable_if_t<std::conjunction_v<QTypeTraits::has_o
 template <typename Container, typename ...T>
 using compare_lt_result_container = std::enable_if_t<std::conjunction_v<QTypeTraits::has_operator_less_than_container<Container, T>...>, bool>;
 
+template<typename T>
+struct has_operator_compare_three_way : std::false_type {};
+template <typename T, typename U>
+struct has_operator_compare_three_way_with : std::false_type {};
+#if defined(__cpp_lib_three_way_comparison) && defined(__cpp_lib_concepts)
+template<std::three_way_comparable T>
+struct has_operator_compare_three_way<T> : std::true_type {};
+template <typename T, typename U>
+    requires std::three_way_comparable_with<T, U>
+struct has_operator_compare_three_way_with<T, U> : std::true_type {};
+#endif // __cpp_lib_three_way_comparison && __cpp_lib_concepts
+template<typename T>
+constexpr inline bool has_operator_compare_three_way_v = has_operator_compare_three_way<T>::value;
+template<typename T, typename U>
+constexpr inline bool has_operator_compare_three_way_with_v = has_operator_compare_three_way_with<T, U>::value;
+
+// Intentionally no 'has_operator_compare_three_way_container', because the
+// compilers fail to determine the proper return type in this case
+// template <typename Container, typename T>
+// using has_operator_compare_three_way_container =
+//         std::disjunction<std::is_base_of<Container, T>, has_operator_compare_three_way<T>>;
+
 namespace detail {
 
 template<typename T>

src/corelib/tools/qlist.h

@@ -6,6 +6,7 @@
 #define QLIST_H
 
 #include <QtCore/qarraydatapointer.h>
+#include <QtCore/qcompare.h>
 #include <QtCore/qnamespace.h>
 #include <QtCore/qhashfunctions.h>
 #include <QtCore/qiterator.h>
@@ -338,6 +339,33 @@ public:
     void swap(QList &other) noexcept { d.swap(other.d); }
 
 #ifndef Q_QDOC
+private:
+    template <typename U = T,
+              Qt::if_has_qt_compare_three_way<U, U> = true>
+    friend auto compareThreeWay(const QList &lhs, const QList &rhs)
+    {
+        return QtOrderingPrivate::lexicographicalCompareThreeWay(lhs.begin(), lhs.end(),
+                                                                 rhs.begin(), rhs.end());
+    }
+
+#if defined(__cpp_lib_three_way_comparison) && defined(__cpp_lib_concepts)
+    template <typename Container, typename U = T>
+    using if_has_op_less_or_op_compare_three_way =
+            std::enable_if_t<
+                std::disjunction_v<QTypeTraits::has_operator_less_than_container<Container, U>,
+                                   QTypeTraits::has_operator_compare_three_way<U>>,
+            bool>;
+
+    template <typename U = T, if_has_op_less_or_op_compare_three_way<QList, U> = true>
+    friend auto operator<=>(const QList &lhs, const QList &rhs)
+    {
+        return std::lexicographical_compare_three_way(lhs.begin(), lhs.end(),
+                                                      rhs.begin(), rhs.end(),
+                                                      QtOrderingPrivate::synthThreeWay);
+    }
+#endif // __cpp_lib_three_way_comparison && __cpp_lib_concepts
+
+public:
     template <typename U = T>
     QTypeTraits::compare_eq_result_container<QList, U> operator==(const QList &other) const
     {
@@ -349,12 +377,14 @@ public:
         // do element-by-element comparison
         return std::equal(begin(), end(), other.begin(), other.end());
     }
+
     template <typename U = T>
     QTypeTraits::compare_eq_result_container<QList, U> operator!=(const QList &other) const
     {
         return !(*this == other);
     }
 
+#ifndef __cpp_lib_three_way_comparison
     template <typename U = T>
     QTypeTraits::compare_lt_result_container<QList, U> operator<(const QList &other) const
         noexcept(noexcept(std::lexicographical_compare<typename QList<U>::const_iterator,
@@ -386,6 +416,7 @@ public:
     {
         return !(*this < other);
     }
+#endif // __cpp_lib_three_way_comparison
 #else
     bool operator==(const QList &other) const;
     bool operator!=(const QList &other) const;
@@ -393,6 +424,7 @@ public:
     bool operator>(const QList &other) const;
     bool operator<=(const QList &other) const;
     bool operator>=(const QList &other) const;
+    friend auto operator<=>(const QList &lhs, const QList &rhs);
 #endif // Q_QDOC
 
     static constexpr qsizetype maxSize() { return Data::maxSize(); }

src/corelib/tools/qlist.qdoc

@@ -426,6 +426,21 @@
     of \c operator<().
 */
 
+/*!
+    \fn template <typename T> auto QList<T>::operator<=>(const QList<T> &lhs, const QList<T> &rhs)
+    \since 6.9
+
+    Compares the contents of \a lhs and \a rhs
+    \l {https://en.cppreference.com/w/cpp/algorithm/lexicographical_compare_three_way}
+    {lexicographically}. Returns the result of the strongest applicable category
+    type, that is \c {decltype(lhs[0] <=> rhs[0])} if \c {operator<=>()} is
+    available for type \c {T}; otherwise \c {std::weak_ordering}.
+
+    \note This operator is only available in C++20 mode, and when the underlying
+    type \c T models the \c {std::three_way_comparable} concept
+    or provides \c {operator<()}.
+*/
+
 /*!
     \fn template <typename T> size_t qHash(const QList<T> &key, size_t seed = 0)
     \since 5.6

tests/auto/corelib/tools/containerapisymmetry/CMakeLists.txt

@@ -14,6 +14,8 @@ endif()
 qt_internal_add_test(tst_containerapisymmetry
     SOURCES
         tst_containerapisymmetry.cpp
+    LIBRARIES
+        Qt::TestPrivate
 )
 
 ## Scopes:

tests/auto/corelib/tools/containerapisymmetry/tst_containerapisymmetry.cpp

@@ -12,6 +12,8 @@
 #include "qstring.h"
 #include "qvarlengtharray.h"
 
+#include <private/qcomparisontesthelper_p.h>
+
 #include <algorithm>
 #include <functional>
 #include <iostream>
@@ -217,6 +219,20 @@ public:
     using QVarLengthArray<T>::QVarLengthArray;
 };
 
+// The class does not provide operator<=> in C++20 mode
+struct LessOnly
+{
+    float val;
+private:
+    friend auto compareThreeWay(LessOnly lhs, LessOnly rhs) noexcept
+    { return Qt::compareThreeWay(lhs.val, rhs.val); }
+
+    friend bool operator==(LessOnly lhs, LessOnly rhs) noexcept
+    { return lhs.val == rhs.val; }
+    friend bool operator<(LessOnly lhs, LessOnly rhs) noexcept
+    { return lhs.val < rhs.val; }
+};
+
 class tst_ContainerApiSymmetry : public QObject
 {
     Q_OBJECT
@@ -455,6 +471,22 @@ private:
 private Q_SLOTS:
     void try_emplace_QHash() { try_emplace_impl<QHash<int, int>>(); }
     void try_emplace_unordered_map() { try_emplace_impl<std::unordered_map<int, int>>(); }
+
+private:
+    template <typename Container, typename Ordering>
+    void comparisonTest_impl();
+
+private Q_SLOTS:
+    void comparisonTest_QList_int()
+    { comparisonTest_impl<QList<int>, Qt::strong_ordering>(); }
+    void comparisonTest_QList_float()
+    { comparisonTest_impl<QList<float>, Qt::partial_ordering>(); }
+    void comparisonTest_QList_QDateTime()
+    { comparisonTest_impl<QList<QDateTime>, Qt::weak_ordering>(); }
+    void comparisonTest_QList_intptr()
+    { comparisonTest_impl<QList<const int *>, Qt::strong_ordering>(); }
+    void comparisonTest_QList_LessOnly()
+    { comparisonTest_impl<QList<LessOnly>, Qt::weak_ordering>(); }
 };
 
 void tst_ContainerApiSymmetry::init()
@@ -1341,5 +1373,138 @@ void tst_ContainerApiSymmetry::try_emplace_impl() const
     QCOMPARE(it->second, V());
 }
 
+template <typename ValueType, typename Ordering>
+std::vector<ValueType> makeComparisonData(Ordering)
+{ return {}; }
+
+template <>
+std::vector<int> makeComparisonData(Qt::strong_ordering order)
+{
+    if (order == Qt::strong_ordering::equivalent)
+        return {1, 2, 3, 4};
+    else if (order == Qt::strong_ordering::less)
+        return {1, 2, 1};
+    else /* greater */
+        return {1, 2, 4};
+}
+
+template <>
+std::vector<float> makeComparisonData(Qt::partial_ordering order)
+{
+    if (order == Qt::partial_ordering::equivalent)
+        return {1.f, 2.f, 3.f, 4.f};
+    else if (order == Qt::partial_ordering::less)
+        return {1.f, 2.f, 1.f};
+    else if (order == Qt::partial_ordering::greater)
+        return {1.f, 2.f, 4.f};
+    else /* unordered */
+        return {std::numeric_limits<float>::quiet_NaN(), 2.f, 3.f, 4.f};
+}
+
+template <>
+std::vector<QDateTime> makeComparisonData(Qt::weak_ordering order)
+{
+    using namespace std::chrono_literals;
+    QTimeZone utcPlusOne = QTimeZone::fromDurationAheadOfUtc(3600s);
+    // These two QDateTimes represent the same moment of time, but using
+    // different time zones. So, they are equivalent, but not equal.
+    QDateTime nullUtc = QDateTime::fromMSecsSinceEpoch(0, QTimeZone::UTC);
+    QDateTime nullUtcPlusOne = QDateTime::fromMSecsSinceEpoch(0, utcPlusOne);
+
+    if (order == Qt::weak_ordering::equivalent)
+        return {nullUtc, nullUtc.addDays(1), nullUtc.addDays(2), nullUtc.addDays(3)};
+    else if (order == Qt::weak_ordering::less)
+        return {nullUtcPlusOne, nullUtc.addDays(1), nullUtc};
+    else /* greater */
+        return {nullUtcPlusOne, nullUtc.addDays(1), nullUtc.addDays(3)};
+}
+
+static constexpr std::array<int, 4> intArray = {0, 0, 0, 0};
+
+template <>
+std::vector<const int *> makeComparisonData(Qt::strong_ordering order)
+{
+    if (order == Qt::strong_ordering::equivalent)
+        return {&intArray[0], &intArray[1], &intArray[2], &intArray[3]};
+    else if (order == Qt::strong_ordering::less)
+        return {&intArray[0], &intArray[1], &intArray[0]};
+    else /* greater */
+        return {&intArray[0], &intArray[1], &intArray[3]};
+}
+
+template <>
+std::vector<LessOnly> makeComparisonData(Qt::weak_ordering order)
+{
+    if (order == Qt::weak_ordering::equivalent)
+        return {LessOnly{1.f}, LessOnly{2.f}, LessOnly{3.f}, LessOnly{4.f}};
+    else if (order == Qt::weak_ordering::less)
+        return {LessOnly{1.f}, LessOnly{2.f}, LessOnly{1.f}};
+    else /* greater */
+        return {LessOnly{1.f}, LessOnly{2.f}, LessOnly{4.f}};
+}
+
+template<typename Container, typename Ordering>
+void tst_ContainerApiSymmetry::comparisonTest_impl()
+{
+    QTestPrivate::testAllComparisonOperatorsCompile<Container>();
+
+    using V = typename Container::value_type;
+    const auto eq_vec = makeComparisonData<V>(Ordering::equivalent);
+
+    Container lhs{eq_vec.begin(), eq_vec.end()};
+    Container rhs{eq_vec.begin(), eq_vec.end()};
+    QCOMPARE_EQ(compareThreeWay(lhs, rhs), Ordering::equivalent);
+    QT_TEST_ALL_COMPARISON_OPS(lhs, rhs, Ordering::equivalent);
+
+
+    const auto lt_vec = makeComparisonData<V>(Ordering::less);
+    rhs = {lt_vec.begin(), lt_vec.end()};
+    QCOMPARE_EQ(compareThreeWay(lhs, rhs), Ordering::greater);
+    QT_TEST_ALL_COMPARISON_OPS(lhs, rhs, Ordering::greater);
+
+    const auto gt_vec = makeComparisonData<V>(Ordering::greater);
+    rhs = {gt_vec.begin(), gt_vec.end()};
+    QCOMPARE_EQ(compareThreeWay(lhs, rhs), Ordering::less);
+    QT_TEST_ALL_COMPARISON_OPS(lhs, rhs, Ordering::less);
+
+    if constexpr (std::is_same_v<Ordering, Qt::partial_ordering>) {
+        const auto un_vec = makeComparisonData<V>(Ordering::unordered);
+        rhs = {un_vec.begin(), un_vec.end()};
+        QCOMPARE_EQ(compareThreeWay(lhs, rhs), Ordering::unordered);
+#ifdef __cpp_lib_three_way_comparison
+        QT_TEST_ALL_COMPARISON_OPS(lhs, rhs, Ordering::unordered);
+#else
+        // partial_ordering::unordered works incorrectly for containers in C++17
+        // mode, but that is in line with how std containers behave
+        QVERIFY(!(lhs == rhs));
+        QVERIFY(lhs != rhs);
+        QVERIFY(!(lhs < rhs));
+        QVERIFY(!(lhs > rhs));
+        // Behaves like this, because in C++17 op<=() and op>=() are calculated
+        // based on op<() and op==().
+        QVERIFY(lhs <= rhs);
+        QVERIFY(lhs >= rhs);
+#endif
+
+        // comparison with itself should still yield unordered
+        lhs = {un_vec.begin(), un_vec.end()};
+        QCOMPARE_EQ(compareThreeWay(lhs, rhs), Ordering::unordered);
+#ifdef __cpp_lib_three_way_comparison
+        QT_TEST_ALL_COMPARISON_OPS(lhs, rhs, Ordering::unordered);
+#else
+        // partial_ordering::unordered works incorrectly for containers in C++17
+        // mode, but that is in line with how std containers behave
+        QVERIFY(!(lhs == rhs));
+        QVERIFY(lhs != rhs);
+        QVERIFY(!(lhs < rhs));
+        QVERIFY(!(lhs > rhs));
+        // Behaves like this, because in C++17 op<=() and op>=() are calculated
+        // based on op<() and op==().
+        QVERIFY(lhs <= rhs);
+        QVERIFY(lhs >= rhs);
+#endif
+    }
+}
+
 QTEST_APPLESS_MAIN(tst_ContainerApiSymmetry)
 #include "tst_containerapisymmetry.moc"