QByteArray(View) comparisons: tame the overload set a bit

QByteArray declares comparison operators against QByteArrayView.
With the addition of the implicit conversion from QByteArray to
std::string_view, those comparison operators are now ambiguous
when comparing a QByteArray against a std::string_view. The
overload set contains:

1) QByteArray<=>QByteArrayView, after converting the string_view
   to a QByteArrayView
2) string_view<=>string_view, after converting the QByteArray to
   string_view.

This is part of the fallout of adding QByteArray->std::string_view
implicit conversions. We accepted the risk of SICs, and here they are.

The first operator should have never been added: comparing a QByteArray
to a QByteArrayView is already possible using QByteArrayView's own
comparison operators (QBAV<=>QBAV, after converting the QBA to it).
Therefore, this commit removes it.

The removal reveals two other problems:

1) there is a comparison defined between QByteArrayView and
const char *. That comparison makes comparing QByteArrayView and
QByteArray ambiguous, because QByteArray can convert to both
QByteArrayView (thus finding the QBAV<=>QBAV comparisosn) and to const
char * (thus finding the QBAV<=>const char * comparison).

Again, the latter overloads should have never been added: const char *
implicitly converts to QByteArrayView and therefore it's served by the
QBAV<=>QBAV comparison operators. This gets removed too.

2) comparisons between QByteArray and types convertible to
QByteArrayView (e.g. std::string) are no longer possible. These were
never meant to be supported: heterogeneous string comparisons must first
and foremost convert everything to a common view type
(...QAnyStringView?), and only then the comparison can proceed.

In general, we should aim at this setup:

1) Owning string classes should only have homogeneous comparisons. They
   might offer heterogeneous comparisons (say, QString <=> const char *)
   on a case-by-case basis. The must never declare comparisons against
   views (instead, view's own comparisons will take care of comparing
   an owning string agains a view!).  Comparing different owning string
   types directly is not meant to be supported (e.g. QString <=>
   std::u16_string), as per above.

2) Views only have homogeneous comparisons. Everything compatible will
   convert to the view type for the comparison. Again, exceptions on
   a case-by-case basis (e.g. QStringView<=>const char *, protected by
   the NO_ASCII macro / ASCII_WARN / etc.).

3) We do not support compare against views coming from other libraries
   (e.g. QByteArrayView<=>std::string_view). One always risks an
   ambiguity, since both convert to each other. SG16 has rejected ideas
   of a "string view protocol"; Qt view classes implicitly build from
   compatible ranges (which likely includes any other string view class
   out there), which makes it a magnet for these ambiguities (say
   against a 3rd party view class that also does the same).

This isn't a complete overhaul: there are still strings and views that
don't follow the above and this commit isn't changing them yet.

Fixes: QTBUG-134902
Change-Id: If181af1fb627490423ca1e467ecce7330f7445fb
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>

src/corelib/text/qbytearray.cpp

@@ -832,7 +832,7 @@ QByteArray qUncompress(const uchar* data, qsizetype nbytes)
     \reentrant
 
     \compares strong
-    \compareswith strong {const char *} QByteArrayView
+    \compareswith strong {const char *}
     \endcompareswith
     \compareswith strong QChar char16_t QString QStringView QLatin1StringView \
                   QUtf8StringView

src/corelib/text/qbytearray.h

@@ -564,7 +564,6 @@ private:
         return Qt::compareThreeWay(res, 0);
     }
     Q_DECLARE_STRONGLY_ORDERED(QByteArray)
-    Q_DECLARE_STRONGLY_ORDERED(QByteArray, QByteArrayView)
     Q_DECLARE_STRONGLY_ORDERED(QByteArray, const char *)
 #if defined(__GLIBCXX__) && defined(__cpp_lib_three_way_comparison)
     // libstdc++ has a bug [0] when `operator const void *()` is preferred over

src/corelib/text/qbytearrayview.h

@@ -366,13 +366,6 @@ private:
     }
     Q_DECLARE_STRONGLY_ORDERED(QByteArrayView)
 
-    friend bool comparesEqual(const QByteArrayView &lhs, const char *rhs) noexcept
-    { return comparesEqual(lhs, QByteArrayView(rhs)); }
-    friend Qt::strong_ordering
-    compareThreeWay(const QByteArrayView &lhs, const char *rhs) noexcept
-    { return compareThreeWay(lhs, QByteArrayView(rhs)); }
-    Q_DECLARE_STRONGLY_ORDERED(QByteArrayView, const char *)
-
     // defined in qstring.cpp
     friend Q_CORE_EXPORT bool
     comparesEqual(const QByteArrayView &lhs, const QChar &rhs) noexcept;

src/corelib/text/qbytearrayview.qdoc

@@ -15,8 +15,6 @@
     \reentrant
 
     \compares strong
-    \compareswith strong QByteArray {const char *}
-    \endcompareswith
     \compareswith strong QString QStringView QUtf8StringView QLatin1StringView \
                   QChar char16_t
     When comparing with string and Unicode character types, the content is

tests/auto/corelib/text/qstringapisymmetry/tst_qstringapisymmetry.cpp

@@ -123,7 +123,7 @@ private:
     //
 
     void compare_data(bool hasConceptOfNullAndEmpty=true);
