walkero
/
cppcheck
1
0
Fork 0
Code Issues Pull Requests Projects Releases 1 Wiki Activity

Fixed #3174 (New check: Useless calls of STL functions)

This commit is contained in:
Marek Zmysłowski 2011-10-24 23:25:23 +02:00 committed by Daniel Marjamäki
parent 467840c6e1
commit 190139f441
3 changed files with 117 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

59
lib/checkstl.cpp
View File

@ -1125,3 +1125,62 @@ void CheckStl::autoPointerArrayError(const Token *tok)
);
}
void CheckStl::uselessCalls()
{
for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) {
if (tok->varId() == 0)
continue;
if (Token::Match(tok, "%var% . compare (") &&
tok->varId() == tok->tokAt(3)->link()->tokAt(-1)->varId()) {
uselessCallsReturnValueError(tok, tok->tokAt(2));
} else if ((Token::Match(tok, "%var% . compare ( %var%") ||
Token::Match(tok, "%var% . find ( %var%") ||
Token::Match(tok, "%var% . rfind ( %var%") ||
Token::Match(tok, "%var% . find_first_not_of ( %var%") ||
Token::Match(tok, "%var% . find_first_of ( %var%") ||
Token::Match(tok, "%var% . find_last_not_of ( %var%") ||
Token::Match(tok, "%var% . find_last_of ( %var%")) &&
tok->varId() == tok->tokAt(4)->varId()) {
uselessCallsReturnValueError(tok->tokAt(4), tok->tokAt(2));
} else if (Token::Match(tok, "%var% . swap ( %var% )") &&
tok->varId() == tok->tokAt(4)->varId()) {
uselessCallsSwapError(tok);
} else if (Token::Match(tok, "%var% . substr ( )")) {
uselessCallsSubstrError(tok);
} else if (Token::Match(tok, "%var% . substr ( 0")) {
if (tok->tokAt(5)->str() == ")" ||
tok->tokAt(3)->link()->tokAt(-1)->str() == "npos")
uselessCallsSubstrError(tok);
}
}
}
void CheckStl::uselessCallsReturnValueError(const Token *tok, const Token *function)
{
std::ostringstream errmsg;
const std::string funname(function ? function->str().c_str() : "");
const std::string varname(tok ? tok->str().c_str() : "");
errmsg << "Function \'"<< funname << "\' useless call. The variable \'"<< varname <<"\' is using function \'"<< funname << "\' against itself. Return of this function depends only on the position.\n" <<
"Function \'"<< funname << "\' useless call. The variable \'"<< varname <<"\' is using function \'"<< funname << "\' against itself. If the \'pos\' argument in the \'"<< funname << "\' function is 0 or there is no this argument then the function returns 0. If the position is greater than 0, the return value is npos.";
reportError(tok, Severity::warning, "uselessCallsCompare", errmsg.str());
}
void CheckStl::uselessCallsSwapError(const Token *tok)
{
std::ostringstream errmsg;
const std::string varname(tok ? tok->str().c_str() : "");
errmsg << "Function \'swap\' useless call. Using \'swap\' function from \'" << varname << "\' against itself doesn't make any changes.\n" <<
"Function \'swap\' useless call. Using \'swap\' function from \'" << varname << "\' against itself doesn't make any changes and can decrease in performance.";
reportError(tok, Severity::performance, "uselessCallsSwap", errmsg.str());
}
void CheckStl::uselessCallsSubstrError(const Token *tok)
{
std::ostringstream errmsg;
const std::string varname(tok ? tok->str().c_str() : "");
errmsg << "Function \'substr\' useless call. Function create copy of the \'" << varname << "\' object.\n" <<
"Function \'substr\' useless call. \'substr\' function create copy of the whole \'" << varname << "\' object which can decrease in performance. Please use \'=\' operator instead.";
reportError(tok, Severity::performance, "uselessCallsSubstr", errmsg.str());
}

15
lib/checkstl.h
View File

