diff --git a/lib/checkclass.cpp b/lib/checkclass.cpp index 121b02d58..cde40fb69 100644 --- a/lib/checkclass.cpp +++ b/lib/checkclass.cpp @@ -2039,6 +2039,42 @@ void CheckClass::initializerListError(const Token *tok1, const Token *tok2, cons "initialization errors.", true); } + +//--------------------------------------------------------------------------- +// Check for self initialization in initialization list +//--------------------------------------------------------------------------- + +void CheckClass::checkSelfInitialization() +{ + const SymbolDatabase *symbolDatabase = _tokenizer->getSymbolDatabase(); + for (std::size_t i = 0; i < symbolDatabase->functionScopes.size(); ++i) { + const Scope* scope = symbolDatabase->functionScopes[i]; + const Function* function = scope->function; + if (!function || !function->isConstructor()) + continue; + + const Token* tok = function->arg->link()->next(); + if (tok->str() != ":") + continue; + + for (; tok != scope->classStart; tok = tok->next()) { + if (Token::Match(tok, "[:,] %var% ( %var% )") && tok->next()->varId() && tok->next()->varId() == tok->tokAt(3)->varId()) { + selfInitializationError(tok, tok->strAt(1)); + } + } + } +} + +void CheckClass::selfInitializationError(const Token* tok, const std::string& name) +{ + reportError(tok, Severity::error, "selfInitialization", "Member variable '" + name + "' is initialized by itself.\n"); +} + + +//--------------------------------------------------------------------------- +// Check for pure virtual function calls +//--------------------------------------------------------------------------- + void CheckClass::checkPureVirtualFunctionCall() { const std::size_t functions = symbolDatabase->functionScopes.size(); @@ -2064,6 +2100,98 @@ void CheckClass::checkPureVirtualFunctionCall() } } +const std::list & CheckClass::callsPureVirtualFunction(const Function & function, + std::map > & callsPureVirtualFunctionMap) +{ + std::pair >::iterator, bool > found = + callsPureVirtualFunctionMap.insert(std::pair >(&function, std::list())); + std::list & pureFunctionCalls = found.first->second; + if (found.second) { + if (function.hasBody) { + for (const Token *tok = function.arg->link(); + tok && tok != function.functionScope->classEnd; + tok = tok->next()) { + if (function.type != Function::eConstructor && + function.type != Function::eCopyConstructor && + function.type != Function::eMoveConstructor && + function.type != Function::eDestructor) { + if ((Token::simpleMatch(tok, ") {") && + tok->link() && + Token::Match(tok->link()->previous(), "if|switch")) || + Token::simpleMatch(tok, "else {") + ) { + // Assume pure virtual function call is prevented by "if|else|switch" condition + tok = tok->linkAt(1); + continue; + } + } + const Function * callFunction = tok->function(); + if (!callFunction || + function.nestedIn != callFunction->nestedIn || + (tok->previous() && tok->previous()->str() == ".")) + continue; + + if (tok->previous() && + tok->previous()->str() == "(") { + const Token * prev = tok->previous(); + if (prev->previous() && + (_settings->library.ignorefunction(tok->str()) + || _settings->library.ignorefunction(prev->previous()->str()))) + continue; + } + + if (isPureWithoutBody(*callFunction)) { + pureFunctionCalls.push_back(tok); + continue; + } + + const std::list & pureFunctionCallsOfTok = callsPureVirtualFunction(*callFunction, + callsPureVirtualFunctionMap); + if (!pureFunctionCallsOfTok.empty()) { + pureFunctionCalls.push_back(tok); + continue; + } + } + } + } + return pureFunctionCalls; +} + +void CheckClass::getFirstPureVirtualFunctionCallStack( + std::map > & callsPureVirtualFunctionMap, + const Token & pureCall, + std::list & pureFuncStack) +{ + if (isPureWithoutBody(*pureCall.function())) { + pureFuncStack.push_back(pureCall.function()->token); + return; + } + std::map >::const_iterator found = callsPureVirtualFunctionMap.find(pureCall.function()); + if (found == callsPureVirtualFunctionMap.end() || + found->second.empty()) { + pureFuncStack.clear(); + return; + } + const Token & firstPureCall = **found->second.begin(); + pureFuncStack.push_back(&firstPureCall); + getFirstPureVirtualFunctionCallStack(callsPureVirtualFunctionMap, firstPureCall, pureFuncStack); +} + +void CheckClass::callsPureVirtualFunctionError( + const Function & scopeFunction, + const std::list & tokStack, + const std::string &purefuncname) +{ + const char * scopeFunctionTypeName = getFunctionTypeName(scopeFunction.type); + reportError(tokStack, Severity::warning, "pureVirtualCall", "Call of pure virtual function '" + purefuncname + "' in " + scopeFunctionTypeName + ".\n" + "Call of pure virtual function '" + purefuncname + "' in " + scopeFunctionTypeName + ". The call will fail during runtime."); +} + + +//--------------------------------------------------------------------------- +// Check for members hiding inherited members with the same name +//--------------------------------------------------------------------------- + void CheckClass::checkDuplInheritedMembers() { if (!_settings->isEnabled("warning")) @@ -2112,91 +2240,3 @@ void CheckClass::duplInheritedMembersError(const Token *tok1, const Token* tok2, std::string(baseIsStruct ? "struct" : "class") + " '" + basename + "'."; reportError(toks, Severity::warning, "duplInheritedMember", message); } - -const std::list & CheckClass::callsPureVirtualFunction(const Function & function, - std::map > & callsPureVirtualFunctionMap) -{ - std::pair >::iterator , bool > found= - callsPureVirtualFunctionMap.insert(std::pair >(&function,std::list())); - std::list & pureFunctionCalls=found.first->second; - if (found.second) { - if (function.hasBody) { - for (const Token *tok = function.arg->link(); - tok && tok != function.functionScope->classEnd; - tok = tok->next()) { - if (function.type != Function::eConstructor && - function.type != Function::eCopyConstructor && - function.type != Function::eMoveConstructor && - function.type != Function::eDestructor) { - if ((Token::simpleMatch(tok,") {") && - tok->link() && - Token::Match(tok->link()->previous(),"if|switch")) || - Token::simpleMatch(tok,"else {") - ) { - // Assume pure virtual function call is prevented by "if|else|switch" condition - tok = tok->linkAt(1); - continue; - } - } - const Function * callFunction=tok->function(); - if (!callFunction || - function.nestedIn != callFunction->nestedIn || - (tok->previous() && tok->previous()->str()==".")) - continue; - - if (tok->previous() && - tok->previous()->str()=="(") { - const Token * prev = tok->previous(); - if (prev->previous() && - (_settings->library.ignorefunction(tok->str()) - || _settings->library.ignorefunction(prev->previous()->str()))) - continue; - } - - if (isPureWithoutBody(*callFunction)) { - pureFunctionCalls.push_back(tok); - continue; - } - - const std::list & pureFunctionCallsOfTok=callsPureVirtualFunction(*callFunction, - callsPureVirtualFunctionMap); - if (!pureFunctionCallsOfTok.empty()) { - pureFunctionCalls.push_back(tok); - continue; - } - } - } - } - return pureFunctionCalls; -} - -void CheckClass::getFirstPureVirtualFunctionCallStack( - std::map > & callsPureVirtualFunctionMap, - const Token & pureCall, - std::list & pureFuncStack) -{ - if (isPureWithoutBody(*pureCall.function())) { - pureFuncStack.push_back(pureCall.function()->token); - return; - } - std::map >::const_iterator found=callsPureVirtualFunctionMap.find(pureCall.function()); - if (found==callsPureVirtualFunctionMap.end() || - found->second.empty()) { - pureFuncStack.clear(); - return; - } - const Token & firstPureCall=**found->second.begin(); - pureFuncStack.push_back(&firstPureCall); - getFirstPureVirtualFunctionCallStack(callsPureVirtualFunctionMap, firstPureCall, pureFuncStack); -} - -void CheckClass::callsPureVirtualFunctionError( - const Function & scopeFunction, - const std::list & tokStack, - const std::string &purefuncname) -{ - const char * scopeFunctionTypeName=getFunctionTypeName(scopeFunction.type); - reportError(tokStack, Severity::warning, "pureVirtualCall", "Call of pure virtual function '" + purefuncname + "' in " + scopeFunctionTypeName + ".\n" - "Call of pure virtual function '" + purefuncname + "' in " + scopeFunctionTypeName + ". The call will fail during runtime."); -} - diff --git a/lib/checkclass.h b/lib/checkclass.h index 5ef1fc2d6..717ab2ad3 100644 --- a/lib/checkclass.h +++ b/lib/checkclass.h @@ -68,6 +68,7 @@ public: checkClass.operatorEqToSelf(); checkClass.initializerListOrder(); checkClass.initializationListUsage(); + checkClass.checkSelfInitialization(); checkClass.virtualDestructor(); checkClass.checkConst(); @@ -116,8 +117,12 @@ public: /** @brief Check initializer list order */ void initializerListOrder(); + /** @brief Suggest using initialization list */ void initializationListUsage(); + /** @brief Check for initialization of a member with itself */ + void checkSelfInitialization(); + void copyconstructors(); /** @brief call of pure virtual funcion */ @@ -150,6 +155,7 @@ private: void checkConstError2(const Token *tok1, const Token *tok2, const std::string &classname, const std::string &funcname, bool suggestStatic); void initializerListError(const Token *tok1,const Token *tok2, const std::string & classname, const std::string &varname); void suggestInitializationList(const Token *tok, const std::string& varname); + void selfInitializationError(const Token* tok, const std::string& name); void callsPureVirtualFunctionError(const Function & scopeFunction, const std::list & tokStack, const std::string &purefuncname); void duplInheritedMembersError(const Token* tok1, const Token* tok2, const std::string &derivedname, const std::string &basename, const std::string &variablename, bool derivedIsStruct, bool baseIsStruct); @@ -174,6 +180,7 @@ private: c.checkConstError(0, "class", "function", true); c.initializerListError(0, 0, "class", "variable"); c.suggestInitializationList(0, "variable"); + c.selfInitializationError(0, "var"); c.duplInheritedMembersError(0, 0, "class", "class", "variable", false, false); } @@ -196,6 +203,7 @@ private: "* Constness for member functions\n" "* Order of initializations\n" "* Suggest usage of initialization list\n" + "* Initialization of a member with itself\n" "* Suspicious subtraction from 'this'\n" "* Call of pure virtual function in constructor/destructor\n" "* Duplicated inherited data members\n"; diff --git a/test/testclass.cpp b/test/testclass.cpp index 56a9b7274..8a17e8b37 100644 --- a/test/testclass.cpp +++ b/test/testclass.cpp @@ -57,6 +57,7 @@ private: TEST_CASE(noConstructor7); // ticket #4391 TEST_CASE(noConstructor8); // ticket #4404 TEST_CASE(noConstructor9); // ticket #4419 + TEST_CASE(forwardDeclaration); // ticket #4290/#3190 TEST_CASE(operatorEq1); TEST_CASE(operatorEq2); @@ -178,8 +179,7 @@ private: TEST_CASE(initializerListOrder); TEST_CASE(initializerListUsage); - - TEST_CASE(forwardDeclaration); // ticket #4290/#3190 + TEST_CASE(selfInitialization); TEST_CASE(pureVirtualFunctionCall); TEST_CASE(pureVirtualFunctionCallOtherClass); @@ -2019,12 +2019,12 @@ private: ASSERT_EQUALS("[test.cpp:3]: (error) Class 'Base' which is inherited by class 'Derived' does not have a virtual destructor.\n", errout.str()); } - void checkNoConstructor(const char code[], const char* level="style") { + void checkNoConstructor(const char code[]) { // Clear the error log errout.str(""); Settings settings; - settings.addEnabled(level); + settings.addEnabled("style"); // Tokenize.. Tokenizer tokenizer(&settings, this); @@ -2124,6 +2124,22 @@ private: ASSERT_EQUALS("", errout.str()); } + // ticket #4290 "False Positive: style (noConstructor): The class 'foo' does not have a constructor." + // ticket #3190 "SymbolDatabase: Parse of sub class constructor fails" + void forwardDeclaration() { + checkNoConstructor("class foo;\n" + "int bar;\n"); + ASSERT_EQUALS("", errout.str()); + + checkNoConstructor("class foo;\n" + "class foo;\n"); + ASSERT_EQUALS("", errout.str()); + + checkNoConstructor("class foo{};\n" + "class foo;\n"); + ASSERT_EQUALS("", errout.str()); + } + void checkNoMemset(const char code[], bool load_std_cfg = false) { // Clear the error log errout.str(""); @@ -5826,19 +5842,52 @@ private: ASSERT_EQUALS("", errout.str()); } - // ticket #4290 "False Positive: style (noConstructor): The class 'foo' does not have a constructor." - // ticket #3190 "SymbolDatabase: Parse of sub class constructor fails" - void forwardDeclaration() { - checkConst("class foo;\n" - "int bar;\n"); - ASSERT_EQUALS("", errout.str()); - checkConst("class foo;\n" - "class foo;\n"); - ASSERT_EQUALS("", errout.str()); + void checkSelfInitialization(const char code []) { + // Clear the error log + errout.str(""); - checkConst("class foo{};\n" - "class foo;\n"); + // Check.. + Settings settings; + + // Tokenize.. + Tokenizer tokenizer(&settings, this); + std::istringstream istr(code); + tokenizer.tokenize(istr, "test.cpp"); + tokenizer.simplifyTokenList2(); + + CheckClass checkClass(&tokenizer, &settings, this); + checkClass.checkSelfInitialization(); + } + + void selfInitialization() { + checkSelfInitialization("class Fred {\n" + " int i;\n" + " Fred() : i(i) {\n" + " }\n" + "};"); + ASSERT_EQUALS("[test.cpp:3]: (error) Member variable 'i' is initialized by itself.\n", errout.str()); + + checkSelfInitialization("class Fred {\n" + " int i;\n" + " Fred();\n" + "};\n" + "Fred::Fred() : i(i) {\n" + "}"); + ASSERT_EQUALS("[test.cpp:5]: (error) Member variable 'i' is initialized by itself.\n", errout.str()); + + checkSelfInitialization("class Fred {\n" + " std::string s;\n" + " Fred() : s(s) {\n" + " }\n" + "};"); + ASSERT_EQUALS("[test.cpp:3]: (error) Member variable 's' is initialized by itself.\n", errout.str()); + + checkSelfInitialization("class Fred {\n" + " std::string s;\n" + " Fred(const std::string& s) : s(s) {\n" + " }\n" + "};"); ASSERT_EQUALS("", errout.str()); } @@ -6052,7 +6101,7 @@ private: ASSERT_THROW(checkNoConstructor("struct R1 {\n" " int a;\n" " R1 () : a { }\n" - "};\n", "warning"), InternalError); + "};\n"), InternalError); } };