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

[Thiago Macieira]

On Tuesday, 25 July 2023 01:42:42 PDT Ivan Solovev via Development wrote:
> > 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.

Ah, I see. It's not an ambiguous overload problem because the hidden friend is 
not even part of the overload set in the first place. That can be solved by 
promoting the function from hidden friend to static member, either as public 
or private, depending on the implementer's choice.

That means it can't be used in ADL contexts without scope qualifications, but I 
don't see that as an API requirement. The API should be the operators.

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

That doesn't seem to be a problem. If we can't make the macros work with 
either, then we can simply add another equals to match the one we want, 
adapting to the one we don't. That is, we'd probably add a static equals that 
calls the non-static one if the latter is already public API and is in the 
ABI.

> 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].

As above: let's just add the static bool equals(two arg) to QLocale:

 private:
     QLocale(QLocalePrivate &dd);
     bool equals(const QLocale &other) const;
+    static bool equals(const QLocale &l1, const QLocale &l2)
+    { return l1.equals(l2); }
     friend class QLocalePrivate;
     friend class QSystemLocale;

The ABI for either function is the same and only differ in mangling. Since 
QLocale is not trivially copyable, we can't implement this with pass-by-value.

> 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.

I disagree with that requirement *because* it's costly. Let them use <=> if 
they want to use generic code.

> The problem with "compare" is the pre-existing QString::compare(), which
> returns an int.

That is not a problem. int is a suitable strong order result type.

> 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; }

I had posted a comment months ago that this definition was wrong, as you'd 
noticed:

>     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); }

You forgot the <=> 0 here. Comparing an int with 0 through the spaceship 
operator produces a std::strong_ordering result. And comparing a 
std::strong_ordering with 0 via the spaceship returns itself too.

https://gcc.godbolt.org/z/ebKe8Eb3a

This works for weak_ordering too. For partial_ordering, the case of unordered 
needs to be handled explicitly, so I'd instead require that the called 
function return either std::partial_ordering or QPartialOrdering, nothing 
else.

-- 
Thiago Macieira - thiago.macieira (AT) intel.com
  Cloud Software Architect - Intel DCAI Cloud Engineering
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 5152 bytes
Desc: not available
URL: <http://lists.qt-project.org/pipermail/development/attachments/20230725/1b2cbf73/attachment.bin>