refs #937 Resolved clazy warnings: unnecessary detaching of containers.

This commit is contained in:
Mathew Sutcliffe
2017-04-15 01:19:26 +01:00
parent ce1730b453
commit fce1513dae
25 changed files with 108 additions and 63 deletions

View File

@@ -162,22 +162,22 @@ namespace BlackSample
QString containsStr("aaa");
number = 0;
timer.start();
for (const auto &str : strList1) { if (newRegex.match(str).hasMatch()) number++; }
for (const auto &str : as_const(strList1)) { if (newRegex.match(str).hasMatch()) number++; }
ms = timer.elapsed();
out << "new regex matched " << number << " of" << strList1.size() << " strings in " << ms << "ms" << endl;
number = 0;
timer.start();
for (const auto &str : strList2) { if (fullRegex.exactMatch(str)) number++; }
for (const auto &str : as_const(strList2)) { if (fullRegex.exactMatch(str)) number++; }
ms = timer.elapsed();
out << "full regex matched " << number << " of" << strList2.size() << " strings in " << ms << "ms" << endl;
number = 0;
timer.start();
for (const auto &str : strList3) { if (wildcardRegex.exactMatch(str)) number++; }
for (const auto &str : as_const(strList3)) { if (wildcardRegex.exactMatch(str)) number++; }
ms = timer.elapsed();
out << "wildcard matched " << number << " of " << strList3.size() << " strings in " << ms << "ms" << endl;
number = 0;
timer.start();
for (const auto &str : strList4) { if (str.contains(containsStr)) number++; }
for (const auto &str : as_const(strList4)) { if (str.contains(containsStr)) number++; }
ms = timer.elapsed();
out << "contains matched " << number << " of " << strList4.size() << " strings in " << ms << "ms" << endl;
@@ -241,7 +241,7 @@ namespace BlackSample
out << "Reads by times / callsigns: " << timer.elapsed() << "ms" << endl;
timer.start();
QHash<CCallsign, CAircraftSituationList> splitList = situations.splitPerCallsign();
const QHash<CCallsign, CAircraftSituationList> splitList = situations.splitPerCallsign();
Q_ASSERT(splitList.size() == numberOfCallsigns);
for (int t = 0; t < numberOfTimes; t++)
{
@@ -318,7 +318,7 @@ namespace BlackSample
Q_UNUSED(b);
timer.start();
QHash<CCallsign, CAircraftSituationList> csSituations = situations.splitPerCallsign();
const QHash<CCallsign, CAircraftSituationList> csSituations = situations.splitPerCallsign();
out << "Split by " << csSituations.size() << " callsigns, " << timer.elapsed() << "ms" << endl;
timer.start();
@@ -422,7 +422,7 @@ namespace BlackSample
upperRegex.optimize();
timer.start();
for (const QString &s : strings)
for (const QString &s : as_const(strings))
{
auto c = containsChar(s, [](QChar c) { return c.isUpper(); });
Q_UNUSED(c);
@@ -430,7 +430,7 @@ namespace BlackSample
out << "Check 100,000 strings for containing uppercase letter: (utility) " << timer.elapsed() << "ms" << endl;
timer.start();
for (const QString &s : strings)
for (const QString &s : as_const(strings))
{
auto c = s.contains(upperRegex);
Q_UNUSED(c);
@@ -438,7 +438,7 @@ namespace BlackSample
out << "Check 100,000 strings for containing uppercase letter: (regex) " << timer.elapsed() << "ms" << endl << endl;
timer.start();
for (const QString &s : strings)
for (const QString &s : as_const(strings))
{
auto i = indexOfChar(s, [](QChar c) { return c.isUpper(); });
Q_UNUSED(i);
@@ -446,7 +446,7 @@ namespace BlackSample
out << "Check 100,000 strings for index of first uppercase letter: (utility) " << timer.elapsed() << "ms" << endl;
timer.start();
for (const QString &s : strings)
for (const QString &s : as_const(strings))
{
auto i = s.indexOf(upperRegex);
Q_UNUSED(i);

View File

@@ -102,13 +102,13 @@ namespace BlackSample
CMetaMemberComparator cmp;
QList<QPair<QString, bool>> list = cmp(station1, station3);
for (const auto &member : list) { out << member.first << (member.second ? " equal" : " NOT equal") << endl; }
for (const auto &member : as_const(list)) { out << member.first << (member.second ? " equal" : " NOT equal") << endl; }
out << endl;
list = cmp(station1, station3, { "controller" });
for (const auto &member : list) { out << member.first << (member.second ? " equal" : " NOT equal") << endl; }
for (const auto &member : as_const(list)) { out << member.first << (member.second ? " equal" : " NOT equal") << endl; }
out << endl;
list = cmp(station1, station3, { "controller", "homebase" });
for (const auto &member : list) { out << member.first << (member.second ? " equal" : " NOT equal") << endl; }
for (const auto &member : as_const(list)) { out << member.first << (member.second ? " equal" : " NOT equal") << endl; }
out << "-----------------------------------------------" << endl;
return 0;

View File

@@ -134,8 +134,8 @@ namespace BlackSample
stream >> cmd;
stream.skipWhiteSpace();
auto found = m_commands.find(cmd);
if (found == m_commands.end())
auto found = m_commands.constFind(cmd);
if (found == m_commands.constEnd())
{
std::cout << "No such command" << std::endl;
}

View File

@@ -163,6 +163,6 @@ const QDBusArgument &operator >>(const QDBusArgument &arg, BlackCore::Context::C
{
QList<CLogSubscriptionPair> listOfPairs;
arg >> listOfPairs;
for (const auto &pair : listOfPairs) { hash.insert(pair.first, pair.second); }
for (const auto &pair : as_const(listOfPairs)) { hash.insert(pair.first, pair.second); }
return arg;
}

View File

@@ -552,7 +552,7 @@ namespace BlackCore
QSharedPointer<IVoiceChannel> CContextAudio::getVoiceChannelBy(const CVoiceRoom &voiceRoom)
{
QSharedPointer<IVoiceChannel> voiceChannel;
for (const auto &channel : m_voiceChannelMapping.values())
for (const auto &channel : as_const(m_voiceChannelMapping))
{
if (channel->getVoiceRoom().getVoiceRoomUrl() == voiceRoom.getVoiceRoomUrl()) voiceChannel = channel;
}

View File

@@ -16,6 +16,8 @@
#include <limits.h>
#include <QtGlobal>
// clazy:excludeall=detaching-member
using namespace BlackInput;
using namespace BlackMisc;
using namespace BlackMisc::Input;
@@ -107,7 +109,7 @@ namespace BlackCore
if (action.isEmpty()) { return; }
if (m_actionRelayingEnabled) emit remoteActionFromLocal(action, isKeyDown);
for (const auto &boundAction : m_boundActions)
for (const auto &boundAction : as_const(m_boundActions))
{
if (boundAction.m_action == action)
{

View File

@@ -97,10 +97,10 @@ namespace BlackCore
void CWeatherManager::fetchNextWeatherData()
{
const WeatherRequest weatherRequest = m_pendingRequests.first();
const WeatherRequest weatherRequest = m_pendingRequests.constFirst();
PhysicalQuantities::CLength maxDistance(100.0, CLengthUnit::km());
for (IWeatherData *plugin : m_weatherDataPlugins)
for (IWeatherData *plugin : as_const(m_weatherDataPlugins))
{
plugin->fetchWeatherData(weatherRequest.weatherGrid, maxDistance);
}
@@ -114,7 +114,7 @@ namespace BlackCore
Q_ASSERT(weatherDataPlugin);
auto fetchedWeatherGrid = weatherDataPlugin->getWeatherData();
const WeatherRequest weatherRequest = m_pendingRequests.first();
const WeatherRequest weatherRequest = m_pendingRequests.constFirst();
CWeatherGrid requestedWeatherGrid = weatherRequest.weatherGrid;
// Interpolation. So far it just picks the closest point without interpolation.

View File

@@ -105,7 +105,7 @@ namespace BlackInput
// FIXME: Take the first device for the time being
// For the future, the user should be able to choose which device
// he wants to use.
CJoystickDeviceData deviceData = m_availableJoystickDevices.first();
const CJoystickDeviceData &deviceData = m_availableJoystickDevices.constFirst();
// Create device
if (FAILED(hr = m_directInput->CreateDevice(deviceData.guidDevice, &m_directInputDevice, nullptr)))

View File

@@ -8,6 +8,7 @@
*/
#include "blackmisc/connectionguard.h"
#include "blackmisc/range.h"
namespace BlackMisc
{
@@ -35,7 +36,7 @@ namespace BlackMisc
{
if (this->m_connections.isEmpty()) { return 0; }
int c = 0;
for (const QMetaObject::Connection &con : this->m_connections)
for (const QMetaObject::Connection &con : as_const(this->m_connections))
{
if (QObject::disconnect(con)) { c++; }
}

View File

@@ -226,7 +226,7 @@ namespace BlackMisc
qSwap(m_queue, queue);
lock.unlock();
for (const auto &pair : queue)
for (const auto &pair : BlackMisc::as_const(queue))
{
m_page->setValuesFromCache(pair.first, pair.second);
}
@@ -299,7 +299,7 @@ namespace BlackMisc
auto msg = m_cache->loadFromFiles(persistentStore(), m_cache->m_revision.keysWithNewerTimestamps(), baseline.toVariantMap(), newValues, m_cache->m_revision.timestampsAsString());
newValues.setTimestamps(m_cache->m_revision.newerTimestamps());
auto missingKeys = m_cache->m_revision.keysWithNewerTimestamps().subtract(newValues.keys());
auto missingKeys = m_cache->m_revision.keysWithNewerTimestamps() - newValues.keys();
if (! missingKeys.isEmpty()) { m_cache->m_revision.writeNewRevision({}, missingKeys); }
msg.setCategories(this);

View File

@@ -64,7 +64,7 @@ namespace BlackMisc
{
// further reduce by VATSIM flag
QStringList furtherReduced;
for (const QString &p : reduced)
for (const QString &p : as_const(reduced))
{
if (CBuildConfig::isVatsimVersion())
{
@@ -203,7 +203,7 @@ namespace BlackMisc
const QJsonObject platforms = json.value("platforms").toObject();
const QStringList platformsKeys = platforms.keys();
if (platformsKeys.isEmpty()) { return CDistribution(); } // no platforms, then the whole distribution is useless
for (const QString platformKey : platformsKeys)
for (const QString platformKey : as_const(platformsKeys))
{
QStringList platformFileNames;
QJsonArray platformFiles = platforms.value(platformKey).toArray();

View File

@@ -183,7 +183,7 @@ namespace BlackMisc
m_connections.insert(connection.name(), connection);
CLogMessage(this).debug() << "New Connection from:" << connection.name();
bool success = true;
for (auto i = m_objects.begin(); i != m_objects.end(); ++i)
for (auto i = m_objects.cbegin(); i != m_objects.cend(); ++i)
{
CLogMessage(this).debug() << "Adding" << i.key() << getDBusInterfaceFromClassInfo(i.value()) << "to the new connection.";
bool ok = connection.registerObject(i.key(), i.value(), registerOptions());
@@ -228,7 +228,7 @@ namespace BlackMisc
break;
case SERVERMODE_P2P:
{
for (QDBusConnection connection : m_connections)
for (QDBusConnection connection : as_const(m_connections))
{
if (connection.registerObject(path, object, registerOptions()))
{
@@ -277,7 +277,7 @@ namespace BlackMisc
break;
case SERVERMODE_P2P:
{
for (QDBusConnection connection : m_connections)
for (QDBusConnection connection : as_const(m_connections))
{
connection.unregisterObject(path);
}

View File

@@ -11,6 +11,7 @@
#include "blackmisc/directoryutils.h"
#include "blackmisc/fileutils.h"
#include "blackmisc/range.h"
#include <QCoreApplication>
#include <QDir>
#include <QUrl>
@@ -176,8 +177,8 @@ namespace BlackMisc
comp.sameNameInTarget = CDirectoryUtils::filesToCanonicalNames(sameNames, sTargetCanonicalFiles);
Q_ASSERT_X(comp.sameNameInSource.size() == comp.sameNameInTarget.size(), Q_FUNC_INFO, "Same sets require same size");
QSet<QString>::const_iterator targetIt = comp.sameNameInTarget.begin();
for (const QString &sourceFile : comp.sameNameInSource)
QSet<QString>::const_iterator targetIt = comp.sameNameInTarget.cbegin();
for (const QString &sourceFile : as_const(comp.sameNameInSource))
{
const QFileInfo source(sourceFile);
const QFileInfo target(*targetIt++);

View File

@@ -130,7 +130,7 @@ namespace BlackMisc
void CLogHandler::logMessage(const CStatusMessage &statusMessage)
{
auto handlers = handlersForMessage(statusMessage);
const auto handlers = handlersForMessage(statusMessage);
if (isFallThroughEnabled(handlers))
{

View File

@@ -289,24 +289,54 @@ namespace BlackMisc
/*!
* Returns a CRange constructed from the begin and end iterators of the given container.
*
* The returned CRange may or may not be const, depending on the constness of the container.
*/
//! @{
template <class T>
auto makeRange(T &&container) -> CRange<decltype(container.begin())>
auto makeRange(T &container) -> CRange<decltype(container.begin())>
{
return { container.begin(), container.end() };
}
template <class T>
auto makeRange(const T &container) -> CRange<decltype(container.begin())>
{
return { container.begin(), container.end() };
}
//! @}
/*!
* Returns a const CRange constructed from the cbegin and cend iterators of the given container.
*/
template <class T>
auto makeConstRange(T &&container) -> CRange<decltype(container.cbegin())>
auto makeConstRange(const T &container) -> CRange<decltype(container.cbegin())>
{
return { container.cbegin(), container.cend() };
}
/*!
* Returns a const CRange for iterating over the keys of an associative container.
*
* This is more efficient than the keys() method of the container, as it doesn't allocate memory.
*/
template <class T>
auto makeKeysRange(const T &container)
{
return makeRange(Iterators::makeKeyIterator(container.cbegin()), container.cend());
}
/*!
* Keys range for a non-const lvalue would be unsafe due to iterator invalidation on detach().
*
* \see http://doc.qt.io/qt-5/containers.html#implicit-sharing-iterator-problem
*/
template <class T>
void makeKeysRange(T &container) = delete;
/*!
* Keys range for a temporary would be unsafe.
*/
template <class T>
void makeKeysRange(const T &&container) = delete;
/*
* Member functions of CRangeBase template defined out of line, because they depend on CRange etc.
*/

View File

@@ -38,11 +38,11 @@ namespace BlackMisc
this->m_splitParts = m_cleanedLine.split(' ');
if (!this->m_splitParts.isEmpty())
{
const QString first = this->m_splitParts.first();
const QString &first = this->m_splitParts.constFirst();
const QString formatted = formatCommand(first);
if (isCommand(first))
{
this->m_commandPart = formatCommand(first);
this->m_commandPart = formatted;
this->m_knownCommand = this->m_knownCommands.contains(formatted);
}
}

View File

@@ -90,7 +90,7 @@ namespace BlackMisc
QStringList remove(toUpper(modelsToBeRemoved));
remove.sort();
QSet<QString> removeSet(knownModels.toSet().intersect(remove.toSet()));
QSet<QString> removeSet(knownModels.toSet() & remove.toSet());
int c = 0;
for (const QString &model : removeSet)
{

View File

@@ -132,10 +132,11 @@ namespace BlackMisc
const QString &CVPilotRulesReader::standardMappingsDirectory()
{
//! \fixme not threadsafe
static QString directory;
if (directory.isEmpty())
{
directory = QStandardPaths::standardLocations(QStandardPaths::DocumentsLocation).first();
directory = QStandardPaths::standardLocations(QStandardPaths::DocumentsLocation).constFirst();
if (!directory.endsWith('/')) { directory.append('/'); }
directory.append("vPilot Files/Model Matching Rule Sets");
}

View File

@@ -262,7 +262,7 @@ namespace BlackMisc
file.close();
parseFullPackage(content, package);
for (const auto &plane : package.planes)
for (const auto &plane : as_const(package.planes))
{
if (installedModels.containsModelString(plane.getModelName()))
{
@@ -295,7 +295,7 @@ namespace BlackMisc
bool CAircraftModelLoaderXPlane::doPackageSub(QString &ioPath)
{
for (auto i = m_cslPackages.begin(); i != m_cslPackages.end(); ++i)
for (auto i = m_cslPackages.cbegin(); i != m_cslPackages.cend(); ++i)
{
if (strncmp(qPrintable(i->name), qPrintable(ioPath), i->name.size()) == 0)
{
@@ -315,8 +315,8 @@ namespace BlackMisc
return false;
}
auto p = std::find_if(m_cslPackages.begin(), m_cslPackages.end(), [&tokens](CSLPackage p) { return p.name == tokens[1]; });
if (p == m_cslPackages.end())
auto p = std::find_if(m_cslPackages.cbegin(), m_cslPackages.cend(), [&tokens](const CSLPackage &p) { return p.name == tokens[1]; });
if (p == m_cslPackages.cend())
{
package.path = path;
package.name = tokens[1];
@@ -338,7 +338,7 @@ namespace BlackMisc
return false;
}
if (std::count_if(m_cslPackages.begin(), m_cslPackages.end(), [&tokens](CSLPackage p) { return p.name == tokens[1]; }) == 0)
if (std::count_if(m_cslPackages.cbegin(), m_cslPackages.cend(), [&tokens](const CSLPackage &p) { return p.name == tokens[1]; }) == 0)
{
CLogMessage(this).warning("WARNING: required package %1 not found. Aborting processing of this package.") << tokens[1];
return false;

View File

@@ -182,14 +182,14 @@ namespace BlackMisc
CValueCache::Element &CValueCache::getElement(const QString &key)
{
QMutexLocker lock(&m_mutex);
return getElement(key, m_elements.lowerBound(key));
return getElement(key, as_const(m_elements).lowerBound(key));
}
CValueCache::Element &CValueCache::getElement(const QString &key, QMap<QString, ElementPtr>::const_iterator pos)
{
QMutexLocker lock(&m_mutex);
if (pos != m_elements.end() && pos.key() == key) { return **pos; }
Q_ASSERT(pos == m_elements.lowerBound(key));
if (pos != m_elements.cend() && pos.key() == key) { return **pos; }
Q_ASSERT(pos == as_const(m_elements).lowerBound(key));
return **m_elements.insert(pos, key, ElementPtr(new Element(key)));
}
@@ -259,8 +259,9 @@ namespace BlackMisc
{
QMutexLocker lock(&m_mutex);
if (values.empty()) { return; }
auto out = m_elements.lowerBound(values.cbegin().key());
auto end = m_elements.upperBound((values.cend() - 1).key());
m_elements.detach(); //! \fixme see http://doc.qt.io/qt-5/containers.html#implicit-sharing-iterator-problem
auto out = as_const(m_elements).lowerBound(values.cbegin().key());
auto end = as_const(m_elements).upperBound((values.cend() - 1).key());
for (auto in = values.cbegin(); in != values.cend(); ++in)
{
while (out != end && out.key() < in.key()) { ++out; }
@@ -295,8 +296,9 @@ namespace BlackMisc
}
CValueCachePacket ratifiedChanges(values.isSaved());
CValueCachePacket ackedChanges(values.isSaved());
auto out = m_elements.lowerBound(values.cbegin().key());
auto end = m_elements.upperBound((values.cend() - 1).key());
m_elements.detach(); //! \fixme see http://doc.qt.io/qt-5/containers.html#implicit-sharing-iterator-problem
auto out = as_const(m_elements).lowerBound(values.cbegin().key());
auto end = as_const(m_elements).upperBound((values.cend() - 1).key());
for (auto in = values.cbegin(); in != values.cend(); ++in)
{
while (out != end && out.key() < in.key()) { ++out; }

View File

@@ -134,10 +134,11 @@ namespace BlackSound
while (chunks)
{
// periodSize-> Returns the period size in bytes.
//! \todo looks wrong: read() will memcpy from m_buffer.constData() to m_buffer.data()
const qint64 len = this->read(m_buffer.data(), this->m_audioOutput->periodSize());
if (len >= 0)
{
this->m_pushModeIODevice->write(m_buffer.data(), len);
this->m_pushModeIODevice->write(m_buffer.constData(), len);
}
if (len != this->m_audioOutput->periodSize())
{
@@ -176,7 +177,7 @@ namespace BlackSound
Q_UNUSED(bytesForAllChannels) // suppress warning in release builds
m_buffer.resize(totalLength);
unsigned char *bufferPointer = reinterpret_cast<unsigned char *>(m_buffer.data());
unsigned char *bufferPointer = reinterpret_cast<unsigned char *>(m_buffer.data()); // clazy:exclude=detaching-member
foreach(Tone t, this->m_tones)
{

View File

@@ -257,7 +257,7 @@ namespace BlackWxPlugin
}
CLogMessage(this).debug() << "Parsed" << messageNo << "GRIB messages.";
for (const GfsGridPoint &gfsGridPoint : m_gfsWeatherGrid)
for (const GfsGridPoint &gfsGridPoint : as_const(m_gfsWeatherGrid))
{
if(QThread::currentThread()->isInterruptionRequested()) { return; }

View File

@@ -25,6 +25,8 @@
#include <cstring>
#include <cmath>
// clazy:excludeall=reserve-candidates
namespace XBus
{
@@ -246,7 +248,7 @@ namespace XBus
void CTraffic::removeAllPlanes()
{
for (auto plane : m_planesByCallsign)
for (auto plane : BlackMisc::as_const(m_planesByCallsign))
{
XPMPDestroyPlane(plane->id);
delete plane;
@@ -334,7 +336,10 @@ namespace XBus
}
else if (callsign.isEmpty())
{
for (const auto &callsign : m_planesByCallsign.keys()) { setInterpolatorMode(callsign, spline); }
for (const auto &callsign : BlackMisc::makeKeysRange(BlackMisc::as_const(m_planesByCallsign)))
{
setInterpolatorMode(callsign, spline);
}
}
}
@@ -379,10 +384,10 @@ namespace XBus
if (plane->hasSurfaces)
{
const auto currentTime = QDateTime::currentMSecsSinceEpoch();
while (! plane->pendingSurfaces.isEmpty() && plane->pendingSurfaces.front().first <= currentTime)
while (! plane->pendingSurfaces.isEmpty() && plane->pendingSurfaces.constFirst().first <= currentTime)
{
//! \todo if gear is currently retracted, look ahead and pull gear position from pendingSurfaces up to 5 seconds in the future
plane->pendingSurfaces.front().second(plane);
plane->pendingSurfaces.constFirst().second(plane);
plane->pendingSurfaces.pop_front();
}
if (plane->surfaces.gearPosition != plane->targetGearPosition)

View File

@@ -15,6 +15,7 @@
*/
#include "expect.h"
#include "blackmisc/range.h"
#include <QCoreApplication>
#include <QEventLoop>
@@ -81,7 +82,8 @@ void Expect::wait(const SourceLocation& srcloc, int timeout)
{
i->onDone([&](const ExpectUnit* u){ unitsCopy.remove(u); });
}
for (auto i : unitsCopy.toList()) // toList is an easy way to make a temporary copy, needed because init might invalidate iterators
// toList is an easy way to make a temporary copy, needed because init might invalidate iterators
for (auto i : unitsCopy.toList()) // clazy:exclude=container-anti-pattern,range-loop
{
i->init();
}
@@ -90,7 +92,7 @@ void Expect::wait(const SourceLocation& srcloc, int timeout)
timer.setSingleShot(true);
QObject::connect(&timer, &QTimer::timeout, [=, &unitsCopy]{
reportTimeout(srcloc, unitsCopy);
for (auto i : unitsCopy)
for (auto i : BlackMisc::as_const(unitsCopy))
{
i->onDone(nullptr); //paranoia
}

View File

@@ -61,7 +61,7 @@ namespace BlackCoreTest
ConnectGuard &operator+= (const QMetaObject::Connection &conn) { m_conns += conn; return *this; }
//! Disconnect and remove all stored connections.
void cleanup() { for (auto i = m_conns.begin(); i != m_conns.end(); ++i) QObject::disconnect(*i); m_conns.clear(); }
void cleanup() { for (auto i = m_conns.cbegin(); i != m_conns.cend(); ++i) QObject::disconnect(*i); m_conns.clear(); }
//! Copying is only allowed when there are no connections stored.
//! @{