Remove checks that are already covered well by most compilers (Unreachable Code; Assignment in Condition; Unused Variable).
This commit is contained in:
parent
53d02c0804
commit
42c608b6f0
|
@ -371,25 +371,6 @@ void CheckOther::InvalidFunctionUsage()
|
|||
//---------------------------------------------------------------------------
|
||||
|
||||
|
||||
//---------------------------------------------------------------------------
|
||||
// Assignment in condition
|
||||
//---------------------------------------------------------------------------
|
||||
|
||||
void CheckOther::CheckIfAssignment()
|
||||
{
|
||||
for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next())
|
||||
{
|
||||
if (Token::Match(tok, "if ( %var% = %num% )") ||
|
||||
Token::Match(tok, "if ( %var% = %str% )") ||
|
||||
Token::Match(tok, "if ( %var% = %var% )"))
|
||||
{
|
||||
_errorLogger->reportErr(ErrorMessage::ifAssignment(_tokenizer, tok));
|
||||
}
|
||||
}
|
||||
}
|
||||
//---------------------------------------------------------------------------
|
||||
|
||||
|
||||
//---------------------------------------------------------------------------
|
||||
// Check for unsigned divisions
|
||||
//---------------------------------------------------------------------------
|
||||
|
@ -797,179 +778,6 @@ void CheckOther::CheckIncompleteStatement()
|
|||
|
||||
|
||||
|
||||
//---------------------------------------------------------------------------
|
||||
// Unreachable code below a 'return'
|
||||
//---------------------------------------------------------------------------
|
||||
|
||||
void CheckOther::unreachableCode()
|
||||
{
|
||||
const Token *tok = _tokenizer->tokens();
|
||||
while ((tok = Token::findmatch(tok, "[;{}] return")))
|
||||
{
|
||||
// Goto the 'return' token
|
||||
tok = tok->next();
|
||||
|
||||
// Locate the end of the 'return' statement
|
||||
while (tok && tok->str() != ";")
|
||||
tok = tok->next();
|
||||
while (tok && tok->next() && tok->next()->str() == ";")
|
||||
tok = tok->next();
|
||||
|
||||
if (!tok)
|
||||
break;
|
||||
|
||||
// If there is a statement below the return it is unreachable
|
||||
if (!Token::Match(tok, "; case|default|}|#") && !Token::Match(tok, "; %var% :")
|
||||
&& (_settings._checkCodingStyle || !Token::simpleMatch(tok, "; break")))
|
||||
{
|
||||
_errorLogger->reportErr(ErrorMessage::unreachableCode(_tokenizer, tok->next()));
|
||||
}
|
||||
}
|
||||
}
|
||||
//---------------------------------------------------------------------------
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
//---------------------------------------------------------------------------
|
||||
// Usage of function variables
|
||||
//---------------------------------------------------------------------------
|
||||
|
||||
static bool isOp(const Token *tok)
|
||||
{
|
||||
return bool(tok &&
|
||||
(tok->str() == "&&" ||
|
||||
tok->str() == "||" ||
|
||||
tok->str() == "==" ||
|
||||
tok->str() == "!=" ||
|
||||
tok->str() == "<" ||
|
||||
tok->str() == "<=" ||
|
||||
tok->str() == ">" ||
|
||||
tok->str() == ">=" ||
|
||||
tok->str() == "<<" ||
|
||||
Token::Match(tok, "[+-*/%&!~|^,[])?:]")));
|
||||
}
|
||||
|
||||
void CheckOther::functionVariableUsage()
|
||||
{
|
||||
// Parse all executing scopes..
|
||||
const Token *tok1 = _tokenizer->tokens();
|
||||
if (!tok1)
|
||||
return;
|
||||
|
||||
while ((tok1 = Token::findmatch(tok1->next(), ") const| {")) != NULL)
|
||||
{
|
||||
// Varname, usage {1=declare, 2=read, 4=write}
|
||||
std::map<std::string, unsigned int> varUsage;
|
||||
static const unsigned int USAGE_DECLARE = 1;
|
||||
static const unsigned int USAGE_READ = 2;
|
||||
static const unsigned int USAGE_WRITE = 4;
|
||||
|
||||
int indentlevel = 0;
|
||||
for (const Token *tok = tok1->next(); tok; tok = tok->next())
|
||||
{
|
||||
if (tok->str() == "{")
|
||||
++indentlevel;
|
||||
else if (tok->str() == "}")
|
||||
{
|
||||
--indentlevel;
|
||||
if (indentlevel <= 0)
|
||||
break;
|
||||
}
|
||||
else if (Token::Match(tok, "struct|union|class {") ||
|
||||
Token::Match(tok, "struct|union|class %type% {"))
|
||||
{
|
||||
int indentlevel0 = indentlevel;
|
||||
|
||||
while (tok->str() != "{")
|
||||
tok = tok->next();
|
||||
|
||||
do
|
||||
{
|
||||
if (tok->str() == "{")
|
||||
indentlevel++;
|
||||
else if (tok->str() == "}")
|
||||
indentlevel--;
|
||||
tok = tok->next();
|
||||
}
|
||||
while (tok && indentlevel > indentlevel0);
|
||||
|
||||
if (! tok)
|
||||
break;
|
||||
}
|
||||
|
||||
if (Token::Match(tok, "[;{}] bool|char|short|int|long|float|double %var% ;|="))
|
||||
varUsage[ tok->strAt(2)] = USAGE_DECLARE;
|
||||
|
||||
else if (Token::Match(tok, "[;{}] bool|char|short|int|long|float|double * %var% ;|="))
|
||||
varUsage[ tok->strAt(3)] = USAGE_DECLARE;
|
||||
|
||||
else if (Token::Match(tok, "delete|return %var%"))
|
||||
varUsage[ tok->strAt(1)] |= USAGE_READ;
|
||||
|
||||
else if (Token::Match(tok, "%var% ="))
|
||||
varUsage[ tok->str()] |= USAGE_WRITE;
|
||||
|
||||
else if (Token::Match(tok, "else %var% ="))
|
||||
varUsage[ tok->strAt(1)] |= USAGE_WRITE;
|
||||
|
||||
else if (Token::Match(tok, ">>|& %var%"))
|
||||
varUsage[ tok->strAt(1)] |= (USAGE_WRITE | USAGE_READ);
|
||||
|
||||
else if ((Token::Match(tok, "[(=&!]") || isOp(tok)) && Token::Match(tok->next(), "%var%"))
|
||||
varUsage[ tok->strAt(1)] |= USAGE_READ;
|
||||
|
||||
else if (Token::Match(tok, "-=|+=|*=|/=|&=|^= %var%") || Token::Match(tok, "|= %var%"))
|
||||
varUsage[ tok->strAt(1)] |= USAGE_READ;
|
||||
|
||||
else if (Token::Match(tok, "%var%") && (tok->next()->str() == ")" || isOp(tok->next())))
|
||||
varUsage[ tok->str()] |= USAGE_READ;
|
||||
|
||||
else if (Token::Match(tok, "[(,] %var% [,)]"))
|
||||
varUsage[ tok->strAt(1)] |= (USAGE_WRITE | USAGE_READ);
|
||||
|
||||
else if (Token::Match(tok, "; %var% ;"))
|
||||
varUsage[ tok->strAt(1)] |= USAGE_READ;
|
||||
}
|
||||
|
||||
// Check usage of all variables in the current scope..
|
||||
for (std::map<std::string, unsigned int>::const_iterator it = varUsage.begin(); it != varUsage.end(); ++it)
|
||||
{
|
||||
std::string varname = it->first;
|
||||
unsigned int usage = it->second;
|
||||
|
||||
if (!std::isalpha(varname[0]))
|
||||
continue;
|
||||
|
||||
if (!(usage & USAGE_DECLARE))
|
||||
continue;
|
||||
|
||||
if (usage == USAGE_DECLARE)
|
||||
{
|
||||
_errorLogger->reportErr(ErrorMessage::unusedVariable(_tokenizer, tok1->next(), varname));
|
||||
}
|
||||
|
||||
else if (!(usage & USAGE_READ))
|
||||
{
|
||||
_errorLogger->reportErr(ErrorMessage::unreadVariable(_tokenizer, tok1->next(), varname));
|
||||
}
|
||||
|
||||
else if (!(usage & USAGE_WRITE))
|
||||
{
|
||||
_errorLogger->reportErr(ErrorMessage::unassignedVariable(_tokenizer, tok1->next(), varname));
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
//---------------------------------------------------------------------------
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
//---------------------------------------------------------------------------
|
||||
// str plus char
|
||||
//---------------------------------------------------------------------------
|
||||
|
|
|
@ -42,9 +42,6 @@ public:
|
|||
// Warning upon: if (condition);
|
||||
void WarningIf();
|
||||
|
||||
// Assignment in condition
|
||||
void CheckIfAssignment();
|
||||
|
||||
// Invalid function usage..
|
||||
void InvalidFunctionUsage();
|
||||
|
||||
|
@ -66,12 +63,6 @@ public:
|
|||
// Incomplete statement. A statement that only contains a constant or variable
|
||||
void CheckIncompleteStatement();
|
||||
|
||||
/** Unreachable code below a 'return' */
|
||||
void unreachableCode();
|
||||
|
||||
/** Unused function variables */
|
||||
void functionVariableUsage();
|
||||
|
||||
/** str plus char */
|
||||
void strPlusChar();
|
||||
|
||||
|
|
|
@ -251,10 +251,6 @@ void CppCheck::checkFile(const std::string &code, const char FileName[])
|
|||
if (ErrorMessage::charArrayIndex(_settings) || ErrorMessage::charBitOp(_settings))
|
||||
checkOther.CheckCharVariable();
|
||||
|
||||
// Usage of local variables
|
||||
if (ErrorMessage::unusedVariable(_settings))
|
||||
checkOther.functionVariableUsage();
|
||||
|
||||
|
||||
|
||||
_tokenizer.simplifyTokenList();
|
||||
|
@ -283,10 +279,6 @@ void CppCheck::checkFile(const std::string &code, const char FileName[])
|
|||
if (ErrorMessage::arrayIndexOutOfBounds(_settings) && ErrorMessage::bufferOverrun(_settings))
|
||||
checkBufferOverrun.bufferOverrun();
|
||||
|
||||
// Check for "if (a=b)"
|
||||
if (ErrorMessage::ifAssignment(_settings))
|
||||
checkOther.CheckIfAssignment();
|
||||
|
||||
// Dangerous functions, such as 'gets' and 'scanf'
|
||||
checkBufferOverrun.dangerousFunctions();
|
||||
|
||||
|
@ -323,10 +315,6 @@ void CppCheck::checkFile(const std::string &code, const char FileName[])
|
|||
if (ErrorMessage::unusedStructMember(_settings))
|
||||
checkOther.CheckStructMemberUsage();
|
||||
|
||||
// Unreachable code below a 'return' statement
|
||||
if (ErrorMessage::unreachableCode())
|
||||
checkOther.unreachableCode();
|
||||
|
||||
// Check if a constant function parameter is passed by value
|
||||
if (ErrorMessage::passedByValue())
|
||||
checkOther.CheckConstantFunctionParameter();
|
||||
|
|
|
@ -247,15 +247,6 @@ public:
|
|||
return s._checkCodingStyle;
|
||||
}
|
||||
|
||||
static std::string unreachableCode(const Tokenizer *tokenizer, const Token *Location)
|
||||
{
|
||||
return msg1(tokenizer, Location) + "Unreachable code below a 'return'";
|
||||
}
|
||||
static bool unreachableCode()
|
||||
{
|
||||
return true;
|
||||
}
|
||||
|
||||
static std::string passedByValue(const Tokenizer *tokenizer, const Token *Location, const std::string &parname)
|
||||
{
|
||||
return msg1(tokenizer, Location) + "Function parameter '" + parname + "' is passed by value. It could be passed by reference instead.";
|
||||
|
@ -265,33 +256,6 @@ public:
|
|||
return true;
|
||||
}
|
||||
|
||||
static std::string unusedVariable(const Tokenizer *tokenizer, const Token *Location, const std::string &varname)
|
||||
{
|
||||
return msg1(tokenizer, Location) + "Unused variable '" + varname + "'";
|
||||
}
|
||||
static bool unusedVariable(const Settings &s)
|
||||
{
|
||||
return s._checkCodingStyle;
|
||||
}
|
||||
|
||||
static std::string unreadVariable(const Tokenizer *tokenizer, const Token *Location, const std::string &varname)
|
||||
{
|
||||
return msg1(tokenizer, Location) + "Variable '" + varname + "' is assigned a value that is never used";
|
||||
}
|
||||
static bool unreadVariable(const Settings &s)
|
||||
{
|
||||
return s._checkCodingStyle;
|
||||
}
|
||||
|
||||
static std::string unassignedVariable(const Tokenizer *tokenizer, const Token *Location, const std::string &varname)
|
||||
{
|
||||
return msg1(tokenizer, Location) + "Variable '" + varname + "' is not assigned a value";
|
||||
}
|
||||
static bool unassignedVariable(const Settings &s)
|
||||
{
|
||||
return s._checkCodingStyle;
|
||||
}
|
||||
|
||||
static std::string constStatement(const Tokenizer *tokenizer, const Token *Location, const std::string &type)
|
||||
{
|
||||
return msg1(tokenizer, Location) + "Redundant code: Found a statement that begins with " + type + " constant";
|
||||
|
@ -328,15 +292,6 @@ public:
|
|||
return false;
|
||||
}
|
||||
|
||||
static std::string ifAssignment(const Tokenizer *tokenizer, const Token *Location)
|
||||
{
|
||||
return msg1(tokenizer, Location) + "Assignment in if-condition";
|
||||
}
|
||||
static bool ifAssignment(const Settings &s)
|
||||
{
|
||||
return s._checkCodingStyle;
|
||||
}
|
||||
|
||||
static std::string conditionAlwaysTrueFalse(const Tokenizer *tokenizer, const Token *Location, const std::string &truefalse)
|
||||
{
|
||||
return msg1(tokenizer, Location) + "Condition is always " + truefalse + "";
|
||||
|
|
|
@ -59,26 +59,6 @@ private:
|
|||
TEST_CASE(structmember3);
|
||||
TEST_CASE(structmember4);
|
||||
TEST_CASE(structmember5);
|
||||
|
||||
TEST_CASE(localvar1);
|
||||
TEST_CASE(localvar2);
|
||||
TEST_CASE(localvar3);
|
||||
TEST_CASE(localvar4);
|
||||
TEST_CASE(localvar5);
|
||||
|
||||
// Don't give false positives for variables in structs/unions
|
||||
TEST_CASE(localvarStruct1);
|
||||
TEST_CASE(localvarStruct2);
|
||||
TEST_CASE(localvarStruct3);
|
||||
TEST_CASE(localvarStruct4); // Ticket #31: sigsegv on incomplete struct
|
||||
|
||||
TEST_CASE(localvarOp); // Usage with arithmetic operators
|
||||
TEST_CASE(localvarInvert); // Usage with inverted variable
|
||||
TEST_CASE(localvarIf); // Usage in if
|
||||
TEST_CASE(localvarIfElse); // return tmp1 ? tmp2 : tmp3;
|
||||
TEST_CASE(localvarOpAssign); // a |= b;
|
||||
TEST_CASE(localvarFor); // for ( ; var; )
|
||||
TEST_CASE(localvarShift); // 1 >> var
|
||||
}
|
||||
|
||||
void structmember1()
|
||||
|
@ -170,203 +150,6 @@ private:
|
|||
"}\n");
|
||||
ASSERT_EQUALS(std::string(""), errout.str());
|
||||
}
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
void functionVariableUsage(const char code[])
|
||||
{
|
||||
// Tokenize..
|
||||
Tokenizer tokenizer;
|
||||
std::istringstream istr(code);
|
||||
tokenizer.tokenize(istr, "test.cpp");
|
||||
tokenizer.simplifyTokenList();
|
||||
|
||||
// Clear the error buffer..
|
||||
errout.str("");
|
||||
|
||||
// Check for unused variables..
|
||||
CheckOther checkOther(&tokenizer, Settings(), this);
|
||||
checkOther.functionVariableUsage();
|
||||
}
|
||||
|
||||
void localvar1()
|
||||
{
|
||||
functionVariableUsage("void foo()\n"
|
||||
"{\n"
|
||||
" int i = 0;\n"
|
||||
"}\n");
|
||||
ASSERT_EQUALS(std::string("[test.cpp:2]: Variable 'i' is assigned a value that is never used\n"), errout.str());
|
||||
}
|
||||
|
||||
void localvar2()
|
||||
{
|
||||
functionVariableUsage("void foo()\n"
|
||||
"{\n"
|
||||
" int i;\n"
|
||||
" return i;\n"
|
||||
"}\n");
|
||||
ASSERT_EQUALS(std::string("[test.cpp:2]: Variable 'i' is not assigned a value\n"), errout.str());
|
||||
}
|
||||
|
||||
void localvar3()
|
||||
{
|
||||
functionVariableUsage("void foo()\n"
|
||||
"{\n"
|
||||
" int i;\n"
|
||||
" if ( abc )\n"
|
||||
" ;\n"
|
||||
" else i = 0;\n"
|
||||
"}\n");
|
||||
ASSERT_EQUALS(std::string("[test.cpp:2]: Variable 'i' is assigned a value that is never used\n"), errout.str());
|
||||
}
|
||||
|
||||
void localvar4()
|
||||
{
|
||||
functionVariableUsage("void foo()\n"
|
||||
"{\n"
|
||||
" int i = 0;\n"
|
||||
" f(i);\n"
|
||||
"}\n");
|
||||
ASSERT_EQUALS(std::string(""), errout.str());
|
||||
}
|
||||
|
||||
void localvar5()
|
||||
{
|
||||
functionVariableUsage("void foo()\n"
|
||||
"{\n"
|
||||
" int a = 0;\n"
|
||||
" b = (char)a;\n"
|
||||
"}\n");
|
||||
ASSERT_EQUALS(std::string(""), errout.str());
|
||||
}
|
||||
|
||||
|
||||
|
||||
void localvarStruct1()
|
||||
{
|
||||
functionVariableUsage("void foo()\n"
|
||||
"{\n"
|
||||
" static const struct{ int x, y, w, h; } bounds = {1,2,3,4};\n"
|
||||
" return bounds.x + bounds.y + bounds.w + bounds.h;\n"
|
||||
"}\n");
|
||||
ASSERT_EQUALS(std::string(""), errout.str());
|
||||
}
|
||||
|
||||
void localvarStruct2()
|
||||
{
|
||||
functionVariableUsage("void foo()\n"
|
||||
"{\n"
|
||||
" struct ABC { int a, b, c; };\n"
|
||||
" struct ABC abc = { 1, 2, 3 };\n"
|
||||
"}\n");
|
||||
ASSERT_EQUALS(std::string(""), errout.str());
|
||||
}
|
||||
|
||||
void localvarStruct3()
|
||||
{
|
||||
functionVariableUsage("void foo()\n"
|
||||
"{\n"
|
||||
" int a = 10;\n"
|
||||
" union { struct { unsigned char x; }; unsigned char z; };\n"
|
||||
" do {\n"
|
||||
" func();\n"
|
||||
" } while(a--);\n"
|
||||
"}\n");
|
||||
ASSERT_EQUALS(std::string(""), errout.str());
|
||||
}
|
||||
|
||||
void localvarStruct4()
|
||||
{
|
||||
/* This must not SIGSEGV: */
|
||||
functionVariableUsage("void foo()\n"
|
||||
"{\n"
|
||||
" struct { \n");
|
||||
errout.str();
|
||||
}
|
||||
|
||||
|
||||
|
||||
void localvarOp()
|
||||
{
|
||||
const char op[] = "+-*/%&|^";
|
||||
for (const char *p = op; *p; ++p)
|
||||
{
|
||||
std::string code("int main()\n"
|
||||
"{\n"
|
||||
" int tmp = 10;\n"
|
||||
" return 123 " + std::string(1, *p) + " tmp;\n"
|
||||
"}\n");
|
||||
functionVariableUsage(code.c_str());
|
||||
ASSERT_EQUALS(std::string(""), errout.str());
|
||||
}
|
||||
}
|
||||
|
||||
void localvarInvert()
|
||||
{
|
||||
functionVariableUsage("int main()\n"
|
||||
"{\n"
|
||||
" int tmp = 10;\n"
|
||||
" return ~tmp;\n"
|
||||
"}\n");
|
||||
ASSERT_EQUALS(std::string(""), errout.str());
|
||||
}
|
||||
|
||||
void localvarIf()
|
||||
{
|
||||
functionVariableUsage("int main()\n"
|
||||
"{\n"
|
||||
" int tmp = 10;\n"
|
||||
" if ( tmp )\n"
|
||||
" return 1;\n"
|
||||
" return 0;\n"
|
||||
"}\n");
|
||||
ASSERT_EQUALS(std::string(""), errout.str());
|
||||
}
|
||||
|
||||
void localvarIfElse()
|
||||
{
|
||||
functionVariableUsage("int foo()\n"
|
||||
"{\n"
|
||||
" int tmp1 = 1;\n"
|
||||
" int tmp2 = 2;\n"
|
||||
" int tmp3 = 3;\n"
|
||||
" return tmp1 ? tmp2 : tmp3;\n"
|
||||
"}\n");
|
||||
ASSERT_EQUALS(std::string(""), errout.str());
|
||||
}
|
||||
|
||||
void localvarOpAssign()
|
||||
{
|
||||
functionVariableUsage("void foo()\n"
|
||||
"{\n"
|
||||
" int a = 1;\n"
|
||||
" int b = 2;\n"
|
||||
" a |= b;\n"
|
||||
"}\n");
|
||||
ASSERT_EQUALS(std::string("[test.cpp:2]: Variable 'a' is assigned a value that is never used\n"), errout.str());
|
||||
}
|
||||
|
||||
void localvarFor()
|
||||
{
|
||||
functionVariableUsage("void foo()\n"
|
||||
"{\n"
|
||||
" int a = 1;\n"
|
||||
" for (;a;);\n"
|
||||
"}\n");
|
||||
ASSERT_EQUALS(std::string(""), errout.str());
|
||||
}
|
||||
|
||||
void localvarShift()
|
||||
{
|
||||
functionVariableUsage("int foo()\n"
|
||||
"{\n"
|
||||
" int var = 1;\n"
|
||||
" return 1 >> var;\n"
|
||||
"}\n");
|
||||
ASSERT_EQUALS(std::string(""), errout.str());
|
||||
}
|
||||
};
|
||||
|
||||
REGISTER_TEST(TestUnusedVar)
|
||||
|
|
|
@ -86,16 +86,11 @@ int main()
|
|||
err.push_back(Message("udivError", Message::always, "Unsigned division. The result will be wrong."));
|
||||
err.push_back(Message("udivWarning", Message::style_all, "Warning: Division with signed and unsigned operators"));
|
||||
err.push_back(Message("unusedStructMember", Message::style, "struct or union member '%1::%2' is never used", "structname", "varname"));
|
||||
err.push_back(Message("unreachableCode", Message::always, "Unreachable code below a 'return'"));
|
||||
err.push_back(Message("passedByValue", Message::always, "Function parameter '%1' is passed by value. It could be passed by reference instead.", "parname"));
|
||||
err.push_back(Message("unusedVariable", Message::style, "Unused variable '%1'", "varname"));
|
||||
err.push_back(Message("unreadVariable", Message::style, "Variable '%1' is assigned a value that is never used", "varname"));
|
||||
err.push_back(Message("unassignedVariable", Message::style, "Variable '%1' is not assigned a value", "varname"));
|
||||
err.push_back(Message("constStatement", Message::style, "Redundant code: Found a statement that begins with %1 constant", "type"));
|
||||
err.push_back(Message("charArrayIndex", Message::style, "Warning - using char variable as array index"));
|
||||
err.push_back(Message("charBitOp", Message::style, "Warning - using char variable in bit operation"));
|
||||
err.push_back(Message("variableScope", Message::never, "The scope of the variable %1 can be limited", "varname"));
|
||||
err.push_back(Message("ifAssignment", Message::style, "Assignment in if-condition"));
|
||||
err.push_back(Message("conditionAlwaysTrueFalse", Message::style, "Condition is always %1", "truefalse"));
|
||||
err.push_back(Message("strPlusChar", Message::always, "Unusual pointer arithmetic"));
|
||||
|
||||
|
|
Loading…
Reference in New Issue