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

Refactoring: Reuse isLikelyStreamRead in isVariableChanged

This commit is contained in:
Daniel Marjamäki 2018-04-18 17:45:43 +02:00
parent fb89c987b0
commit 7ee636b934
6 changed files with 29 additions and 43 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
lib/astutils.cpp
View File

@ -528,7 +528,7 @@ bool isVariableChangedByFunctionCall(const Token *tok, const Settings *settings,
return arg && !arg->isConst() && arg->isReference(); return arg && !arg->isConst() && arg->isReference();
} }
bool isVariableChanged(const Token *start, const Token *end, const unsigned int varid, bool globalvar, const Settings *settings) bool isVariableChanged(const Token *start, const Token *end, const unsigned int varid, bool globalvar, const Settings *settings, bool cpp)
{ {
for (const Token *tok = start; tok != end; tok = tok->next()) { for (const Token *tok = start; tok != end; tok = tok->next()) {
if (tok->varId() != varid) { if (tok->varId() != varid) {
@ -544,14 +544,8 @@ bool isVariableChanged(const Token *start, const Token *end, const unsigned int
if (Token::Match(tok->previous(), "++|-- %name%")) if (Token::Match(tok->previous(), "++|-- %name%"))
return true; return true;
if (Token::simpleMatch(tok->previous(), ">>")) { if (isLikelyStreamRead(cpp, tok->previous()))
const Token *shr = tok->previous(); return true;
if (Token::simpleMatch(shr->astParent(), ">>"))
return true;
const Token *lhs = shr->astOperand1();
if (!lhs || !lhs->valueType() || !lhs->valueType()->isIntegral())
return true;
}
const Token *ftok = tok; const Token *ftok = tok;
while (ftok && !Token::Match(ftok, "[({[]")) while (ftok && !Token::Match(ftok, "[({[]"))

2
lib/astutils.h
View File

@ -100,7 +100,7 @@ bool isVariableChangedByFunctionCall(const Token *tok, unsigned int varid, const
bool isVariableChangedByFunctionCall(const Token *tok, const Settings *settings, bool *inconclusive); bool isVariableChangedByFunctionCall(const Token *tok, const Settings *settings, bool *inconclusive);
/** Is variable changed in block of code? */ /** Is variable changed in block of code? */
bool isVariableChanged(const Token *start, const Token *end, const unsigned int varid, bool globalvar, const Settings *settings); bool isVariableChanged(const Token *start, const Token *end, const unsigned int varid, bool globalvar, const Settings *settings, bool cpp);
/** Determines the number of arguments - if token is a function call or macro /** Determines the number of arguments - if token is a function call or macro
* @param start token which is supposed to be the function/macro name. * @param start token which is supposed to be the function/macro name.

4
lib/checkcondition.cpp
View File

@ -168,7 +168,7 @@ bool CheckCondition::assignIfParseScope(const Token * const assignTok,
// is variable changed in loop? // is variable changed in loop?
const Token *bodyStart = tok2->linkAt(1)->next(); const Token *bodyStart = tok2->linkAt(1)->next();
const Token *bodyEnd = bodyStart ? bodyStart->link() : nullptr; const Token *bodyEnd = bodyStart ? bodyStart->link() : nullptr;
if (!bodyEnd || bodyEnd->str() != "}" || isVariableChanged(bodyStart, bodyEnd, varid, !islocal, _settings)) if (!bodyEnd || bodyEnd->str() != "}" || isVariableChanged(bodyStart, bodyEnd, varid, !islocal, _settings, _tokenizer->isCPP()))
continue; continue;
} }
@ -629,7 +629,7 @@ void CheckCondition::multiCondition2()
} }
bool changed = false; bool changed = false;
for (std::set<unsigned int>::const_iterator it = vars.begin(); it != vars.end(); ++it) { for (std::set<unsigned int>::const_iterator it = vars.begin(); it != vars.end(); ++it) {
if (isVariableChanged(tok1, tok2, *it, nonlocal, _settings)) { if (isVariableChanged(tok1, tok2, *it, nonlocal, _settings, _tokenizer->isCPP())) {
changed = true; changed = true;
break; break;
} }

36
lib/valueflow.cpp
View File

@ -1228,7 +1228,7 @@ static void valueFlowReverse(TokenList *tokenlist,
const Token *start = tok2; const Token *start = tok2;
const Token *end = start->link(); const Token *end = start->link();
if (isVariableChanged(start,end,varid,var->isGlobal(),settings)) { if (isVariableChanged(start,end,varid,var->isGlobal(),settings, tokenlist->isCPP())) {
if (settings->debugwarnings) if (settings->debugwarnings)
bailout(tokenlist, errorLogger, tok2, "variable " + var->name() + " is assigned in loop. so valueflow analysis bailout when start of loop is reached."); bailout(tokenlist, errorLogger, tok2, "variable " + var->name() + " is assigned in loop. so valueflow analysis bailout when start of loop is reached.");
break; break;
@ -1308,7 +1308,7 @@ static void valueFlowBeforeCondition(TokenList *tokenlist, SymbolDatabase *symbo
// Variable changed in 3rd for-expression // Variable changed in 3rd for-expression
if (Token::simpleMatch(tok2->previous(), "for (")) { if (Token::simpleMatch(tok2->previous(), "for (")) {
if (tok2->astOperand2() && tok2->astOperand2()->astOperand2() && isVariableChanged(tok2->astOperand2()->astOperand2(), tok2->link(), varid, var->isGlobal(), settings)) { if (tok2->astOperand2() && tok2->astOperand2()->astOperand2() && isVariableChanged(tok2->astOperand2()->astOperand2(), tok2->link(), varid, var->isGlobal(), settings, tokenlist->isCPP())) {
varid = 0U; varid = 0U;
if (settings->debugwarnings) if (settings->debugwarnings)
bailout(tokenlist, errorLogger, tok, "variable " + var->name() + " used in loop"); bailout(tokenlist, errorLogger, tok, "variable " + var->name() + " used in loop");
@ -1320,7 +1320,7 @@ static void valueFlowBeforeCondition(TokenList *tokenlist, SymbolDatabase *symbo
const Token * const start = tok2->link()->next(); const Token * const start = tok2->link()->next();
const Token * const end = start->link(); const Token * const end = start->link();
if (isVariableChanged(start,end,varid,var->isGlobal(),settings)) { if (isVariableChanged(start,end,varid,var->isGlobal(),settings, tokenlist->isCPP())) {
varid = 0U; varid = 0U;
if (settings->debugwarnings) if (settings->debugwarnings)
bailout(tokenlist, errorLogger, tok, "variable " + var->name() + " used in loop"); bailout(tokenlist, errorLogger, tok, "variable " + var->name() + " used in loop");
@ -1425,7 +1425,7 @@ static void handleKnownValuesInLoop(const Token *startToken,
for (std::list<ValueFlow::Value>::iterator it = values->begin(); it != values->end(); ++it) { for (std::list<ValueFlow::Value>::iterator it = values->begin(); it != values->end(); ++it) {
if (it->isKnown()) { if (it->isKnown()) {
if (!isChanged) { if (!isChanged) {
if (!isVariableChanged(startToken, endToken, varid, globalvar, settings)) if (!isVariableChanged(startToken, endToken, varid, globalvar, settings, true))
break; break;
isChanged = true; isChanged = true;
} }
@ -1530,7 +1530,7 @@ static bool valueFlowForward(Token * const startToken,
bailoutflag = true; bailoutflag = true;
break; break;
} }
if (iselse && it->isPossible() && isVariableChanged(start1, start1->link(), varid, var->isGlobal(), settings)) if (iselse && it->isPossible() && isVariableChanged(start1, start1->link(), varid, var->isGlobal(), settings, tokenlist->isCPP()))
values.erase(it++); values.erase(it++);
else else
++it; ++it;
@ -1546,7 +1546,7 @@ static bool valueFlowForward(Token * const startToken,
} else if (indentlevel <= 0 && } else if (indentlevel <= 0 &&
Token::simpleMatch(tok2->link()->previous(), "else {") && Token::simpleMatch(tok2->link()->previous(), "else {") &&
!isReturnScope(tok2->link()->tokAt(-2)) && !isReturnScope(tok2->link()->tokAt(-2)) &&
isVariableChanged(tok2->link(), tok2, varid, var->isGlobal(), settings)) { isVariableChanged(tok2->link(), tok2, varid, var->isGlobal(), settings, tokenlist->isCPP())) {
changeKnownToPossible(values); changeKnownToPossible(values);
} }
} }
@ -1599,7 +1599,7 @@ static bool valueFlowForward(Token * const startToken,
if (Token::simpleMatch(end, "} while (")) if (Token::simpleMatch(end, "} while ("))
end = end->linkAt(2); end = end->linkAt(2);
if (isVariableChanged(start, end, varid, var->isGlobal(), settings)) { if (isVariableChanged(start, end, varid, var->isGlobal(), settings, tokenlist->isCPP())) {
if (settings->debugwarnings) if (settings->debugwarnings)
bailout(tokenlist, errorLogger, tok2, "variable " + var->name() + " valueFlowForward, assignment in do-while"); bailout(tokenlist, errorLogger, tok2, "variable " + var->name() + " valueFlowForward, assignment in do-while");
return false; return false;
@ -1611,7 +1611,7 @@ static bool valueFlowForward(Token * const startToken,
// conditional block of code that assigns variable.. // conditional block of code that assigns variable..
else if (!tok2->varId() && Token::Match(tok2, "%name% (") && Token::simpleMatch(tok2->linkAt(1), ") {")) { else if (!tok2->varId() && Token::Match(tok2, "%name% (") && Token::simpleMatch(tok2->linkAt(1), ") {")) {
// is variable changed in condition? // is variable changed in condition?
if (isVariableChanged(tok2->next(), tok2->next()->link(), varid, var->isGlobal(), settings)) { if (isVariableChanged(tok2->next(), tok2->next()->link(), varid, var->isGlobal(), settings, tokenlist->isCPP())) {
if (settings->debugwarnings) if (settings->debugwarnings)
bailout(tokenlist, errorLogger, tok2, "variable " + var->name() + " valueFlowForward, assignment in condition"); bailout(tokenlist, errorLogger, tok2, "variable " + var->name() + " valueFlowForward, assignment in condition");
return false; return false;
@ -1674,7 +1674,7 @@ static bool valueFlowForward(Token * const startToken,
errorLogger, errorLogger,
settings); settings);
if (!condAlwaysFalse && isVariableChanged(startToken1, startToken1->link(), varid, var->isGlobal(), settings)) { if (!condAlwaysFalse && isVariableChanged(startToken1, startToken1->link(), varid, var->isGlobal(), settings, tokenlist->isCPP())) {
removeValues(values, truevalues); removeValues(values, truevalues);
changeKnownToPossible(values); changeKnownToPossible(values);
} }
@ -1702,7 +1702,7 @@ static bool valueFlowForward(Token * const startToken,
errorLogger, errorLogger,
settings); settings);
if (!condAlwaysTrue && isVariableChanged(startTokenElse, startTokenElse->link(), varid, var->isGlobal(), settings)) { if (!condAlwaysTrue && isVariableChanged(startTokenElse, startTokenElse->link(), varid, var->isGlobal(), settings, tokenlist->isCPP())) {
removeValues(values, falsevalues); removeValues(values, falsevalues);
changeKnownToPossible(values); changeKnownToPossible(values);
} }
@ -1723,7 +1723,7 @@ static bool valueFlowForward(Token * const startToken,
Token * const start = tok2->linkAt(1)->next(); Token * const start = tok2->linkAt(1)->next();
Token * const end = start->link(); Token * const end = start->link();
const bool varusage = (indentlevel >= 0 && constValue && number_of_if == 0U) ? const bool varusage = (indentlevel >= 0 && constValue && number_of_if == 0U) ?
isVariableChanged(start,end,varid,var->isGlobal(),settings) : isVariableChanged(start,end,varid,var->isGlobal(),settings, tokenlist->isCPP()) :
(nullptr != Token::findmatch(start, "%varid%", end, varid)); (nullptr != Token::findmatch(start, "%varid%", end, varid));
if (!read) { if (!read) {
read = bool(nullptr != Token::findmatch(tok2, "%varid% !!=", end, varid)); read = bool(nullptr != Token::findmatch(tok2, "%varid% !!=", end, varid));
@ -1792,7 +1792,7 @@ static bool valueFlowForward(Token * const startToken,
return false; return false;
} }
if (isVariableChanged(start, end, varid, var->isGlobal(), settings)) { if (isVariableChanged(start, end, varid, var->isGlobal(), settings, tokenlist->isCPP())) {
if ((!read || number_of_if == 0) && if ((!read || number_of_if == 0) &&
Token::simpleMatch(tok2, "if (") && Token::simpleMatch(tok2, "if (") &&
!(Token::simpleMatch(end, "} else {") && !(Token::simpleMatch(end, "} else {") &&
@ -1941,8 +1941,8 @@ static bool valueFlowForward(Token * const startToken,
} }
tok2 = tok2->next(); tok2 = tok2->next();
if (isVariableChanged(questionToken, questionToken->astOperand2(), varid, false, settings) && if (isVariableChanged(questionToken, questionToken->astOperand2(), varid, false, settings, tokenlist->isCPP()) &&
isVariableChanged(questionToken->astOperand2(), tok2, varid, false, settings)) { isVariableChanged(questionToken->astOperand2(), tok2, varid, false, settings, tokenlist->isCPP())) {
if (settings->debugwarnings) if (settings->debugwarnings)
bailout(tokenlist, errorLogger, tok2, "variable " + var->name() + " valueFlowForward, assignment in condition"); bailout(tokenlist, errorLogger, tok2, "variable " + var->name() + " valueFlowForward, assignment in condition");
return false; return false;
@ -2117,7 +2117,7 @@ static bool valueFlowForward(Token * const startToken,
Token::simpleMatch(tok2->linkAt(1), "] (") && Token::simpleMatch(tok2->linkAt(1), "] (") &&
Token::simpleMatch(tok2->linkAt(1)->linkAt(1), ") {")) { Token::simpleMatch(tok2->linkAt(1)->linkAt(1), ") {")) {
const Token *bodyStart = tok2->linkAt(1)->linkAt(1)->next(); const Token *bodyStart = tok2->linkAt(1)->linkAt(1)->next();
if (isVariableChanged(bodyStart, bodyStart->link(), varid, var->isGlobal(), settings)) { if (isVariableChanged(bodyStart, bodyStart->link(), varid, var->isGlobal(), settings, tokenlist->isCPP())) {
if (settings->debugwarnings) if (settings->debugwarnings)
bailout(tokenlist, errorLogger, tok2, "valueFlowForward, " + var->name() + " is changed in lambda function"); bailout(tokenlist, errorLogger, tok2, "valueFlowForward, " + var->name() + " is changed in lambda function");
return false; return false;
@ -2486,7 +2486,7 @@ static void valueFlowAfterCondition(TokenList *tokenlist, SymbolDatabase* symbol
// does condition reassign variable? // does condition reassign variable?
if (tok != top->astOperand2() && if (tok != top->astOperand2() &&
Token::Match(top->astOperand2(), "%oror%|&&") && Token::Match(top->astOperand2(), "%oror%|&&") &&
isVariableChanged(top, top->link(), varid, var->isGlobal(), settings)) { isVariableChanged(top, top->link(), varid, var->isGlobal(), settings, tokenlist->isCPP())) {
if (settings->debugwarnings) if (settings->debugwarnings)
bailout(tokenlist, errorLogger, tok, "assignment in condition"); bailout(tokenlist, errorLogger, tok, "assignment in condition");
continue; continue;
@ -2540,7 +2540,7 @@ static void valueFlowAfterCondition(TokenList *tokenlist, SymbolDatabase* symbol
valueFlowForward(startTokens[i]->next(), startTokens[i]->link(), var, varid, values, true, false, tokenlist, errorLogger, settings); valueFlowForward(startTokens[i]->next(), startTokens[i]->link(), var, varid, values, true, false, tokenlist, errorLogger, settings);
values.front().setPossible(); values.front().setPossible();
if (isVariableChanged(startTokens[i], startTokens[i]->link(), varid, var->isGlobal(), settings)) { if (isVariableChanged(startTokens[i], startTokens[i]->link(), varid, var->isGlobal(), settings, tokenlist->isCPP())) {
// TODO: The endToken should not be startTokens[i]->link() in the valueFlowForward call // TODO: The endToken should not be startTokens[i]->link() in the valueFlowForward call
if (settings->debugwarnings) if (settings->debugwarnings)
bailout(tokenlist, errorLogger, startTokens[i]->link(), "valueFlowAfterCondition: " + var->name() + " is changed in conditional block"); bailout(tokenlist, errorLogger, startTokens[i]->link(), "valueFlowAfterCondition: " + var->name() + " is changed in conditional block");
@ -2854,7 +2854,7 @@ static void valueFlowForLoopSimplify(Token * const bodyStart, const unsigned int
const Token * const bodyEnd = bodyStart->link(); const Token * const bodyEnd = bodyStart->link();
// Is variable modified inside for loop // Is variable modified inside for loop
if (isVariableChanged(bodyStart, bodyEnd, varid, globalvar, settings)) if (isVariableChanged(bodyStart, bodyEnd, varid, globalvar, settings, tokenlist->isCPP()))
return; return;
for (Token *tok2 = bodyStart->next(); tok2 != bodyEnd; tok2 = tok2->next()) { for (Token *tok2 = bodyStart->next(); tok2 != bodyEnd; tok2 = tok2->next()) {

11
test/testastutils.cpp
View File

@ -64,16 +64,15 @@ private:
tokenizer.tokenize(istr, "test.cpp"); tokenizer.tokenize(istr, "test.cpp");
const Token * const tok1 = Token::findsimplematch(tokenizer.tokens(), startPattern); const Token * const tok1 = Token::findsimplematch(tokenizer.tokens(), startPattern);
const Token * const tok2 = Token::findsimplematch(tokenizer.tokens(), endPattern); const Token * const tok2 = Token::findsimplematch(tokenizer.tokens(), endPattern);
return ::isVariableChanged(tok1,tok2,1,false,&settings); return ::isVariableChanged(tok1,tok2,1,false,&settings,true);
} }
void isVariableChanged() { void isVariableChanged() {
// #8211 - no lhs for >> , do not crash // #8211 - no lhs for >> , do not crash
ASSERT_EQUALS(true, isVariableChanged("void f() {\n"
isVariableChanged("void f() {\n" " int b;\n"
" int b;\n" " if (b) { (int)((INTOF(8))result >> b); }\n"
" if (b) { (int)((INTOF(8))result >> b); }\n" "}", "if", "}");
"}", "if", "}"));
} }
bool isVariableChangedByFunctionCall(const char code[], const char pattern[], bool *inconclusive) { bool isVariableChangedByFunctionCall(const char code[], const char pattern[], bool *inconclusive) {

7
test/testunusedvar.cpp
View File

@ -1783,13 +1783,6 @@ private:
"}"); "}");
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("", errout.str());
functionVariableUsage("void f() {\n"
" int x, y;\n"
" std::cin >> (x >> y);\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (style) Variable 'x' is not assigned a value.\n"
"[test.cpp:2]: (style) Variable 'y' is not assigned a value.\n", errout.str());
// ticket #8494 // ticket #8494
functionVariableUsage("void f(C c) {\n" functionVariableUsage("void f(C c) {\n"
" int x;\n" " int x;\n"