From 8647e4c0d0aa2745b5ba09e35dde1ff4ef08436b Mon Sep 17 00:00:00 2001 From: Zachary Blair Date: Fri, 18 Jan 2013 16:34:15 -0800 Subject: [PATCH] Fixed a false positive in #4109 (if (c == 1) c == 0; Isn't picked up) --- lib/checkother.cpp | 12 +++++++----- test/testother.cpp | 21 +++++++++++++++++++++ 2 files changed, 28 insertions(+), 5 deletions(-) diff --git a/lib/checkother.cpp b/lib/checkother.cpp index a8c39086d..57d69a100 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -1117,6 +1117,8 @@ void CheckOther::checkSuspiciousEqualityComparison() for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) { if (Token::simpleMatch(tok, "for (")) { + const Token* const openParen = tok->next(); + const Token* const closeParen = tok->linkAt(1); // Search for any suspicious equality comparison in the initialization // or increment-decrement parts of the for() loop. @@ -1124,8 +1126,8 @@ void CheckOther::checkSuspiciousEqualityComparison() // for (i == 2; i < 10; i++) // or // for (i = 0; i < 10; i == a) - const Token* tok2 = Token::findmatch(tok->next(), "[;(] %var% == %any% [;)]", tok->linkAt(1)); - if (tok2 && (tok2->str() == "(" || tok2->strAt(4) == ")")) { + const Token* tok2 = Token::findmatch(openParen, "[;(] %var% == %any% [;)]", closeParen); + if (tok2 && (tok2 == openParen || tok2->tokAt(4) == closeParen)) { suspiciousEqualityComparisonError(tok2->tokAt(2)); } @@ -1134,14 +1136,14 @@ void CheckOther::checkSuspiciousEqualityComparison() // in the initialization or increment-decrement parts of the for() loop. // For example: // for (!i; i < 10; i++) - const Token* tok3 = Token::findmatch(tok->next(), "[;(] ! %var% [;)]", tok->linkAt(1)); - if (tok3 && (tok3->str() == "(" || tok3->strAt(3) == ")")) { + const Token* tok3 = Token::findmatch(openParen, "[;(] ! %var% [;)]", closeParen); + if (tok3 && (tok3 == openParen || tok3->tokAt(3) == closeParen)) { suspiciousEqualityComparisonError(tok3->tokAt(2)); } // Skip over for() loop conditions because "for (;running==1;)" // is a bit strange, but not necessarily incorrect. - tok = tok->linkAt(1); + tok = closeParen; } if (Token::Match(tok, "[;{}] *| %var% == %any% ;")) { diff --git a/test/testother.cpp b/test/testother.cpp index 46cb92fff..b03ed6316 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -2864,6 +2864,27 @@ private: " printf(\"%i\n\", ({x == 0; x > 0 ? 10 : 20}));\n" "}"); ASSERT_EQUALS("[test.cpp:2]: (warning, inconclusive) Found suspicious equality comparison. Did you intend to assign a value instead?\n", errout.str()); + + check("void foo(int x) {\n" + " for (const Token* const end = tok->link(); tok != end; tok = (tok == end) ? end : tok->next()) {\n" + " x++;\n" + " }\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + check("void foo(int x) {\n" + " for (int i = (x == 0) ? 0 : 5; i < 10; i ++) {\n" + " x++;\n" + " }\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + check("void foo(int x) {\n" + " for (int i = 0; i < 10; i += (x == 5) ? 1 : 2) {\n" + " x++;\n" + " }\n" + "}"); + ASSERT_EQUALS("", errout.str()); } void selfAssignment() {