Compile without deprecated foreach
ClosedPublic

Authored by ahmadsamir on Oct 2 2019, 8:14 PM.

Details

Summary

src/core/*
src/urifilters/*
src/widgets/*

Test Plan

make && ctest

Diff Detail

Repository
R241 KIO
Branch
arcpatch-D24372 (branched from master)
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 17624
Build 17642: arc lint + arc unit
ahmadsamir created this revision.Oct 2 2019, 8:14 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptOct 2 2019, 8:14 PM
ahmadsamir requested review of this revision.Oct 2 2019, 8:14 PM
dfaure requested changes to this revision.Oct 8 2019, 5:17 AM
dfaure added inline comments.
src/core/job.cpp
181–182

You need a local const var to hold the result of the subjobs() method call.

(repeats)

src/core/ksslcertificatemanager.cpp
408–410

userList would be a better variable name

src/core/ktcpsocket.cpp
729

or just iterate over ciphers, which is already const

src/core/scheduler.cpp
214

qAsConst not needed, this method is const

299–301

I'm afraid that kill() ends up removing the job from waitingList. I would iterate over a copy to be safe.

379

Did you try enabling this to make sure your ported code compiles?

src/widgets/dropjob.cpp
270

qAsConst

329

qAsConst

361

qAsConst

src/widgets/kdirmodel.cpp
151

qAsConst not needed, method is const

src/widgets/kfileitemdelegate.cpp
233

not needed, method is const and informationList is a member

This revision now requires changes to proceed.Oct 8 2019, 5:17 AM
ahmadsamir updated this revision to Diff 67510.EditedOct 8 2019, 2:59 PM
ahmadsamir marked 7 inline comments as done.
  • Use more descriptive var names other than list2
  • qAsConst isn't needed if the method is const and the container is a member var
ahmadsamir added inline comments.Oct 8 2019, 11:59 PM
src/core/job.cpp
181–182

IIUC, subjobs() returns a const QList &, do we still need a local const var?
https://api.kde.org/frameworks/kcoreaddons/html/classKCompositeJob.html#aaec8d9b05c7c4194c5ba121d43f2997e

src/core/ktcpsocket.cpp
729

Yep.

src/core/scheduler.cpp
214

(... and m_runningJobs is a member var).

379

Yes, I did. (I, like everyone else, hate to be embarrassed, so I always make sure it builds and passes unittests whenever I change anything except maybe comments :)).

src/widgets/dropjob.cpp
270

m_urls is declared const in DropJobPrivate: https://cgit.kde.org/kio.git/tree/src/widgets/dropjob.cpp#n142

src/widgets/kfileitemdelegate.cpp
233

"member" is what made that concept finally click in my head; (I kept thinking calling begin() on a qt container won't call the const overload, but it will if the container is a member and the this pointer is a pointer to const).

dfaure accepted this revision.Oct 12 2019, 7:35 PM
dfaure added inline comments.
src/core/job.cpp
181–182

Well spotted. Unusual....

src/widgets/dropjob.cpp
270

Oh, interesting :-)

This revision is now accepted and ready to land.Oct 12 2019, 7:35 PM
ahmadsamir added a comment.EditedOct 12 2019, 9:04 PM

I had to fix one conflict when rebasing, and basically D24419 ate the foreach src/core/ksslcertificatemanager.cpp@line 128 (in D24419, not here). By returning the result of calling contains() on the container, looks neater :).

Edit: Thanks for looking at the patch :). Should I just land it?

dfaure accepted this revision.Oct 12 2019, 9:09 PM
This revision was automatically updated to reflect the committed changes.