Port Scene away from foreach
AbandonedPublic

Authored by zzag on Jan 13 2019, 3:29 PM.

Details

Reviewers
None
Group Reviewers
KWin
Summary

foreach is deprecated.

Diff Detail

Repository
R108 KWin
Branch
port-scene-away-from-foreach
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 7049
Build 7067: arc lint + arc unit
zzag created this revision.Jan 13 2019, 3:29 PM
Restricted Application added a project: KWin. · View Herald TranscriptJan 13 2019, 3:29 PM
Restricted Application added a subscriber: kwin. · View Herald Transcript
zzag requested review of this revision.Jan 13 2019, 3:29 PM

I wouldn't start porting away from foreach before Qt 6 has it removed. Quite simply I don't think it will be removed. foreach and extended for behave differently as the qAsConst things show. Using extended for on Qt container is extremely expensive (let's ignore the fact that we know that stuff, think of the average developer with hardly any Qt background).

So as long as it's not possible to swap foreach with the extended for I doubt Qt will remove it. What's worse is that we don't get deprecation warnings. So if Qt really would remove it, this would end up as a big surprise for many, many Qt users who never heard of this. Given that it's not straight forward to port, this would create a huge burden for migrating to Qt 6 which many teams would not be able to do. Just imagine asking for money to do three weeks of Q_FOREACH porting. No manager understands the need.

Qt won't harm itself in such a stupid way. I remember that I discussed this specific topic with Qt devs at an Akademy. Other KDE devs were very outspoken about it (e.g. Krita).

Due to the risk with porting to extended for loop and the introduced code ugliness (qAsConst) my recommendation would be to not touch those loops till it's actually removed and we know how Qt 6 containers are going to be operated in an extended for. And if it becomes necessary we have to port then.

zzag abandoned this revision.Jan 13 2019, 4:23 PM

Ok, fair enough. :-)