Compile without Q_FOREACH
Needs RevisionPublic

Authored by tcanabrava on May 10 2019, 12:25 PM.

Details

Reviewers
hindenburg
sandsmark
mglb
Group Reviewers
Konsole

Diff Detail

Repository
R319 Konsole
Branch
removeForeach
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 11722
Build 11740: arc lint + arc unit
tcanabrava created this revision.May 10 2019, 12:25 PM
Restricted Application added a project: Konsole. · View Herald TranscriptMay 10 2019, 12:25 PM
Restricted Application added a subscriber: konsole-devel. · View Herald Transcript
tcanabrava requested review of this revision.May 10 2019, 12:25 PM

Thanks for doing this - I was going to eventually get around to it.

Note that this generates a lot of these warnings

warning: c++11 range-loop might detach Qt container (QList) [-Wclazy-range-loop]

I’ll look the warnings, none of them appear on Gcc. Should be a clazy
warning

Em sáb, 11 de mai de 2019 às 16:28, Kurt Hindenburg <
noreply@phabricator.kde.org> escreveu:

hindenburg added a comment. View Revision
https://phabricator.kde.org/D21123

Thanks for doing this - I was going to eventually get around to it.

Note that this generates a lot of these warnings

warning: c++11 range-loop might detach Qt container (QList)
[-Wclazy-range-loop]

*REPOSITORY*
R319 Konsole

*REVISION DETAIL*
https://phabricator.kde.org/D21123

*To: *tcanabrava, Konsole, hindenburg, sandsmark
*Cc: *konsole-devel, gennad, thsurrel, ngraham, maximilianocuria,
hindenburg

You'll have to throw in a bunch of qAsConst() to silence clazy and the code ends up less readable imho. So I actually like Q_FOREACH better if you want to make clazy shut up as well (but in general I think clazy is a bit too triggerhappy, readable code is more important than fast code, if it is too slow you bring out a profiler and fix where the problems are).

mglb requested changes to this revision.Jul 8 2019, 8:12 PM
mglb added a subscriber: mglb.

You can't turn QT_NO_FOREACH on without bumping KF5 version - some headers (e.g. kconfiggroup.h in KF5 Config 5.47) use Q_FOREACH.

@sandsmark: Q_FOREACH/foreach is "discouraged"/deprecated, we'll have to change it in the future anyway.

This revision now requires changes to proceed.Jul 8 2019, 8:12 PM