From 37b37218cf01e83d8210c9ca835ce1f79f2e12e1 Mon Sep 17 00:00:00 2001 From: Robert Reif Date: Mon, 19 Jul 2010 08:40:46 +0200 Subject: [PATCH 1/7] Fixed #1882 (false negative: function can be declared const) --- lib/checkclass.cpp | 2 +- test/testclass.cpp | 21 +++++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/lib/checkclass.cpp b/lib/checkclass.cpp index e6297755b..c3c06e75e 100644 --- a/lib/checkclass.cpp +++ b/lib/checkclass.cpp @@ -2329,7 +2329,7 @@ bool CheckClass::checkConstFunc(const std::string &classname, const Var *varlist } // function call.. - else if ((tok1->str() != "return" && Token::Match(tok1, "%var% (") && tok1->str() != "c_str") || + else if ((Token::Match(tok1, "%var% (") && !Token::Match(tok1, "return|c_str|if")) || Token::Match(tok1, "%var% < %any% > (")) { isconst = false; diff --git a/test/testclass.cpp b/test/testclass.cpp index 7be54272f..5fc91fb93 100644 --- a/test/testclass.cpp +++ b/test/testclass.cpp @@ -129,6 +129,7 @@ private: TEST_CASE(const24); // ticket #1708 TEST_CASE(const25); // ticket #1724 TEST_CASE(const26); // ticket #1847 + TEST_CASE(const27); // ticket #1882 TEST_CASE(constoperator1); // operator< can often be const TEST_CASE(constoperator2); // operator<< TEST_CASE(constincdec); // increment/decrement => non-const @@ -3611,6 +3612,26 @@ private: ASSERT_EQUALS("", errout.str()); } + void const27() // ticket #1882 + { + checkConst("class A {\n" + "public:\n" + " A(){m_d=1.0; m_iRealVal=2.0;}\n" + " double dGetValue();\n" + "private:\n" + " double m_d;\n" + " double m_iRealVal;\n" + "};\n" + "double A::dGetValue() {\n" + " double dRet = m_iRealVal;\n" + " if( m_d != 0 )\n" + " return dRet / m_d;\n" + " return dRet;\n" + "};\n" + ); + ASSERT_EQUALS("[test.cpp:9] -> [test.cpp:4]: (style) The function 'A::dGetValue' can be const\n", errout.str()); + } + // increment/decrement => not const void constincdec() { From 95fa267c0eb2b0d6a80149c42442e66de93cdf83 Mon Sep 17 00:00:00 2001 From: Kimmo Varis Date: Mon, 19 Jul 2010 10:54:53 +0300 Subject: [PATCH 2/7] Remove misleading comment. --- lib/errorlogger.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/errorlogger.h b/lib/errorlogger.h index 385e96b8d..7e3a23986 100644 --- a/lib/errorlogger.h +++ b/lib/errorlogger.h @@ -16,10 +16,10 @@ * along with this program. If not, see . */ -// THIS FILE IS GENERATED BY MACHINE, SEE ../tools/errmsg.cpp ! #ifndef errorloggerH #define errorloggerH + #include #include #include "settings.h" From bbf2c6c6e6cac3fb5559611234b5111f192c92d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Mon, 19 Jul 2010 10:03:54 +0200 Subject: [PATCH 3/7] Fixed #1880 (false positive: Uninitialized array (initialized in subfunction)) --- lib/checkother.cpp | 16 ++++++++++++++++ test/testother.cpp | 2 +- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/lib/checkother.cpp b/lib/checkother.cpp index 0cab94dbf..c1a0f30b7 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -3395,6 +3395,22 @@ private: // it is possible that the variable is initialized here if (Token::Match(tok2->previous(), "[(,] %var% [,)]")) bailouts.insert(tok2->varId()); + + // array initialization.. + if (Token::Match(tok2->previous(), "[,(] %var% +")) + { + // if var is array, bailout + for (std::list::const_iterator it = checks.begin(); it != checks.end(); ++it) + { + if ((*it)->varId == tok2->varId()) + { + const CheckUninitVar *c = dynamic_cast(*it); + if (c && c->array) + bailouts.insert(tok2->varId()); + break; + } + } + } } } diff --git a/test/testother.cpp b/test/testother.cpp index 10b312e89..b3bd301f5 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -1930,7 +1930,7 @@ private: " y + 1 );\n" " s = y[0]*y[1];\n" "}\n"); - TODO_ASSERT_EQUALS("", errout.str()); + ASSERT_EQUALS("", errout.str()); } // alloc.. From 9f78177914d3649b649124f5a658de56ce462d06 Mon Sep 17 00:00:00 2001 From: Kimmo Varis Date: Mon, 19 Jul 2010 11:04:30 +0300 Subject: [PATCH 4/7] Remove unused methods from ErrorLogger. --- lib/errorlogger.h | 183 ---------------------------------------------- 1 file changed, 183 deletions(-) diff --git a/lib/errorlogger.h b/lib/errorlogger.h index 7e3a23986..9487a198a 100644 --- a/lib/errorlogger.h +++ b/lib/errorlogger.h @@ -153,189 +153,6 @@ public: */ virtual void reportStatus(unsigned int index, unsigned int max) = 0; - static bool outOfBounds() - { - return true; - } - - static bool stlOutOfBounds() - { - return true; - } - - static bool noConstructor(const Settings &s) - { - return s._checkCodingStyle; - } - - static bool uninitVar(const Settings &s) - { - return s._checkCodingStyle; - } - - static bool unusedPrivateFunction(const Settings &s) - { - return s._checkCodingStyle; - } - - static bool memsetClass() - { - return true; - } - - static bool memsetStruct() - { - return true; - } - - static bool operatorEq(const Settings &s) - { - return s._checkCodingStyle; - } - - static bool virtualDestructor() - { - return true; - } - - static bool mismatchAllocDealloc() - { - return true; - } - - static bool memleak() - { - return true; - } - - static bool resourceLeak() - { - return true; - } - - static bool deallocDealloc() - { - return true; - } - - static bool deallocuse() - { - return true; - } - - static bool mismatchSize() - { - return true; - } - - static bool cstyleCast(const Settings &s) - { - return s._checkCodingStyle; - } - - - static bool redundantIfDelete0(const Settings &s) - { - return s._checkCodingStyle; - } - - - static bool redundantIfRemove(const Settings &s) - { - return s._checkCodingStyle; - } - - static bool dangerousUsageStrtol() - { - return true; - } - - static bool ifNoAction(const Settings &s) - { - return s._checkCodingStyle; - } - - static bool sprintfOverlappingData() - { - return true; - } - - - static bool udivError() - { - return true; - } - - - static bool unusedStructMember(const Settings &s) - { - return s._checkCodingStyle; - } - - static bool passedByValue(const Settings &s) - { - return s._checkCodingStyle; - } - - static bool constStatement(const Settings &s) - { - return s._checkCodingStyle; - } - - - static bool charArrayIndex(const Settings &s) - { - return s._checkCodingStyle; - } - - - static bool charBitOp(const Settings &s) - { - return s._checkCodingStyle; - } - - - static bool variableScope() - { - return false; - } - - static bool conditionAlwaysTrueFalse(const Settings &s) - { - return s._checkCodingStyle; - } - - - static bool strPlusChar() - { - return true; - } - - - static bool returnLocalVariable() - { - return true; - } - - - static bool dangerousFunctionmktemp(const Settings &s) - { - return s._checkCodingStyle; - } - - - static bool dangerousFunctiongets(const Settings &s) - { - return s._checkCodingStyle; - } - - - static bool dangerousFunctionscanf(const Settings &s) - { - return s._checkCodingStyle; - } - - static std::string callStackToString(const std::list &callStack); }; From 7ef0296f9753d7f07dcc1012b406f5d25993cd09 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Mon, 19 Jul 2010 11:30:01 +0200 Subject: [PATCH 5/7] --verbose: added more information for the variableScope error message. --- lib/checkother.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/checkother.cpp b/lib/checkother.cpp index c1a0f30b7..ad36087a2 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -3961,7 +3961,11 @@ void CheckOther::charBitOpError(const Token *tok) void CheckOther::variableScopeError(const Token *tok, const std::string &varname) { - reportError(tok, Severity::style, "variableScope", "The scope of the variable " + varname + " can be reduced"); + reportError(tok, + Severity::style, + "variableScope", + "The scope of the variable " + varname + " can be reduced\n" + "Be very careful when you reduce the scope! The logical behaviour can be changed by mistake, for example when reducing the scope in a loop."); } void CheckOther::conditionAlwaysTrueFalse(const Token *tok, const std::string &truefalse) From d4d0bc050a92e582d90fb6f21b7498714773ea1e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Mon, 19 Jul 2010 12:06:20 +0200 Subject: [PATCH 6/7] Fixed #1865 (Tokenizer::simplifyRedundantParantheses: wrong handling of 'operator delete') --- lib/tokenize.cpp | 5 +++-- test/testtokenize.cpp | 17 ++++++++++++++++- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/lib/tokenize.cpp b/lib/tokenize.cpp index 21f01524b..bdb27491d 100644 --- a/lib/tokenize.cpp +++ b/lib/tokenize.cpp @@ -5735,8 +5735,9 @@ bool Tokenizer::simplifyRedundantParanthesis() ret = true; } - if ((Token::simpleMatch(tok->previous(), "delete (") && Token::Match(tok->link(), ") ;|,")) || - (Token::simpleMatch(tok->previous(), "; (") && Token::Match(tok->link(), ") ;"))) + if (!Token::simpleMatch(tok->tokAt(-2), "operator delete") && + Token::Match(tok->previous(), "delete|; (") && + Token::Match(tok->link(), ") ;|,")) { tok->link()->deleteThis(); tok->deleteThis(); diff --git a/test/testtokenize.cpp b/test/testtokenize.cpp index 330ba12ea..51f0d677f 100644 --- a/test/testtokenize.cpp +++ b/test/testtokenize.cpp @@ -165,13 +165,14 @@ private: TEST_CASE(simplify_function_parameters); - TEST_CASE(removeParantheses1); // Ticket #61 + TEST_CASE(removeParantheses1); // Ticket #61 TEST_CASE(removeParantheses2); TEST_CASE(removeParantheses3); TEST_CASE(removeParantheses4); // Ticket #390 TEST_CASE(removeParantheses5); // Ticket #392 TEST_CASE(removeParantheses6); TEST_CASE(removeParantheses7); + TEST_CASE(removeParantheses8); // Ticket #1865 TEST_CASE(tokenize_double); TEST_CASE(tokenize_strings); @@ -3014,6 +3015,20 @@ private: ASSERT_EQUALS(" ; delete p ; ( p ) = 0 ;", ostr.str()); } + void removeParantheses8() + { + const char code[] = "struct foo {\n" + " void operator delete(void *obj, size_t sz);\n" + "}\n"; + const std::string actual(tokenizeAndStringify(code)); + + const char expected[] = "struct foo {\n" + "void operator delete ( void * obj , size_t sz ) ;\n" + "}"; + + ASSERT_EQUALS(expected, actual); + } + void tokenize_double() { const char code[] = "void f()\n" From 4cf92992a8ea3b7216a431be2c22a17baf9bf3fb Mon Sep 17 00:00:00 2001 From: Robert Reif Date: Mon, 19 Jul 2010 13:16:11 +0200 Subject: [PATCH 7/7] Fixed #1883 (false positive: (style) The function 'A::SetPos' can be const) --- lib/checkclass.cpp | 73 ++++++++++++++++++++++++++++++++++++++++++---- lib/checkclass.h | 4 +-- test/testclass.cpp | 20 +++++++++++++ 3 files changed, 89 insertions(+), 8 deletions(-) diff --git a/lib/checkclass.cpp b/lib/checkclass.cpp index c3c06e75e..2669a1a89 100644 --- a/lib/checkclass.cpp +++ b/lib/checkclass.cpp @@ -1992,7 +1992,7 @@ void CheckClass::checkConst() } // if nothing non-const was found. write error.. - if (checkConstFunc(classname, varlist, paramEnd)) + if (checkConstFunc(info.className, info.derivedFrom, varlist, paramEnd)) { for (int i = nestInfo.size() - 2; i >= 0; i--) classname = std::string(nestInfo[i].className + "::" + classname); @@ -2027,7 +2027,7 @@ void CheckClass::checkConst() if (sameFunc(namespaceLevel, tok2, paramEnd)) { // if nothing non-const was found. write error.. - if (checkConstFunc(classname, varlist, paramEnd)) + if (checkConstFunc(info.className, info.derivedFrom, varlist, paramEnd)) { for (int k = nestInfo.size() - 2; k >= 0; k--) classname = std::string(nestInfo[k].className + "::" + classname); @@ -2254,7 +2254,7 @@ bool CheckClass::isMemberFunc(const Token *tok) return false; } -bool CheckClass::isMemberVar(const std::string &classname, const Var *varlist, const Token *tok) +bool CheckClass::isMemberVar(const std::string &classname, const std::vector &derivedFrom, const Var *varlist, const Token *tok) { while (tok->previous() && !Token::Match(tok->previous(), "}|{|;|public:|protected:|private:|return|:|?")) { @@ -2282,10 +2282,71 @@ bool CheckClass::isMemberVar(const std::string &classname, const Var *varlist, c } } + // not found in this class + if (!derivedFrom.empty()) + { + // check each base class + for (unsigned int i = 0; i < derivedFrom.size(); ++i) + { + std::string className; + + if (derivedFrom[i].find("::") != std::string::npos) + { + /** @todo handle nested base classes and namespaces */ + } + else + className = derivedFrom[i]; + + std::string classPattern = std::string("class|struct ") + className + std::string(" {|:"); + + // find the base class + const Token *classToken = Token::findmatch(_tokenizer->tokens(), classPattern.c_str()); + + // find the function in the base class + if (classToken) + { + std::vector baseList; + const Token * tok1 = classToken; + + while (tok1->str() != "{") + { + // check for base classes + if (Token::Match(tok1, ":|, public|protected|private")) + { + // jump to base class name + tok1 = tok1->tokAt(2); + + std::string base; + + // handle nested base classea and namespacess + while (Token::Match(tok1, "%var% ::")) + { + base += tok1->str(); + base += " :: "; + tok1 = tok1->tokAt(2); + } + + base += tok1->str(); + + // save pattern for base class name + baseList.push_back(base); + } + tok1 = tok1->next(); + } + + // Get class variables... + Var *varlist1 = getVarList(classToken); + + if (isMemberVar(classToken->next()->str(), baseList, varlist1, tok)) + return true; + } + } + } + return false; } -bool CheckClass::checkConstFunc(const std::string &classname, const Var *varlist, const Token *tok) +bool CheckClass::checkConstFunc(const std::string &classname, const std::vector &derivedFrom, const Var *varlist, const Token *tok) { // if the function doesn't have any assignment nor function call, // it can be a const function.. @@ -2307,7 +2368,7 @@ bool CheckClass::checkConstFunc(const std::string &classname, const Var *varlist (tok1->str().find("=") == 1 && tok1->str().find_first_of("") == std::string::npos)) { - if (isMemberVar(classname, varlist, tok1->previous())) + if (isMemberVar(classname, derivedFrom, varlist, tok1->previous())) { isconst = false; break; @@ -2315,7 +2376,7 @@ bool CheckClass::checkConstFunc(const std::string &classname, const Var *varlist } // streaming: << - else if (tok1->str() == "<<" && isMemberVar(classname, varlist, tok1->previous())) + else if (tok1->str() == "<<" && isMemberVar(classname, derivedFrom, varlist, tok1->previous())) { isconst = false; break; diff --git a/lib/checkclass.h b/lib/checkclass.h index 4c33b8e7e..0c0b811b5 100644 --- a/lib/checkclass.h +++ b/lib/checkclass.h @@ -165,8 +165,8 @@ private: bool sameFunc(int nest, const Token *firstEnd, const Token *secondEnd); bool isMemberFunc(const Token *tok); - bool isMemberVar(const std::string &classname, const Var *varlist, const Token *tok); - bool checkConstFunc(const std::string &classname, const Var *varlist, const Token *tok); + bool isMemberVar(const std::string &classname, const std::vector &derivedFrom, const Var *varlist, const Token *tok); + bool checkConstFunc(const std::string &classname, const std::vector &derivedFrom, const Var *varlist, const Token *tok); /** @brief check if this function is virtual in the base classes */ bool isVirtual(const std::vector &derivedFrom, const Token *functionToken) const; diff --git a/test/testclass.cpp b/test/testclass.cpp index 5fc91fb93..c56401fa9 100644 --- a/test/testclass.cpp +++ b/test/testclass.cpp @@ -130,6 +130,7 @@ private: TEST_CASE(const25); // ticket #1724 TEST_CASE(const26); // ticket #1847 TEST_CASE(const27); // ticket #1882 + TEST_CASE(const28); // ticket #1883 TEST_CASE(constoperator1); // operator< can often be const TEST_CASE(constoperator2); // operator<< TEST_CASE(constincdec); // increment/decrement => non-const @@ -3612,6 +3613,25 @@ private: ASSERT_EQUALS("", errout.str()); } + void const28() // ticket #1883 + { + checkConst("class P {\n" + "public:\n" + " P() { x=0.0; y=0.0; }\n" + " double x,y;\n" + "};\n" + "class A : public P {\n" + "public:\n" + " A():P(){}\n" + " void SetPos(double xPos, double yPos) {\n" + " x=xPos;\n" + " y=yPos;\n" + " }\n" + "};\n" + ); + ASSERT_EQUALS("", errout.str()); + } + void const27() // ticket #1882 { checkConst("class A {\n"