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

[Ivan Solovev]

> What I meant is that the product API of using the macros are the set of
> operators. The methods that those operators called are not API and users are
> not expected to use them in their code. In fact, if conflicting names are a
> problem, then we should uglify the names we're asking for in the macros by
> inserting a "q_" or "qt_" prefix.

I do not completely agree with that, see my arguments below.

> We still need *some* level of public API to produce ordering results because
> users will need those methods to produce their own comparison functions, such
> as composing on top of our string or date/time classes. If they want to also
> be compatible with C++17, then they can't use the spaceship, and instead they
> will need to call a named function of some sorts.

Right, totally agree with this part.

> If we are going to expose
> API, then said API must have proper names and thus must be "equals" and
> "compare".
>
> But again, these do not need to be the functions that the macros call
> directly. The macros may call adaptor functions with ugly names.

What I cannot understand is why we should duplicate the functions? It would
be much easier if we could use the same functions both for macros and the
user-specific code.

Basically, what you suggest is that for every pair of comparable Qt types
we would need to double the amount of work that we do - provide not only
the helper functions for the macros, but also the overload for some
public functions for the end-users.

Of course, we could try to simplify it. I already started to introduce
Qt::order() overloads for built-in types in [0].
So, we could probably provide a general case like this:

    template <typename L, typename R>
    auto order(const L &lhs, const R &rhs) noexcept
    {
        // or any other uglified name that we choose
        return qt_order(lhs, rhs);
    }

and just let the template fail if qt_order() is not defined for L and R.

With such approach, we could use qt_equals() and qt_compare() as helper
functions, and Qt::equals() and Qt::compare() as public API for the users.

But then, if we use qt_*() functions, their names will not clash with any
of the existing APIs, and we can just let the end-users use these
functions directly.

[0]: https://codereview.qt-project.org/c/qt/qtbase/+/478199

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

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: Wednesday, July 26, 2023 5:18 PM
To: development at qt-project.org
Cc: Ivan Solovev
Subject: Re: [Development] C++20 comparisons @ Qt (was: Re: C++20 @ Qt)

On Wednesday, 26 July 2023 02:58:51 PDT Ivan Solovev via Development wrote:
> > 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.
> We introduce new public macros, which means that users will be using them in
> their custom classes in order to achieve the same unified comparison
> behavior between C++17 and C++20.
> This means that the users will have to implement the helper functions
> required for these macros. I agree that for the equal()/equals() helper
> function, the op==() and op!=() are mostly usable.
> But what about the order()/compare() function? I assume that the users would
> NOT be able to use op<=>(), because they will write C++17-compatible code.
> And testing for Unordered in terms of C++17 relational operators looks like
> an unnecessarily large amount of code, especially if we already have
> written this code for them. So why not just let them use it?
> Of course, you can argue that they can use the public APIs directly (if they
> exist). But what if the user develops a generic class?

What I meant is that the product API of using the macros are the set of
operators. The methods that those operators called are not API and users are
not expected to use them in their code. In fact, if conflicting names are a
problem, then we should uglify the names we're asking for in the macros by
inserting a "q_" or "qt_" prefix.

This means C++17 users are able to use all operators to produce a boolean
result, but can't get a QXxxOrdering result with homogeneous API from the
macros.

We still need *some* level of public API to produce ordering results because
users will need those methods to produce their own comparison functions, such
as composing on top of our string or date/time classes. If they want to also
be compatible with C++17, then they can't use the spaceship, and instead they
will need to call a named function of some sorts. If we are going to expose
API, then said API must have proper names and thus must be "equals" and
"compare".

But again, these do not need to be the functions that the macros call
directly. The macros may call adaptor functions with ugly names.

> What we can do is to have both private/public static two-arg member and a
> hidden friend, which would simply call the static method. We will have to
> apply this hack only to the classes that already have public APIs with the
> clashing names. For other classes we can just use hidden friends directly.

I don't see the need for anything to be a hidden friend. We need a *name* that
compiles, that's all.

> > 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.
>
> Well, the idea was to avoid unnecessary operations, and basically optimize
> operator<=>() to simply call the helper function in C++20 case.

Agreed, but your argument does not refute my proposal. What I suggested is
still optimal for strong and weak ordering.

The only problem is partial ordering because QPartialOrdering::Unordered's
value does not match two of the three Standard Libraries' values (for Qt 7, we
can fix this now that we know what those values are). And even then, this only
applies if someone is actually storing that number or passing it through an
opaque ABI boundary. If instead everything is inline, the compiler's optimiser
should simply emit optimal code.

> But anyway, that's a nice approach, and I should consider using it in my
> patches. Specially if it allows us to "unblock" compare() as a name for the
> helper function.

--
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/20230731/07b7c12e/attachment-0001.htm>