Skip to content

Commit 68c129d

Browse files
chrchr-githubchrchr-github
authored
Fix #11493 FN useStlAlgorithm with indices (danmar#7317)
1 parent 1852b10 commit 68c129d

File tree

9 files changed

+188
-54
lines changed

9 files changed

+188
-54
lines changed

‎.selfcheck_suppressions‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,3 +35,4 @@ invalidPrintfArgType_uint:externals/tinyxml2/tinyxml2.cpp
3535
funcArgNamesDifferent:externals/tinyxml2/tinyxml2.cpp
3636
nullPointerRedundantCheck:externals/tinyxml2/tinyxml2.cpp
3737
knownConditionTrueFalse:externals/tinyxml2/tinyxml2.cpp
38+
useStlAlgorithm:externals/simplecpp/simplecpp.cpp

‎gui/projectfiledialog.cpp‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -945,11 +945,10 @@ void ProjectFileDialog::editSuppression(const QModelIndex & /*index*/)
945945
int ProjectFileDialog::getSuppressionIndex(const QString &shortText) const
946946
{
947947
const std::string s = shortText.toStdString();
948-
for (int i = 0; i < mSuppressions.size(); ++i) {
949-
if (mSuppressions[i].getText() == s)
950-
return i;
951-
}
952-
return -1;
948+
auto it = std::find_if(mSuppressions.cbegin(), mSuppressions.cend(), [&](const SuppressionList::Suppression& sup) {
949+
return sup.getText() == s;
950+
});
951+
return it == mSuppressions.cend() ? -1 : static_cast<int>(std::distance(mSuppressions.cbegin(), it));
953952
}
954953

955954
void ProjectFileDialog::browseMisraFile()

‎gui/resultstree.cpp‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1290,12 +1290,10 @@ void ResultsTree::saveErrors(Report *report, const QStandardItem *fileItem) cons
12901290

12911291
static int indexOf(const QList<ErrorItem> &list, const ErrorItem &item)
12921292
{
1293-
for (int i = 0; i < list.size(); i++) {
1294-
if (ErrorItem::sameCID(item, list[i])) {
1295-
return i;
1296-
}
1297-
}
1298-
return -1;
1293+
auto it = std::find_if(list.cbegin(), list.cend(), [&](const ErrorItem& e) {
1294+
return ErrorItem::sameCID(item, e);
1295+
});
1296+
return it == list.cend() ? -1 : static_cast<int>(std::distance(list.cbegin(), it));
12991297
}
13001298

13011299
void ResultsTree::updateFromOldReport(const QString &filename)

‎gui/translationhandler.cpp‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -177,12 +177,8 @@ void TranslationHandler::addTranslation(const char *name, const char *filename)
177177

178178
int TranslationHandler::getLanguageIndexByCode(const QString &code) const
179179
{
180-
int index = -1;
181-
for (int i = 0; i < mTranslations.size(); i++) {
182-
if (mTranslations[i].mCode == code || mTranslations[i].mCode == code.left(2)) {
183-
index = i;
184-
break;
185-
}
186-
}
187-
return index;
180+
auto it = std::find_if(mTranslations.cbegin(), mTranslations.cend(), [&](const TranslationInfo& ti) {
181+
return ti.mCode == code || ti.mCode == code.left(2);
182+
});
183+
return it == mTranslations.cend() ? -1 : static_cast<int>(std::distance(mTranslations.cbegin(), it));
188184
}

‎lib/checkstl.cpp‎

Lines changed: 83 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2598,7 +2598,9 @@ static const Token *singleStatement(const Token *start)
25982598
return endStatement;
25992599
}
26002600

