[Development] C++20 comparisons @ Qt (was: Re: C++20 @ Qt)

[Ivan Solovev]

> Then don't use that as the hidden friend. The public API requirements take
> precedence. So long as the problem is not insurmountable, we should make the
> effort to have a clean API because we'll be stuck with it for the next 10
> years.
>
> > The public methods, in turn, are actually an issue, because the compiler
> > will
> > try to use them instead of a defined hidden friend, even if the number of
> > arguments or their types do not match.
>
> And why is that a problem? So long as the proper overload exists, it'll be
> called. There's only a problem if the overloads are ambiguous.

The problem is that there might be no "proper" overload, but the compiler
would still try to use the public API methods, ignoring the hidden friend
with a correct signature. That's what I tried to illustrate with the links
from my previous email.

Also, we have inconsistency among our classes, as some of them use two-arg static
equals(), and others use one-arg non-static equals().

In my patch[0] I introduce several helper macros that use helper functions
to provide the actual implementation.
Since we are talking about "equal()/equals()", let's take
Q_DECLARE_EQUALITY_COMPARABLE as an example. Omitting all the details,
and supposing that we use "equals" as a helepr function name, it expands into
something like this:

    friend bool operator==(const MyType &lhs, const MyType &rhs) noexcept
    { return equals(lhs, rhs); }

As you can see, we selected the two-arg version of the helper function.
This will work without any additional changes for the classes like
QBluetoothDeviceInfo, which has a private static two-arg equals[1], but would
fail for classes like QLocale, because it has a one-arg non-static equals[2].

I could re-write the macro to use the one-arg version (which does not make
much sense for me), but that would just "reverse" the problem.

And in any case, none of the existing public APIs would be able to work with
the generic code. And I keep repeating that it's something that we want to
provide for our users as building blocks for implementing three-way-comparison
with C++17.

[0]: https://codereview.qt-project.org/c/qt/qtbase/+/475447
[1]: https://github.com/qt/qtconnectivity/blob/dev/src/bluetooth/qbluetoothdeviceinfo.h#L233
[2]: https://github.com/qt/qtbase/blob/dev/src/corelib/text/qlocale.h#L1125

> As I said: we should strive for the proper naming.
>
> This also applies to the ordering function be called "compare".

The problem with "compare" is the pre-existing QString::compare(), which
returns an int.
Again, let's consider an example macro from [0].
Let's take Q_DECLARE_STRONGLY_ORDERED, which expands into all 6 relational
operators under C++17. I'll consider only operator>() and operator<() here for simplicity:

    friend bool operator<(const MyType &lhs, const MyType &rhs) noexcept
    { return order(lhs, rhs) == QStrongOrdering::Less; }
    friend bool operator>(const MyType &lhs, const MyType &rhs) noexcept
    { return order(lhs, rhs) == QStrongOrdering::Greater; }

Here we expect that the order() function returns one of the Q*Ordering types.

I agree that it can be re-written to work with the compare() function
returning an int:

    friend bool operator<(const MyType &lhs, const MyType &rhs) noexcept
    { return compare(lhs, rhs) < 0; }
    friend bool operator>(const MyType &lhs, const MyType &rhs) noexcept
    { return compare(lhs, rhs) > 0; }

HOWEVER, that will work ONLY for C++17.

For C++20 we want the Q_DECLARE_STRONGLY_ORDERED marco to simply expand into
operator<=>():

    friend auto operator<=>(const MyType &lhs, const MyType &rhs) noexcept
    { return order(lhs, rhs); }

Obviously, we want the helper function to remain the same for both C++17 and C++20.
I don't see how we can re-use a compare() function returning an int here.

Best regards,

------------------------------

Ivan Solovev
Senior Software Engineer

The Qt Company GmbH
Erich-Thilo-Str. 10
12489 Berlin, Germany
ivan.solovev at qt.io
www.qt.io

Geschäftsführer: Mika Pälsi,
Juha Varelius, Jouni Lintunen
Sitz der Gesellschaft: Berlin,
Registergericht: Amtsgericht
Charlottenburg, HRB 144331 B

________________________________
From: Thiago Macieira
Sent: Tuesday, July 25, 2023 2:11 AM
To: development at qt-project.org
Cc: Ivan Solovev
Subject: Re: [Development] C++20 comparisons @ Qt (was: Re: C++20 @ Qt)

On Monday, 24 July 2023 05:34:58 PDT Ivan Solovev via Development wrote:
> > Why doesn't "equals" work?
> >
> > We've been over this in the code review: our current naming conventions
> > would dictate "equals" even for two arguments, as the most common
> > phrasing is "A equals B" instead of "A and B equal". When you compare
> > things, you always compare at a minimum two of them, so there being two
> > is implied by the act of comparing anyway. And I don't see why we can't
> > provide both static and non- static members, like QString::compare does
>
> IIUC, the main problem that Marc has in mind is that the existing class
> member equals() would conflict with the two-arg hidden friend equals(),
> which we would use as a helper for operator==() and operator!=().

Then don't use that as the hidden friend. The public API requirements take
precedence. So long as the problem is not insurmountable, we should make the
effort to have a clean API because we'll be stuck with it for the next 10
years.

> The public methods, in turn, are actually an issue, because the compiler
> will
> try to use them instead of a defined hidden friend, even if the number of
> arguments or their types do not match.

And why is that a problem? So long as the proper overload exists, it'll be
called. There's only a problem if the overloads are ambiguous.

> Probably this approach would allow us to use "equals()" as a name for the E
> function.
> What do you think?

As I said: we should strive for the proper naming.

This also applies to the ordering function be called "compare".

--
Thiago Macieira - thiago.macieira (AT) intel.com
  Cloud Software Architect - Intel DCAI Cloud Engineering
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.qt-project.org/pipermail/development/attachments/20230725/05e845fd/attachment-0001.htm>