From a4b5824dec9d7a9dd4dd9935a0b91e09ed25bcdb Mon Sep 17 00:00:00 2001 From: PKEuS Date: Fri, 7 Sep 2012 12:36:40 +0200 Subject: [PATCH] New internal check: checkRedundantNextPrevious(). Fixed findings by new internal check --- lib/checkbufferoverrun.cpp | 2 +- lib/checkinternal.cpp | 30 +++++++++++++++++++++ lib/checkinternal.h | 8 +++++- lib/checknullpointer.cpp | 4 +-- lib/checkother.cpp | 8 +++--- lib/templatesimplifier.cpp | 2 +- lib/tokenize.cpp | 2 +- test/testinternal.cpp | 53 ++++++++++++++++++++++++++++++++++++++ 8 files changed, 99 insertions(+), 10 deletions(-) diff --git a/lib/checkbufferoverrun.cpp b/lib/checkbufferoverrun.cpp index 37737f738..071376898 100644 --- a/lib/checkbufferoverrun.cpp +++ b/lib/checkbufferoverrun.cpp @@ -274,7 +274,7 @@ static const Token *for_init(const Token *tok, unsigned int &varid, std::string } varid = tok->tokAt(2)->varId(); - varname = tok->tokAt(2)->str(); + varname = tok->strAt(2); tok = tok->tokAt(6); } else return 0; diff --git a/lib/checkinternal.cpp b/lib/checkinternal.cpp index 80e92ad1f..d9ce8ff1b 100644 --- a/lib/checkinternal.cpp +++ b/lib/checkinternal.cpp @@ -214,6 +214,30 @@ void CheckInternal::checkUnknownPattern() } } +void CheckInternal::checkRedundantNextPrevious() +{ + for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) { + if (Token::Match(tok, ". previous ( ) . next|tokAt|strAt|linkAt (") || Token::Match(tok, ". next ( ) . previous|tokAt|strAt|linkAt (") || + (Token::simpleMatch(tok, ". tokAt (") && Token::Match(tok->linkAt(2), ") . previous|next|tokAt|strAt|linkAt|str|link ("))) { + const std::string& func1 = tok->strAt(1); + const std::string& func2 = tok->linkAt(2)->strAt(2); + + if ((func2 == "previous" || func2 == "next" || func2 == "str" || func2 == "link") && tok->linkAt(2)->strAt(4) != ")") + continue; + + redundantNextPreviousError(tok, func1, func2); + } else if (Token::Match(tok, ". next|previous ( ) . next|previous ( ) . next|previous|linkAt|strAt|link|str (")) { + const std::string& func1 = tok->strAt(1); + const std::string& func2 = tok->strAt(9); + + if ((func2 == "previous" || func2 == "next" || func2 == "str" || func2 == "link") && tok->strAt(11) != ")") + continue; + + redundantNextPreviousError(tok, func1, func2); + } + } +} + void CheckInternal::simplePatternError(const Token* tok, const std::string& pattern, const std::string &funcname) { reportError(tok, Severity::warning, "simplePatternError", @@ -241,4 +265,10 @@ void CheckInternal::unknownPatternError(const Token* tok, const std::string& pat "Unknown pattern used: \"" + pattern + "\""); } +void CheckInternal::redundantNextPreviousError(const Token* tok, const std::string& func1, const std::string& func2) +{ + reportError(tok, Severity::style, "redundantNextPrevious", + "Call to 'Token::" + func1 + "()' followed by 'Token::" + func2 + "()' can be simplified."); +} + #endif // #ifndef NDEBUG diff --git a/lib/checkinternal.h b/lib/checkinternal.h index 1f5625557..3d05adf21 100644 --- a/lib/checkinternal.h +++ b/lib/checkinternal.h @@ -54,6 +54,7 @@ public: checkInternal.checkTokenSimpleMatchPatterns(); checkInternal.checkMissingPercentCharacter(); checkInternal.checkUnknownPattern(); + checkInternal.checkRedundantNextPrevious(); } /** @brief %Check if a simple pattern is used inside Token::Match or Token::findmatch */ @@ -65,14 +66,18 @@ public: /** @brief %Check for missing % end character in Token::Match pattern */ void checkMissingPercentCharacter(); - /** @brief %Check for for unknown (invalid) complex patterns like "%typ%" */ + /** @brief %Check for unknown (invalid) complex patterns like "%typ%" */ void checkUnknownPattern(); + /** @brief %Check for inefficient usage of Token::next(), Token::previous() and Token::tokAt() */ + void checkRedundantNextPrevious(); + private: void simplePatternError(const Token *tok, const std::string &pattern, const std::string &funcname); void complexPatternError(const Token *tok, const std::string &pattern, const std::string &funcname); void missingPercentCharacterError(const Token *tok, const std::string &pattern, const std::string &funcname); void unknownPatternError(const Token* tok, const std::string& pattern); + void redundantNextPreviousError(const Token* tok, const std::string& func1, const std::string& func2); void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const { CheckInternal c(0, settings, errorLogger); @@ -80,6 +85,7 @@ private: c.complexPatternError(0, "%type% ( )", "Match"); c.missingPercentCharacterError(0, "%num", "Match"); c.unknownPatternError(0, "%typ"); + c.redundantNextPreviousError(0, "previous", "next"); } static std::string myName() { diff --git a/lib/checknullpointer.cpp b/lib/checknullpointer.cpp index 06dee53e7..62e594fda 100644 --- a/lib/checknullpointer.cpp +++ b/lib/checknullpointer.cpp @@ -849,8 +849,8 @@ void CheckNullPointer::nullPointerByCheckAndDeRef() vartok = tok->next(); } else if (Token::Match(tok, "( ! ( %var% =")) { vartok = tok->tokAt(3); - if (Token::simpleMatch(tok->tokAt(2)->link(), ") &&")) - checkConditionStart = tok->tokAt(2)->link(); + if (Token::simpleMatch(tok->linkAt(2), ") &&")) + checkConditionStart = tok->linkAt(2); } else continue; diff --git a/lib/checkother.cpp b/lib/checkother.cpp index 18a66fca0..415648290 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -2176,7 +2176,7 @@ void CheckOther::checkCCTypeFunctions() if (tok->varId() == 0 && Token::Match(tok, "isalnum|isalpha|iscntrl|isdigit|isgraph|islower|isprint|ispunct|isspace|isupper|isxdigit ( %num% ,|)") && MathLib::isNegative(tok->strAt(2))) { - cctypefunctionCallError(tok, tok->str(), tok->tokAt(2)->str()); + cctypefunctionCallError(tok, tok->str(), tok->strAt(2)); } } } @@ -2481,12 +2481,12 @@ void CheckOther::checkDoubleFree() if (var) { if (Token::Match(tok, "free|g_free")) { if (freedVariables.find(var) != freedVariables.end()) - doubleFreeError(tok, tok->tokAt(2)->str()); + doubleFreeError(tok, tok->strAt(2)); else freedVariables.insert(var); } else if (tok->str() == "closedir") { if (closeDirVariables.find(var) != closeDirVariables.end()) - doubleCloseDirError(tok, tok->tokAt(2)->str()); + doubleCloseDirError(tok, tok->strAt(2)); else closeDirVariables.insert(var); } @@ -2500,7 +2500,7 @@ void CheckOther::checkDoubleFree() unsigned int var = tok->tokAt(varIdx)->varId(); if (var) { if (freedVariables.find(var) != freedVariables.end()) - doubleFreeError(tok, tok->tokAt(varIdx)->str()); + doubleFreeError(tok, tok->strAt(varIdx)); else freedVariables.insert(var); } diff --git a/lib/templatesimplifier.cpp b/lib/templatesimplifier.cpp index 074870637..7d769d6da 100644 --- a/lib/templatesimplifier.cpp +++ b/lib/templatesimplifier.cpp @@ -375,7 +375,7 @@ std::list TemplateSimplifier::simplifyTemplatesGetTemplateDeclarations( for (Token *tok = tokens; tok; tok = tok->next()) { // TODO: handle namespaces. Right now we don't instantiate templates that are defined in namespaces. if (Token::Match(tok, "namespace %type% {")) - tok = tok->tokAt(2)->link(); + tok = tok->linkAt(2); if (Token::simpleMatch(tok, "template <")) { codeWithTemplates = true; diff --git a/lib/tokenize.cpp b/lib/tokenize.cpp index 276483129..961c98e9f 100644 --- a/lib/tokenize.cpp +++ b/lib/tokenize.cpp @@ -8657,7 +8657,7 @@ void Tokenizer::simplifyNamespaceStd() if (_settings->standards.cpp == Standards::CPP11 && Token::simpleMatch(tok, "std :: tr1 ::")) Token::eraseTokens(tok, tok->tokAt(3)); - else if (Token::Match(tok, "using namespace std ;")) { + else if (Token::simpleMatch(tok, "using namespace std ;")) { Token::eraseTokens(tok, tok->tokAt(4)); tok->deleteThis(); } diff --git a/test/testinternal.cpp b/test/testinternal.cpp index dc99257c8..e207b775e 100644 --- a/test/testinternal.cpp +++ b/test/testinternal.cpp @@ -38,6 +38,7 @@ private: TEST_CASE(simplePatternAlternatives) TEST_CASE(missingPercentCharacter) TEST_CASE(unknownPattern) + TEST_CASE(redundantNextPrevious) TEST_CASE(internalError) } @@ -244,6 +245,58 @@ private: ASSERT_EQUALS("", errout.str()); } + void redundantNextPrevious() { + check("void f() {\n" + " return tok->next()->previous();\n" + "}"); + ASSERT_EQUALS("[test.cpp:2]: (style) Call to 'Token::next()' followed by 'Token::previous()' can be simplified.\n", errout.str()); + + check("void f() {\n" + " return tok->tokAt(5)->previous();\n" + "}"); + ASSERT_EQUALS("[test.cpp:2]: (style) Call to 'Token::tokAt()' followed by 'Token::previous()' can be simplified.\n", errout.str()); + + check("void f() {\n" + " return tok->previous()->linkAt(5);\n" + "}"); + ASSERT_EQUALS("[test.cpp:2]: (style) Call to 'Token::previous()' followed by 'Token::linkAt()' can be simplified.\n", errout.str()); + + check("void f() {\n" + " tok->next()->previous(foo);\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + check("void f() {\n" + " return tok->next()->next();\n" // Simplification won't make code much shorter/readable + "}"); + ASSERT_EQUALS("", errout.str()); + + check("void f() {\n" + " return tok->previous()->previous();\n" // Simplification won't make code much shorter/readable + "}"); + ASSERT_EQUALS("", errout.str()); + + check("void f() {\n" + " return tok->tokAt(foo+bar)->tokAt();\n" + "}"); + ASSERT_EQUALS("[test.cpp:2]: (style) Call to 'Token::tokAt()' followed by 'Token::tokAt()' can be simplified.\n", errout.str()); + + check("void f() {\n" + " return tok->tokAt(foo+bar)->link();\n" + "}"); + ASSERT_EQUALS("[test.cpp:2]: (style) Call to 'Token::tokAt()' followed by 'Token::link()' can be simplified.\n", errout.str()); + + check("void f() {\n" + " tok->tokAt(foo+bar)->link(foo);\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + check("void f() {\n" + " return tok->next()->next()->str();\n" + "}"); + ASSERT_EQUALS("[test.cpp:2]: (style) Call to 'Token::next()' followed by 'Token::str()' can be simplified.\n", errout.str()); + } + void internalError() { // Make sure cppcheck does not raise an internal error of Token::Match ( Ticket #3727 ) check("class DELPHICLASS X;\n"