D24372: Compile without deprecated foreach

David Faure noreply at phabricator.kde.org
Tue Oct 8 06:17:13 BST 2019


dfaure requested changes to this revision.
dfaure added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> job.cpp:181
>      // kill all subjobs, without triggering their result slot
> -    Q_FOREACH (KJob *it, subjobs()) {
> -        it->kill(KJob::Quietly);
> +    for (KJob *job : subjobs()) {
> +        job->kill(KJob::Quietly);

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

(repeats)

> ksslcertificatemanager.cpp:380
>  
> -    foreach (const QSslCertificate &cert, QSslCertificate::fromPath(userCertDir + QLatin1Char('*'),
> -             QSsl::Pem, QRegExp::Wildcard)) {
> +    const QList<QSslCertificate> list2 = QSslCertificate::fromPath(userCertDir + QLatin1Char('*'), QSsl::Pem,
> +                                                                   QRegExp::Wildcard);

`userList` would be a better variable name

> ktcpsocket.cpp:729
>      cl.reserve(d->ciphers.size());
> -    foreach (const KSslCipher &c, d->ciphers) {
> +    for (const KSslCipher &c : qAsConst(d->ciphers)) {
>          cl.append(d->ccc.converted(c));

or just iterate over `ciphers`, which is already const

> scheduler.cpp:214
>      ret.reserve(m_runningJobs.size());
> -    Q_FOREACH (SimpleJob *job, m_runningJobs) {
> +    for (SimpleJob *job : qAsConst(m_runningJobs)) {
>          Slave *slave = jobSlave(job);

qAsConst not needed, this method is const

> scheduler.cpp:299
>      PerSlaveQueue &jobs = it.value();
> -    Q_FOREACH (SimpleJob *job, jobs.waitingList) {
> +    for (SimpleJob *job : qAsConst(jobs.waitingList)) {
>          // ### for compatibility with the old scheduler we don't touch the running job, if any.

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

> scheduler.cpp:377
>      Q_UNUSED(runningJobsCount);
>  #ifdef SCHEDULER_DEBUG
>      int realRunningJobsCount = 0;

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

> dropjob.cpp:270
>      bool containsTrashRoot = false;
> -    foreach (const QUrl &url, m_urls) {
> +    for (const QUrl &url : m_urls) {
>          const bool local = url.isLocalFile();

qAsConst

> dropjob.cpp:329
>      fileItems.reserve(m_urls.size());
> -    foreach (const QUrl &url, m_urls) {
> +    for (const QUrl &url : m_urls) {
>          fileItems.append(KFileItem(url));

qAsConst

> dropjob.cpp:361
>          bool equalDestination = true;
> -        foreach (const QUrl &src, m_urls) {
> +        for (const QUrl &src : m_urls) {
>              if (!m_destUrl.matches(src.adjusted(QUrl::RemoveFilename), QUrl::StripTrailingSlash)) {

qAsConst

> kdirmodel.cpp:151
>          urls.reserve(urls.size() + m_childNodes.size());
> -        Q_FOREACH (KDirModelNode *node, m_childNodes) {
> +        for (KDirModelNode *node : qAsConst(m_childNodes)) {
>              const KFileItem &item = node->item();

qAsConst not needed, method is const

> kfileitemdelegate.cpp:233
>  
> -    foreach (KFileItemDelegate::Information info, informationList) {
> +    for (KFileItemDelegate::Information info : qAsConst(informationList)) {
>          if (info == KFileItemDelegate::NoInformation) {

not needed, method is const and informationList is a member

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D24372

To: ahmadsamir, dfaure
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20191008/578a382a/attachment-0001.html>


More information about the Kde-frameworks-devel mailing list