diff --git a/lib/checkassignif.cpp b/lib/checkassignif.cpp index 484d587af..deffba3ed 100644 --- a/lib/checkassignif.cpp +++ b/lib/checkassignif.cpp @@ -54,29 +54,39 @@ void CheckAssignIf::assignIf() for (const Token *tok2 = tok->tokAt(4); tok2; tok2 = tok2->next()) { if (tok2->str() == "(" || tok2->str() == "}" || tok2->str() == "=") break; - if (Token::Match(tok2,"if ( (| %varid% %any% %num% &&|%oror%|)", varid)) { - const Token *vartok = tok2->tokAt(tok2->strAt(2)=="(" ? 3 : 2); - const std::string& op(vartok->strAt(1)); - const MathLib::bigint num2 = MathLib::toLongNumber(vartok->strAt(2)); - std::string condition; - if (Token::simpleMatch(tok2, "if ( (")) - condition = "'" + vartok->str() + op + vartok->strAt(2) + "'"; - if (op == "==" && (num & num2) != ((bitop=='&') ? num2 : num)) - assignIfError(tok2, condition, false); - else if (op == "!=" && (num & num2) != ((bitop=='&') ? num2 : num)) - assignIfError(tok2, condition, true); - 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); + } + } } } } } } -void CheckAssignIf::assignIfError(const Token *tok, const std::string &condition, bool result) +void CheckAssignIf::assignIfError(const Token *tok1, const Token *tok2, const std::string &condition, bool result) { - reportError(tok, Severity::style, + std::list locations; + locations.push_back(tok1); + locations.push_back(tok2); + + reportError(locations, + Severity::style, "assignIfError", - "Mismatching assignment and comparison, comparison " + condition + (condition.empty()?"":" ") + "is always " + std::string(result ? "true" : "false") + "."); + "Mismatching assignment and comparison, comparison '" + condition + "' is always " + std::string(result ? "true" : "false") + "."); } diff --git a/lib/checkassignif.h b/lib/checkassignif.h index 7c1f50d89..a31745f7b 100644 --- a/lib/checkassignif.h +++ b/lib/checkassignif.h @@ -63,7 +63,7 @@ public: private: - void assignIfError(const Token *tok, const std::string &condition, bool result); + void assignIfError(const Token *tok1, const Token *tok2, const std::string &condition, bool result); void comparisonError(const Token *tok, const std::string &bitop, @@ -76,7 +76,7 @@ private: void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const { CheckAssignIf c(0, settings, errorLogger); - c.assignIfError(0, "", false); + c.assignIfError(0, 0, "", false); c.comparisonError(0, "&", 6, "==", 1, false); c.multiConditionError(0,1); } diff --git a/test/testassignif.cpp b/test/testassignif.cpp index 5701687aa..33fe6fc03 100644 --- a/test/testassignif.cpp +++ b/test/testassignif.cpp @@ -65,33 +65,46 @@ private: " int y = x & 4;\n" " if (y == 3);\n" "}\n"); - ASSERT_EQUALS("[test.cpp:4]: (style) Mismatching assignment and comparison, comparison is always false.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (style) Mismatching assignment and comparison, comparison 'y==3' is always false.\n", errout.str()); check("void foo(int x)\n" "{\n" " int y = x & 4;\n" " if (y != 3);\n" "}\n"); - ASSERT_EQUALS("[test.cpp:4]: (style) Mismatching assignment and comparison, comparison is always true.\n", errout.str()); - - check("void foo(int x) {\n" - " int y = x & 4;\n" - " if ((y == 3) && (z == 1));\n" - "}\n"); - ASSERT_EQUALS("[test.cpp:3]: (style) Mismatching assignment and comparison, comparison 'y==3' is always false.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (style) Mismatching assignment and comparison, comparison 'y!=3' is always true.\n", errout.str()); // | check("void foo(int x) {\n" " int y = x | 0x14;\n" " if (y == 0x710);\n" "}"); - ASSERT_EQUALS("[test.cpp:3]: (style) Mismatching assignment and comparison, comparison is always false.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) Mismatching assignment and comparison, comparison 'y==1808' is always false.\n", errout.str()); check("void foo(int x) {\n" " int y = x | 0x14;\n" " if (y == 0x71f);\n" "}"); ASSERT_EQUALS("", errout.str()); + + // multiple conditions + check("void foo(int x) {\n" + " int y = x & 4;\n" + " if ((y == 3) && (z == 1));\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) Mismatching assignment and comparison, comparison 'y==3' is always false.\n", errout.str()); + + check("void foo(int x) {\n" + " int y = x & 4;\n" + " if ((x==123) || ((y == 3) && (z == 1)));\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) Mismatching assignment and comparison, comparison 'y==3' is always false.\n", errout.str()); + + check("void f(int x) {\n" + " int y = x & 7;\n" + " if (setvalue(&y) && y != 8);\n" + "}"); + ASSERT_EQUALS("", errout.str()); } void compare() {