-    template <typename LHS, typename RHS>
+    template <typename LHS, typename RHS, bool CheckCompareThreeWay = true>
     void compare_impl() const;
 
 private Q_SLOTS:
@@ -184,6 +184,8 @@ private Q_SLOTS:
     void compare_QString_QByteArrayView() { compare_impl<QString, QByteArrayView>(); }
     void compare_QString_const_char_star_data() { compare_data(); }
     void compare_QString_const_char_star() { compare_impl<QString, const char *>(); }
+    void compare_QString_std_u16_string_view_data() { compare_data(); }
+    void compare_QString_std_u16_string_view() { compare_impl<QString, std::u16string_view, false>(); }
 
     void compare_QStringView_QChar_data() { compare_data(false); }
     void compare_QStringView_QChar() { compare_impl<QStringView, QChar>(); }
@@ -203,6 +205,8 @@ private Q_SLOTS:
     void compare_QStringView_QByteArrayView() { compare_impl<QStringView, QByteArrayView>(); }
     void compare_QStringView_const_char_star_data() { compare_data(); }
     void compare_QStringView_const_char_star() { compare_impl<QStringView, const char *>(); }
+    void compare_QStringView_std_u16_string_data() { compare_data(); }
+    void compare_QStringView_std_u16_string() { compare_impl<QStringView, std::u16string, false>(); }
 
     void compare_QUtf8StringView_QChar_data() { compare_data(false); }
     void compare_QUtf8StringView_QChar() { compare_impl<QUtf8StringView, QChar>(); }
@@ -260,6 +264,13 @@ private Q_SLOTS:
     void compare_QByteArray_QByteArrayView() { compare_impl<QByteArray, QByteArrayView>(); }
     void compare_QByteArray_const_char_star_data() { compare_data(); }
     void compare_QByteArray_const_char_star() { compare_impl<QByteArray, const char *>(); }
+    void compare_QByteArray_std_string_view_data() { compare_data(); }
+    void compare_QByteArray_std_string_view()
+    {
+#ifdef QT_BYTEARRAY_CONVERTS_TO_STD_STRING_VIEW
+        compare_impl<QByteArray, std::string_view, false>();
+#endif
+    }
 
     void compare_QByteArrayView_QChar_data() { compare_data(false); }
     void compare_QByteArrayView_QChar() { compare_impl<QByteArrayView, QChar>(); }
@@ -279,6 +290,8 @@ private Q_SLOTS:
     void compare_QByteArrayView_QByteArrayView() { compare_impl<QByteArrayView, QByteArrayView>(); }
     void compare_QByteArrayView_const_char_star_data() { compare_data(); }
     void compare_QByteArrayView_const_char_star() { compare_impl<QByteArrayView, const char *>(); }
+    void compare_QByteArrayView_std_string_data() { compare_data(); }
+    void compare_QByteArrayView_std_string() { compare_impl<QByteArrayView, std::string, false>(); }
 
     void compare_const_char_star_QChar_data() { compare_data(false); }
     void compare_const_char_star_QChar() { compare_impl<const char *, QChar>(); }
@@ -1630,6 +1643,13 @@ MAKE(QUtf8StringView)        { return u8; }
 MAKE(QAnyStringViewUsingL1)  { return {QAnyStringView{l1}}; }
 MAKE(QAnyStringViewUsingU8)  { return {QAnyStringView{u8}}; }
 MAKE(QAnyStringViewUsingU16) { return {QAnyStringView{sv}}; }
+MAKE(std::string)            { return u8.toStdString(); }
+#ifdef QT_BYTEARRAY_CONVERTS_TO_STD_STRING_VIEW
+MAKE(std::string_view)       { return u8; }
+#else
+MAKE(std::string_view)       { return std::string_view(u8.data(), size_t(u8.size())); }
+#endif
+MAKE(std::u16string_view)    { return sv; }
 #undef MAKE
 
 // Some types have ASCII-only case-insensitive compare, but are handled as containing
@@ -1642,7 +1662,7 @@ template <> constexpr bool is_bytearray_like_v<QByteArrayView> = true;
 template <typename LHS, typename RHS>
 constexpr bool has_nothrow_member_compare_v = is_bytearray_like_v<LHS> == is_bytearray_like_v<RHS>;
 
-template <typename LHS, typename RHS>
+template <typename LHS, typename RHS, bool CheckCompareThreeWay>
 void tst_QStringApiSymmetry::compare_impl() const
 {
     QFETCH(QStringView, lhsUnicode);
@@ -1686,11 +1706,14 @@ void tst_QStringApiSymmetry::compare_impl() const
     CHECK(<=);
     CHECK(>=);
 #undef CHECK
-    // Test that all string-like types implemente compareThreeWay() as a friend
+
-    // function.
+    if constexpr (CheckCompareThreeWay) {
-    const Qt::strong_ordering expectedOrdering =
+        // Test that all string-like types implemente compareThreeWay() as a friend
-            Qt::compareThreeWay(caseSensitiveCompareResult, 0);
+        // function.
-    QCOMPARE_EQ(qCompareThreeWay(lhs, rhs), expectedOrdering);
+        const Qt::strong_ordering expectedOrdering =
+                Qt::compareThreeWay(caseSensitiveCompareResult, 0);
+        QCOMPARE_EQ(qCompareThreeWay(lhs, rhs), expectedOrdering);
+    }
 }
 
 template <typename LHS, typename RHS>