Skip to content

Commit

Permalink
Fix QVariant/QMetaType::compare APIs
Browse files Browse the repository at this point in the history
std::optional<int> is the wrong datatype to use for compare.

First and foremost, it can't be used in the idiomatic form of

  auto r = a.compare(b);
  if (r < 0) ~~~ // a is less than b
  if (r > 0) ~~~ // a is greater than b

which we *already* feature in Qt (QString, QByteArray).

Also, std::optional<int> (explicitly) converts to bool, which is
a trap, because the result of the comparison can be accidentally
tested as a bool:

  if (a.compare(b)) ~~~ // oops! does NOT mean a<b

Not to mention extending this to algorithms:

  auto lessThan = [](QVariant a, QVariant b) { return a.compare(b); }; // oops!
  std::ranges::sort(vectorOfVariants, lessThan);

which thankfully doesn't compile as is -- std::optional has
an *explicit* operator bool, and the Compare concept requires an
implicit conversion. However, the error the user is going to face
will be "cannot convert to bool because the operator is explicit",
which is deceiving because the fix is NOT supposed to be:

  auto lessThan = [](QVariant a, QVariant b) { return (bool)a.compare(b); }; // big oops!

Instead: backport to Qt the required subset of C++20's <compare>
API, and use that. This commits just adds the necessary parts
for compare() (i.e. partial ordering), the rest of <compare>
(classes, functions, conversions) can be added to 6.1.

Change-Id: I2b5522da47854da39f79993e1207fad033786f00
Reviewed-by: Volker Hilsheimer <[email protected]>
(cherry picked from commit 3e59c97)
Reviewed-by: Qt Cherry-pick Bot <[email protected]>
  • Loading branch information