2601-
static const Token *singleAssignInScope(const Token *start, nonneg int varid, bool &input, bool &hasBreak, const Settings& settings)
2601+
enum class LoopType : std::uint8_t { OTHER, RANGE, ITERATOR, INDEX };
2602+
2603+
static const Token *singleAssignInScope(const Token *start, nonneg int varid, bool &input, bool &hasBreak, LoopType loopType, const Settings& settings)
26022604
{
26032605
const Token *endStatement = singleStatement(start);
26042606
if (!endStatement)
@@ -2612,6 +2614,23 @@ static const Token *singleAssignInScope(const Token *start, nonneg int varid, bo
26122614
return nullptr;
26132615
input = Token::findmatch(assignTok->next(), "%varid%", endStatement, varid) || !Token::Match(start->next(), "%var% =");
26142616
hasBreak = Token::simpleMatch(endStatement->previous(), "break");
2617+
2618+
if (loopType == LoopType::INDEX) { // check for container access
2619+
nonneg int containerId{};
2620+
for (const Token* tok = assignTok->next(); tok != endStatement; tok = tok->next()) {
2621+
if (tok->varId() == varid) {
2622+
if (!Token::simpleMatch(tok->astParent(), "["))
2623+
return nullptr;
2624+
const Token* contTok = tok->astParent()->astOperand1();
2625+
if (!contTok->valueType() || !contTok->valueType()->container || contTok->varId() == 0)
2626+
return nullptr;
2627+
if (containerId > 0 && containerId != contTok->varId()) // allow only one container
2628+
return nullptr;
2629+
containerId = contTok->varId();
2630+
}
2631+
}
2632+
return containerId > 0 ? assignTok : nullptr;
2633+
}
26152634
return assignTok;
26162635
}
26172636

@@ -2652,7 +2671,7 @@ static const Token *singleIncrementInScope(const Token *start, nonneg int varid,
26522671
return varTok;
26532672
}
26542673

2655-
static const Token *singleConditionalInScope(const Token *start, nonneg int varid, const Settings& settings)
2674+
static const Token *singleConditionalInScope(const Token *start, nonneg int varid, LoopType loopType, const Settings& settings)
26562675
{
26572676
if (start->str() != "{")
26582677
return nullptr;
@@ -2671,6 +2690,22 @@ static const Token *singleConditionalInScope(const Token *start, nonneg int vari
26712690
return nullptr;
26722691
if (isVariableChanged(start, bodyTok, varid, /*globalvar*/ false, settings))
26732692
return nullptr;
2693+
if (loopType == LoopType::INDEX) { // check for container access
2694+
nonneg int containerId{};
2695+
for (const Token* tok = start->tokAt(2); tok != start->linkAt(2); tok = tok->next()) {
2696+
if (tok->varId() == varid) {
2697+
if (!Token::simpleMatch(tok->astParent(), "["))
2698+
return nullptr;
2699+
const Token* contTok = tok->astParent()->astOperand1();
2700+
if (!contTok->valueType() || !contTok->valueType()->container || contTok->varId() == 0)
2701+
return nullptr;
2702+
if (containerId > 0 && containerId != contTok->varId()) // allow only one container
2703+
return nullptr;
2704+
containerId = contTok->varId();
2705+
}
2706+
}
2707+
return containerId > 0 ? bodyTok : nullptr;
2708+
}
26742709
return bodyTok;
26752710
}
26762711

@@ -2735,11 +2770,9 @@ static std::string flipMinMax(const std::string &algo)
27352770
return algo;
27362771
}
27372772

2738-
static std::string minmaxCompare(const Token *condTok, nonneg int loopVar, nonneg int assignVar, bool invert = false)
2773+
static std::string minmaxCompare(const Token *condTok, nonneg int loopVar, nonneg int assignVar, LoopType loopType, bool invert = false)
27392774
{
2740-
if (!Token::Match(condTok, "<|<=|>=|>"))
2741-
return "std::accumulate";
2742-
if (!hasVarIds(condTok, loopVar, assignVar))
2775+
if (loopType == LoopType::RANGE && !hasVarIds(condTok, loopVar, assignVar))
27432776
return "std::accumulate";
27442777
std::string algo = "std::max_element";
27452778
if (Token::Match(condTok, "<|<="))
@@ -2751,6 +2784,38 @@ static std::string minmaxCompare(const Token *condTok, nonneg int loopVar, nonne
27512784
return algo;
27522785
}
27532786

2787+
static bool isTernaryAssignment(const Token* assignTok, nonneg int loopVarId, nonneg int assignVarId, LoopType loopType, std::string& algo)
2788+
{
2789+
if (!Token::simpleMatch(assignTok->astOperand2(), "?"))
2790+
return false;
2791+
const Token* condTok = assignTok->astOperand2()->astOperand1();
2792+
if (!Token::Match(condTok, "<|<=|>=|>"))
2793+
return false;
2794+
2795+
const Token* colon = assignTok->astOperand2()->astOperand2();
2796+
if (loopType == LoopType::RANGE) {
2797+
if (!(condTok->astOperand1()->varId() && condTok->astOperand2()->varId() && colon->astOperand1()->varId() && colon->astOperand2()->varId()))
2798+
return false;
2799+
}
2800+
else if (loopType == LoopType::INDEX) {
2801+
int nVar = 0, nCont = 0;
2802+
for (const Token* tok : { condTok->astOperand1(), condTok->astOperand2(), colon->astOperand1(), colon->astOperand2() }) {
2803+
if (tok->varId())
2804+
++nVar;
2805+
else if (tok->str() == "[" && tok->astOperand1()->varId() && tok->astOperand1()->valueType() && tok->astOperand1()->valueType()->container &&
2806+
tok->astOperand2()->varId() == loopVarId)
2807+
++nCont;
2808+
}
2809+
if (nVar != 2 || nCont != 2)
2810+
return false;
2811+
}
2812+
else
2813+
return false;
2814+
2815+
algo = minmaxCompare(condTok, loopVarId, assignVarId, loopType, colon->astOperand1()->varId() == assignVarId);
2816+
return true;
2817+
}
2818+
27542819
namespace {
27552820
struct LoopAnalyzer {
27562821
const Token* bodyTok = nullptr;
@@ -2940,13 +3005,14 @@ void CheckStl::useStlAlgorithm()
29403005
const Token *bodyTok = tok->linkAt(1)->next();
29413006
const Token *splitTok = tok->next()->astOperand2();
29423007
const Token* loopVar{};
2943-
bool isIteratorLoop = false;
3008+
LoopType loopType{};
29443009
if (Token::simpleMatch(splitTok, ":")) {
29453010
loopVar = splitTok->previous();
29463011
if (loopVar->varId() == 0)
29473012
continue;
29483013
if (Token::simpleMatch(splitTok->astOperand2(), "{"))
29493014
continue;
3015+
loopType = LoopType::RANGE;
29503016
}
29513017
else { // iterator-based loop?
29523018
const Token* initTok = getInitTok(tok);
@@ -2955,18 +3021,19 @@ void CheckStl::useStlAlgorithm()
29553021
if (!initTok || !condTok || !stepTok)
29563022
continue;
29573023
loopVar = Token::Match(condTok, "%comp%") ? condTok->astOperand1() : nullptr;
2958-
if (!Token::Match(loopVar, "%var%") || !loopVar->valueType() || loopVar->valueType()->type != ValueType::Type::ITERATOR)
3024+
if (!Token::Match(loopVar, "%var%") || !loopVar->valueType() ||
3025+
(loopVar->valueType()->type != ValueType::Type::ITERATOR && !loopVar->valueType()->isIntegral()))
29593026
continue;
29603027
if (!Token::simpleMatch(initTok, "=") || !Token::Match(initTok->astOperand1(), "%varid%", loopVar->varId()))
29613028
continue;
29623029
if (!stepTok->isIncDecOp())
29633030
continue;
2964-
isIteratorLoop = true;
3031+
loopType = (loopVar->valueType()->type == ValueType::Type::ITERATOR) ? LoopType::ITERATOR : LoopType::INDEX;
29653032
}
29663033

29673034
// Check for single assignment
29683035
bool useLoopVarInAssign{}, hasBreak{};
2969-
const Token *assignTok = singleAssignInScope(bodyTok, loopVar->varId(), useLoopVarInAssign, hasBreak, *mSettings);
3036+
const Token *assignTok = singleAssignInScope(bodyTok, loopVar->varId(), useLoopVarInAssign, hasBreak, loopType, *mSettings);
29703037
if (assignTok) {
29713038
if (!checkAssignee(assignTok->astOperand1()))
29723039
continue;
@@ -2986,8 +3053,8 @@ void CheckStl::useStlAlgorithm()
29863053
algo = "std::distance";
29873054
else if (accumulateBool(assignTok, assignVarId))
29883055
algo = "std::any_of, std::all_of, std::none_of, or std::accumulate";
2989-
else if (Token::Match(assignTok, "= %var% <|<=|>=|> %var% ? %var% : %var%") && hasVarIds(assignTok->tokAt(6), loopVar->varId(), assignVarId))
2990-
algo = minmaxCompare(assignTok->tokAt(2), loopVar->varId(), assignVarId, assignTok->tokAt(5)->varId() == assignVarId);
3056+
else if (isTernaryAssignment(assignTok, loopVar->varId(), assignVarId, loopType, algo))
3057+
;
29913058
else if (isAccumulation(assignTok, assignVarId))
29923059
algo = "std::accumulate";
29933060
else
@@ -2999,7 +3066,7 @@ void CheckStl::useStlAlgorithm()
29993066
// Check for container calls
30003067
bool useLoopVarInMemCall;
30013068
const Token *memberAccessTok = singleMemberCallInScope(bodyTok, loopVar->varId(), useLoopVarInMemCall, *mSettings);
3002-
if (memberAccessTok && !isIteratorLoop) {
3069+
if (memberAccessTok && loopType == LoopType::RANGE) {
30033070
const Token *memberCallTok = memberAccessTok->astOperand2();
30043071
const int contVarId = memberAccessTok->astOperand1()->varId();
30053072
if (contVarId == loopVar->varId())
@@ -3031,10 +3098,10 @@ void CheckStl::useStlAlgorithm()
30313098
}
30323099

30333100
// Check for conditionals
3034-
const Token *condBodyTok = singleConditionalInScope(bodyTok, loopVar->varId(), *mSettings);
3101+
const Token *condBodyTok = singleConditionalInScope(bodyTok, loopVar->varId(), loopType, *mSettings);
30353102
if (condBodyTok) {
30363103
// Check for single assign
3037-
assignTok = singleAssignInScope(condBodyTok, loopVar->varId(), useLoopVarInAssign, hasBreak, *mSettings);
3104+
assignTok = singleAssignInScope(condBodyTok, loopVar->varId(), useLoopVarInAssign, hasBreak, loopType, *mSettings);
30383105
if (assignTok) {
30393106
if (!checkAssignee(assignTok->astOperand1()))
30403107
continue;
@@ -3108,7 +3175,7 @@ void CheckStl::useStlAlgorithm()
31083175
const Token *loopVar2 = Token::findmatch(condBodyTok, "%varid%", condBodyTok->link(), loopVar->varId());
31093176
std::string algo;
31103177
if (loopVar2 ||
3111-
(isIteratorLoop && loopVar->variable() && precedes(loopVar->variable()->nameToken(), tok))) // iterator declared outside the loop
3178+
(loopType == LoopType::ITERATOR && loopVar->variable() && precedes(loopVar->variable()->nameToken(), tok))) // iterator declared outside the loop
31123179
algo = "std::find_if";
31133180
else
31143181
algo = "std::any_of";

‎lib/clangimport.cpp‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -906,15 +906,14 @@ Token *clangimport::AstNode::createTokens(TokenList &tokenList)
906906
varDecl->children.clear();
907907
Token *expr1 = varDecl->createTokens(tokenList);
908908
Token *colon = addtoken(tokenList, ":");
909-
AstNodePtr range;
910-
for (std::size_t i = 0; i < 2; i++) {
911-
if (children[i] && children[i]->nodeType == DeclStmt && children[i]->getChild(0)->nodeType == VarDecl) {
912-
range = children[i]->getChild(0)->getChild(0);
913-
break;
914-
}
915-
}
916-
if (!range)
909+
910+
auto it = std::find_if(children.cbegin(), children.cbegin() + 2, [&](const AstNodePtr& c) {
911+
return c && c->nodeType == DeclStmt && c->getChild(0)->nodeType == VarDecl;
912+
});
913+
if (it == children.cbegin() + 2)
917914
throw InternalError(tokenList.back(), "Failed to import CXXForRangeStmt. Range?");
915+
AstNodePtr range = (*it)->getChild(0)->getChild(0);
916+
918917
Token *expr2 = range->createTokens(tokenList);
919918
Token *par2 = addtoken(tokenList, ")");
920919

‎lib/tokenize.cpp‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7463,12 +7463,9 @@ void Tokenizer::simplifyStaticConst()
74637463
Token* leftTok = tok;
74647464
bool behindOther = false;
74657465
for (; leftTok; leftTok = leftTok->previous()) {
7466-
for (std::size_t j = 0; j <= i; j++) {
7467-
if (leftTok->str() == qualifiers[j]) {
7468-
behindOther = true;
7469-
break;
7470-
}
7471-
}
7466+
behindOther = std::any_of(qualifiers.cbegin(), qualifiers.cbegin() + i + 1, [&](const std::string& q) {
7467+
return q == leftTok->str();
7468+
});
74727469
if (behindOther)
74737470
break;
74747471
if (isCPP() && Token::simpleMatch(leftTok, ">")) {

‎lib/tokenlist.cpp‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -113,9 +113,11 @@ void TokenList::determineCppC()
113113
int TokenList::appendFileIfNew(std::string fileName)
114114
{
115115
// Has this file been tokenized already?
116-
for (int i = 0; i < mFiles.size(); ++i)
117-
if (Path::sameFileName(mFiles[i], fileName))
118-
return i;
116+
auto it = std::find_if(mFiles.cbegin(), mFiles.cend(), [&](const std::string& f) {
117+
return Path::sameFileName(f, fileName);
118+
});
119+
if (it != mFiles.cend())
120+
return static_cast<int>(std::distance(mFiles.cbegin(), it));
119121

120122
// The "mFiles" vector remembers what files have been tokenized..
121123
mFiles.push_back(std::move(fileName));

0 commit comments

Comments
 (0)