Fixed #2652 (container .size() check too strict)

This commit is contained in:
Daniel Marjamäki 2013-02-14 16:59:58 +01:00
parent 76e1464fab
commit 08ada4cc63
3 changed files with 21 additions and 11 deletions

View File

@ -894,8 +894,10 @@ void CheckStl::if_findError(const Token *tok, bool str)
} }
/**
bool CheckStl::isStlContainer(const Token *tok) const * Is container.size() slow?
*/
static bool isContainerSizeSlow(const Token *tok)
{ {
// find where this token is defined // find where this token is defined
const Variable *var = tok->variable(); const Variable *var = tok->variable();
@ -910,11 +912,8 @@ bool CheckStl::isStlContainer(const Token *tok) const
if (Token::simpleMatch(type, "std ::")) if (Token::simpleMatch(type, "std ::"))
type = type->tokAt(2); type = type->tokAt(2);
// all possible stl containers as a token
static const char STL_CONTAINER_LIST[] = "array|bitset|deque|list|forward_list|map|multimap|multiset|priority_queue|queue|set|stack|vector|hash_map|hash_multimap|hash_set|unordered_map|unordered_multimap|unordered_set|unordered_multiset|basic_string";
// check if it's an stl template // check if it's an stl template
if (Token::Match(type, STL_CONTAINER_LIST)) if (Token::Match(type, "array|bitset|list|forward_list|map|multimap|multiset|priority_queue|queue|set|stack|hash_map|hash_multimap|hash_set|unordered_map|unordered_multimap|unordered_set|unordered_multiset|basic_string"))
return true; return true;
return false; return false;
@ -944,21 +943,21 @@ void CheckStl::size()
// check for comparison to zero // check for comparison to zero
if ((tok->previous() && !tok->previous()->isArithmeticalOp() && Token::Match(end, "==|<=|!=|> 0")) || if ((tok->previous() && !tok->previous()->isArithmeticalOp() && Token::Match(end, "==|<=|!=|> 0")) ||
(end->next() && !end->next()->isArithmeticalOp() && Token::Match(tok->tokAt(-2), "0 ==|>=|!=|<"))) { (end->next() && !end->next()->isArithmeticalOp() && Token::Match(tok->tokAt(-2), "0 ==|>=|!=|<"))) {
if (isStlContainer(tok1)) if (isContainerSizeSlow(tok1))
sizeError(tok1); sizeError(tok1);
} }
// check for comparison to one // check for comparison to one
if ((tok->previous() && !tok->previous()->isArithmeticalOp() && Token::Match(end, ">=|< 1")) || if ((tok->previous() && !tok->previous()->isArithmeticalOp() && Token::Match(end, ">=|< 1")) ||
(end->next() && !end->next()->isArithmeticalOp() && Token::Match(tok->tokAt(-2), "1 <=|>"))) { (end->next() && !end->next()->isArithmeticalOp() && Token::Match(tok->tokAt(-2), "1 <=|>"))) {
if (isStlContainer(tok1)) if (isContainerSizeSlow(tok1))
sizeError(tok1); sizeError(tok1);
} }
// check for using as boolean expression // check for using as boolean expression
else if ((Token::Match(tok->tokAt(-2), "if|while (") && end->str() == ")") || else if ((Token::Match(tok->tokAt(-2), "if|while (") && end->str() == ")") ||
(tok->previous()->type() == Token::eLogicalOp && Token::Match(end, "&&|)|,|;|%oror%"))) { (tok->previous()->type() == Token::eLogicalOp && Token::Match(end, "&&|)|,|;|%oror%"))) {
if (isStlContainer(tok1)) if (isContainerSizeSlow(tok1))
sizeError(tok1); sizeError(tok1);
} }
} }

View File

@ -225,8 +225,6 @@ private:
"* using auto pointer (auto_ptr)\n" "* using auto pointer (auto_ptr)\n"
"* useless calls of string and STL functions\n"; "* useless calls of string and STL functions\n";
} }
bool isStlContainer(const Token *tok) const;
}; };
/// @} /// @}
//--------------------------------------------------------------------------- //---------------------------------------------------------------------------

View File

@ -101,6 +101,7 @@ private:
TEST_CASE(size1); TEST_CASE(size1);
TEST_CASE(size2); TEST_CASE(size2);
TEST_CASE(size3); TEST_CASE(size3);
TEST_CASE(size4); // #2652 - don't warn about vector/deque
// Redundant conditions.. // Redundant conditions..
// if (ints.find(123) != ints.end()) ints.remove(123); // if (ints.find(123) != ints.end()) ints.remove(123);
@ -1620,6 +1621,18 @@ private:
ASSERT_EQUALS("[test.cpp:10]: (performance) Possible inefficient checking for 'x' emptiness.\n", errout.str()); ASSERT_EQUALS("[test.cpp:10]: (performance) Possible inefficient checking for 'x' emptiness.\n", errout.str());
} }
void size4() { // #2652 - don't warn about vector/deque
check("void f(std::vector<int> &v) {\n"
" if (v.size() > 0U) {}\n"
"}");
ASSERT_EQUALS("", errout.str());
check("void f(std::deque<int> &v) {\n"
" if (v.size() > 0U) {}\n"
"}");
ASSERT_EQUALS("", errout.str());
}
void redundantCondition2() { void redundantCondition2() {
check("void f(string haystack)\n" check("void f(string haystack)\n"
"{\n" "{\n"