dangelog committed Nov 30, 2020
1 parent 81e893f commit 405244f
Show file tree
Hide file tree
Showing 14 changed files with 511 additions and 29 deletions.
1 change: 1 addition & 0 deletions src/corelib/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ qt_internal_add_module(Core
EXCEPTIONS
SOURCES
global/archdetect.cpp
global/qcompare.h
global/qcompilerdetection.h
global/qcontainerinfo.h
global/qendian.cpp global/qendian.h global/qendian_p.h
Expand Down
1 change: 1 addition & 0 deletions src/corelib/global/global.pri
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ HEADERS += \
global/qoperatingsystemversion.h \
global/qoperatingsystemversion_p.h \
global/qsystemdetection.h \
global/qcompare.h \
global/qcompilerdetection.h \
global/qcontainerinfo.h \
global/qprocessordetection.h \
Expand Down
143 changes: 143 additions & 0 deletions src/corelib/global/qcompare.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,143 @@
/****************************************************************************
**
** Copyright (C) 2020 Klarälvdalens Datakonsult AB, a KDAB Group company, [email protected], author Giuseppe D'Angelo <[email protected]>
** Contact: http://www.qt.io/licensing/
**
** This file is part of the QtCore module of the Qt Toolkit.
**
** $QT_BEGIN_LICENSE:LGPL$
** Commercial License Usage
** Licensees holding valid commercial Qt licenses may use this file in
** accordance with the commercial license agreement provided with the
** Software or, alternatively, in accordance with the terms contained in
** a written agreement between you and The Qt Company. For licensing terms
** and conditions see https://www.qt.io/terms-conditions. For further
** information use the contact form at https://www.qt.io/contact-us.
**
** GNU Lesser General Public License Usage
** Alternatively, this file may be used under the terms of the GNU Lesser
** General Public License version 3 as published by the Free Software
** Foundation and appearing in the file LICENSE.LGPL3 included in the
** packaging of this file. Please review the following information to
** ensure the GNU Lesser General Public License version 3 requirements
** will be met: https://www.gnu.org/licenses/lgpl-3.0.html.
**
** GNU General Public License Usage
** Alternatively, this file may be used under the terms of the GNU
** General Public License version 2.0 or (at your option) the GNU General
** Public license version 3 or any later version approved by the KDE Free
** Qt Foundation. The licenses are as published by the Free Software
** Foundation and appearing in the file LICENSE.GPL2 and LICENSE.GPL3
** included in the packaging of this file. Please review the following
** information to ensure the GNU General Public License requirements will
** be met: https://www.gnu.org/licenses/gpl-2.0.html and
** https://www.gnu.org/licenses/gpl-3.0.html.
**
** $QT_END_LICENSE$
**
****************************************************************************/

#ifndef QCOMPARE_H
#define QCOMPARE_H

#if 0
#pragma qt_class(QtCompare)
#endif

#include <QtCore/qglobal.h>

QT_BEGIN_NAMESPACE

namespace QtPrivate {
using CompareUnderlyingType = qint8;

// [cmp.categories.pre] / 1
enum class Ordering : CompareUnderlyingType
{
Equal = 0,
Equivalent = Equal,
Less = -1,
Greater = 1
};

enum class Uncomparable : CompareUnderlyingType
{
Unordered = -127
};

// [cmp.categories.pre] / 3, but using a safe bool trick
// and also rejecting std::nullptr_t (unlike the example)
class CompareAgainstLiteralZero {
public:
using SafeZero = void (CompareAgainstLiteralZero::*)();
Q_IMPLICIT constexpr CompareAgainstLiteralZero(SafeZero) noexcept {}

template <typename T, std::enable_if_t<!std::is_same_v<T, int>, bool> = false>
CompareAgainstLiteralZero(T) = delete;
};
} // namespace QtPrivate

// [cmp.partialord]
class QPartialOrdering
{
public:
static const QPartialOrdering Less;
static const QPartialOrdering Equivalent;
static const QPartialOrdering Greater;
static const QPartialOrdering Unordered;

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

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

friend constexpr bool operator==(QPartialOrdering p1, QPartialOrdering p2) noexcept
{ return p1.m_order == p2.m_order; }
friend constexpr bool operator!=(QPartialOrdering p1, QPartialOrdering p2) noexcept
{ return p1.m_order != p2.m_order; }

private:
constexpr explicit QPartialOrdering(QtPrivate::Ordering order) noexcept
: m_order(static_cast<QtPrivate::CompareUnderlyingType>(order))
{}
constexpr explicit QPartialOrdering(QtPrivate::Uncomparable order) noexcept
: m_order(static_cast<QtPrivate::CompareUnderlyingType>(order))
{}

// instead of the exposition only is_ordered member in [cmp.partialord],
// use a private function
constexpr bool isOrdered() noexcept
{ return m_order != static_cast<QtPrivate::CompareUnderlyingType>(QtPrivate::Uncomparable::Unordered); }

QtPrivate::CompareUnderlyingType m_order;
};

inline constexpr QPartialOrdering QPartialOrdering::Less(QtPrivate::Ordering::Less);
inline constexpr QPartialOrdering QPartialOrdering::Equivalent(QtPrivate::Ordering::Equivalent);
inline constexpr QPartialOrdering QPartialOrdering::Greater(QtPrivate::Ordering::Greater);
inline constexpr QPartialOrdering QPartialOrdering::Unordered(QtPrivate::Uncomparable::Unordered);

QT_END_NAMESPACE

#endif // QCOMPARE_H
147 changes: 147 additions & 0 deletions src/corelib/global/qcompare.qdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,147 @@
/****************************************************************************
**
** Copyright (C) 2020 Klarälvdalens Datakonsult AB, a KDAB Group company, [email protected], author Giuseppe D'Angelo <[email protected]>
** Contact: http://www.qt.io/licensing/
**
** This file is part of the documentation of the Qt Toolkit.
**
** $QT_BEGIN_LICENSE:FDL$
** Commercial License Usage
** Licensees holding valid commercial Qt licenses may use this file in
** accordance with the commercial license agreement provided with the
** Software or, alternatively, in accordance with the terms contained in
** a written agreement between you and The Qt Company. For licensing terms
** and conditions see https://www.qt.io/terms-conditions. For further
** information use the contact form at https://www.qt.io/contact-us.
**
** GNU Free Documentation License Usage
** Alternatively, this file may be used under the terms of the GNU Free
** Documentation License version 1.3 as published by the Free Software
** Foundation and appearing in the file included in the packaging of
** this file. Please review the following information to ensure
** the GNU Free Documentation License version 1.3 requirements
** will be met: https://www.gnu.org/licenses/fdl-1.3.html.
** $QT_END_LICENSE$
**
****************************************************************************/

/*!
\class QPartialOrdering
\inmodule QtCore
\brief QPartialOrdering represents the result of a comparison that allows for unordered results.
\since 6.0

A value of type QPartialOrdering is typically returned from a
three-way comparison function. Such a function compares two
objects, and it may either establish that the two objects are
ordered relative to each other, or that they are not ordered. The
QPartialOrdering value returned from the comparison function
represents one of those possibilities.

The possible values of type QPartialOrdering are, in fact, fully
represented by the following four static values:

\list

\li \c QPartialOrdering::Less represents that the first object is
less than the second;

\li \c QPartialOrdering::Equivalent represents that the first
object is equivalent to the second;

\li \c QPartialOrdering::Greater represents that the first object
is equivalent to the second;

\li \c QPartialOrdering::Unordered represents that the first object
is \e{not ordered} with respect to the second.

\endlist

QPartialOrdering is idiomatically used by comparing an instance
against a literal zero, for instance like this:

\code

// given a, b, c, d as objects of some type that allows for a 3-way compare

QPartialOrdering result = a.compare(b);
if (result < 0) {
// a is less than b
}

if (c.compare(d) >= 0) {
// c is greater than or equal to d
}

\endcode

A QPartialOrdering value which represents an unordered result will
always return false when compared against literal 0.
*/

/*!
\fn bool operator==(QPartialOrdering p1, QPartialOrdering p2) noexcept
\relates QPartialOrdering

Return true if \a p1 and \a p2 represent the same result;
otherwise, returns false.
*/

/*!
\fn bool operator!=(QPartialOrdering p1, QPartialOrdering p2) noexcept
\relates QPartialOrdering

Return true if \a p1 and \a p2 represent different results;
otherwise, returns true.
*/

/*!
\fn bool operator==(QPartialOrdering p, QtPrivate::CompareAgainstLiteralZero) noexcept
\fn bool operator!=(QPartialOrdering p, QtPrivate::CompareAgainstLiteralZero) noexcept
\fn bool operator< (QPartialOrdering p, QtPrivate::CompareAgainstLiteralZero) noexcept
\fn bool operator<=(QPartialOrdering p, QtPrivate::CompareAgainstLiteralZero) noexcept
\fn bool operator> (QPartialOrdering p, QtPrivate::CompareAgainstLiteralZero) noexcept
\fn bool operator>=(QPartialOrdering p, QtPrivate::CompareAgainstLiteralZero) noexcept

\fn bool operator==(QtPrivate::CompareAgainstLiteralZero, QPartialOrdering p) noexcept
\fn bool operator!=(QtPrivate::CompareAgainstLiteralZero, QPartialOrdering p) noexcept
\fn bool operator< (QtPrivate::CompareAgainstLiteralZero, QPartialOrdering p) noexcept
\fn bool operator<=(QtPrivate::CompareAgainstLiteralZero, QPartialOrdering p) noexcept
\fn bool operator> (QtPrivate::CompareAgainstLiteralZero, QPartialOrdering p) noexcept
\fn bool operator>=(QtPrivate::CompareAgainstLiteralZero, QPartialOrdering p) noexcept
\relates QPartialOrdering
\internal
*/

/*!
\value QPartialOrdering::Less
\relates QPartialOrdering

Represents the result of a comparison where the value on the left
hand side is less than the value on right hand side.
*/

/*!
\value QPartialOrdering::Equivalent
\relates QPartialOrdering

Represents the result of a comparison where the value on the left
hand side is equivalent to the value on right hand side.
*/

/*!
\value QPartialOrdering::Greater
\relates QPartialOrdering

Represents the result of a comparison where the value on the left
hand side is greater than the value on right hand side.
*/

/*!
\value QPartialOrdering::Unordered
\relates QPartialOrdering

Represents the result of a comparison where the value on the left
hand side is not ordered with respect to the value on right hand
side.
*/
36 changes: 24 additions & 12 deletions src/corelib/kernel/qmetatype.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -666,12 +666,24 @@ void QMetaType::destruct(void *data) const
}
}

