diff --git a/gui/checkthread.cpp b/gui/checkthread.cpp index 0da46e6fa..e7d682ba0 100644 --- a/gui/checkthread.cpp +++ b/gui/checkthread.cpp @@ -177,7 +177,7 @@ void CheckThread::runAddonsAndTools(const FileSettings *fileSettings, const QStr const QString clangPath = CheckThread::clangTidyCmd(); if (!clangPath.isEmpty()) { QDir dir(clangPath + "/../lib/clang"); - for (QString ver : dir.entryList()) { + for (const QString& ver : dir.entryList()) { QString includePath = dir.absolutePath() + '/' + ver + "/include"; if (ver[0] != '.' && QDir(includePath).exists()) { args << "-isystem" << includePath; diff --git a/gui/cppchecklibrarydata.cpp b/gui/cppchecklibrarydata.cpp index 7c7d20261..58caadf03 100644 --- a/gui/cppchecklibrarydata.cpp +++ b/gui/cppchecklibrarydata.cpp @@ -697,14 +697,14 @@ static void writeFunction(QXmlStreamWriter &xmlWriter, const CppcheckLibraryData } if (!function.notOverlappingDataArgs.isEmpty()) { xmlWriter.writeStartElement("not-overlapping-data"); - foreach (const QString value, function.notOverlappingDataArgs) { + foreach (const QString& value, function.notOverlappingDataArgs) { xmlWriter.writeAttribute(function.notOverlappingDataArgs.key(value), value); } xmlWriter.writeEndElement(); } if (!function.containerAttributes.isEmpty()) { xmlWriter.writeStartElement("container"); - foreach (const QString value, function.containerAttributes) { + foreach (const QString& value, function.containerAttributes) { xmlWriter.writeAttribute(function.containerAttributes.key(value), value); } xmlWriter.writeEndElement(); diff --git a/lib/checkother.cpp b/lib/checkother.cpp index 041b9d022..3e324a683 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -1244,10 +1244,14 @@ void CheckOther::checkPassByReference() const SymbolDatabase * const symbolDatabase = mTokenizer->getSymbolDatabase(); for (const Variable* var : symbolDatabase->variableList()) { - if (!var || !var->isArgument() || !var->isClass() || var->isPointer() || var->isArray() || var->isReference() || var->isEnumType()) + if (!var || !var->isClass() || var->isPointer() || var->isArray() || var->isReference() || var->isEnumType()) continue; - if (var->scope() && var->scope()->function->arg->link()->strAt(-1) == "...") + const bool isRangeBasedFor = astIsRangeBasedForDecl(var->nameToken()); + if (!var->isArgument() && !isRangeBasedFor) + continue; + + if (!isRangeBasedFor && var->scope() && var->scope()->function->arg->link()->strAt(-1) == "...") continue; // references could not be used as va_start parameters (#5824) const Token * const varDeclEndToken = var->declEndToken(); @@ -1275,25 +1279,27 @@ void CheckOther::checkPassByReference() const bool isConst = var->isConst(); if (isConst) { - passedByValueError(var, inconclusive); + passedByValueError(var, inconclusive, isRangeBasedFor); continue; } // Check if variable could be const - if (!var->scope() || var->scope()->function->isImplicitlyVirtual()) + if (!isRangeBasedFor && (!var->scope() || var->scope()->function->isImplicitlyVirtual())) continue; if (!isVariableChanged(var, mSettings, mTokenizer->isCPP())) { - passedByValueError(var, inconclusive); + passedByValueError(var, inconclusive, isRangeBasedFor); } } } -void CheckOther::passedByValueError(const Variable* var, bool inconclusive) +void CheckOther::passedByValueError(const Variable* var, bool inconclusive, bool isRangeBasedFor) { - std::string id = "passedByValue"; - std::string msg = "$symbol:" + (var ? var->name() : "") + "\n" - "Function parameter '$symbol' should be passed by const reference."; + std::string id = isRangeBasedFor ? "iterateByValue" : "passedByValue"; + const std::string action = isRangeBasedFor ? "declared as": "passed by"; + const std::string type = isRangeBasedFor ? "Range variable" : "Function parameter"; + std::string msg = "$symbol:" + (var ? var->name() : "") + "\n" + + type + " '$symbol' should be " + action + " const reference."; ErrorPath errorPath; if (var && var->scope() && var->scope()->function && var->scope()->function->functionPointerUsage) { id += "Callback"; @@ -1302,7 +1308,10 @@ void CheckOther::passedByValueError(const Variable* var, bool inconclusive) } if (var) errorPath.emplace_back(var->nameToken(), msg); - msg += "\nParameter '$symbol' is passed by value. It could be passed as a const reference which is usually faster and recommended in C++."; + if (isRangeBasedFor) + msg += "\nVariable '$symbol' is used to iterate by value. It could be declared as a const reference which is usually faster and recommended in C++."; + else + msg += "\nParameter '$symbol' is passed by value. It could be passed as a const reference which is usually faster and recommended in C++."; reportError(errorPath, Severity::performance, id.c_str(), msg, CWE398, inconclusive ? Certainty::inconclusive : Certainty::normal); } diff --git a/lib/checkother.h b/lib/checkother.h index 2499ef1b5..8f8400703 100644 --- a/lib/checkother.h +++ b/lib/checkother.h @@ -241,7 +241,7 @@ private: void clarifyStatementError(const Token* tok); void cstyleCastError(const Token *tok); void invalidPointerCastError(const Token* tok, const std::string& from, const std::string& to, bool inconclusive, bool toIsInt); - void passedByValueError(const Variable* var, bool inconclusive); + void passedByValueError(const Variable* var, bool inconclusive, bool isRangeBasedFor = false); void constVariableError(const Variable *var, const Function *function); void constStatementError(const Token *tok, const std::string &type, bool inconclusive); void signedCharArrayIndexError(const Token *tok); diff --git a/test/testother.cpp b/test/testother.cpp index 75977e4dd..d22157015 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -289,6 +289,7 @@ private: TEST_CASE(constVariableArrayMember); // #10371 TEST_CASE(knownPointerToBool); + TEST_CASE(iterateByValue); } #define check(...) check_(__FILE__, __LINE__, __VA_ARGS__) @@ -11707,6 +11708,16 @@ private: "}\n"); ASSERT_EQUALS("", errout.str()); } + + void iterateByValue() { + check("void f() {\n" // #9684 + " const std::set ss = { \"a\", \"b\", \"c\" };\n" + " for (auto s : ss)\n" + " (void)s.size();\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:3]: (performance) Range variable 's' should be declared as const reference.\n", + errout.str()); + } }; REGISTER_TEST(TestOther)