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.m...@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 -- Development mailing list Development@qt-project.org https://lists.qt-project.org/listinfo/development
