From e305a155afd529de70ae129bac70d1b84d342b17 Mon Sep 17 00:00:00 2001 From: Robert Reif Date: Mon, 7 Mar 2011 19:49:43 -0500 Subject: [PATCH] convert CheckStl::size() to use symbol database, fix false positives, and remove inconclusive --- lib/checkstl.cpp | 41 +++++++++++++++---------------------- test/teststl.cpp | 53 +++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 68 insertions(+), 26 deletions(-) diff --git a/lib/checkstl.cpp b/lib/checkstl.cpp index e52a4b79f..3509386ca 100644 --- a/lib/checkstl.cpp +++ b/lib/checkstl.cpp @@ -17,8 +17,8 @@ */ #include "checkstl.h" -#include "token.h" #include "executionpath.h" +#include "symboldatabase.h" #include // Register this check class (by creating a static instance of it) @@ -799,11 +799,13 @@ bool CheckStl::isStlContainer(const Token *tok) if (tok->varId()) { // find where this token is defined - const Token *type = Token::findmatch(_tokenizer->tokens(), "%varid%", tok->varId()); + const Variable *var = _tokenizer->getSymbolDatabase()->getVariableFromVarId(tok->varId()); + + if (!var) + return false; // find where this tokens type starts - while (type->previous() && !Token::Match(type->previous(), "[;{,(]")) - type = type->previous(); + const Token *type = var->typeStartToken(); // ignore "const" if (type->str() == "const") @@ -829,38 +831,27 @@ bool CheckStl::isStlContainer(const Token *tok) void CheckStl::size() { - if (!_settings->_checkCodingStyle || !_settings->inconclusive) + if (!_settings->_checkCodingStyle) return; for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) { if (Token::Match(tok, "%var% . size ( )")) { - if (Token::Match(tok->tokAt(5), "==|!=|> 0")) + // check for comparison to zero + if (Token::Match(tok->tokAt(5), "==|!=|> 0") || + Token::Match(tok->tokAt(-2), "0 ==|!=|<")) { if (isStlContainer(tok)) sizeError(tok); } - else if ((tok->tokAt(5)->str() == ")" || - tok->tokAt(5)->str() == "&&" || - tok->tokAt(5)->str() == "||" || - tok->tokAt(5)->str() == "!") && - (tok->tokAt(-1)->str() == "(" || - tok->tokAt(-1)->str() == "&&" || - tok->tokAt(-1)->str() == "||" || - tok->tokAt(-1)->str() == "!")) + + // check for using as boolean expression + else if ((Token::Match(tok->tokAt(-2), "if|while (") || + Token::Match(tok->tokAt(-3), "if|while ( !")) && + tok->strAt(5) == ")") { - if (tok->tokAt(-1)->str() == "(" && - tok->tokAt(5)->str() == ")") - { - // check for passing size to function call - if (Token::Match(tok->tokAt(-2), "if|while")) - { - if (isStlContainer(tok)) - sizeError(tok); - } - } - else if (isStlContainer(tok)) + if (isStlContainer(tok)) sizeError(tok); } } diff --git a/test/teststl.cpp b/test/teststl.cpp index 748c46ac0..15b68fd36 100644 --- a/test/teststl.cpp +++ b/test/teststl.cpp @@ -113,7 +113,6 @@ private: errout.str(""); Settings settings; - settings.inconclusive = true; settings._checkCodingStyle = true; // Tokenize.. @@ -1064,6 +1063,23 @@ private: void size1() { + check("struct Fred {\n" + " void foo();\n" + " std::list x;\n" + "};\n" + "void Fred::foo()\n" + "{\n" + " if (x.size() == 0) {}\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:7]: (performance) Possible inefficient checking for 'x' emptiness.\n", errout.str()); + + check("std::list x;\n" + "void f()\n" + "{\n" + " if (x.size() == 0) {}\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:4]: (performance) Possible inefficient checking for 'x' emptiness.\n", errout.str()); + check("void f()\n" "{\n" " std::list x;\n" @@ -1071,6 +1087,13 @@ private: "}\n"); ASSERT_EQUALS("[test.cpp:4]: (performance) Possible inefficient checking for 'x' emptiness.\n", errout.str()); + check("void f()\n" + "{\n" + " std::list x;\n" + " if (0 == x.size()) {}\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:4]: (performance) Possible inefficient checking for 'x' emptiness.\n", errout.str()); + check("void f()\n" "{\n" " std::list x;\n" @@ -1078,6 +1101,13 @@ private: "}\n"); ASSERT_EQUALS("[test.cpp:4]: (performance) Possible inefficient checking for 'x' emptiness.\n", errout.str()); + check("void f()\n" + "{\n" + " std::list x;\n" + " if (0 != x.size()) {}\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:4]: (performance) Possible inefficient checking for 'x' emptiness.\n", errout.str()); + check("void f()\n" "{\n" " std::list x;\n" @@ -1085,6 +1115,13 @@ private: "}\n"); ASSERT_EQUALS("[test.cpp:4]: (performance) Possible inefficient checking for 'x' emptiness.\n", errout.str()); + check("void f()\n" + "{\n" + " std::list x;\n" + " if (0 < x.size()) {}\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:4]: (performance) Possible inefficient checking for 'x' emptiness.\n", errout.str()); + check("void f()\n" "{\n" " std::list x;\n" @@ -1092,12 +1129,26 @@ private: "}\n"); ASSERT_EQUALS("[test.cpp:4]: (performance) Possible inefficient checking for 'x' emptiness.\n", errout.str()); + check("void f()\n" + "{\n" + " std::list x;\n" + " if (!x.size()) {}\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:4]: (performance) Possible inefficient checking for 'x' emptiness.\n", errout.str()); + check("void f()\n" "{\n" " std::list x;\n" " fun(x.size());\n" "}\n"); ASSERT_EQUALS("", errout.str()); + + check("void f()\n" + "{\n" + " std::list x;\n" + " fun(!x.size());\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); } void redundantCondition1()