static QPartialOrdering threeWayCompare(const void *ptr1, const void *ptr2)
{
std::less<const void *> less;
if (less(ptr1, ptr2))
return QPartialOrdering::Less;
if (less(ptr2, ptr1))
return QPartialOrdering::Greater;
return QPartialOrdering::Equivalent;
}

/*!
Compares the objects at \a lhs and \a rhs for ordering.
Returns an empty optional if comparison is not supported or the values are unordered.
Otherwise, returns -1, 0 or +1 according as \a lhs is less than, equal to or greater
than \a rhs.
Returns QPartialOrdering::Unordered if comparison is not supported
or the values are unordered. Otherwise, returns
QPartialOrdering::Less, QPartialOrdering::Equivalent or
QPartialOrdering::Greater if \a lhs is less than, equivalent
to or greater than \a rhs, respectively.
Both objects must be of the type described by this metatype. If either \a lhs
or \a rhs is \nullptr, the values are unordered. Comparison is only supported
Expand All @@ -690,24 +702,24 @@ void QMetaType::destruct(void *data) const
\since 6.0
\sa equals(), isOrdered()
*/
std::optional<int> QMetaType::compare(const void *lhs, const void *rhs) const
QPartialOrdering QMetaType::compare(const void *lhs, const void *rhs) const
{
if (!lhs || !rhs)
return std::optional<int>{};
return QPartialOrdering::Unordered;
if (d_ptr->flags & QMetaType::IsPointer)
return std::less<const void *>()(*reinterpret_cast<const void * const *>(lhs),
*reinterpret_cast<const void * const *>(rhs));
return threeWayCompare(*reinterpret_cast<const void * const *>(lhs),
*reinterpret_cast<const void * const *>(rhs));
if (d_ptr && d_ptr->lessThan) {
if (d_ptr->equals && d_ptr->equals(d_ptr, lhs, rhs))
return 0;
return QPartialOrdering::Equivalent;
if (d_ptr->lessThan(d_ptr, lhs, rhs))
return -1;
return QPartialOrdering::Less;
if (d_ptr->lessThan(d_ptr, rhs, lhs))
return 1;
return QPartialOrdering::Greater;
if (!d_ptr->equals)
return 0;
return QPartialOrdering::Equivalent;
}
return std::optional<int>{};
return QPartialOrdering::Unordered;
}

/*!
Expand Down
Loading

0 comments on commit 405244f

Please sign in to comment.