Hi,

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

One more question here - do we want our users to be able to do (1), or
is it enough to expose only (4) to them?
The reason for my question is (again) naming, see below.

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

I was about to suggest a = `qt_equals`, b = `Qt`, c = `qEquals`, but that
would result in ugly `Qt::qt_equals()` calls.

But if we say that the users should only use `qEquals()` and `qCompare` in
their code, then we can use `QtPrivate` as the namespace​:
a = `qt_equals`,    b = `QtPrivate`, c = `qEquals`     for equality, and
a = `qt_compare`, b = `QtPrivate`, c = `qCompare` for ordering.

AFAIK, the most critical drawback of qSwap() was the compilation speed
when having it in every value class, so I would not expect Qt itself to use
`qEquals` and `qCompare` everywhere.
But would it be ok for the user code?

What do you think?

Other idea that came to my mind while writing this - just do not provide (4),
since it has known issues (at least with compilation speed). Then we can
use
a = `qEquals`,     b = `Qt` for equality, and
a = `qCompare`, b = `Qt` for ordering.
We already have `QTest::qCompare()`, so `Qt::qCompare` would not be
a much worse naming.

Anyway, it would be great to come to some kind of consensus about the
naming of the APIs, and move on to the actual implementation.

Best regards,
Ivan

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

Ivan Solovev
Senior Software Engineer

The Qt Company GmbH
Erich-Thilo-Str. 10
12489 Berlin, Germany
ivan.solo...@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: Development <development-boun...@qt-project.org> on behalf of Marc Mutz 
via Development <development@qt-project.org>
Sent: Thursday, September 14, 2023 12:47 PM
To: development@qt-project.org <development@qt-project.org>
Subject: Re: [Development] C++20 comparisons @ Qt (was: Re: C++20 @ Qt)

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

--
Development mailing list
Development@qt-project.org
https://lists.qt-project.org/listinfo/development
-- 
Development mailing list
Development@qt-project.org
https://lists.qt-project.org/listinfo/development

Reply via email to