[Development] How qAsConst and qExchange lead to qNN

[Volker Hilsheimer]

> On 4 Nov 2022, at 16:00, Marc Mutz via Development <development at qt-project.org> wrote:
> 
> Hi,
> 
> After getting my head washed by Volker, lemme provide background on 
> these two functions.

Thanks for the context, Marc!

> TL;DR: we created real maintenance and porting problems by not removing 
> stop-gap functionality in a timely fashion, qNN presented as a way to 
> ensure this won't happen again.
> 
> Both qAsConst and qExchange are 1:1 reimplementations of std 
> functionality we couldn't rely on back then (as_const is C++17 and 
> exchange is C++14), so they're exactly equivalent to the std versions. 
> Or were, when they were added.
> 
> Neither std::as_const nor qAsConst have changed, so the replacement is 
> easy: s/qAsConst/std::as_const/. It cannot not compile because qAsConst 
> comes from qglobal.h and that transitively includes <utility>. This is 
> what was intended from the get-go:


The open question is whether and when we should deprecate such a stop-gap 1:1 reimplementations of std functionality. How to deprecate is now well documented, but the wiki starts with the process of doing so once we concluded that we shall. It doesn’t give us any guidance yet on how to come to that conclusion.

As qAsConst has shown, it is of course useful and pragmatic for us to introduce certain functionality before the respective C++ standard is generally available to us. Let’s then assume - for the sake of argument - that we might be able to require a new standard 2 years after all generally available compilers fully implement it (C++20 has perhaps reached that point [1] if we ignore Apple clang’s limping behind on library features, so let’s say 2024). Then there’ll likely be a lot of code in Qt that is using our temporary implementation. Again, qAsConst is a good example.

[1] https://en.cppreference.com/w/cpp/compiler_support

The questions is now what we do. Replacing all qAsConst with std::as_const is mechanically straight forward, also thanks to the tooling that Marc and others have been building for that purpose. So as long as we follow the proposed rules, then making the change as such not the problem. And neither is reviewing - with well-tested tooling, reviewing every line of diff produced adds little value.

However, such a changes will touch a lot of code, across all repositories. And that introduces conflicts when cherry-picking changes back to stable branches (and it messes up the git blame history, which for one-liners is perhaps rarely an issue). If we consider the s/count/size, s/qAsConst/std::as_const, s/QStringLiteral/u””_s, … replacement activities, then each of them might not be a big issue, but the sum of lines changed by all of them together quickly makes this become a source of bother and discontent. To a certain degree, the argument is the same as for making coding-style fixes: we don’t, unless we touch the respective code anyway.

That’s not an entirely fair comparison, because we do have to maintain our Qt replacements for as long as they get used in Qt. With qAsConst, maintaining 4 lines of code would perhaps have been acceptable if it saved us that noise. With the 9 lines of qExchange, Marc has already pointed out that we have diverged from std. If we consider std::span or std::expected - we clearly don’t want to drag those implementations around with us for longer than absolutely necessary.

So, there probably can’t be a single rule that fits every situation. And removing something from everyone’s toolbox, like qAsConst, might anyway deserve creating a bit more awareness. Hence, a suggestion:


When it’s time to phase out one of our own qNN implementations, then

1) propose the change here first to raise awareness, and to give people time to ask questions and/or raise objections

Even if the people doing the work all agree, a lot of maintainers and contributors will still be impacted (at least by the tool being removed). The proposal should come with some data about how prevalent the usage of the relevant construct is in Qt. It makes a difference whether we’d have to touch a few dozen lines, or several hundred to remove all usage.

2) If possible, add a warning to the sanity bot so that no new usage is added

This is trivial in some cases, not so trivial in others. Rationale: For changes that impact a larger amount of code, there’ll be plenty of time between those changes getting merged, and the old Qt-implementation ultimately getting removed or fully deprecated (which we can’t/shouldn’t do while we still have usage in Qt itself). For example, we now have some qAsConst back in the qtbase code.


Whether we then do a bulk replacement in Qt, or whether we just stop using old stuff in new code and phase it out over time as we touch code (until here’s perhaps little enough left to make a bulk change), depends on the discussion. If we do make a bulk change, then making that change in stable branches to avoid cherry-picking conflicts would probably be ok as well (unless those branch can’t use the new C++ version yet).


Volker