[Development] RFC: more liberal 'auto' rules?

Marc Mutz marc.mutz at kdab.com
Tue Dec 22 17:08:52 CET 2015


On Tuesday 22 December 2015 15:40:01 Matthew Woehlke wrote:
> On 2015-12-22 09:50, Marc Mutz wrote:
> >     foreach(const QString &s : someLongExpression()->fooBar().names())
> >     
> >     const auto strings = someLongExpression()->fooBar().names();
> >     for (const QString &s : strings)
> 
> On that note, is there any case in which Q_FOREACH is superior to a
> range-based for? (Besides the obvious one where the original list is
> modified from within the loop?)

No. range-for and for (auto it = begin(), end = end(); it != end; ++it) are 
equivalent. indexed access is less efficient (both from text size pov as well as 
address generation, where e.g. Haswell, where offsetting into L1 has latency 4 
whereas going though a pointer has latency 3), and foreach, due to the copy, 
is always much, much worse. Even when the RHS is an rvalue and you do the 
replacement you quoted above, the range-for is more efficient, probably 
Q_FOREACH forces another copy while assigning to const-auto uses (N)RVO.

Even in the case where the original list is modified in the loop, taking a copy 
and then using range-for may be more efficient. It is surely more readable, 
because you can (and should) comment on why the copy is necessary. That said, 
such loops as you describe will usually have very poor performance. Either due 
to the detach() that necessarily happens, or due to quadratic behaviour, e.g. 
calling erase(it) on a vector or QList while iterating.

Try to port all your Q_FOREACH to range-for loops. You will be surprised how 
much digging is required if you don't always want to take the hit of taking a 
copy.

In a change of mine, I estimated that by removing all Q_FOREACH uses in 
qtbase/src (all 880+ of them) would save 80KiB in text size across the qtbase 
libraries. I now think that it will be more, because QtCore is compiled with 
exceptions enabled, and then Q_FOREACH is really, really, horrendous. I posted 
side-by-side asm here: https://codereview.qt-project.org/112431

Thiago claims to have fixed that particular code explosion, but I think that 
the correct fix is to deprecate and port all Q_FOREACH/foreach to range-for.

Thanks,
Marc

-- 
Marc Mutz <marc.mutz at kdab.com> | Senior Software Engineer
KDAB (Deutschland) GmbH & Co.KG, a KDAB Group Company
Tel: +49-30-521325470
KDAB - The Qt Experts



More information about the Development mailing list