Skip to content

Commit

Permalink
Fix partial_ordering::unordered != 0 comparison
Browse files Browse the repository at this point in the history 
The standard does not provide any details on implementing operator!=(),
because it relies on the fact that it will be synthesized by the
compiler.

Our C++17 implementation, however, has to provide operator!=(), and we
made the mistake of copy-pasting the implementation from other
operators.
However, the isOrdered() check does not make sense for operator!=(),
because an unordered value is never equal to literal zero.

Fix the implementation for both Qt::partial_ordering and legacy
QPartialOrdering.

Amends 405244f (for QPartialOrdering)
and bdd41f4 (for Qt::partial_ordering).

[ChangeLog][QtCore][QtCompare] Fixed a bug where
partial_ordering::unordered != 0 comparison produced an incorrect
result.

Fixes: QTBUG-127759
Pick-to: 6.8 6.7 6.5 6.2 5.15
Change-Id: I5008f72831c17dc7fa4ae181bfc8115198a691f0
Reviewed-by: Marc Mutz <[email protected]>
  • Loading branch information
@isolovev
isolovev committed Aug 12, 2024
1 parent fed1099 commit d39441a
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 22 deletions.
8 changes: 4 additions & 4 deletions src/corelib/global/qcompare.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ class partial_ordering

friend constexpr bool operator!=(partial_ordering lhs,
QtPrivate::CompareAgainstLiteralZero) noexcept
{ return lhs.isOrdered() && lhs.m_order != 0; }
{ return !lhs.isOrdered() || lhs.m_order != 0; }

friend constexpr bool operator< (partial_ordering lhs,
QtPrivate::CompareAgainstLiteralZero) noexcept
Expand All @@ -113,7 +113,7 @@ class partial_ordering

friend constexpr bool operator!=(QtPrivate::CompareAgainstLiteralZero,
partial_ordering rhs) noexcept
{ return rhs.isOrdered() && 0 != rhs.m_order; }
{ return !rhs.isOrdered() || 0 != rhs.m_order; }

friend constexpr bool operator< (QtPrivate::CompareAgainstLiteralZero,
partial_ordering rhs) noexcept
Expand Down Expand Up @@ -730,7 +730,7 @@ class QPartialOrdering

friend constexpr bool operator!=(QPartialOrdering lhs,
QtPrivate::CompareAgainstLiteralZero) noexcept
{ return lhs.isOrdered() && lhs.m_order != 0; }
{ return !lhs.isOrdered() || lhs.m_order != 0; }

friend constexpr bool operator< (QPartialOrdering lhs,
QtPrivate::CompareAgainstLiteralZero) noexcept
Expand All @@ -755,7 +755,7 @@ class QPartialOrdering

friend constexpr bool operator!=(QtPrivate::CompareAgainstLiteralZero,
QPartialOrdering rhs) noexcept
{ return rhs.isOrdered() && 0 != rhs.m_order; }
{ return !rhs.isOrdered() || 0 != rhs.m_order; }

friend constexpr bool operator< (QtPrivate::CompareAgainstLiteralZero,
QPartialOrdering rhs) noexcept
Expand Down
24 changes: 6 additions & 18 deletions tests/auto/corelib/global/qcompare/tst_qcompare.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,21 +53,21 @@ void tst_QCompare::legacyPartialOrdering()
static_assert(QPartialOrdering::Greater == QPartialOrdering::Greater);

static_assert(!is_eq (QPartialOrdering::Unordered));
static_assert(!is_neq (QPartialOrdering::Unordered));
static_assert( is_neq (QPartialOrdering::Unordered));
static_assert(!is_lt (QPartialOrdering::Unordered));
static_assert(!is_lteq(QPartialOrdering::Unordered));
static_assert(!is_gt (QPartialOrdering::Unordered));
static_assert(!is_gteq(QPartialOrdering::Unordered));

static_assert(!(QPartialOrdering::Unordered == 0));
static_assert(!(QPartialOrdering::Unordered != 0));
static_assert( (QPartialOrdering::Unordered != 0));
static_assert(!(QPartialOrdering::Unordered < 0));
static_assert(!(QPartialOrdering::Unordered <= 0));
static_assert(!(QPartialOrdering::Unordered > 0));
static_assert(!(QPartialOrdering::Unordered >= 0));

