From 5ac64ab2cdc1b88d74dc9164166627650121953e Mon Sep 17 00:00:00 2001 From: Berthold Stoeger Date: Sat, 16 Mar 2024 16:50:43 +0100 Subject: [PATCH] cleanup: replace Q_FOREACH and foreach by range base for Q_FOREACH and foreach are anachronisms. Range based for may cause a performance regression: it can lead to a copy of shared containers (one reason why Qt's COW containers are broken). However, as long as there is no user noticeable delay, there is no point in analyzing each case. And also no point in slapping an 'asConst' on every container that is looped over. Signed-off-by: Berthold Stoeger --- .clang-format | 2 +- core/downloadfromdcthread.cpp | 6 +++--- core/parse-gpx.cpp | 2 +- core/qt-ble.cpp | 17 ++++++++--------- core/qthelper.cpp | 2 +- desktop-widgets/divelistview.cpp | 4 ++-- desktop-widgets/divelogimportdialog.cpp | 4 ++-- desktop-widgets/groupedlineedit.cpp | 4 ++-- desktop-widgets/kmessagewidget.cpp | 7 +++---- desktop-widgets/mainwindow.cpp | 4 ++-- .../preferences/preferences_language.cpp | 2 +- desktop-widgets/templatelayout.cpp | 2 +- mobile-widgets/qmlmanager.cpp | 4 ++-- qt-models/maplocationmodel.cpp | 3 +-- qt-models/models.cpp | 2 +- scripts/whitespace.pl | 2 +- 16 files changed, 32 insertions(+), 35 deletions(-) diff --git a/.clang-format b/.clang-format index 662eb1d80..bd73416d4 100644 --- a/.clang-format +++ b/.clang-format @@ -10,7 +10,7 @@ ColumnLimit: 0 ConstructorInitializerAllOnOneLineOrOnePerLine: true ConstructorInitializerIndentWidth: 8 ContinuationIndentWidth: 8 -ForEachMacros: [ 'foreach', 'for_each_dc', 'for_each_relevant_dc', 'for_each_dive', 'for_each_line', 'Q_FOREACH' ] +ForEachMacros: [ 'for_each_dc', 'for_each_relevant_dc', 'for_each_dive', 'for_each_line' ] IndentFunctionDeclarationAfterType: false #personal taste, good for long methods IndentWidth: 8 MaxEmptyLinesToKeep: 2 diff --git a/core/downloadfromdcthread.cpp b/core/downloadfromdcthread.cpp index 7b5707372..0004b8b42 100644 --- a/core/downloadfromdcthread.cpp +++ b/core/downloadfromdcthread.cpp @@ -156,7 +156,7 @@ void fill_computer_list() descriptorLookup[QString(vendor).toLower() + QString(product).toLower()] = descriptor; } dc_iterator_free(iterator); - Q_FOREACH (QString vendor, vendorList) { + for (const QString &vendor: vendorList) { auto &l = productList[vendor]; std::sort(l.begin(), l.end()); } @@ -194,9 +194,9 @@ void show_computer_list() { unsigned int transportMask = get_supported_transports(NULL); qDebug() << "Supported dive computers:"; - Q_FOREACH (QString vendor, vendorList) { + for (const QString &vendor: vendorList) { QString msg = vendor + ": "; - Q_FOREACH (QString product, productList[vendor]) { + for (const QString &product: productList[vendor]) { dc_descriptor_t *descriptor = descriptorLookup[vendor.toLower() + product.toLower()]; unsigned int transport = dc_descriptor_get_transports(descriptor) & transportMask; QString transportString = getTransportString(transport); diff --git a/core/parse-gpx.cpp b/core/parse-gpx.cpp index bf93a3101..49471c7f3 100644 --- a/core/parse-gpx.cpp +++ b/core/parse-gpx.cpp @@ -47,7 +47,7 @@ int getCoordsFromGPXFile(struct dive_coords *coords, const QString &fileName) if (nameCmp(gpxReader, "trkpt") == 0) { trkpt_found = true; line++; - foreach (const QXmlStreamAttribute &attr, gpxReader.attributes()) { + for (const QXmlStreamAttribute &attr: gpxReader.attributes()) { if (attr.name().toString() == QLatin1String("lat")) lat = attr.value().toString().toDouble(); else if (attr.name().toString() == QLatin1String("lon")) diff --git a/core/qt-ble.cpp b/core/qt-ble.cpp index 6dbb9ab9f..abe1e7169 100644 --- a/core/qt-ble.cpp +++ b/core/qt-ble.cpp @@ -308,7 +308,7 @@ dc_status_t BLEObject::write(const void *data, size_t size, size_t *actual) } while (!receivedPackets.isEmpty()); } - foreach (const QLowEnergyCharacteristic &c, preferredService()->characteristics()) { + for (const QLowEnergyCharacteristic &c: preferredService()->characteristics()) { if (!is_write_characteristic(c)) continue; @@ -398,7 +398,7 @@ dc_status_t BLEObject::read(void *data, size_t size, size_t *actual) dc_status_t BLEObject::select_preferred_service(void) { // Wait for each service to finish discovering - foreach (const QLowEnergyService *s, services) { + for (const QLowEnergyService *s: services) { #if QT_VERSION >= QT_VERSION_CHECK(6, 0, 0) WAITFOR(s->state() != QLowEnergyService::RemoteServiceDiscovering, BLE_TIMEOUT); if (s->state() == QLowEnergyService::RemoteServiceDiscovering) @@ -410,19 +410,19 @@ dc_status_t BLEObject::select_preferred_service(void) } // Print out the services for debugging - foreach (const QLowEnergyService *s, services) { + for (const QLowEnergyService *s: services) { qDebug() << "Found service" << s->serviceUuid() << s->serviceName(); - foreach (const QLowEnergyCharacteristic &c, s->characteristics()) { + for (const QLowEnergyCharacteristic &c: s->characteristics()) { qDebug() << " c:" << c.uuid(); - foreach (const QLowEnergyDescriptor &d, c.descriptors()) + for (const QLowEnergyDescriptor &d: c.descriptors()) qDebug() << " d:" << d.uuid(); } } // Pick the preferred one - foreach (QLowEnergyService *s, services) { + for (QLowEnergyService *s: services) { #if QT_VERSION >= QT_VERSION_CHECK(6, 0, 0) if (s->state() != QLowEnergyService::RemoteServiceDiscovered) #else @@ -434,7 +434,7 @@ dc_status_t BLEObject::select_preferred_service(void) bool haswrite = false; QBluetoothUuid uuid = s->serviceUuid(); - foreach (const QLowEnergyCharacteristic &c, s->characteristics()) { + for (const QLowEnergyCharacteristic &c: s->characteristics()) { hasread |= is_read_characteristic(c); haswrite |= is_write_characteristic(c); } @@ -615,9 +615,8 @@ dc_status_t qt_ble_open(void **io, dc_context_t *, const char *devaddr, device_d // Finish discovering the services, then add all those services and discover their characteristics. ble->connect(controller, &QLowEnergyController::discoveryFinished, [=] { qDebug() << "finished service discovery, start discovering characteristics"; - foreach(QBluetoothUuid s, controller->services()) { + for (QBluetoothUuid s: controller->services()) ble->addService(s); - } }); #if QT_VERSION >= QT_VERSION_CHECK(6, 0, 0) ble->connect(controller, &QLowEnergyController::errorOccurred, [=](QLowEnergyController::Error newError) { diff --git a/core/qthelper.cpp b/core/qthelper.cpp index 6cf4fb345..9c4bd4b77 100644 --- a/core/qthelper.cpp +++ b/core/qthelper.cpp @@ -1176,7 +1176,7 @@ QStringList mediaExtensionFilters() QStringList imageExtensionFilters() { QStringList filters; - foreach (const QString &format, QImageReader::supportedImageFormats()) + for (QString format: QImageReader::supportedImageFormats()) filters.append("*." + format); return filters; } diff --git a/desktop-widgets/divelistview.cpp b/desktop-widgets/divelistview.cpp index 5d71df8b0..495f6f597 100644 --- a/desktop-widgets/divelistview.cpp +++ b/desktop-widgets/divelistview.cpp @@ -486,7 +486,7 @@ void DiveListView::selectionChanged(const QItemSelection &selected, const QItemS QItemSelection newSelected = selected.size() ? selected : selectionModel()->selection(); std::vector addToSelection, removeFromSelection; - Q_FOREACH (const QModelIndex &index, newDeselected.indexes()) { + for (const QModelIndex &index: newDeselected.indexes()) { if (index.column() != 0) continue; const QAbstractItemModel *model = index.model(); @@ -499,7 +499,7 @@ void DiveListView::selectionChanged(const QItemSelection &selected, const QItemS removeFromSelection.push_back(trip->dives.dives[i]); } } - Q_FOREACH (const QModelIndex &index, newSelected.indexes()) { + for (const QModelIndex &index: newSelected.indexes()) { if (index.column() != 0) continue; diff --git a/desktop-widgets/divelogimportdialog.cpp b/desktop-widgets/divelogimportdialog.cpp index cbfd323a7..2aedca101 100644 --- a/desktop-widgets/divelogimportdialog.cpp +++ b/desktop-widgets/divelogimportdialog.cpp @@ -495,7 +495,7 @@ void DiveLogImportDialog::loadFileContents(int value, whatChanged triggeredBy) firstLine = f.readLine().trimmed(); currColumns = firstLine.split(';'); - Q_FOREACH (QString columnText, currColumns) { + for (const QString &columnText: currColumns) { if (columnText == "Time") { headers.append("Sample time"); } else if (columnText == "Depth") { @@ -613,7 +613,7 @@ void DiveLogImportDialog::loadFileContents(int value, whatChanged triggeredBy) if (line.length() > 0) columns = line.split(separator); // now try and guess the columns - Q_FOREACH (QString columnText, currColumns) { + for (QString columnText: currColumns) { count++; /* * We have to skip the conversion of 2 to ₂ for APD Log diff --git a/desktop-widgets/groupedlineedit.cpp b/desktop-widgets/groupedlineedit.cpp index 0601d40ca..64e6b0a0f 100644 --- a/desktop-widgets/groupedlineedit.cpp +++ b/desktop-widgets/groupedlineedit.cpp @@ -99,7 +99,7 @@ void GroupedLineEdit::addColor(QColor color) QStringList GroupedLineEdit::getBlockStringList() { QStringList retList; - foreach (const Private::Block &block, d->blocks) + for (const Private::Block &block: d->blocks) retList.append(block.text); return retList; } @@ -179,7 +179,7 @@ void GroupedLineEdit::paintEvent(QPaintEvent *e) QVectorIterator i(d->colors); i.toFront(); - foreach (const Private::Block &block, d->blocks) { + for (const Private::Block &block: d->blocks) { qreal start_x = line.cursorToX(block.start, QTextLine::Leading); qreal end_x = line.cursorToX(block.end-1, QTextLine::Trailing); diff --git a/desktop-widgets/kmessagewidget.cpp b/desktop-widgets/kmessagewidget.cpp index c64635220..715a148de 100644 --- a/desktop-widgets/kmessagewidget.cpp +++ b/desktop-widgets/kmessagewidget.cpp @@ -111,7 +111,7 @@ void KMessageWidgetPrivate::createLayout() qDeleteAll(buttons); buttons.clear(); - Q_FOREACH (QAction *action, q->actions()) { + for (QAction *action: q->actions()) { QToolButton *button = new QToolButton(content); button->setDefaultAction(action); button->setToolButtonStyle(Qt::ToolButtonTextBesideIcon); @@ -131,7 +131,7 @@ void KMessageWidgetPrivate::createLayout() QHBoxLayout *buttonLayout = new QHBoxLayout; buttonLayout->addStretch(); - Q_FOREACH (QToolButton *button, buttons) { + for (QToolButton *button: buttons) { // For some reason, calling show() is necessary if wordwrap is true, // otherwise the buttons do not show up. It is not needed if // wordwrap is false. @@ -145,9 +145,8 @@ void KMessageWidgetPrivate::createLayout() layout->addWidget(iconLabel); layout->addWidget(textLabel); - Q_FOREACH (QToolButton *button, buttons) { + for (QToolButton *button: buttons) layout->addWidget(button); - } layout->addWidget(closeButton); }; diff --git a/desktop-widgets/mainwindow.cpp b/desktop-widgets/mainwindow.cpp index 1a3ab6c12..5e3a58a5f 100644 --- a/desktop-widgets/mainwindow.cpp +++ b/desktop-widgets/mainwindow.cpp @@ -379,7 +379,7 @@ void MainWindow::on_actionOpen_triggered() QStringList cleanFilenames; QRegularExpression reg(".*\\[[^]]+]\\.ssrf", QRegularExpression::CaseInsensitiveOption); - Q_FOREACH (QString filename, filenames) { + for (QString filename: filenames) { if (reg.match(filename).hasMatch()) filename.remove(QRegularExpression("\\.ssrf$", QRegularExpression::CaseInsensitiveOption)); cleanFilenames << filename; @@ -1071,7 +1071,7 @@ void MainWindow::loadRecentFiles() recentFiles.clear(); QSettings s; s.beginGroup("Recent_Files"); - foreach (const QString &key, s.childKeys()) { + for (const QString &key: s.childKeys()) { // TODO Sorting only correct up to 9 entries. Currently, only 4 used, so no problem. if (!key.startsWith("File_")) continue; diff --git a/desktop-widgets/preferences/preferences_language.cpp b/desktop-widgets/preferences/preferences_language.cpp index 84ee74180..80caaa686 100644 --- a/desktop-widgets/preferences/preferences_language.cpp +++ b/desktop-widgets/preferences/preferences_language.cpp @@ -27,7 +27,7 @@ PreferencesLanguage::PreferencesLanguage() : AbstractPreferencesWidget(tr("Langu dateFormatShortMap.insert("MM/dd/yyyy", "M/d/yy"); dateFormatShortMap.insert("dd.MM.yyyy", "d.M.yy"); dateFormatShortMap.insert("yyyy-MM-dd", "yy-M-d"); - foreach (QString format, dateFormatShortMap.keys()) + for (const QString &format: dateFormatShortMap.keys()) ui->dateFormatEntry->addItem(format); ui->dateFormatEntry->completer()->setCaseSensitivity(Qt::CaseSensitive); connect(ui->dateFormatEntry, &QComboBox::currentTextChanged, diff --git a/desktop-widgets/templatelayout.cpp b/desktop-widgets/templatelayout.cpp index 9edc15a40..0c3cd3f91 100644 --- a/desktop-widgets/templatelayout.cpp +++ b/desktop-widgets/templatelayout.cpp @@ -55,7 +55,7 @@ void set_bundled_templates_as_read_only() listStats[i] = stats + QDir::separator() + listStats.at(i); list += listStats; - foreach (const QString& f, list) + for (const QString &f: list) QFile::setPermissions(pathUser + QDir::separator() + f, QFileDevice::ReadOwner | QFileDevice::ReadUser); } diff --git a/mobile-widgets/qmlmanager.cpp b/mobile-widgets/qmlmanager.cpp index ae9bbcfeb..2ca85b4e7 100644 --- a/mobile-widgets/qmlmanager.cpp +++ b/mobile-widgets/qmlmanager.cpp @@ -2365,13 +2365,13 @@ QStringList QMLManager::cloudCacheList() const QDir localCacheDir(QString("%1/cloudstorage/").arg(system_default_directory())); QStringList dirs = localCacheDir.entryList(); QStringList result; - foreach(QString dir, dirs) { + for (const QString &dir: dirs) { QString originsDir = QString("%1/cloudstorage/%2/.git/refs/remotes/origin/").arg(system_default_directory()).arg(dir); QDir remote(originsDir); if (dir == "localrepo") { result << QString("localrepo[master]"); } else { - foreach(QString branch, remote.entryList().filter(QRegularExpression("...+"))) + for (const QString &branch: remote.entryList().filter(QRegularExpression("...+"))) result << QString("%1[%2]").arg(dir).arg(branch); } } diff --git a/qt-models/maplocationmodel.cpp b/qt-models/maplocationmodel.cpp index 1b45a45f0..96ef0d960 100644 --- a/qt-models/maplocationmodel.cpp +++ b/qt-models/maplocationmodel.cpp @@ -216,8 +216,7 @@ void MapLocationModel::setSelected(const QVector &divesites) MapLocation *MapLocationModel::getMapLocation(const struct dive_site *ds) { - MapLocation *location; - foreach(location, m_mapLocations) { + for (MapLocation *location: m_mapLocations) { if (ds == location->divesite) return location; } diff --git a/qt-models/models.cpp b/qt-models/models.cpp index 58ce7ca64..cc0482d1e 100644 --- a/qt-models/models.cpp +++ b/qt-models/models.cpp @@ -64,7 +64,7 @@ LanguageModel *LanguageModel::instance() LanguageModel::LanguageModel(QObject *parent) : QAbstractListModel(parent) { QDir d(getSubsurfaceDataPath("translations")); - Q_FOREACH (const QString &s, d.entryList()) { + for (const QString &s: d.entryList()) { if (s.startsWith("subsurface_") && s.endsWith(".qm")) { languages.push_back((s == "subsurface_source.qm") ? "English" : s); } diff --git a/scripts/whitespace.pl b/scripts/whitespace.pl index 2cb01b5bc..be1c4416a 100755 --- a/scripts/whitespace.pl +++ b/scripts/whitespace.pl @@ -3,7 +3,7 @@ my $input = $ARGV[0]; my $source = `clang-format $input`; -# for_each_dive (...) and Q_FOREACH and friends... +# for_each_dive (...) and friends... $source =~ s/(?:\G|^)(.*each.*\(.*) \* (\S.*\))$/$1 *$2/img; # if a variable is declared in the argument, '*' is an indicator for a pointer, not arithmatic $source =~ s/(?:\G|^)(.*each.*\(.*) \& (\S.*\))$/$1 &$2/img; # if a variable is declared in the argument, '&' is an indicator for a reference, not bit logic $source =~ s/(?:\G|^)(.*each[^\s(]*)\s*(\(.*)$/$1 $2/img; # we want exactly one space between keyword and opening parenthesis '('