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

[Marc Mutz]

Hi,

Sorry for being absent this discussion for the last months, but getting 
6.6 in shape was more pressing than even fundamental 6.7 work.

Whenever you want to add an operation to _all_ types in the C++, 
built-in, as well as user-defined one, you obviously cannot use 
something that only works in classes: member functions don't work and 
neither do class-static functions. Simply because you can add neither to 
the type `int`, or the type `int[10]`.

The gold standard, crystallized from 30 years of C++ experience _and_ 30 
years of Qt experience, requires four things:

1. an ADL-able calling convention:   `using std::swap; swap(lhs, rhs);`
2. an implementation for built-in types: `std::swap`
3. an implementation for user-defined types: ADL `swap`
4. (optionally) a convenience wrapper: `std::ranges::swap()`, `qSwap`

In addition, experience tells us that we cannot afford to use the 
convenience wrapper outside generic code, because it's too slow to 
compile (cf. https://codereview.qt-project.org/q/topic:qSwap).

This gold standard has three variables: the names
a. `swap` for the implementation functions (2+3)
b. `std` as the namespace containing the impl for built-ins (2)
c. `std::ranges::swap`/`qSwap` as the name of the convenience wrapper

Within this framework, we're basically free to pick any names, though 
`Qt` is probably set as the name of the namespace containing the impl 
for built-in types (2).

My original proposal had, for equality, a = equal, b = Qt, c = qEquals, 
and for ordered types, a = order, b = Qt and c = qOrder (or qCompare, 
don't remember which).

Within the framework, I don't really care about the names.

The rest of the email is only interesting if you disagree with the 
algorithm laid out so far.

Thanks,
Marc

So, you disagree with the framework as-is. Each puzzle piece has it's 
place there, though, and I believe it's minimal, so you're free to poke 
holes into it, if you can. Here are a few rebuttals for replies I 
anticipate:

"We don't need (1), we can just always call std::swap".

This means all user-defined types would have to either specialize 
std::swap (only possible for concrete types, there's no partial 
specialization for function templates) or overload it (in namespace 
std). If we do that, it would work, but it wouldn't solve the problem of 
the pages of error messages thrown at users when they get it wrong. We 
need hidden friend for that, and hidden friend are only found via ADL 
and ADL is only active for non-qualified calls, and a name with `::` is 
a qualified name, and so ADL doesn't kick in.

"We can put swap() at the global namespace, then, and always call 
swap(lhs, rhs)."

This would require us to have a q/q_/qt_ prefix, but we might want that, 
anyway, so not really a problem there. But this has the same problem 
with 100s of overloads and the compiler printing them all on error. In 
addition, I _think_ (and the language lawyers can speak up whether I'm 
right or wrong), name lookup will go up the scopes, find (one of the) 
`swap`s at the global scope and stop there. Since it found something, it 
will not consider ADL.

If you have more, let's hear it.

Thanks,
Marc

On 17.08.23 19:41, Thiago Macieira wrote:
> On Monday, 14 August 2023 03:16:37 PDT Ivan Solovev via Development wrote:
>>> 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.
>>
>> The new macros are supposed to be publicly available. This means that the
>> users will be able to use them in their custom classes to implement the
>> unified comparison behavior between C++17 and C++20.
>>
>> This means that the users will need to provide their implementations of
>> the helper methods. The idea is to explicitly mention it in the docs,
>> so the methods cannot be considered "Qt internal" anymore.
>> Can we use the "q_" or "qt_" prefix for these helper functions in this case?
> 
> The methods aren't "Qt internal" but they they don't have to be API either for
> those libraries any more than they have to for us. They are used by library
> implementers (like us), so an uglier name is not a big deal.
> 
> But maybe we should opt for a name like qHash and the older qLess.
> 
>> Or do you suggest making the macros Qt-private? Not that we *really* can
>> do it, because the macros are exposed in public headers, but we can at least
>> hide the documentation, and do not advertise the macros...
>> But IMO such decision would significantly decrease the value of the new
>> feature.
> 
> I'm unsure. How widely used is this going to be even inside Qt? Have you found
> any use for it inside Qt Creator?
> 
> I can actually see this more used in the KF6 libraries than in Qt Creator
> because they are libraries (Qt Creator has some, but they are all internal
> with in-tree users). We may want to keep them documented with \internal for
> the first release and take a look if we find uses for it outside of Qt. That way
> we can also collect any issues with colliding symbol names and adapt them
> before having to guarantee source compatibility.
> 
>>> 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".
>>
>> The initial idea was that the helper functions could also become the public
>> API for producing ordering results. IIUC, that's also why Marc suggested to
>> make them hidden friends. Using private 2-arg static function would work for
>> the comparison operators but wouldn't work as a public API. Hidden friends
>> will work for both cases.
> 
> And I think it's a good idea for there to be a public function users can use
> in C++17 mode. But as I said, that requires them to have proper names. Whether
> they are the same functions that the macros call is up for debate. They don't
> have to be: one set can call the other without trouble, just with a bit of
> boilerplate.
> 
> The question to answer is whether we want to support users writing generic
> code in C++17 mode (i.e., without the spaceship). I don't particularly think
> it's necessary and thus don't think it compels us to make compromises in our
> API for this use-case. Third party code would either need to require C++20 or
> forego the ability to be generic. So like above: I think we need to know
> whether there's any real use out there of this.
> 
> If we do think we should provide a generic API, then I'd argue such API should
> be namespaced with a "q" prefix: qEquals and qCompare (note: we have
> QTest::qCompare, but I don't think there's any clash).
> 
>> If we advertise only the public API which is not supported by the macros,
>> then the users will still need to implement all six (or 12 in case of
>> mixed-type comparison) relational operators instead of just calling one
>> macro and implementing two helper functions.
> 
> 
-- 
Marc Mutz <marc.mutz at qt.io>
Principal Software Engineer

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

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