From 7ee636b9340f76cbfa7ec8f448809d1c7771f5e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Wed, 18 Apr 2018 17:45:43 +0200 Subject: [PATCH] Refactoring: Reuse isLikelyStreamRead in isVariableChanged --- lib/astutils.cpp | 12 +++--------- lib/astutils.h | 2 +- lib/checkcondition.cpp | 4 ++-- lib/valueflow.cpp | 36 ++++++++++++++++++------------------ test/testastutils.cpp | 11 +++++------ test/testunusedvar.cpp | 7 ------- 6 files changed, 29 insertions(+), 43 deletions(-) diff --git a/lib/astutils.cpp b/lib/astutils.cpp index 6c806d5b1..dc56763bc 100644 --- a/lib/astutils.cpp +++ b/lib/astutils.cpp @@ -528,7 +528,7 @@ bool isVariableChangedByFunctionCall(const Token *tok, const Settings *settings, 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()) { 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%")) return true; - if (Token::simpleMatch(tok->previous(), ">>")) { - const Token *shr = tok->previous(); - if (Token::simpleMatch(shr->astParent(), ">>")) - return true; - const Token *lhs = shr->astOperand1(); - if (!lhs || !lhs->valueType() || !lhs->valueType()->isIntegral()) - return true; - } + if (isLikelyStreamRead(cpp, tok->previous())) + return true; const Token *ftok = tok; while (ftok && !Token::Match(ftok, "[({[]")) diff --git a/lib/astutils.h b/lib/astutils.h index c9759865b..912c05587 100644 --- a/lib/astutils.h +++ b/lib/astutils.h @@ -100,7 +100,7 @@ bool isVariableChangedByFunctionCall(const Token *tok, unsigned int varid, const bool isVariableChangedByFunctionCall(const Token *tok, const Settings *settings, bool *inconclusive); /** 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 * @param start token which is supposed to be the function/macro name. diff --git a/lib/checkcondition.cpp b/lib/checkcondition.cpp index 2438a3ac7..a1081e8a4 100644 --- a/lib/checkcondition.cpp +++ b/lib/checkcondition.cpp @@ -168,7 +168,7 @@ bool CheckCondition::assignIfParseScope(const Token * const assignTok, // is variable changed in loop? const Token *bodyStart = tok2->linkAt(1)->next(); 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; } @@ -629,7 +629,7 @@ void CheckCondition::multiCondition2() } bool changed = false; for (std::set::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; break; } diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index c56c76c03..393611556 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -1228,7 +1228,7 @@ static void valueFlowReverse(TokenList *tokenlist, const Token *start = tok2; 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) bailout(tokenlist, errorLogger, tok2, "variable " + var->name() + " is assigned in loop. so valueflow analysis bailout when start of loop is reached."); break; @@ -1308,7 +1308,7 @@ static void valueFlowBeforeCondition(TokenList *tokenlist, SymbolDatabase *symbo // Variable changed in 3rd for-expression 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; if (settings->debugwarnings) 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 end = start->link(); - if (isVariableChanged(start,end,varid,var->isGlobal(),settings)) { + if (isVariableChanged(start,end,varid,var->isGlobal(),settings, tokenlist->isCPP())) { varid = 0U; if (settings->debugwarnings) bailout(tokenlist, errorLogger, tok, "variable " + var->name() + " used in loop"); @@ -1425,7 +1425,7 @@ static void handleKnownValuesInLoop(const Token *startToken, for (std::list::iterator it = values->begin(); it != values->end(); ++it) { if (it->isKnown()) { if (!isChanged) { - if (!isVariableChanged(startToken, endToken, varid, globalvar, settings)) + if (!isVariableChanged(startToken, endToken, varid, globalvar, settings, true)) break; isChanged = true; } @@ -1530,7 +1530,7 @@ static bool valueFlowForward(Token * const startToken, bailoutflag = true; 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++); else ++it; @@ -1546,7 +1546,7 @@ static bool valueFlowForward(Token * const startToken, } else if (indentlevel <= 0 && Token::simpleMatch(tok2->link()->previous(), "else {") && !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); } } @@ -1599,7 +1599,7 @@ static bool valueFlowForward(Token * const startToken, if (Token::simpleMatch(end, "} while (")) 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) bailout(tokenlist, errorLogger, tok2, "variable " + var->name() + " valueFlowForward, assignment in do-while"); return false; @@ -1611,7 +1611,7 @@ static bool valueFlowForward(Token * const startToken, // conditional block of code that assigns variable.. else if (!tok2->varId() && Token::Match(tok2, "%name% (") && Token::simpleMatch(tok2->linkAt(1), ") {")) { // 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) bailout(tokenlist, errorLogger, tok2, "variable " + var->name() + " valueFlowForward, assignment in condition"); return false; @@ -1674,7 +1674,7 @@ static bool valueFlowForward(Token * const startToken, errorLogger, 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); changeKnownToPossible(values); } @@ -1702,7 +1702,7 @@ static bool valueFlowForward(Token * const startToken, errorLogger, 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); changeKnownToPossible(values); } @@ -1723,7 +1723,7 @@ static bool valueFlowForward(Token * const startToken, Token * const start = tok2->linkAt(1)->next(); Token * const end = start->link(); 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)); if (!read) { read = bool(nullptr != Token::findmatch(tok2, "%varid% !!=", end, varid)); @@ -1792,7 +1792,7 @@ static bool valueFlowForward(Token * const startToken, 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) && Token::simpleMatch(tok2, "if (") && !(Token::simpleMatch(end, "} else {") && @@ -1941,8 +1941,8 @@ static bool valueFlowForward(Token * const startToken, } tok2 = tok2->next(); - if (isVariableChanged(questionToken, questionToken->astOperand2(), varid, false, settings) && - isVariableChanged(questionToken->astOperand2(), tok2, varid, false, settings)) { + if (isVariableChanged(questionToken, questionToken->astOperand2(), varid, false, settings, tokenlist->isCPP()) && + isVariableChanged(questionToken->astOperand2(), tok2, varid, false, settings, tokenlist->isCPP())) { if (settings->debugwarnings) bailout(tokenlist, errorLogger, tok2, "variable " + var->name() + " valueFlowForward, assignment in condition"); return false; @@ -2117,7 +2117,7 @@ static bool valueFlowForward(Token * const startToken, Token::simpleMatch(tok2->linkAt(1), "] (") && Token::simpleMatch(tok2->linkAt(1)->linkAt(1), ") {")) { 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) bailout(tokenlist, errorLogger, tok2, "valueFlowForward, " + var->name() + " is changed in lambda function"); return false; @@ -2486,7 +2486,7 @@ static void valueFlowAfterCondition(TokenList *tokenlist, SymbolDatabase* symbol // does condition reassign variable? if (tok != top->astOperand2() && 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) bailout(tokenlist, errorLogger, tok, "assignment in condition"); 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); 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 if (settings->debugwarnings) 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(); // Is variable modified inside for loop - if (isVariableChanged(bodyStart, bodyEnd, varid, globalvar, settings)) + if (isVariableChanged(bodyStart, bodyEnd, varid, globalvar, settings, tokenlist->isCPP())) return; for (Token *tok2 = bodyStart->next(); tok2 != bodyEnd; tok2 = tok2->next()) { diff --git a/test/testastutils.cpp b/test/testastutils.cpp index 8faabe3cb..b07454f21 100644 --- a/test/testastutils.cpp +++ b/test/testastutils.cpp @@ -64,16 +64,15 @@ private: tokenizer.tokenize(istr, "test.cpp"); const Token * const tok1 = Token::findsimplematch(tokenizer.tokens(), startPattern); 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() { // #8211 - no lhs for >> , do not crash - ASSERT_EQUALS(true, - isVariableChanged("void f() {\n" - " int b;\n" - " if (b) { (int)((INTOF(8))result >> b); }\n" - "}", "if", "}")); + isVariableChanged("void f() {\n" + " int b;\n" + " if (b) { (int)((INTOF(8))result >> b); }\n" + "}", "if", "}"); } bool isVariableChangedByFunctionCall(const char code[], const char pattern[], bool *inconclusive) { diff --git a/test/testunusedvar.cpp b/test/testunusedvar.cpp index 10f206dcf..e92961195 100644 --- a/test/testunusedvar.cpp +++ b/test/testunusedvar.cpp @@ -1783,13 +1783,6 @@ private: "}"); 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 functionVariableUsage("void f(C c) {\n" " int x;\n"