/* * Cppcheck - A tool for static C/C++ code analysis * Copyright (C) 2007-2011 Daniel Marjamäki and Cppcheck team. * * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by * the Free Software Foundation, either version 3 of the License, or * (at your option) any later version. * * This program is distributed in the hope that it will be useful, * but WITHOUT ANY WARRANTY; without even the implied warranty of * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the * GNU General Public License for more details. * * You should have received a copy of the GNU General Public License * along with this program. If not, see . */ //--------------------------------------------------------------------------- #include "checkclass.h" #include "tokenize.h" #include "token.h" #include "errorlogger.h" #include "symboldatabase.h" #include #include #include #include #include //--------------------------------------------------------------------------- // Register CheckClass.. namespace { CheckClass instance; } //--------------------------------------------------------------------------- CheckClass::CheckClass(const Tokenizer *tokenizer, const Settings *settings, ErrorLogger *errorLogger) : Check(tokenizer, settings, errorLogger), symbolDatabase(NULL) { } void CheckClass::createSymbolDatabase() { // Multiple calls => bail out if (symbolDatabase) return; symbolDatabase = _tokenizer->getSymbolDatabase(); } //--------------------------------------------------------------------------- // ClassCheck: Check that all class constructors are ok. //--------------------------------------------------------------------------- void CheckClass::constructors() { if (!_settings->_checkCodingStyle) return; createSymbolDatabase(); std::list::const_iterator i; for (i = symbolDatabase->scopeList.begin(); i != symbolDatabase->scopeList.end(); ++i) { const Scope *scope = *i; // only check classes and structures if (!scope->isClassOrStruct()) continue; // There are no constructors. if (scope->numConstructors == 0) { // If there is a private variable, there should be a constructor.. std::list::const_iterator var; for (var = scope->varlist.begin(); var != scope->varlist.end(); ++var) { if (var->isPrivate() && !var->isClass() && !var->isStatic()) { noConstructorError(scope->classDef, scope->className, scope->classDef->str() == "struct"); break; } } } std::list::const_iterator func; std::vector usage(scope->varlist.size()); for (func = scope->functionList.begin(); func != scope->functionList.end(); ++func) { if (!func->hasBody || !(func->type == Function::eConstructor || func->type == Function::eCopyConstructor || func->type == Function::eOperatorEqual)) continue; // Mark all variables not used clearAllVar(usage); std::list callstack; initializeVarList(*func, callstack, scope, usage); // Check if any variables are uninitialized std::list::const_iterator var; unsigned int count = 0; for (var = scope->varlist.begin(); var != scope->varlist.end(); ++var, ++count) { if (usage[count].assign || usage[count].init || var->isStatic()) continue; if (var->isConst() && var->nameToken()->previous()->str() != "*") continue; // Check if this is a class constructor if (var->isClass() && func->type == Function::eConstructor) { // Unknown type so assume it is initialized if (!var->type()) continue; // Known type that doesn't need initialization or // known type that has member variables of an unknown type else if (var->type()->needInitialization != Scope::True) continue; } // Check if type can't be copied if (var->type() && canNotCopy(var->type())) continue; // It's non-static and it's not initialized => error if (func->type == Function::eOperatorEqual) { const Token *operStart = 0; if (func->token->str() == "=") operStart = func->token->tokAt(1); else operStart = func->token->tokAt(3); bool classNameUsed = false; for (const Token *operTok = operStart; operTok != operStart->link(); operTok = operTok->next()) { if (operTok->str() == scope->className) { classNameUsed = true; break; } } if (classNameUsed) operatorEqVarError(func->token, scope->className, var->name()); } else if (func->access != Private) uninitVarError(func->token, scope->className, var->name()); } } } } bool CheckClass::canNotCopy(const Scope *scope) const { std::list::const_iterator func; bool privateAssign = false; bool privateCopy = false; for (func = scope->functionList.begin(); func != scope->functionList.end(); ++func) { if (func->type == Function::eCopyConstructor && func->access == Private) privateCopy = true; else if (func->type == Function::eOperatorEqual && func->access == Private) privateAssign = true; } return privateAssign && privateCopy; } void CheckClass::assignVar(const std::string &varname, const Scope *scope, std::vector &usage) { std::list::const_iterator var; unsigned int count = 0; for (var = scope->varlist.begin(); var != scope->varlist.end(); ++var, ++count) { if (var->name() == varname) { usage[count].assign = true; return; } } } void CheckClass::initVar(const std::string &varname, const Scope *scope, std::vector &usage) { std::list::const_iterator var; unsigned int count = 0; for (var = scope->varlist.begin(); var != scope->varlist.end(); ++var, ++count) { if (var->name() == varname) { usage[count].init = true; return; } } } void CheckClass::assignAllVar(std::vector &usage) { for (size_t i = 0; i < usage.size(); ++i) usage[i].assign = true; } void CheckClass::clearAllVar(std::vector &usage) { for (size_t i = 0; i < usage.size(); ++i) { usage[i].assign = false; usage[i].init = false; } } bool CheckClass::isBaseClassFunc(const Token *tok, const Scope *scope) { // Iterate through each base class... for (size_t i = 0; i < scope->derivedFrom.size(); ++i) { const Scope *derivedFrom = scope->derivedFrom[i].scope; // Check if base class exists in database if (derivedFrom) { std::list::const_iterator func; for (func = derivedFrom->functionList.begin(); func != derivedFrom->functionList.end(); ++func) { if (func->tokenDef->str() == tok->str()) return true; } } // Base class not found so assume it is in it. else return true; } return false; } void CheckClass::initializeVarList(const Function &func, std::list &callstack, const Scope *scope, std::vector &usage) { bool Assign = false; unsigned int indentlevel = 0; const Token *ftok = func.token; for (; ftok; ftok = ftok->next()) { if (!ftok->next()) break; // Class constructor.. initializing variables like this // clKalle::clKalle() : var(value) { } if (indentlevel == 0) { if (Assign && Token::Match(ftok, "%var% (")) { initVar(ftok->str(), scope, usage); // assignment in the initializer.. // : var(value = x) if (Token::Match(ftok->tokAt(2), "%var% =")) assignVar(ftok->strAt(2), scope, usage); } Assign |= (ftok->str() == ":"); } if (ftok->str() == "{") { ++indentlevel; Assign = false; } else if (ftok->str() == "}") { if (indentlevel <= 1) break; --indentlevel; } if (indentlevel < 1) continue; // Variable getting value from stream? if (Token::Match(ftok, ">> %var%")) { assignVar(ftok->strAt(1), scope, usage); } // Before a new statement there is "[{};)=]" if (! Token::Match(ftok, "[{};()=]")) continue; if (Token::simpleMatch(ftok, "( !")) ftok = ftok->next(); // Using the operator= function to initialize all variables.. if (Token::simpleMatch(ftok->next(), "* this = ")) { assignAllVar(usage); break; } // Calling member variable function? if (Token::Match(ftok->next(), "%var% . %var% (")) { std::list::const_iterator var; for (var = scope->varlist.begin(); var != scope->varlist.end(); ++var) { if (var->varId() == ftok->next()->varId()) { /** @todo false negative: we assume function changes variable state */ assignVar(ftok->next()->str(), scope, usage); continue; } } ftok = ftok->tokAt(2); } if (!Token::Match(ftok->next(), "%var%") && !Token::Match(ftok->next(), "this . %var%") && !Token::Match(ftok->next(), "* %var% =") && !Token::Match(ftok->next(), "( * this ) . %var%")) continue; // Goto the first token in this statement.. ftok = ftok->next(); // Skip "( * this )" if (Token::simpleMatch(ftok, "( * this ) .")) { ftok = ftok->tokAt(5); } // Skip "this->" if (Token::simpleMatch(ftok, "this .")) ftok = ftok->tokAt(2); // Skip "classname :: " if (Token::Match(ftok, "%var% ::")) ftok = ftok->tokAt(2); // Clearing all variables.. if (Token::simpleMatch(ftok, "memset ( this ,")) { assignAllVar(usage); return; } // Clearing array.. else if (Token::Match(ftok, "memset ( %var% ,")) { assignVar(ftok->strAt(2), scope, usage); ftok = ftok->next()->link(); continue; } // Calling member function? else if (Token::simpleMatch(ftok, "operator = (") && ftok->previous()->str() != "::") { // check if member function exists std::list::const_iterator it; for (it = scope->functionList.begin(); it != scope->functionList.end(); ++it) { if (ftok->next()->str() == it->tokenDef->str() && it->type != Function::eConstructor) break; } // member function found if (it != scope->functionList.end()) { // member function has implementation if (it->hasBody) { // initialize variable use list using member function callstack.push_back(ftok->str()); initializeVarList(*it, callstack, scope, usage); callstack.pop_back(); } // there is a called member function, but it has no implementation, so we assume it initializes everything else { assignAllVar(usage); } } // using default operator =, assume everything initialized else { assignAllVar(usage); } } else if (Token::Match(ftok, "%var% (") && ftok->str() != "if") { // Passing "this" => assume that everything is initialized for (const Token *tok2 = ftok->next()->link(); tok2 && tok2 != ftok; tok2 = tok2->previous()) { if (tok2->str() == "this") { assignAllVar(usage); return; } } // recursive call / calling overloaded function // assume that all variables are initialized if (std::find(callstack.begin(), callstack.end(), ftok->str()) != callstack.end()) { assignAllVar(usage); return; } // check if member function std::list::const_iterator it; for (it = scope->functionList.begin(); it != scope->functionList.end(); ++it) { if (ftok->str() == it->tokenDef->str() && it->type != Function::eConstructor) break; } // member function found if (it != scope->functionList.end()) { // member function has implementation if (it->hasBody) { // initialize variable use list using member function callstack.push_back(ftok->str()); initializeVarList(*it, callstack, scope, usage); callstack.pop_back(); } // there is a called member function, but it has no implementation, so we assume it initializes everything else { assignAllVar(usage); } } // not member function else { // could be a base class virtual function, so we assume it initializes everything if (func.type != Function::eConstructor && isBaseClassFunc(ftok, scope)) { /** @todo False Negative: we should look at the base class functions to see if they * call any derived class virtual functions that change the derived class state */ assignAllVar(usage); } // has friends, so we assume it initializes everything if (!scope->friendList.empty()) assignAllVar(usage); // the function is external and it's neither friend nor inherited virtual function. // assume all variables that are passed to it are initialized.. else { unsigned int indentlevel2 = 0; for (const Token *tok = ftok->tokAt(2); tok; tok = tok->next()) { if (tok->str() == "(") ++indentlevel2; else if (tok->str() == ")") { if (indentlevel2 == 0) break; --indentlevel2; } if (tok->isName()) { assignVar(tok->str(), scope, usage); } } } } } // Assignment of member variable? else if (Token::Match(ftok, "%var% =")) { assignVar(ftok->str(), scope, usage); } // Assignment of array item of member variable? else if (Token::Match(ftok, "%var% [ %any% ] =")) { assignVar(ftok->str(), scope, usage); } // Assignment of member of array item of member variable? else if (Token::Match(ftok, "%var% [ %any% ] . %var% =") || Token::Match(ftok, "%var% [ %any% ] . %var% . %var% =")) { assignVar(ftok->str(), scope, usage); } // Assignment of array item of member variable? else if (Token::Match(ftok, "%var% [ %any% ] [ %any% ] =")) { assignVar(ftok->str(), scope, usage); } // Assignment of array item of member variable? else if (Token::Match(ftok, "* %var% =")) { assignVar(ftok->next()->str(), scope, usage); } // Assignment of struct member of member variable? else if (Token::Match(ftok, "%var% . %any% =")) { assignVar(ftok->str(), scope, usage); } // The functions 'clear' and 'Clear' are supposed to initialize variable. if (Token::Match(ftok, "%var% . clear|Clear (")) { assignVar(ftok->str(), scope, usage); } } } void CheckClass::noConstructorError(const Token *tok, const std::string &classname, bool isStruct) { // For performance reasons the constructor might be intentionally missing. Therefore this is not a "warning" reportError(tok, Severity::style, "noConstructor", "The " + std::string(isStruct ? "struct" : "class") + " '" + classname + "' does not have a constructor.\n" "The " + std::string(isStruct ? "struct" : "class") + " '" + classname + " 'does not have a constructor but it has attributes. " "The attributes are not initialized which may cause bugs or undefined behavior."); } void CheckClass::uninitVarError(const Token *tok, const std::string &classname, const std::string &varname) { reportError(tok, Severity::warning, "uninitVar", "Member variable '" + classname + "::" + varname + "' is not initialised in the constructor."); } void CheckClass::operatorEqVarError(const Token *tok, const std::string &classname, const std::string &varname) { reportError(tok, Severity::warning, "operatorEqVarError", "Member variable '" + classname + "::" + varname + "' is not assigned a value in '" + classname + "::operator=" + "'"); } //--------------------------------------------------------------------------- // ClassCheck: Unused private functions //--------------------------------------------------------------------------- void CheckClass::privateFunctions() { if (!_settings->_checkCodingStyle) return; // don't check code that contains templates. Templates that are // "unused" are removed from the code. #2067 if (_tokenizer->codeWithTemplates()) return; // dont check borland classes with properties.. if (Token::findmatch(_tokenizer->tokens(), "; __property ;")) return; // #2407 calls from operator() is not detected // TODO: Don't bailout. Detect the call. if (Token::findmatch(_tokenizer->tokens(), "operator ( )")) return; createSymbolDatabase(); std::list::const_iterator i; for (i = symbolDatabase->scopeList.begin(); i != symbolDatabase->scopeList.end(); ++i) { const Scope *scope = *i; // only check classes and structures if (!scope->isClassOrStruct()) continue; // don’t check derived classes if (!scope->derivedFrom.empty()) continue; // Locate some class const Token *tok1 = scope->classDef; // check that the whole class implementation is seen bool whole = true; std::list::const_iterator func; for (func = scope->functionList.begin(); func != scope->functionList.end(); ++func) { if (!func->hasBody) { // empty private copy constructors and assignment operators are OK if ((func->type == Function::eCopyConstructor || func->type == Function::eOperatorEqual) && func->access == Private) continue; whole = false; break; } } if (!whole) continue; const std::string &classname = tok1->next()->str(); std::list FuncList; /** @todo embedded class have access to private functions */ if (!scope->getNestedNonFunctions()) { for (func = scope->functionList.begin(); func != scope->functionList.end(); ++func) { // Get private functions.. if (func->type == Function::eFunction && func->access == Private && func->hasBody) FuncList.push_back(func->tokenDef); } } // Check that all private functions are used.. bool HasFuncImpl = false; bool inclass = false; int indent_level = 0; for (const Token *ftok = _tokenizer->tokens(); ftok; ftok = ftok->next()) { if (ftok->str() == "{") ++indent_level; else if (ftok->str() == "}") { if (indent_level > 0) --indent_level; if (indent_level == 0) inclass = false; } else if (ftok->str() == "class" && ftok->next()->str() == classname && Token::Match(ftok->tokAt(2), ":|{")) { indent_level = 0; inclass = true; } // Check member class functions to see what functions are used.. else if ((inclass && indent_level == 1 && Token::Match(ftok, "%var% (")) || (ftok->str() == classname && Token::Match(ftok->next(), ":: ~| %var% ("))) { while (ftok && ftok->str() != ")") ftok = ftok->next(); if (!ftok) break; if (Token::Match(ftok, ") : %var% (")) { while (!Token::Match(ftok->next(), "[{};]")) { if (Token::Match(ftok, "::|,|( %var% ,|)")) { // Remove function from FuncList std::list::iterator it = FuncList.begin(); while (it != FuncList.end()) { if (ftok->next()->str() == (*it)->str()) FuncList.erase(it++); else ++it; } } ftok = ftok->next(); } } if (!Token::Match(ftok, ") const| {")) continue; if (ftok->fileIndex() == 0) HasFuncImpl = true; // Parse function.. int indentlevel2 = 0; for (const Token *tok2 = ftok; tok2; tok2 = tok2->next()) { if (tok2->str() == "{") ++indentlevel2; else if (tok2->str() == "}") { --indentlevel2; if (indentlevel2 < 1) break; } else if (Token::Match(tok2, "%var% (")) { // Remove function from FuncList std::list::iterator it = FuncList.begin(); while (it != FuncList.end()) { if (tok2->str() == (*it)->str()) FuncList.erase(it++); else ++it; } } } } } while (HasFuncImpl && !FuncList.empty()) { // Final check; check if the function pointer is used somewhere.. const std::string _pattern("return|(|)|,|= " + FuncList.front()->str()); // or if the function address is used somewhere... // eg. sigc::mem_fun(this, &className::classFunction) const std::string _pattern2("& " + classname + " :: " + FuncList.front()->str()); const std::string methodAsArgument("(|, " + classname + " :: " + FuncList.front()->str() + " ,|)"); if (!Token::findmatch(_tokenizer->tokens(), _pattern.c_str()) && !Token::findmatch(_tokenizer->tokens(), _pattern2.c_str()) && !Token::findmatch(_tokenizer->tokens(), methodAsArgument.c_str()) ) { unusedPrivateFunctionError(FuncList.front(), classname, FuncList.front()->str()); } FuncList.pop_front(); } } } void CheckClass::unusedPrivateFunctionError(const Token *tok, const std::string &classname, const std::string &funcname) { reportError(tok, Severity::style, "unusedPrivateFunction", "Unused private function '" + classname + "::" + funcname + "'"); } //--------------------------------------------------------------------------- // ClassCheck: Check that memset is not used on classes //--------------------------------------------------------------------------- void CheckClass::noMemset() { // Locate all 'memset' tokens.. for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) { if (!Token::Match(tok, "memset|memcpy|memmove")) continue; std::string type; if (Token::Match(tok, "memset ( %var% , %num% , sizeof ( %type% ) )")) type = tok->strAt(8); else if (Token::Match(tok, "memset ( & %var% , %num% , sizeof ( %type% ) )")) type = tok->strAt(9); else if (Token::Match(tok, "memset ( %var% , %num% , sizeof ( struct %type% ) )")) type = tok->strAt(9); else if (Token::Match(tok, "memset ( & %var% , %num% , sizeof ( struct %type% ) )")) type = tok->strAt(10); else if (Token::Match(tok, "%type% ( %var% , %var% , sizeof ( %type% ) )")) type = tok->strAt(8); // No type defined => The tokens didn't match if (type.empty()) continue; // Warn if type is a class or struct that contains any std::* variables const std::string pattern2(std::string("struct|class ") + type + " {"); for (const Token *tstruct = Token::findmatch(_tokenizer->tokens(), pattern2.c_str()); tstruct; tstruct = tstruct->next()) { if (tstruct->str() == "}") break; // struct with function? skip function body.. if (Token::simpleMatch(tstruct, ") {")) { tstruct = tstruct->next()->link(); if (!tstruct) break; } // before a statement there must be either: // * private:|protected:|public: // * { } ; if (Token::Match(tstruct, "[;{}]") || tstruct->str().find(":") != std::string::npos) { if (Token::Match(tstruct->next(), "std :: %type% %var% ;")) memsetStructError(tok, tok->str(), tstruct->strAt(3)); else if (Token::Match(tstruct->next(), "std :: %type% < ")) { // backup the type const std::string typestr(tstruct->strAt(3)); // check if it's a pointer variable.. unsigned int level = 0; while (0 != (tstruct = tstruct->next())) { if (tstruct->str() == "<") ++level; else if (tstruct->str() == ">") { if (level <= 1) break; --level; } else if (tstruct->str() == "(") tstruct = tstruct->link(); } if (!tstruct) break; // found error => report if (Token::Match(tstruct, "> %var% ;")) memsetStructError(tok, tok->str(), typestr); } } } } } void CheckClass::memsetClassError(const Token *tok, const std::string &memfunc) { reportError(tok, Severity::error, "memsetClass", "Using '" + memfunc + "' on class"); } void CheckClass::memsetStructError(const Token *tok, const std::string &memfunc, const std::string &classname) { reportError(tok, Severity::error, "memsetStruct", "Using '" + memfunc + "' on struct that contains a 'std::" + classname + "'"); } //--------------------------------------------------------------------------- // ClassCheck: "void operator=(" and "const type & operator=(" //--------------------------------------------------------------------------- void CheckClass::operatorEq() { if (!_settings->_checkCodingStyle) return; createSymbolDatabase(); std::list::const_iterator i; for (i = symbolDatabase->scopeList.begin(); i != symbolDatabase->scopeList.end(); ++i) { std::list::const_iterator it; for (it = (*i)->functionList.begin(); it != (*i)->functionList.end(); ++it) { if (it->type == Function::eOperatorEqual && it->access != Private) { if (it->token->strAt(-2) == "void") operatorEqReturnError(it->token->tokAt(-2)); } } } } void CheckClass::operatorEqReturnError(const Token *tok) { reportError(tok, Severity::style, "operatorEq", "'operator=' should return something"); } //--------------------------------------------------------------------------- // ClassCheck: "C& operator=(const C&) { ... return *this; }" // operator= should return a reference to *this //--------------------------------------------------------------------------- void CheckClass::checkReturnPtrThis(const Scope *scope, const Function *func, const Token *tok, const Token *last) { bool foundReturn = false; for (; tok && tok != last; tok = tok->next()) { // check for return of reference to this if (tok->str() == "return") { foundReturn = true; std::string cast("( " + scope->className + " & )"); if (Token::Match(tok->next(), cast.c_str())) tok = tok->tokAt(4); // check if a function is called if (Token::Match(tok->tokAt(1), "%any% (") && tok->tokAt(2)->link()->next()->str() == ";") { std::list::const_iterator it; // check if it is a member function for (it = scope->functionList.begin(); it != scope->functionList.end(); ++it) { // check for a regular function with the same name and a body if (it->type == Function::eFunction && it->hasBody && it->token->str() == tok->next()->str()) { // check for the proper return type if (it->tokenDef->previous()->str() == "&" && it->tokenDef->strAt(-2) == scope->className) { // make sure it's not a const function if (it->arg->link()->next()->str() != "const") { /** @todo make sure argument types match */ // make sure it's not the same function if (&*it != func) checkReturnPtrThis(scope, &*it, it->arg->link()->next(), it->arg->link()->next()->link()); // just bail for now else return; } } } } } // check of *this is returned else if (!(Token::Match(tok->tokAt(1), "(| * this ;|=") || Token::Match(tok->tokAt(1), "(| * this +=") || Token::Match(tok->tokAt(1), "operator = ("))) operatorEqRetRefThisError(func->token); } } if (!foundReturn) operatorEqRetRefThisError(func->token); } void CheckClass::operatorEqRetRefThis() { if (!_settings->_checkCodingStyle) return; createSymbolDatabase(); std::list::const_iterator i; for (i = symbolDatabase->scopeList.begin(); i != symbolDatabase->scopeList.end(); ++i) { const Scope *scope = *i; // only check classes and structures if (scope->isClassOrStruct()) { std::list::const_iterator func; for (func = scope->functionList.begin(); func != scope->functionList.end(); ++func) { if (func->type == Function::eOperatorEqual && func->hasBody) { // make sure return signature is correct if (Token::Match(func->tokenDef->tokAt(-4), ";|}|{|public:|protected:|private: %type% &") && func->tokenDef->strAt(-3) == scope->className) { // find the ')' const Token *tok = func->token->next()->link(); checkReturnPtrThis(scope, &(*func), tok->tokAt(2), tok->next()->link()); } } } } } } void CheckClass::operatorEqRetRefThisError(const Token *tok) { reportError(tok, Severity::style, "operatorEqRetRefThis", "'operator=' should return reference to self"); } //--------------------------------------------------------------------------- // ClassCheck: "C& operator=(const C& rhs) { if (this == &rhs) ... }" // operator= should check for assignment to self // // For simple classes, an assignment to self check is only a potential optimization. // // For classes that allocate dynamic memory, assignment to self can be a real error // if it is deallocated and allocated again without being checked for. // // This check is not valid for classes with multiple inheritance because a // class can have multiple addresses so there is no trivial way to check for // assignment to self. //--------------------------------------------------------------------------- void CheckClass::operatorEqToSelf() { if (!_settings->_checkCodingStyle) return; createSymbolDatabase(); std::list::const_iterator i; for (i = symbolDatabase->scopeList.begin(); i != symbolDatabase->scopeList.end(); ++i) { const Scope *scope = *i; std::list::const_iterator it; // skip classes with multiple inheritance if (scope->derivedFrom.size() > 1) continue; for (it = scope->functionList.begin(); it != scope->functionList.end(); ++it) { if (it->type == Function::eOperatorEqual && it->hasBody) { // make sure return signature is correct if (Token::Match(it->tokenDef->tokAt(-4), ";|}|{|public:|protected:|private: %type% &") && it->tokenDef->strAt(-3) == scope->className) { // check for proper function parameter signature if ((Token::Match(it->tokenDef->next(), "( const %var% & )") || Token::Match(it->tokenDef->next(), "( const %var% & %var% )")) && it->tokenDef->strAt(3) == scope->className) { // find the parameter name const Token *rhs = it->token; while (rhs->str() != "&") rhs = rhs->next(); rhs = rhs->next(); // find the ')' const Token *tok = it->token->next()->link(); const Token *tok1 = tok; if (tok1 && tok1->tokAt(1) && tok1->tokAt(1)->str() == "{" && tok1->tokAt(1)->link()) { const Token *first = tok1->tokAt(1); const Token *last = first->link(); if (!hasAssignSelf(first, last, rhs)) { if (hasDeallocation(first, last)) operatorEqToSelfError(tok); } } } } } } } } bool CheckClass::hasDeallocation(const Token *first, const Token *last) { // This function is called when no simple check was found for assignment // to self. We are currently looking for a specific sequence of: // deallocate member ; ... member = allocate // This check is far from ideal because it can cause false negatives. // Unfortunately, this is necessary to prevent false positives. // This check needs to do careful analysis someday to get this // correct with a high degree of certainty. for (const Token *tok = first; tok && (tok != last); tok = tok->next()) { // check for deallocating memory if (Token::Match(tok, "{|;|, free ( %var%")) { const Token *var = tok->tokAt(3); // we should probably check that var is a pointer in this class const Token *tok1 = tok->tokAt(4); while (tok1 && (tok1 != last)) { if (Token::Match(tok1, "%var% =")) { if (tok1->str() == var->str()) return true; } tok1 = tok1->next(); } } else if (Token::Match(tok, "{|;|, delete [ ] %var%")) { const Token *var = tok->tokAt(4); // we should probably check that var is a pointer in this class const Token *tok1 = tok->tokAt(5); while (tok1 && (tok1 != last)) { if (Token::Match(tok1, "%var% = new %type% [")) { if (tok1->str() == var->str()) return true; } tok1 = tok1->next(); } } else if (Token::Match(tok, "{|;|, delete %var%")) { const Token *var = tok->tokAt(2); // we should probably check that var is a pointer in this class const Token *tok1 = tok->tokAt(3); while (tok1 && (tok1 != last)) { if (Token::Match(tok1, "%var% = new")) { if (tok1->str() == var->str()) return true; } tok1 = tok1->next(); } } } return false; } bool CheckClass::hasAssignSelf(const Token *first, const Token *last, const Token *rhs) { for (const Token *tok = first; tok && tok != last; tok = tok->next()) { if (Token::Match(tok, "if (")) { const Token *tok1 = tok->tokAt(2); const Token *tok2 = tok->tokAt(1)->link(); if (tok1 && tok2) { for (; tok1 && tok1 != tok2; tok1 = tok1->next()) { if (Token::Match(tok1, "this ==|!= & %var%")) { if (tok1->tokAt(3)->str() == rhs->str()) return true; } else if (Token::Match(tok1, "& %var% ==|!= this")) { if (tok1->tokAt(1)->str() == rhs->str()) return true; } } } } } return false; } void CheckClass::operatorEqToSelfError(const Token *tok) { reportError(tok, Severity::warning, "operatorEqToSelf", "'operator=' should check for assignment to self"); } //--------------------------------------------------------------------------- // A destructor in a base class should be virtual //--------------------------------------------------------------------------- void CheckClass::virtualDestructor() { // This error should only be given if: // * base class doesn't have virtual destructor // * derived class has non-empty destructor // * base class is deleted if (!_settings->inconclusive) return; createSymbolDatabase(); std::list::const_iterator i; for (i = symbolDatabase->scopeList.begin(); i != symbolDatabase->scopeList.end(); ++i) { const Scope *scope = *i; // Skip base classes and namespaces if (scope->derivedFrom.empty()) continue; // Find the destructor const Function *destructor = scope->getDestructor(); // Check for destructor with implementation if (!destructor || !destructor->hasBody) continue; // Empty destructor if (destructor->token->tokAt(3)->link() == destructor->token->tokAt(4)) continue; const Token *derived = scope->classDef; const Token *derivedClass = derived->tokAt(1); // Iterate through each base class... for (unsigned int j = 0; j < scope->derivedFrom.size(); ++j) { // Check if base class is public and exists in database if (scope->derivedFrom[j].access != Private && scope->derivedFrom[j].scope) { const Scope *derivedFrom = scope->derivedFrom[j].scope; // Name of base class.. const std::string baseName = derivedFrom->className; // Find the destructor declaration for the base class. const Function *base_destructor = derivedFrom->getDestructor(); const Token *base = 0; if (base_destructor) base = base_destructor->token; // Check that there is a destructor.. if (!base_destructor) { if (derivedFrom->derivedFrom.empty()) virtualDestructorError(derivedFrom->classDef, baseName, derivedClass->str()); } else if (!base_destructor->isVirtual) { // TODO: This is just a temporary fix, better solution is needed. // Skip situations where base class has base classes of its own, because // some of the base classes might have virtual destructor. // Proper solution is to check all of the base classes. If base class is not // found or if one of the base classes has virtual destructor, error should not // be printed. See TODO test case "virtualDestructorInherited" if (derivedFrom->derivedFrom.empty()) { // Make sure that the destructor is public (protected or private // would not compile if inheritance is used in a way that would // cause the bug we are trying to find here.) if (base_destructor->access == Public) virtualDestructorError(base, baseName, derivedClass->str()); } } } } } } void CheckClass::virtualDestructorError(const Token *tok, const std::string &Base, const std::string &Derived) { reportError(tok, Severity::error, "virtualDestructor", "Class " + Base + " which is inherited by class " + Derived + " does not have a virtual destructor"); } //--------------------------------------------------------------------------- // warn for "this-x". The indented code may be "this->x" //--------------------------------------------------------------------------- void CheckClass::thisSubtraction() { if (!_settings->_checkCodingStyle) return; const Token *tok = _tokenizer->tokens(); for (;;) { tok = Token::findmatch(tok, "this - %var%"); if (!tok) break; if (!Token::simpleMatch(tok->previous(), "*")) thisSubtractionError(tok); tok = tok->next(); } } void CheckClass::thisSubtractionError(const Token *tok) { reportError(tok, Severity::warning, "thisSubtraction", "Suspicious pointer subtraction"); } //--------------------------------------------------------------------------- // can member function be const? //--------------------------------------------------------------------------- void CheckClass::checkConst() { if (!_settings->isEnabled("information") || _settings->ifcfg) return; // Don't check C# and JAVA classes if (_tokenizer->isJavaOrCSharp()) { return; } createSymbolDatabase(); std::list::const_iterator it; for (it = symbolDatabase->scopeList.begin(); it != symbolDatabase->scopeList.end(); ++it) { const Scope *scope = *it; // only check classes and structures if (!scope->isClassOrStruct()) continue; std::list::const_iterator func; for (func = scope->functionList.begin(); func != scope->functionList.end(); ++func) { // does the function have a body? if (func->type == Function::eFunction && func->hasBody && !func->isFriend && !func->isStatic && !func->isConst && !func->isVirtual) { // get last token of return type const Token *previous = func->tokenDef->isName() ? func->token->previous() : func->token->tokAt(-2); while (previous && previous->str() == "::") previous = previous->tokAt(-2); // does the function return a pointer or reference? if (Token::Match(previous, "*|&")) { const Token *temp = func->token->previous(); while (!Token::Match(temp->previous(), ";|}|{|public:|protected:|private:")) temp = temp->previous(); if (temp->str() != "const") continue; } else if (Token::Match(previous->previous(), "*|& >")) { const Token *temp = func->token->previous(); while (!Token::Match(temp->previous(), ";|}|{|public:|protected:|private:")) { temp = temp->previous(); if (temp->str() == "const") break; } if (temp->str() != "const") continue; } else { // don't warn for unknown types.. // LPVOID, HDC, etc if (previous->isName()) { bool allupper = true; const std::string s(previous->str()); for (std::string::size_type pos = 0; pos < s.size(); ++pos) { const char ch = s[pos]; if (!(ch == '_' || (ch >= 'A' && ch <= 'Z'))) { allupper = false; break; } } if (allupper && previous->str().size() > 2) continue; } } const Token *paramEnd = func->arg->link(); // check if base class function is virtual if (!scope->derivedFrom.empty()) { if (isVirtualFunc(scope, func->tokenDef)) continue; } // if nothing non-const was found. write error.. if (checkConstFunc(scope, paramEnd)) { std::string classname = scope->className; const Scope *nest = scope->nestedIn; while (nest && nest->type != Scope::eGlobal) { classname = std::string(nest->className + "::" + classname); nest = nest->nestedIn; } // get function name std::string functionName((func->tokenDef->isName() ? "" : "operator") + func->tokenDef->str()); if (func->tokenDef->str() == "(") functionName += ")"; else if (func->tokenDef->str() == "[") functionName += "]"; if (func->isInline) checkConstError(func->token, classname, functionName); else // not inline checkConstError2(func->token, func->tokenDef, classname, functionName); } } } } } bool CheckClass::isMemberVar(const Scope *scope, const Token *tok) { const Token *tok1 = tok; while (tok->previous() && !Token::Match(tok->previous(), "}|{|;|public:|protected:|private:|return|:|?")) { if (Token::Match(tok->previous(), "* this")) return true; tok = tok->previous(); } if (tok->str() == "this") return true; if (Token::Match(tok, "( * %var% ) [") || (Token::Match(tok, "( * %var% ) <<") && tok1->next()->str() == "<<")) tok = tok->tokAt(2); // ignore class namespace if (tok->str() == scope->className && tok->next()->str() == "::") tok = tok->tokAt(2); std::list::const_iterator var; for (var = scope->varlist.begin(); var != scope->varlist.end(); ++var) { if (var->name() == tok->str()) { return !var->isMutable(); } } // not found in this class if (!scope->derivedFrom.empty()) { // check each base class for (unsigned int i = 0; i < scope->derivedFrom.size(); ++i) { // find the base class const Scope *derivedFrom = scope->derivedFrom[i].scope; // find the function in the base class if (derivedFrom) { if (isMemberVar(derivedFrom, tok)) return true; } } } return false; } bool CheckClass::isConstMemberFunc(const Scope *scope, const Token *tok) { std::list::const_iterator func; for (func = scope->functionList.begin(); func != scope->functionList.end(); ++func) { if (func->tokenDef->str() == tok->str() && func->isConst) return true; } // not found in this class if (!scope->derivedFrom.empty()) { // check each base class for (unsigned int i = 0; i < scope->derivedFrom.size(); ++i) { // find the base class const Scope *derivedFrom = scope->derivedFrom[i].scope; // find the function in the base class if (derivedFrom) { if (isConstMemberFunc(derivedFrom, tok)) return true; } } } return false; } bool CheckClass::checkConstFunc(const Scope *scope, const Token *tok) { // if the function doesn't have any assignment nor function call, // it can be a const function.. unsigned int indentlevel = 0; bool isconst = true; for (const Token *tok1 = tok; tok1; tok1 = tok1->next()) { if (tok1->str() == "{") ++indentlevel; else if (tok1->str() == "}") { if (indentlevel <= 1) break; --indentlevel; } // assignment.. = += |= .. else if (tok1->str() == "=" || (tok1->str().find("=") == 1 && tok1->str().find_first_of("") == std::string::npos)) { if (tok1->previous()->varId() == 0 && !scope->derivedFrom.empty()) { isconst = false; break; } else if (isMemberVar(scope, tok1->previous())) { isconst = false; break; } else if (tok1->previous()->str() == "]") { // TODO: I assume that the assigned variable is a member variable // don't assume it isconst = false; break; } else if (tok1->next()->str() == "this") { isconst = false; break; } // FIXME: I assume that a member union/struct variable is assigned. else if (Token::Match(tok1->tokAt(-2), ". %var%")) { isconst = false; break; } } // streaming: << else if (tok1->str() == "<<" && isMemberVar(scope, tok1->previous())) { isconst = false; break; } // increment/decrement (member variable?).. else if (Token::Match(tok1, "++|--")) { isconst = false; break; } // function call.. else if (Token::Match(tok1, "%var% (") && !(Token::Match(tok1, "return|c_str|if|string") || tok1->isStandardType())) { if (!isConstMemberFunc(scope, tok1)) { isconst = false; break; } } else if (Token::Match(tok1, "%var% < %any% > (")) { isconst = false; break; } // delete.. else if (tok1->str() == "delete") { isconst = false; break; } } return isconst; } //--------------------------------------------------------------------------- // check if this function is defined virtual in the base classes bool CheckClass::isVirtualFunc(const Scope *scope, const Token *functionToken) const { // check each base class for (unsigned int i = 0; i < scope->derivedFrom.size(); ++i) { // check if base class exists in database if (scope->derivedFrom[i].scope) { const Scope *derivedFrom = scope->derivedFrom[i].scope; std::list::const_iterator func; // check if function defined in base class for (func = derivedFrom->functionList.begin(); func != derivedFrom->functionList.end(); ++func) { if (func->isVirtual) { const Token *tok = func->tokenDef; if (tok->str() == functionToken->str()) { const Token *temp1 = tok->previous(); const Token *temp2 = functionToken->previous(); bool returnMatch = true; // check for matching return parameters while (temp1->str() != "virtual") { if (temp1->str() != temp2->str()) { returnMatch = false; break; } temp1 = temp1->previous(); temp2 = temp2->previous(); } // check for matching function parameters if (returnMatch && symbolDatabase->argsMatch(scope, tok->tokAt(2), functionToken->tokAt(2), std::string(""), 0)) { return true; } } } } if (!derivedFrom->derivedFrom.empty()) { if (isVirtualFunc(derivedFrom, functionToken)) return true; } } else { // unable to find base class so assume it has a virtual function return true; } } return false; } void CheckClass::checkConstError(const Token *tok, const std::string &classname, const std::string &funcname) { reportError(tok, Severity::information, "functionConst", "Technically the member function '" + classname + "::" + funcname + "' can be const.\n" "The member function '" + classname + "::" + funcname + "' can be made a const " "function. Making this function const function should not cause compiler errors. " "Even though the function can be made const function technically it may not make " "sense conceptually. Think about your design and task of the function first - is " "it a function that must not change object internal state?"); } void CheckClass::checkConstError2(const Token *tok1, const Token *tok2, const std::string &classname, const std::string &funcname) { std::list toks; toks.push_back(tok1); toks.push_back(tok2); reportError(toks, Severity::information, "functionConst", "Technically the member function '" + classname + "::" + funcname + "' can be const.\n" "The member function '" + classname + "::" + funcname + "' can be made a const " "function. Making this function const function should not cause compiler errors. " "Even though the function can be made const function technically it may not make " "sense conceptually. Think about your design and task of the function first - is " "it a function that must not change object internal state?"); }