On Monday, 29 October 2018 04:43:09 PDT Olivier Goffart wrote: > > 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.
True, which is why we didn't deprecate, but did add qAsConst. > > 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/ I'd solve this the other way around, by making the macro: if (const auto &_container_ = (container); true) \ for (variable : _container_) This creates a scope-only const reference to the original container and then uses the ranged for on it. We should be able to depend on this for Qt 6.4 or something. > > I disagree. The optimisation problem is not solved. > > I'm ok with discouraging Q_FOREACH, but deprecating such a function need > more thinking. I'm with you. -- Thiago Macieira - thiago.macieira (AT) intel.com Software Architect - Intel Open Source Technology Center _______________________________________________ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development