[Development] Q_FOREACH, again

[Marc Mutz]

Forgot to add sample commits for each of the cases. Added inline below.

Also, please use topic:Q_FOREACH in Gerrit.

Module maintainers: if your module's .cmake.conf doesn't already contain the QT_NO_FOREACH=1, and no Gerrit change is up that adds it, either add it yourself to check locally or wait for the default to switch. You can use https://codereview.qt-project.org/c/qt/qtwebengine/+/494757 as a template for whitelisting, should that be necessary. Prefer whitelisting over fixing for now (see bottom of my thread starter email). The use of NO_PCH_SOURCES in tests requires https://codereview.qt-project.org/q/I8ead238a8d9e55da632b2929638b67724a42d73c (the absense of which in qt5.git  is why most module top-level changes aren't merged, yet). If you don't have it, you can use NO_UNITY_BUILD_SOURCES instead.

On 14.08.23 19:32, Marc Mutz via Development wrote:
> There's _one_ situation which I call "trivial": When the loop is over a
> _const_ local container (local _value_, const-references or data members
> do _not_ count). For me, it's ok to say in the commit message that these
> are of the trivial kind (but _say_ it in the commit message), it's
> trivially(!) verified by a reviewer that it's correct.

https://codereview.qt-project.org/c/qt/qtbase/+/495076

> There's also what I call a "simple" case: where you iterate over a local
> container that's _not_ const, but where the loop body _clearly_ doesn't
> modify the container. That must be _clearly_ visible. If it calls an
> out-of-line function, that should already spark an explanation in the
> commit message (unless it's something every Qt developer should know,
> like QString::toUtf8() doesn't call unknown code).

https://codereview.qt-project.org/c/qt/qtbase/+/495113

> If it's not one the two, you should do two things:
> 
> - port only one loop (or at most a group of loops over the same
>     variable) per commit. This allows us to quickly revert such a change
>     if it goes south, or even find it, with git-bisect, in the first
>     place.
> - _proove_ that the loop body cannot modify the container. Write the
>     proof in the _commit message_. Watch out for signal emissions,
>     callbacks, events being sent, etc under the hood that can cause
>     unknown code to be called. If you find something like this, you _must_
>     take a copy and iterate over that, or use an indexed loop. Unless the
>     modification is obvious, add a _code comment_ that explains why we
>     take a copy/use an indexed loop.

https://codereview.qt-project.org/c/qt/qtbase/+/495111

> If you can't proove it, don't do the change, or take a copy and say "//
> ### needed?". Just don't do that _everywhere_.

https://codereview.qt-project.org/c/qt/qtbase/+/495080

-- 
Marc Mutz <marc.mutz at qt.io>
Principal Software Engineer

The Qt Company
Erich-Thilo-Str. 10 12489
Berlin, Germany
www.qt.io

Geschäftsführer: Mika Pälsi, Juha Varelius, Jouni Lintunen
Sitz der Gesellschaft: Berlin,
Registergericht: Amtsgericht Charlottenburg,
HRB 144331 B