static_assert(!(0 == QPartialOrdering::Unordered));
static_assert(!(0 != QPartialOrdering::Unordered));
static_assert( (0 != QPartialOrdering::Unordered));
static_assert(!(0 < QPartialOrdering::Unordered));
static_assert(!(0 <= QPartialOrdering::Unordered));
static_assert(!(0 > QPartialOrdering::Unordered));
Expand Down Expand Up @@ -225,21 +225,21 @@ void tst_QCompare::partialOrdering()
static_assert(Qt::partial_ordering::greater == Qt::partial_ordering::greater);

static_assert(!is_eq (Qt::partial_ordering::unordered));
static_assert(!is_neq (Qt::partial_ordering::unordered));
static_assert( is_neq (Qt::partial_ordering::unordered));
static_assert(!is_lt (Qt::partial_ordering::unordered));
static_assert(!is_lteq(Qt::partial_ordering::unordered));
static_assert(!is_gt (Qt::partial_ordering::unordered));
static_assert(!is_gteq(Qt::partial_ordering::unordered));

static_assert(!(Qt::partial_ordering::unordered == 0));
static_assert(!(Qt::partial_ordering::unordered != 0));
static_assert( (Qt::partial_ordering::unordered != 0));
static_assert(!(Qt::partial_ordering::unordered < 0));
static_assert(!(Qt::partial_ordering::unordered <= 0));
static_assert(!(Qt::partial_ordering::unordered > 0));
static_assert(!(Qt::partial_ordering::unordered >= 0));

static_assert(!(0 == Qt::partial_ordering::unordered));
static_assert(!(0 != Qt::partial_ordering::unordered));
static_assert( (0 != Qt::partial_ordering::unordered));
static_assert(!(0 < Qt::partial_ordering::unordered));
static_assert(!(0 <= Qt::partial_ordering::unordered));
static_assert(!(0 > Qt::partial_ordering::unordered));
Expand Down Expand Up @@ -859,32 +859,20 @@ void tst_QCompare::unorderedNeqLiteralZero()
QVERIFY(0 != stdUnordered);
QVERIFY(is_neq(stdUnordered));

QEXPECT_FAIL("", "QTBUG-127759", Continue);
QCOMPARE_EQ(qtUnordered != 0, stdUnordered != 0);
QEXPECT_FAIL("", "QTBUG-127759", Continue);
QCOMPARE_EQ(0 != qtUnordered, 0 != stdUnordered);
QEXPECT_FAIL("", "QTBUG-127759", Continue);
QCOMPARE_EQ(is_neq(qtUnordered), is_neq(stdUnordered));

QEXPECT_FAIL("", "QTBUG-127759", Continue);
QCOMPARE_EQ(qtLegacyUnordered != 0, stdUnordered != 0);
QEXPECT_FAIL("", "QTBUG-127759", Continue);
QCOMPARE_EQ(0 != qtLegacyUnordered, 0 != stdUnordered);
QEXPECT_FAIL("", "QTBUG-127759", Continue);
QCOMPARE_EQ(is_neq(qtLegacyUnordered), is_neq(stdUnordered));
#endif
QEXPECT_FAIL("", "QTBUG-127759", Continue);
QVERIFY(qtUnordered != 0);
QEXPECT_FAIL("", "QTBUG-127759", Continue);
QVERIFY(0 != qtUnordered);
QEXPECT_FAIL("", "QTBUG-127759", Continue);
QVERIFY(is_neq(qtUnordered));

QEXPECT_FAIL("", "QTBUG-127759", Continue);
QVERIFY(qtLegacyUnordered != 0);
QEXPECT_FAIL("", "QTBUG-127759", Continue);
QVERIFY(0 != qtLegacyUnordered);
QEXPECT_FAIL("", "QTBUG-127759", Continue);
QVERIFY(is_neq(qtLegacyUnordered));
}

Expand Down

0 comments on commit d39441a

Please sign in to comment.