From 12cfdee61bf6aff310e87240026b25db8eb72a1e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Sat, 29 Sep 2012 10:33:54 +0200 Subject: [PATCH] AssignIf: Check into scopes recursively --- lib/checkassignif.cpp | 65 ++++++++++++++++++++++++++++--------------- lib/checkassignif.h | 7 +++++ test/testassignif.cpp | 8 ++++++ 3 files changed, 58 insertions(+), 22 deletions(-) diff --git a/lib/checkassignif.cpp b/lib/checkassignif.cpp index deffba3ed..641a70693 100644 --- a/lib/checkassignif.cpp +++ b/lib/checkassignif.cpp @@ -51,32 +51,53 @@ void CheckAssignIf::assignIf() if (num < 0) continue; - for (const Token *tok2 = tok->tokAt(4); tok2; tok2 = tok2->next()) { - if (tok2->str() == "(" || tok2->str() == "}" || tok2->str() == "=") - break; - if (Token::Match(tok2, "if|while|for (")) { - // parse condition - const Token * const end = tok2->next()->link(); - for (; tok2 != end; tok2 = tok2->next()) { - if (Token::Match(tok2, "[(,] &| %varid% [,)]", varid)) - break; - if (Token::Match(tok2,"&&|%oror%|( %varid% %any% %num% &&|%oror%|)", varid)) { - const Token *vartok = tok2->next(); - const std::string& op(vartok->strAt(1)); - const MathLib::bigint num2 = MathLib::toLongNumber(vartok->strAt(2)); - const std::string condition(vartok->str() + op + vartok->strAt(2)); - if (op == "==" && (num & num2) != ((bitop=='&') ? num2 : num)) - assignIfError(tok, tok2, condition, false); - else if (op == "!=" && (num & num2) != ((bitop=='&') ? num2 : num)) - assignIfError(tok, tok2, condition, true); - } - } - } - } + assignIfParseScope(tok, tok->tokAt(4), varid, bitop, num); } } } +/** parse scopes recursively */ +bool CheckAssignIf::assignIfParseScope(const Token * const assignTok, + const Token * const startTok, + const unsigned int varid, + const char bitop, + const MathLib::bigint num) +{ + for (const Token *tok2 = startTok; tok2; tok2 = tok2->next()) { + if (Token::Match(tok2, "[(,] &| %varid% [,)]")) + return true; + if (tok2->str() == "}") + return false; + if (Token::Match(tok2, "if (")) { + // parse condition + const Token * const end = tok2->next()->link(); + for (; tok2 != end; tok2 = tok2->next()) { + if (Token::Match(tok2, "[(,] &| %varid% [,)]", varid)) { + return true; + } + if (Token::Match(tok2,"&&|%oror%|( %varid% %any% %num% &&|%oror%|)", varid)) { + const Token *vartok = tok2->next(); + const std::string& op(vartok->strAt(1)); + const MathLib::bigint num2 = MathLib::toLongNumber(vartok->strAt(2)); + const std::string condition(vartok->str() + op + vartok->strAt(2)); + if (op == "==" && (num & num2) != ((bitop=='&') ? num2 : num)) + assignIfError(assignTok, tok2, condition, false); + else if (op == "!=" && (num & num2) != ((bitop=='&') ? num2 : num)) + assignIfError(assignTok, tok2, condition, true); + } + } + + bool ret1 = assignIfParseScope(assignTok, end->tokAt(2), varid, bitop, num); + bool ret2 = false; + if (Token::simpleMatch(end->next()->link(), "} else {")) + ret2 = assignIfParseScope(assignTok, end->next()->link()->tokAt(3), varid, bitop, num); + if (ret1 || ret2) + return true; + } + } + return false; +} + void CheckAssignIf::assignIfError(const Token *tok1, const Token *tok2, const std::string &condition, bool result) { std::list locations; diff --git a/lib/checkassignif.h b/lib/checkassignif.h index a31745f7b..901e6dbe9 100644 --- a/lib/checkassignif.h +++ b/lib/checkassignif.h @@ -55,6 +55,13 @@ public: /** mismatching assignment / comparison */ void assignIf(); + /** parse scopes recursively */ + bool assignIfParseScope(const Token * const assignTok, + const Token * const startTok, + const unsigned int varid, + const char bitop, + const MathLib::bigint num); + /** mismatching lhs and rhs in comparison */ void comparison(); diff --git a/test/testassignif.cpp b/test/testassignif.cpp index 33fe6fc03..80f053d0d 100644 --- a/test/testassignif.cpp +++ b/test/testassignif.cpp @@ -105,6 +105,14 @@ private: " if (setvalue(&y) && y != 8);\n" "}"); ASSERT_EQUALS("", errout.str()); + + // recursive checking into scopes + check("void f(int x) {\n" + " int y = x & 7;\n" + " if (z) y=0;\n" + " else if (y==8);\n" // always false + "}"); + ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:4]: (style) Mismatching assignment and comparison, comparison 'y==8' is always false.\n", errout.str()); } void compare() {