From 58eb644003630c3dba3d9c4656fe5c53e4d21944 Mon Sep 17 00:00:00 2001 From: Harald Scheidl Date: Sat, 8 Oct 2016 01:57:09 +0200 Subject: [PATCH] Improved Check: Warn about number and char literals in boolean expressions (#7750) --- lib/checkcondition.cpp | 9 ++++++--- lib/tokenize.cpp | 12 ++++++------ lib/tokenize.h | 2 +- test/testcondition.cpp | 16 +++++++++++++++- 4 files changed, 28 insertions(+), 11 deletions(-) diff --git a/lib/checkcondition.cpp b/lib/checkcondition.cpp index ede34d27a..0b29b74e0 100644 --- a/lib/checkcondition.cpp +++ b/lib/checkcondition.cpp @@ -975,7 +975,12 @@ void CheckCondition::alwaysTrueFalse() for (std::size_t i = 0; i < functions; ++i) { const Scope * scope = symbolDatabase->functionScopes[i]; for (const Token* tok = scope->classStart->next(); tok != scope->classEnd; tok = tok->next()) { - if (!Token::Match(tok, "%comp%|!")) + + const bool constValCond = Token::Match(tok->tokAt(-2), "if|while ( %num%|%char% )"); // just one number or char inside if|while + const bool constValExpr = Token::Match(tok, "%num%|%char%") && tok->astParent() && Token::Match(tok->astParent(),"&&|%oror%|?"); // just one number or char in boolean expression + const bool compExpr = Token::Match(tok, "%comp%|!"); // a compare expression + + if (!(constValCond || constValExpr || compExpr)) continue; if (tok->link()) // don't write false positives when templates are used continue; @@ -983,8 +988,6 @@ void CheckCondition::alwaysTrueFalse() continue; if (!tok->values.front().isKnown()) continue; - if (!tok->astParent() || !Token::Match(tok->astParent()->previous(), "%name% (")) - continue; // Don't warn in assertions. Condition is often 'always true' by intention. // If platform,defines,etc cause 'always false' then that is not dangerous neither. diff --git a/lib/tokenize.cpp b/lib/tokenize.cpp index 9f7732231..7ea58c03b 100644 --- a/lib/tokenize.cpp +++ b/lib/tokenize.cpp @@ -1859,14 +1859,14 @@ void Tokenizer::combineOperators() } } -void Tokenizer::combineStrings() +void Tokenizer::combineStringAndCharLiterals() { - // Combine wide strings + // Combine wide strings and wide characters for (Token *tok = list.front(); tok; tok = tok->next()) { - while (tok->str() == "L" && tok->next() && tok->next()->tokType() == Token::eString) { - // Combine 'L "string"' + if (tok->str() == "L" && tok->next() && (tok->next()->tokType() == Token::eString || tok->next()->tokType() == Token::eChar)) { + // Combine 'L "string"' and 'L 'c'' tok->str(tok->next()->str()); tok->deleteNext(); tok->isLong(true); @@ -3326,8 +3326,8 @@ bool Tokenizer::simplifyTokenList1(const char FileName[]) // remove MACRO in variable declaration: MACRO int x; removeMacroInVarDecl(); - // Combine strings - combineStrings(); + // Combine strings and character literals, e.g. L"string", L'c', "string1" "string2" + combineStringAndCharLiterals(); // replace inline SQL with "asm()" (Oracle PRO*C). Ticket: #1959 simplifySQL(); diff --git a/lib/tokenize.h b/lib/tokenize.h index f6457abef..bf805e759 100644 --- a/lib/tokenize.h +++ b/lib/tokenize.h @@ -466,7 +466,7 @@ public: void combineOperators(); - void combineStrings(); + void combineStringAndCharLiterals(); void simplifyNull(); diff --git a/test/testcondition.cpp b/test/testcondition.cpp index 4c0143f19..4bdfb9b50 100644 --- a/test/testcondition.cpp +++ b/test/testcondition.cpp @@ -1552,7 +1552,8 @@ private: check("void f() {\n" " if (x & 3 == 2) {}\n" "}"); - ASSERT_EQUALS("[test.cpp:2]: (style) Suspicious condition (bitwise operator + comparison); Clarify expression with parentheses.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:2]: (style) Suspicious condition (bitwise operator + comparison); Clarify expression with parentheses.\n" + "[test.cpp:2]: (style) Condition '3==2' is always false\n", errout.str()); check("void f() {\n" " if (a & fred1.x == fred2.y) {}\n" @@ -1726,6 +1727,19 @@ private: " assert(x == 0);\n" "}"); ASSERT_EQUALS("", errout.str()); + + // #7750 warn about number and char literals in boolean expressions + check("void f() {\n" + " if('a'){}\n" + " if(L'b'){}\n" + " if(1 && 'c'){}\n" + " int x = 'd' ? 1 : 2;\n" + "}"); + ASSERT_EQUALS("[test.cpp:2]: (style) Condition ''a'' is always true\n" + "[test.cpp:3]: (style) Condition ''b'' is always true\n" + "[test.cpp:4]: (style) Condition '1' is always true\n" + "[test.cpp:4]: (style) Condition ''c'' is always true\n" + "[test.cpp:5]: (style) Condition ''d'' is always true\n", errout.str()); } void checkInvalidTestForOverflow() {