walkero
/
cppcheck
1
0
Fork 0
Code Issues Pull Requests Projects Releases 1 Wiki Activity

Fix FP useStlAlgorithm (don't suggest std::accumulate when nothing is accumulated) (#4647)

This commit is contained in:
chrchr-github 2022-12-18 16:52:04 +01:00 committed by GitHub
parent 63e30d1b8c
commit b1abaf8809
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 42 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
lib/astutils.cpp
View File

@ -1922,7 +1922,7 @@ bool isConstFunctionCall(const Token* ftok, const Library& library)
return true; return true;
} }
bool isConstExpression(const Token *tok, const Library& library, bool pure, bool cpp) bool isConstExpression(const Token *tok, const Library& library, bool cpp)
{ {
if (!tok) if (!tok)
return true; return true;
@ -1941,7 +1941,7 @@ bool isConstExpression(const Token *tok, const Library& library, bool pure, bool
// bailout when we see ({..}) // bailout when we see ({..})
if (tok->str() == "{") if (tok->str() == "{")
return false; return false;
return isConstExpression(tok->astOperand1(), library, pure, cpp) && isConstExpression(tok->astOperand2(), library, pure, cpp); return isConstExpression(tok->astOperand1(), library, cpp) && isConstExpression(tok->astOperand2(), library, cpp);
} }
bool isWithoutSideEffects(bool cpp, const Token* tok, bool checkArrayAccess, bool checkReference) bool isWithoutSideEffects(bool cpp, const Token* tok, bool checkArrayAccess, bool checkReference)

2
lib/astutils.h
View File

@ -266,7 +266,7 @@ bool isOppositeExpression(bool cpp, const Token * const tok1, const Token * cons
bool isConstFunctionCall(const Token* ftok, const Library& library); bool isConstFunctionCall(const Token* ftok, const Library& library);
bool isConstExpression(const Token *tok, const Library& library, bool pure, bool cpp); bool isConstExpression(const Token *tok, const Library& library, bool cpp);
bool isWithoutSideEffects(bool cpp, const Token* tok, bool checkArrayAccess = false, bool checkReference = true); bool isWithoutSideEffects(bool cpp, const Token* tok, bool checkArrayAccess = false, bool checkReference = true);

2
lib/checkbool.cpp
View File

@ -116,7 +116,7 @@ void CheckBool::checkBitwiseOnBoolean()
if (tok->str() == "|" && !isConvertedToBool(tok) && !(isBoolOp1 && isBoolOp2)) if (tok->str() == "|" && !isConvertedToBool(tok) && !(isBoolOp1 && isBoolOp2))
continue; continue;
// first operand will always be evaluated // first operand will always be evaluated
if (!isConstExpression(tok->astOperand2(), mSettings->library, true, mTokenizer->isCPP())) if (!isConstExpression(tok->astOperand2(), mSettings->library, mTokenizer->isCPP()))
continue; continue;
if (tok->astOperand2()->variable() && tok->astOperand2()->variable()->nameToken() == tok->astOperand2()) if (tok->astOperand2()->variable() && tok->astOperand2()->variable()->nameToken() == tok->astOperand2())
continue; continue;

4
lib/checkfunctions.cpp
View File

@ -725,10 +725,10 @@ void CheckFunctions::useStandardLibrary()
const auto& secondOp = condToken->str(); const auto& secondOp = condToken->str();
const bool isLess = "<" == secondOp && const bool isLess = "<" == secondOp &&
isConstExpression(condToken->astOperand2(), mSettings->library, true, mTokenizer->isCPP()) && isConstExpression(condToken->astOperand2(), mSettings->library, mTokenizer->isCPP()) &&
condToken->astOperand1()->varId() == idxVarId; condToken->astOperand1()->varId() == idxVarId;
const bool isMore = ">" == secondOp && const bool isMore = ">" == secondOp &&
isConstExpression(condToken->astOperand1(), mSettings->library, true, mTokenizer->isCPP()) && isConstExpression(condToken->astOperand1(), mSettings->library, mTokenizer->isCPP()) &&
condToken->astOperand2()->varId() == idxVarId; condToken->astOperand2()->varId() == idxVarId;
if (!(isLess || isMore)) if (!(isLess || isMore))

2
lib/checkother.cpp
View File

@ -2521,7 +2521,7 @@ void CheckOther::checkDuplicateExpression()
&errorPath) && &errorPath) &&
isWithoutSideEffects(mTokenizer->isCPP(), tok->astOperand2())) isWithoutSideEffects(mTokenizer->isCPP(), tok->astOperand2()))
duplicateExpressionError(tok->astOperand2(), tok->astOperand1()->astOperand2(), tok, errorPath); duplicateExpressionError(tok->astOperand2(), tok->astOperand1()->astOperand2(), tok, errorPath);
else if (tok->astOperand2() && isConstExpression(tok->astOperand1(), mSettings->library, true, mTokenizer->isCPP())) { else if (tok->astOperand2() && isConstExpression(tok->astOperand1(), mSettings->library, mTokenizer->isCPP())) {
auto checkDuplicate = [&](const Token* exp1, const Token* exp2, const Token* ast1) { auto checkDuplicate = [&](const Token* exp1, const Token* exp2, const Token* ast1) {
if (isSameExpression(mTokenizer->isCPP(), true, exp1, exp2, mSettings->library, true, true, &errorPath) && if (isSameExpression(mTokenizer->isCPP(), true, exp1, exp2, mSettings->library, true, true, &errorPath) &&
isWithoutSideEffects(mTokenizer->isCPP(), exp1) && isWithoutSideEffects(mTokenizer->isCPP(), exp1) &&

12
lib/checkstl.cpp
View File

@ -2708,6 +2708,12 @@ void CheckStl::useStlAlgorithm()
return !astIsContainer(tok); // don't warn for containers, where overloaded operators can be costly return !astIsContainer(tok); // don't warn for containers, where overloaded operators can be costly
}; };
auto isConditionWithoutSideEffects = [this](const Token* tok) -> bool {
if (!Token::simpleMatch(tok, "{") || !Token::simpleMatch(tok->previous(), ")"))
return false;
return isConstExpression(tok->previous()->link()->astOperand2(), mSettings->library, true);
};
for (const Scope *function : mTokenizer->getSymbolDatabase()->functionScopes) { for (const Scope *function : mTokenizer->getSymbolDatabase()->functionScopes) {
for (const Token *tok = function->bodyStart; tok != function->bodyEnd; tok = tok->next()) { for (const Token *tok = function->bodyStart; tok != function->bodyEnd; tok = tok->next()) {
// Parse range-based for loop // Parse range-based for loop
@ -2824,8 +2830,12 @@ void CheckStl::useStlAlgorithm()
algo = "std::count_if"; algo = "std::count_if";
else if (accumulateBoolLiteral(assignTok, assignVarId)) else if (accumulateBoolLiteral(assignTok, assignVarId))
algo = "std::any_of, std::all_of, std::none_of, or std::accumulate"; algo = "std::any_of, std::all_of, std::none_of, or std::accumulate";
else else if (assignTok->str() != "=")
algo = "std::accumulate"; algo = "std::accumulate";
else if (isConditionWithoutSideEffects(condBodyTok))
algo = "std::any_of, std::all_of, std::none_of";
else
continue;
} }
useStlAlgorithmError(assignTok, algo); useStlAlgorithmError(assignTok, algo);
continue; continue;

2
lib/reverseanalyzer.cpp
View File

@ -244,7 +244,7 @@ struct ReverseTraversal {
// Assignment to // Assignment to
} else if (lhsAction.matches() && !assignTok->astOperand2()->hasKnownIntValue() && } else if (lhsAction.matches() && !assignTok->astOperand2()->hasKnownIntValue() &&
assignTok->astOperand2()->exprId() > 0 && assignTok->astOperand2()->exprId() > 0 &&
isConstExpression(assignTok->astOperand2(), settings->library, true, true)) { isConstExpression(assignTok->astOperand2(), settings->library, true)) {
const std::string info = "Assignment to '" + assignTok->expressionString() + "'"; const std::string info = "Assignment to '" + assignTok->expressionString() + "'";
ValuePtr<Analyzer> a = analyzer->reanalyze(assignTok->astOperand2(), info); ValuePtr<Analyzer> a = analyzer->reanalyze(assignTok->astOperand2(), info);
if (a) { if (a) {

8
lib/valueflow.cpp
View File

@ -5039,7 +5039,7 @@ static void valueFlowConditionExpressions(TokenList *tokenlist, SymbolDatabase*
continue; continue;
if (condTok->hasKnownIntValue()) if (condTok->hasKnownIntValue())
continue; continue;
if (!isConstExpression(condTok, settings->library, true, tokenlist->isCPP())) if (!isConstExpression(condTok, settings->library, tokenlist->isCPP()))
continue; continue;
const bool is1 = (condTok->isComparisonOp() || condTok->tokType() == Token::eLogicalOp || astIsBool(condTok)); const bool is1 = (condTok->isComparisonOp() || condTok->tokType() == Token::eLogicalOp || astIsBool(condTok));
@ -5166,7 +5166,7 @@ static void valueFlowSymbolic(TokenList* tokenlist, SymbolDatabase* symboldataba
continue; continue;
if (tok->astOperand2()->exprId() == 0) if (tok->astOperand2()->exprId() == 0)
continue; continue;
if (!isConstExpression(tok->astOperand2(), tokenlist->getSettings()->library, true, tokenlist->isCPP())) if (!isConstExpression(tok->astOperand2(), tokenlist->getSettings()->library, tokenlist->isCPP()))
continue; continue;
if (tok->astOperand1()->valueType() && tok->astOperand2()->valueType()) { if (tok->astOperand1()->valueType() && tok->astOperand2()->valueType()) {
if (isTruncated( if (isTruncated(
@ -5918,7 +5918,7 @@ struct ConditionHandler {
continue; continue;
if (cond.true_values.empty() || cond.false_values.empty()) if (cond.true_values.empty() || cond.false_values.empty())
continue; continue;
if (!isConstExpression(cond.vartok, tokenlist->getSettings()->library, true, tokenlist->isCPP())) if (!isConstExpression(cond.vartok, tokenlist->getSettings()->library, tokenlist->isCPP()))
continue; continue;
f(cond, tok, scope); f(cond, tok, scope);
} }
@ -6625,7 +6625,7 @@ struct SymbolicConditionHandler : SimpleConditionHandler {
return {}; return {};
if (!tok->astOperand2() || tok->astOperand2()->hasKnownIntValue() || tok->astOperand2()->isLiteral()) if (!tok->astOperand2() || tok->astOperand2()->hasKnownIntValue() || tok->astOperand2()->isLiteral())
return {}; return {};
if (!isConstExpression(tok, settings->library, true, true)) if (!isConstExpression(tok, settings->library, true))
return {}; return {};
std::vector<Condition> result; std::vector<Condition> result;

19
test/teststl.cpp
View File

@ -5200,6 +5200,25 @@ private:
"}\n", "}\n",
true); true);
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("", errout.str());
check("bool g(int);\n"
"int f(const std::vector<int>& v) {\n"
" int ret = 0;\n"
" for (const auto i : v)\n"
" if (!g(i))\n"
" ret = 1;\n"
" return ret;\n"
"}\n");
ASSERT_EQUALS("", errout.str());
check("int f(const std::vector<int>& v) {\n"
" int ret = 0;\n"
" for (const auto i : v)\n"
" if (i < 5)\n"
" ret = 1;\n"
" return ret;\n"
"}\n");
ASSERT_EQUALS("[test.cpp:5]: (style) Consider using std::any_of, std::all_of, std::none_of algorithm instead of a raw loop.\n", errout.str());
} }
void loopAlgoMinMax() { void loopAlgoMinMax() {