From d999e646929889a0e25df2be4a097196c9ea3a49 Mon Sep 17 00:00:00 2001 From: Ivan Solovev Date: Fri, 22 Mar 2024 09:53:55 +0100 Subject: [PATCH] QPoint(F): use comparison helper macros Explicitly add QPointF vs QPoint comparison. Previously such comparison was implicitly converting QPoint to QPointF, and doing the fuzzy comparison. We have to keep the old behavior to avoid breaking user code, so explicitly use fuzzy comparison in the new operators. As a drive-by: move the friend functions into the private section, so that they are actually hidden friends. Task-number: QTBUG-120308 Change-Id: I471a890b8332455e8b2dc1b99e5fba4ada168a30 Reviewed-by: Tatiana Borisova --- src/corelib/tools/qpoint.cpp | 27 ++++++++++----- src/corelib/tools/qpoint.h | 21 +++++++----- .../auto/corelib/tools/qpoint/CMakeLists.txt | 2 ++ .../auto/corelib/tools/qpoint/tst_qpoint.cpp | 17 +++++++--- .../auto/corelib/tools/qpointf/CMakeLists.txt | 2 ++ .../corelib/tools/qpointf/tst_qpointf.cpp | 33 ++++++++++++------- 6 files changed, 68 insertions(+), 34 deletions(-) diff --git a/src/corelib/tools/qpoint.cpp b/src/corelib/tools/qpoint.cpp index d1f3b12a68..ce7514cddd 100644 --- a/src/corelib/tools/qpoint.cpp +++ b/src/corelib/tools/qpoint.cpp @@ -15,6 +15,10 @@ QT_BEGIN_NAMESPACE \ingroup painting \reentrant + \compares equality + \compareswith equality QPointF + \endcompareswith + \brief The QPoint class defines a point in the plane using integer precision. @@ -208,16 +212,17 @@ QT_BEGIN_NAMESPACE */ /*! - \fn bool QPoint::operator==(const QPoint &p1, const QPoint &p2) + \fn bool QPoint::operator==(const QPoint &lhs, const QPoint &rhs) - Returns \c true if \a p1 and \a p2 are equal; otherwise returns - false. + Returns \c true if \a lhs and \a rhs are equal; otherwise returns + \c false. */ /*! - \fn bool QPoint::operator!=(const QPoint &p1, const QPoint &p2) + \fn bool QPoint::operator!=(const QPoint &lhs, const QPoint &rhs) - Returns \c true if \a p1 and \a p2 are not equal; otherwise returns \c false. + Returns \c true if \a lhs and \a rhs are not equal; otherwise returns + \c false. */ /*! @@ -463,6 +468,10 @@ size_t qHash(QPoint key, size_t seed) noexcept \ingroup painting \reentrant + \compares equality + \compareswith equality QPoint + \endcompareswith + \brief The QPointF class defines a point in the plane using floating point precision. @@ -730,9 +739,9 @@ size_t qHash(QPoint key, size_t seed) noexcept */ /*! - \fn bool QPointF::operator==(const QPointF &p1, const QPointF &p2) + \fn bool QPointF::operator==(const QPointF &lhs, const QPointF &rhs) - Returns \c true if \a p1 is approximately equal to \a p2; otherwise + Returns \c true if \a lhs is approximately equal to \a rhs; otherwise returns \c false. \warning This function does not check for strict equality; instead, @@ -742,9 +751,9 @@ size_t qHash(QPoint key, size_t seed) noexcept */ /*! - \fn bool QPointF::operator!=(const QPointF &p1, const QPointF &p2); + \fn bool QPointF::operator!=(const QPointF &lhs, const QPointF &rhs) - Returns \c true if \a p1 is sufficiently different from \a p2; + Returns \c true if \a lhs is sufficiently different from \a rhs; otherwise returns \c false. \warning This function does not check for strict inequality; instead, diff --git a/src/corelib/tools/qpoint.h b/src/corelib/tools/qpoint.h index 7df4d49005..2960033c6d 100644 --- a/src/corelib/tools/qpoint.h +++ b/src/corelib/tools/qpoint.h @@ -4,6 +4,7 @@ #ifndef QPOINT_H #define QPOINT_H +#include #include #include @@ -51,10 +52,10 @@ public: constexpr static inline int dotProduct(const QPoint &p1, const QPoint &p2) { return p1.xp * p2.xp + p1.yp * p2.yp; } - friend constexpr inline bool operator==(const QPoint &p1, const QPoint &p2) noexcept +private: + friend constexpr bool comparesEqual(const QPoint &p1, const QPoint &p2) noexcept { return p1.xp == p2.xp && p1.yp == p2.yp; } - friend constexpr inline bool operator!=(const QPoint &p1, const QPoint &p2) noexcept - { return p1.xp != p2.xp || p1.yp != p2.yp; } + Q_DECLARE_EQUALITY_COMPARABLE_LITERAL_TYPE(QPoint) friend constexpr inline QPoint operator+(const QPoint &p1, const QPoint &p2) noexcept { return QPoint(p1.xp + p2.xp, p1.yp + p2.yp); } friend constexpr inline QPoint operator-(const QPoint &p1, const QPoint &p2) noexcept @@ -78,6 +79,7 @@ public: friend constexpr inline QPoint operator/(const QPoint &p, qreal c) { return QPoint(qRound(p.xp / c), qRound(p.yp / c)); } +public: #if defined(Q_OS_DARWIN) || defined(Q_QDOC) [[nodiscard]] Q_CORE_EXPORT CGPoint toCGPoint() const noexcept; #endif @@ -241,19 +243,19 @@ public: return p1.xp * p2.xp + p1.yp * p2.yp; } +private: QT_WARNING_PUSH QT_WARNING_DISABLE_FLOAT_COMPARE - friend constexpr inline bool operator==(const QPointF &p1, const QPointF &p2) + friend constexpr bool comparesEqual(const QPointF &p1, const QPointF &p2) noexcept { return ((!p1.xp || !p2.xp) ? qFuzzyIsNull(p1.xp - p2.xp) : qFuzzyCompare(p1.xp, p2.xp)) && ((!p1.yp || !p2.yp) ? qFuzzyIsNull(p1.yp - p2.yp) : qFuzzyCompare(p1.yp, p2.yp)); } - friend constexpr inline bool operator!=(const QPointF &p1, const QPointF &p2) - { - return !(p1 == p2); - } QT_WARNING_POP - + Q_DECLARE_EQUALITY_COMPARABLE_LITERAL_TYPE(QPointF) + friend constexpr bool comparesEqual(const QPointF &p1, const QPoint &p2) noexcept + { return comparesEqual(p1, p2.toPointF()); } + Q_DECLARE_EQUALITY_COMPARABLE_LITERAL_TYPE(QPointF, QPoint) friend constexpr inline QPointF operator+(const QPointF &p1, const QPointF &p2) { return QPointF(p1.xp + p2.xp, p1.yp + p2.yp); } friend constexpr inline QPointF operator-(const QPointF &p1, const QPointF &p2) @@ -272,6 +274,7 @@ public: return QPointF(p.xp / divisor, p.yp / divisor); } +public: constexpr QPoint toPoint() const; #if defined(Q_OS_DARWIN) || defined(Q_QDOC) diff --git a/tests/auto/corelib/tools/qpoint/CMakeLists.txt b/tests/auto/corelib/tools/qpoint/CMakeLists.txt index f1402d8815..82ece1fc15 100644 --- a/tests/auto/corelib/tools/qpoint/CMakeLists.txt +++ b/tests/auto/corelib/tools/qpoint/CMakeLists.txt @@ -14,4 +14,6 @@ endif() qt_internal_add_test(tst_qpoint SOURCES tst_qpoint.cpp + LIBRARIES + Qt::TestPrivate ) diff --git a/tests/auto/corelib/tools/qpoint/tst_qpoint.cpp b/tests/auto/corelib/tools/qpoint/tst_qpoint.cpp index 7fea787131..4763c1bf07 100644 --- a/tests/auto/corelib/tools/qpoint/tst_qpoint.cpp +++ b/tests/auto/corelib/tools/qpoint/tst_qpoint.cpp @@ -5,6 +5,7 @@ #ifdef QVARIANT_H # error "This test requires qpoint.h to not include qvariant.h" #endif +#include // don't assume template @@ -71,6 +72,7 @@ private slots: void operator_unary_minus_data(); void operator_unary_minus(); + void operatorsCompile(); void operator_eq_data(); void operator_eq(); @@ -155,6 +157,8 @@ void tst_QPoint::toPointF() QFETCH(const QPointF, result); QCOMPARE(input.toPointF(), result); + // test also mixed-type comparison + QT_TEST_EQUALITY_OPS(input, result, true); } void tst_QPoint::transposed() @@ -350,6 +354,12 @@ void tst_QPoint::operator_unary_minus() QCOMPARE(-point, expected); } +void tst_QPoint::operatorsCompile() +{ + // Mixed-type comparison is tested in tst_QPointF. + QTestPrivate::testEqualityOperatorsCompile(); +} + void tst_QPoint::operator_eq_data() { QTest::addColumn("point1"); @@ -371,12 +381,9 @@ void tst_QPoint::operator_eq() QFETCH(QPoint, point2); QFETCH(bool, expectEqual); - bool equal = point1 == point2; - QCOMPARE(equal, expectEqual); - bool notEqual = point1 != point2; - QCOMPARE(notEqual, !expectEqual); + QT_TEST_EQUALITY_OPS(point1, point2, expectEqual); - if (equal) + if (expectEqual) QCOMPARE(qHash(point1), qHash(point2)); } diff --git a/tests/auto/corelib/tools/qpointf/CMakeLists.txt b/tests/auto/corelib/tools/qpointf/CMakeLists.txt index 16e5a9036a..28cbe185b2 100644 --- a/tests/auto/corelib/tools/qpointf/CMakeLists.txt +++ b/tests/auto/corelib/tools/qpointf/CMakeLists.txt @@ -14,4 +14,6 @@ endif() qt_internal_add_test(tst_qpointf SOURCES tst_qpointf.cpp + LIBRARIES + Qt::TestPrivate ) diff --git a/tests/auto/corelib/tools/qpointf/tst_qpointf.cpp b/tests/auto/corelib/tools/qpointf/tst_qpointf.cpp index 392c22c70a..e75dd9d5ab 100644 --- a/tests/auto/corelib/tools/qpointf/tst_qpointf.cpp +++ b/tests/auto/corelib/tools/qpointf/tst_qpointf.cpp @@ -5,6 +5,7 @@ #ifdef QVARIANT_H # error "This test requires qpoint.h to not include qvariant.h" #endif +#include // don't assume template @@ -71,6 +72,7 @@ private slots: void operator_unary_minus_data(); void operator_unary_minus(); + void operatorsCompile(); void operator_eq_data(); void operator_eq(); @@ -345,21 +347,29 @@ void tst_QPointF::operator_unary_minus() QCOMPARE(-point, expected); } +void tst_QPointF::operatorsCompile() +{ + QTestPrivate::testEqualityOperatorsCompile(); + QTestPrivate::testEqualityOperatorsCompile(); +} + void tst_QPointF::operator_eq_data() { QTest::addColumn("point1"); QTest::addColumn("point2"); QTest::addColumn("expectEqual"); + QTest::addColumn("expectIntEqual"); - QTest::newRow("(0, 0) == (0, 0)") << QPointF(0, 0) << QPointF(0, 0) << true; - QTest::newRow("(-1, 0) == (-1, 0)") << QPointF(-1, 0) << QPointF(-1, 0) << true; - QTest::newRow("(-1, 0) != (0, 0)") << QPointF(-1, 0) << QPointF(0, 0) << false; - QTest::newRow("(-1, 0) != (0, -1)") << QPointF(-1, 0) << QPointF(0, -1) << false; - QTest::newRow("(-1.125, 0.25) == (-1.125, 0.25)") << QPointF(-1.125, 0.25) << QPointF(-1.125, 0.25) << true; + QTest::newRow("(0, 0) == (0, 0)") << QPointF(0, 0) << QPointF(0, 0) << true << true; + QTest::newRow("(-1, 0) == (-1, 0)") << QPointF(-1, 0) << QPointF(-1, 0) << true << true; + QTest::newRow("(-1, 0) != (0, 0)") << QPointF(-1, 0) << QPointF(0, 0) << false << false; + QTest::newRow("(-1, 0) != (0, -1)") << QPointF(-1, 0) << QPointF(0, -1) << false << false; + QTest::newRow("(-1.125, 0.25) == (-1.125, 0.25)") + << QPointF(-1.125, 0.25) << QPointF(-1.125, 0.25) << true << false; QTest::newRow("(QREAL_MIN, QREAL_MIN) == (QREAL_MIN, QREAL_MIN)") - << QPointF(QREAL_MIN, QREAL_MIN) << QPointF(QREAL_MIN, QREAL_MIN) << true; + << QPointF(QREAL_MIN, QREAL_MIN) << QPointF(QREAL_MIN, QREAL_MIN) << true << true; QTest::newRow("(QREAL_MAX, QREAL_MAX) == (QREAL_MAX, QREAL_MAX)") - << QPointF(QREAL_MAX, QREAL_MAX) << QPointF(QREAL_MAX, QREAL_MAX) << true; + << QPointF(QREAL_MAX, QREAL_MAX) << QPointF(QREAL_MAX, QREAL_MAX) << true << false; } void tst_QPointF::operator_eq() @@ -367,11 +377,12 @@ void tst_QPointF::operator_eq() QFETCH(QPointF, point1); QFETCH(QPointF, point2); QFETCH(bool, expectEqual); + QFETCH(bool, expectIntEqual); - bool equal = point1 == point2; - QCOMPARE(equal, expectEqual); - bool notEqual = point1 != point2; - QCOMPARE(notEqual, !expectEqual); + QT_TEST_EQUALITY_OPS(point1, point2, expectEqual); + + const QPoint intPoint2 = point2.toPoint(); + QT_TEST_EQUALITY_OPS(point1, intPoint2, expectIntEqual); } void tst_QPointF::toPoint_data()