From f53074c74702a39ad8fcadfa8ba0e1618a352efd Mon Sep 17 00:00:00 2001
From: Ivan Solovev <ivan.solovev@qt.io>
Date: Tue, 23 Jul 2024 15:49:54 +0200
Subject: [PATCH] QArrayDataOps: fix FP equality comparison

We cannot use memcmp() for that, as it incorrectly results in two
NaN values being equal.
Simply replace the whole self-written algorithm with std::equal(),
as it's doing the same thing, but lets the compiler decide when
to do the optimizations.

Amends db89349bdba2fcc03b2f7e2d23f549a9ec5dc0e3.

[ChangeLog][QtCore][QList] Fixed a bug when two QLists holding NaN
values were considered to be equal.

Fixes: QTBUG-127473
Pick-to: 6.7 6.5 6.2
Change-Id: If7e77549b4cbeb96711893d14344b9e0a40c1a87
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
(cherry picked from commit cb3faeba3d9411ba82c311751bd7de36d0fec939)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
---
 src/corelib/tools/qarraydataops.h             | 29 ++-----------------
 .../tst_containerapisymmetry.cpp              | 26 +++++++++++++++++
 2 files changed, 29 insertions(+), 26 deletions(-)

diff --git a/src/corelib/tools/qarraydataops.h b/src/corelib/tools/qarraydataops.h
index 6dcdba66195..7eca084f9a7 100644
--- a/src/corelib/tools/qarraydataops.h
+++ b/src/corelib/tools/qarraydataops.h
@@ -272,23 +272,8 @@ public:
 
     bool compare(const T *begin1, const T *begin2, size_t n) const
     {
-        // only use memcmp for fundamental types or pointers.
-        // Other types could have padding in the data structure or custom comparison
-        // operators that would break the comparison using memcmp
-        if constexpr (QArrayDataPointer<T>::pass_parameter_by_value) {
-            return ::memcmp(begin1, begin2, n * sizeof(T)) == 0;
-        } else {
-            const T *end1 = begin1 + n;
-            while (begin1 != end1) {
-                if (*begin1 == *begin2) {
-                    ++begin1;
-                    ++begin2;
-                } else {
-                    return false;
-                }
-            }
-            return true;
-        }
+        const T *end1 = begin1 + n;
+        return std::equal(begin1, end1, begin2);
     }
 
     void reallocate(qsizetype alloc, QArrayData::AllocationOption option)
@@ -665,15 +650,7 @@ public:
     bool compare(const T *begin1, const T *begin2, size_t n) const
     {
         const T *end1 = begin1 + n;
-        while (begin1 != end1) {
-            if (*begin1 == *begin2) {
-                ++begin1;
-                ++begin2;
-            } else {
-                return false;
-            }
-        }
-        return true;
+        return std::equal(begin1, end1, begin2);
     }
 };
 
diff --git a/tests/auto/corelib/tools/containerapisymmetry/tst_containerapisymmetry.cpp b/tests/auto/corelib/tools/containerapisymmetry/tst_containerapisymmetry.cpp
index 5eb9dbfa362..8cb494f94b2 100644
--- a/tests/auto/corelib/tools/containerapisymmetry/tst_containerapisymmetry.cpp
+++ b/tests/auto/corelib/tools/containerapisymmetry/tst_containerapisymmetry.cpp
@@ -429,6 +429,21 @@ private Q_SLOTS:
     void keyValueRange_QMultiMap() { keyValueRange_impl<QMultiMap<int, int>>(); }
     void keyValueRange_QHash() { keyValueRange_impl<QHash<int, int>>(); }
     void keyValueRange_QMultiHash() { keyValueRange_impl<QMultiHash<int, int>>(); }
+
+private:
+    template <typename Container>
+    void opEqNaN_impl() const;
+
+private Q_SLOTS:
+    void opEqNaN_QList_Float() { opEqNaN_impl<QList<float>>(); }
+    void opEqNaN_QList_Float16() { opEqNaN_impl<QList<qfloat16>>(); }
+    void opEqNaN_QList_Double() { opEqNaN_impl<QList<double>>(); }
+    void opEqNaN_QVarLengthArray_Float() { opEqNaN_impl<QVarLengthArray<float>>(); }
+    void opEqNaN_QVarLengthArray_Float16() { opEqNaN_impl<QVarLengthArray<qfloat16>>(); }
+    void opEqNaN_QVarLengthArray_Double() { opEqNaN_impl<QVarLengthArray<double>>(); }
+    void opEqNaN_QSet_Float() { opEqNaN_impl<QSet<float>>(); }
+    void opEqNaN_QSet_Float16() { opEqNaN_impl<QSet<qfloat16>>(); }
+    void opEqNaN_QSet_Double() { opEqNaN_impl<QSet<double>>(); }
 };
 
 void tst_ContainerApiSymmetry::init()
@@ -1263,5 +1278,16 @@ void tst_ContainerApiSymmetry::keyValueRange_impl() const
     QVERIFY(verify(values, COUNT, 2));
 }
 
+template<typename Container>
+void tst_ContainerApiSymmetry::opEqNaN_impl() const
+{
+    using V = typename Container::value_type;
+    static_assert(std::is_floating_point_v<V> || std::is_same_v<V, qfloat16>);
+    const Container lhs {std::numeric_limits<V>::quiet_NaN()};
+    const Container rhs {std::numeric_limits<V>::quiet_NaN()};
+
+    QCOMPARE_NE(lhs, rhs);
+}
+
 QTEST_APPLESS_MAIN(tst_ContainerApiSymmetry)
 #include "tst_containerapisymmetry.moc"