[Development] How qAsConst and qExchange lead to qNN

Alex Blasche alexander.blasche at qt.io
Tue Nov 8 09:46:30 CET 2022



--
Alex

> -----Original Message-----
> From: Development <development-bounces at qt-project.org> On Behalf Of
> Volker Hilsheimer via Development
> Sent: Monday, 7 November 2022 16:51
> To: Marc Mutz <marc.mutz at qt.io>; development at qt-project.org
> Subject: Re: [Development] How qAsConst and qExchange lead to qNN
> 
> > 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.

I had to ask Volker which wiki he refers to and maybe it is the same for somebody else too. The URL is 

https://wiki.qt.io/Deprecation

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

I support this. In particular, the open communication before the fact is the key here. Let's face it, Qt is large enough that it cannot be expected that everybody knows what's going on in all the modules and such changes may never hit a developer's radar until after the merge and its enforcement.

I propose we add the gist of Volker's proposal to the deprecation wiki mentioned above.

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

Though Volker kind of implies that his two rules might lead to cherry-picking back into older releases, I would like to see this as explicitly mentioned option wherever we document the rules.

> From: Development <development-bounces at qt-project.org> On Behalf Of
> Marc Mutz via Development

>    #define Q_FOREACH(decl, expr) \
>       for (const auto _q_foreach_c = expr; decl : _q_foreach_c)
> 
> And I'd probably approve it, because then that thing can actually just be replaced
> with the expansion everywhere, and then, finally, be deleted.

Considering the above, this kind of change would have to be brought up to the mailing list and be discussed before any approval is given. 

--
Alex


More information about the Development mailing list