Skip to content

Commit 77b653b

Browse files
danmardanmar
committed
Clarify warnings when char literals are converted to bool in conditions
1 parent f862cf6 commit 77b653b

5 files changed

Lines changed: 28 additions & 13 deletions

File tree

‎lib/checkcondition.cpp‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1184,16 +1184,18 @@ void CheckCondition::alwaysTrueFalse()
11841184
continue;
11851185
if (!tok->hasKnownIntValue())
11861186
continue;
1187-
if (Token::Match(tok, "%num%|%bool%"))
1187+
if (Token::Match(tok, "%num%|%bool%|%char%"))
11881188
continue;
1189-
if (Token::Match(tok, "! %num%|%bool%"))
1189+
if (Token::Match(tok, "! %num%|%bool%|%char%"))
1190+
continue;
1191+
if (Token::Match(tok, "%oror%|&&"))
11901192
continue;
11911193

11921194
const bool constIfWhileExpression =
11931195
tok->astParent()
11941196
&& Token::Match(tok->astParent()->astOperand1(), "if|while")
11951197
&& !tok->isBoolean();
1196-
const bool constValExpr = Token::Match(tok, "%num%|%char%") && Token::Match(tok->astParent(),"%oror%|&&|?"); // just one number or char in boolean expression
1198+
const bool constValExpr = tok->isNumber() && Token::Match(tok->astParent(),"%oror%|&&|?"); // just one number in boolean expression
11971199
const bool compExpr = Token::Match(tok, "%comp%|!"); // a compare expression
11981200

11991201
if (!(constIfWhileExpression || constValExpr || compExpr))

‎lib/checkstring.cpp‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -305,9 +305,9 @@ void CheckString::checkIncorrectStringCompare()
305305
incorrectStringCompareError(tok->next(), "substr", end->strAt(1));
306306
}
307307
}
308-
} else if (Token::Match(tok, "&&|%oror%|( %str% &&|%oror%|)") && !Token::Match(tok, "( %str% )")) {
308+
} else if (Token::Match(tok, "&&|%oror%|( %str%|%char% &&|%oror%|)") && !Token::Match(tok, "( %str%|%char% )")) {
309309
incorrectStringBooleanError(tok->next(), tok->strAt(1));
310-
} else if (Token::Match(tok, "if|while ( %str% )")) {
310+
} else if (Token::Match(tok, "if|while ( %str%|%char% )")) {
311311
incorrectStringBooleanError(tok->tokAt(2), tok->strAt(2));
312312
}
313313
}
@@ -321,7 +321,11 @@ void CheckString::incorrectStringCompareError(const Token *tok, const std::strin
321321

322322
void CheckString::incorrectStringBooleanError(const Token *tok, const std::string& string)
323323
{
324-
reportError(tok, Severity::warning, "incorrectStringBooleanError", "Conversion of string literal " + string + " to bool always evaluates to true.", CWE571, false);
324+
const bool charLiteral = string[0] == '\'';
325+
reportError(tok,
326+
Severity::warning,
327+
charLiteral ? "incorrectCharBooleanError" : "incorrectStringBooleanError",
328+
"Conversion of " + std::string(charLiteral ? "char" : "string") + " literal " + string + " to bool always evaluates to true.", CWE571, false);
325329
}
326330

327331
//---------------------------------------------------------------------------

‎lib/checkstring.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,7 @@ class CPPCHECKLIB CheckString : public Check {
113113
c.suspiciousStringCompareError(nullptr, "foo");
114114
c.suspiciousStringCompareError_char(nullptr, "foo");
115115
c.incorrectStringBooleanError(nullptr, "\"Hello World\"");
116+
c.incorrectStringBooleanError(nullptr, "\'x\'");
116117
c.alwaysTrueFalseStringCompareError(nullptr, "str1", "str2");
117118
c.alwaysTrueStringVariableCompareError(nullptr, "varname1", "varname2");
118119
c.overlappingStrcmpError(nullptr, nullptr);
@@ -127,7 +128,7 @@ class CPPCHECKLIB CheckString : public Check {
127128
"- overlapping buffers passed to sprintf as source and destination\n"
128129
"- incorrect length arguments for 'substr' and 'strncmp'\n"
129130
"- suspicious condition (runtime comparison of string literals)\n"
130-
"- suspicious condition (string literals as boolean)\n"
131+
"- suspicious condition (string/char literals as boolean)\n"
131132
"- suspicious comparison of a string literal with a char* variable\n"
132133
"- suspicious comparison of '\\0' with a char* variable\n"
133134
"- overlapping strcmp() expression\n";

‎test/testcondition.cpp‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2419,18 +2419,14 @@ class TestCondition : public TestFixture {
24192419
"}\n");
24202420
ASSERT_EQUALS("", errout.str());
24212421

2422-
// #7750 warn about char literals in boolean expressions
2422+
// #7750 char literals in boolean expressions
24232423
check("void f() {\n"
24242424
" if('a'){}\n"
24252425
" if(L'b'){}\n"
24262426
" if(1 && 'c'){}\n"
24272427
" int x = 'd' ? 1 : 2;\n"
24282428
"}");
2429-
ASSERT_EQUALS("[test.cpp:2]: (style) Condition ''a'' is always true\n"
2430-
"[test.cpp:3]: (style) Condition 'L'b'' is always true\n"
2431-
"[test.cpp:4]: (style) Condition '1&&'c'' is always true\n"
2432-
"[test.cpp:4]: (style) Condition ''c'' is always true\n"
2433-
"[test.cpp:5]: (style) Condition ''d'' is always true\n", errout.str());
2429+
ASSERT_EQUALS("", errout.str());
24342430

24352431
// Skip literals
24362432
check("void f() { if(true) {} }");

‎test/teststring.cpp‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -579,6 +579,18 @@ class TestString : public TestFixture {
579579
" return f2(\"Hello\");\n"
580580
"}");
581581
ASSERT_EQUALS("", errout.str());
582+
583+
// #7750 warn about char literals in boolean expressions
584+
check("void f() {\n"
585+
" if('a'){}\n"
586+
" if(L'b'){}\n"
587+
" if(1 && 'c'){}\n"
588+
" int x = 'd' ? 1 : 2;\n" // <- TODO
589+
"}");
590+
ASSERT_EQUALS("[test.cpp:2]: (warning) Conversion of char literal 'a' to bool always evaluates to true.\n"
591+
"[test.cpp:3]: (warning) Conversion of char literal 'b' to bool always evaluates to true.\n"
592+
"[test.cpp:4]: (warning) Conversion of char literal 'c' to bool always evaluates to true.\n"
593+
, errout.str());
582594
}
583595

584596
void deadStrcmp() {

0 commit comments

Comments
 (0)