@ -55,6 +55,7 @@ public:
checkStl.if_find();
checkStl.string_c_str();
checkStl.checkAutoPointer();
checkStl.uselessCalls();
// Style check
checkStl.size();
@ -137,6 +138,10 @@ public:
/** @brief %Check for use and copy auto pointer */
void checkAutoPointer();
/** @brief %Check calls that using them is useless */
void uselessCalls();
private:
@ -162,6 +167,10 @@ private:
void autoPointerContainerError(const Token *tok);
void autoPointerArrayError(const Token *tok);
void uselessCallsReturnValueError(const Token *tok, const Token *function);
void uselessCallsSwapError(const Token *tok);
void uselessCallsSubstrError(const Token *tok);
void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) {
CheckStl c(0, settings, errorLogger);
c.invalidIteratorError(0, "iterator");
@ -181,6 +190,9 @@ private:
c.autoPointerError(0);
c.autoPointerContainerError(0);
c.autoPointerArrayError(0);
c.uselessCallsReturnValueError(0, 0);
c.uselessCallsSwapError(0);
c.uselessCallsSubstrError(0);
}
std::string myName() const {
@ -198,7 +210,8 @@ private:
"* suspicious condition when using find\n"
"* redundant condition\n"
"* common mistakes when using string::c_str()\n"
"* using auto pointer (auto_ptr)";
"* using auto pointer (auto_ptr)\n"
"* useless calls of string functions";
}
bool isStlContainer(unsigned int varid);

46
test/teststl.cpp
View File

@ -111,6 +111,8 @@ private:
TEST_CASE(autoPointer);
TEST_CASE(uselessCalls);
}
void check(const std::string &code) {
@ -1187,7 +1189,7 @@ private:
}
void redundantCondition1() {
check("void f()\n"
check("void f(string haystack)\n"
"{\n"
" if (haystack.find(needle) != haystack.end())\n"
" haystack.remove(needle);"
@ -1211,7 +1213,7 @@ private:
}
void redundantCondition2() {
check("void f()\n"
check("void f(string haystack)\n"
"{\n"
" if (haystack.find(needle) != haystack.end())\n"
" {\n"
@ -1437,7 +1439,47 @@ private:
ASSERT_EQUALS("", errout.str());
}
void uselessCalls() {
check("void f()\n"
"{\n"
" string s1, s2;\n"
" s1.swap(s2);\n"
" s2.swap(s2);\n"
" \n"
"};\n");
ASSERT_EQUALS("[test.cpp:5]: (performance) Function \'swap\' useless call. Using 'swap' function "
"from \'s2\' against itself doesn't make any changes.\n", errout.str());
check("void f()\n"
"{\n"
" string s1, s2;\n"
" s1.compare(s2);\n"
" s2.compare(s2);\n"
" s1.compare(s2.c_str());\n"
" s1.compare(0, s1.size(), s1);\n"
" \n"
"};\n");
ASSERT_EQUALS("[test.cpp:5]: (warning) Function 'compare' useless call. The variable 's2' is using "
"function \'compare\' against itself. Return of this function depends only on the position.\n"
"[test.cpp:7]: (warning) Function 'compare' useless call. The variable 's1' is using "
"function \'compare\' against itself. Return of this function depends only on the position.\n", errout.str());
check("void f()\n"
"{\n"
" int x=1;\n"
" string s1, s2;\n"
" s1 = s1.substr();\n"
" s2 = s1.substr(x);\n"
" s1 = s2.substr(0, x);\n"
" s1 = s2.substr(0,std::string::npos);\n"
" \n"
"};\n");
ASSERT_EQUALS("[test.cpp:5]: (performance) Function \'substr\' useless call. Function create copy "
"of the \'s1\' object.\n"
"[test.cpp:8]: (performance) Function \'substr\' useless call. Function create copy "
"of the \'s2\' object.\n",errout.str());
}
};
REGISTER_TEST(TestStl)