On 10/28/18 8:17 PM, Thiago Macieira wrote:
On Sunday, 28 October 2018 11:49:08 PDT Olivier Goffart wrote:
It is a bit ironic that one reason given to deprecate Q_FOREACH is that it
may copy the container in some cases, while the alternative has the same
problem in much more common cases. (It is my impression that implicitly
shared container such as QList/QVector are by far much more used than the
one that are not within a typical Qt code base)
There are two problems with Q_FOREACH:
1) it will copy containers. For Qt containers, that's rather cheap (two atomic
refcount operations), but it's not free. And for Standard Library containers,
that is likely very expensive.
But using for(:) with a Qt container will cause a detach in the most common
case, so I'd say is even worse.
Which is why i'm saying using this reason is a bit ironic.
2) it's implemented by way of a for loop inside a for loop, which is known to
throw optimisers out, generating slightly worse code
I would consider that the missed optimization is quite small, if not negligible.
And it can be solved in C++17:
https://codereview.qt-project.org/243984/
Our rule already was: Don't use foreach in Qt code. (it was fine in examples)
Using iterators and now using the ranged for may need more typing, but
produces optimal code.
What could be done is to only deprecate partial specialization of
qMakeForeachContainer for QVarLenghtArray and the standard containers.
Or for containers that do not have a 'detach' function.
That way, users would get a warning for the problematic containers, but
would continue to work just fine with with the containers most Qt developer
use.
I disagree. The optimisation problem is not solved.
I'm ok with discouraging Q_FOREACH, but deprecating such a function need more
thinking.
Deprecating means you will force user to port all their codebase away from it,
which is a huge work. If the rationale is just that they will save a couple of
atomic operations, i do not think it is worth it.
Deprecating it only for non-shared container seems more logical, since we then
warn only when there is actually a problem.
And for the Qt shared container case, using foreach is less typing, but also
less error prone. (do i have to use qAsConst? or make a copy? or even return a
const object which even lead to more problems)
--
Olivier
Woboq - Qt services and support - https://woboq.com - https://code.woboq.org
_______________________________________________
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development
