Tested kwin_x11 a bit, seems to work.
Details
Diff Detail
- Repository
- R108 KWin
- Lint
Lint Skipped - Unit
Unit Tests Skipped
The hard part of moving to the new for syntax isn't changing the code.
The hard part is spotting where there's a behavioural difference that could come from any code inside the loop (including potentially called functions) between for and foreach.
composite.cpp | ||
---|---|---|
725–727 | No!!!!! This is exactly why a blind port of all for loops is very dangerous. foreach takes an implicit copy of the list when it starts. windows.removeAll(t) would remove from the original list, but the one we're iterrating over gets detached and therefore unaffected. This new code will either miss some windows or crash violently, especially as you've const casted it. |
In fairness, I don't see how foreach will do much better with removeAll in the middle, this one should be ported to iterators anyway.
In fairness, I don't see how foreach will do much better with removeAll in the middle
Then please don't port any foreach loops till you do.
Qt automatically takes a copy of the container when it enters a foreach loop. If you modify the container as you are iterating, that won't affect the loop. (If you do not modify the container, the copy still takes place, but thanks to implicit sharing copying a container is very fast.)
In fairness, I don't see how foreach will do much better with removeAll in the middle, this one should be ported to iterators anyway.
Q_FOREACH makes an implicitly shared copy. The moment you do sth. destructive on the original list, the iterated list becomes a deep copy and remains unmodified. Operating on iterators is usually™ more performant (you spare the deep copy but need to maintain the iterator consistency) but at least this on is a very rare occasion for an actual requirement to alter the list.
Just for sale of completeness: I'm against changes like this one as we gain nothing but introduce a huge risk of breakage.