/* * Cppcheck - A tool for static C/C++ code analysis * Copyright (C) 2007-2012 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 //--------------------------------------------------------------------------- // Register CheckClass.. namespace { CheckClass instance; } //--------------------------------------------------------------------------- CheckClass::CheckClass(const Tokenizer *tokenizer, const Settings *settings, ErrorLogger *errorLogger) : Check(myName(), tokenizer, settings, errorLogger), symbolDatabase(tokenizer?tokenizer->getSymbolDatabase():NULL) { } //--------------------------------------------------------------------------- // ClassCheck: Check that all class constructors are ok. //--------------------------------------------------------------------------- void CheckClass::constructors() { if (!_settings->isEnabled("style")) return; const std::size_t classes = symbolDatabase->classAndStructScopes.size(); for (std::size_t i = 0; i < classes; ++i) { const Scope * scope = symbolDatabase->classAndStructScopes[i]; // skip forward declarations if (scope->isForwardDeclaration()) continue; // don't check uninstantiated template classes if (scope->classDef->strAt(-1) == ">") 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; } } } // #3196 => bailout if there are nested unions // TODO: handle union variables better { bool bailout = false; for (std::list::const_iterator it = scope->nestedList.begin(); it != scope->nestedList.end(); ++it) { const Scope * const nestedScope = *it; if (nestedScope->type == Scope::eUnion) { bailout = true; break; } } if (bailout) continue; } 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) { bool inconclusive = false; if (usage[count].assign || usage[count].init || var->isStatic()) continue; if (var->isConst() && func->isOperator) // We can't set const members in assignment operator continue; // Check if this is a class constructor if (!var->isPointer() && 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->isPointer() && var->type() && canNotCopy(var->type())) continue; // Don't warn about unknown types in copy constructors since we // don't know if they can be copied or not.. if (!var->isPointer() && !(var->type() && var->type()->needInitialization != Scope::True) && (func->type == Function::eCopyConstructor || func->type == Function::eOperatorEqual)) { bool stdtype = false; for (const Token *type = var->typeStartToken(); type && type->isName(); type = type->next()) stdtype |= type->isStandardType(); if (!stdtype) { if (_settings->inconclusive) inconclusive = true; else continue; } } // It's non-static and it's not initialized => error if (func->type == Function::eOperatorEqual) { const Token *operStart = func->arg; 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(), inconclusive); } else if (func->access != Private) { const Scope *varType = var->type(); if (!varType || varType->type != Scope::eUnion) uninitVarError(func->token, scope->className, var->name(), inconclusive); } } } } } void CheckClass::copyconstructors() { if (!_settings->isEnabled("style")) return; const std::size_t classes = symbolDatabase->classAndStructScopes.size(); for (std::size_t i = 0; i < classes; ++i) { const Scope * scope = symbolDatabase->classAndStructScopes[i]; std::map allocatedVars; for (std::list::const_iterator func = scope->functionList.begin(); func != scope->functionList.end(); ++func) { if (func->type == Function::eConstructor && func->functionScope) { for (const Token* tok = func->functionScope->classStart; tok!=func->functionScope->classEnd; tok=tok->next()) { if (Token::Match(tok, "%var% = new|malloc|g_malloc|g_try_malloc|realloc|g_realloc|g_try_realloc")) { const Variable* var = symbolDatabase->getVariableFromVarId(tok->varId()); if (var && var->isPointer() && var->scope() == &*scope) allocatedVars[tok->varId()] = tok; } } } } std::set copiedVars; const Token* copyCtor = 0; for (std::list::const_iterator func = scope->functionList.begin(); func != scope->functionList.end(); ++func) { if (func->type == Function::eCopyConstructor) { copyCtor = func->tokenDef; if (func->functionScope) { const Token* tok = func->tokenDef->linkAt(1)->next(); if (tok->str()==":") { tok=tok->next(); while (Token::Match(tok, "%var% (")) { if (allocatedVars.find(tok->varId()) != allocatedVars.end()) { if (tok->varId() && Token::Match(tok->tokAt(2), "%var% . %var% )")) copiedVars.insert(tok); else if (!Token::Match(tok->tokAt(2), "%any% )")) allocatedVars.erase(tok->varId()); // Assume memory is allocated } tok = tok->linkAt(1)->tokAt(2); } } for (tok=func->functionScope->classStart; tok!=func->functionScope->classEnd; tok=tok->next()) { if (Token::Match(tok, "%var% = new|malloc|g_malloc|g_try_malloc|realloc|g_realloc|g_try_realloc")) { allocatedVars.erase(tok->varId()); } else if (Token::Match(tok, "%var% = %var% . %var% ;") && allocatedVars.find(tok->varId()) != allocatedVars.end()) { copiedVars.insert(tok); } } } else // non-copyable or implementation not seen allocatedVars.clear(); break; } } if (!copyCtor) { if (!allocatedVars.empty() && scope->derivedFrom.empty()) // TODO: Check if base class is non-copyable noCopyConstructorError(scope->classDef, scope->className, scope->type == Scope::eStruct); } else { if (!copiedVars.empty()) { for (std::set::const_iterator it = copiedVars.begin(); it != copiedVars.end(); ++it) { copyConstructorShallowCopyError(*it, (*it)->str()); } } // throw error if count mismatch /* FIXME: This doesn't work. See #4154 for (std::map::const_iterator i = allocatedVars.begin(); i != allocatedVars.end(); ++i) { copyConstructorMallocError(copyCtor, i->second, i->second->str()); } */ } } } /* This doesn't work. See #4154 void CheckClass::copyConstructorMallocError(const Token *cctor, const Token *alloc, const std::string& varname) { std::list callstack; callstack.push_back(cctor); callstack.push_back(alloc); reportError(callstack, Severity::warning, "copyCtorNoAllocation", "Copy constructor does not allocate memory for member '" + varname + "' although memory has been allocated in other constructors."); } */ void CheckClass::copyConstructorShallowCopyError(const Token *tok, const std::string& varname) { reportError(tok, Severity::style, "copyCtorPointerCopying", "Value of pointer '" + varname + "', which points to allocated memory, is copied in copy constructor instead of allocating new memory."); } void CheckClass::noCopyConstructorError(const Token *tok, const std::string &classname, bool isStruct) { // The constructor might be intentionally missing. Therefore this is not a "warning" reportError(tok, Severity::style, "noCopyConstructor", "'" + std::string(isStruct ? "struct" : "class") + " " + classname + "' does not have a copy constructor which is recommended since the class contains a pointer to allocated memory."); } bool CheckClass::canNotCopy(const Scope *scope) { std::list::const_iterator func; bool constructor = false; bool publicAssign = false; bool publicCopy = false; for (func = scope->functionList.begin(); func != scope->functionList.end(); ++func) { if (func->type == Function::eConstructor || func->type == Function::eCopyConstructor) constructor = true; if (func->type == Function::eCopyConstructor && func->access == Public) publicCopy = true; else if (func->type == Function::eOperatorEqual && func->access == Public) publicAssign = true; } return constructor && !(publicAssign || publicCopy); } 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 (std::size_t i = 0; i < usage.size(); ++i) usage[i].assign = true; } void CheckClass::clearAllVar(std::vector &usage) { for (std::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 (std::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 initList = true; const Token *ftok = func.arg->link()->next(); int level = 0; for (; ftok != func.functionScope->classEnd; ftok = ftok->next()) { // Class constructor.. initializing variables like this // clKalle::clKalle() : var(value) { } if (initList) { if (level == 0 && Token::Match(ftok, "%var% (")) { if (ftok->strAt(2) != ")") initVar(ftok->str(), scope, usage); } else if (level == 0 && Token::Match(ftok, "%var% {") && ftok->str() != "const" && Token::Match(ftok->next()->link()->next(), ",|{|%name%")) { initVar(ftok->str(), scope, usage); ftok = ftok->linkAt(1); } else if (level != 0 && Token::Match(ftok, "%var% =")) // assignment in the initializer: var(value = x) assignVar(ftok->str(), scope, usage); else if (ftok->str() == "(") level++; else if (ftok->str() == ")") level--; else if (ftok->str() == "{") { if (level == 0) initList = false; else ftok = ftok->link(); } } if (initList) 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::Match(ftok->next(), "return| (| * this )| =")) { assignAllVar(usage); break; } // Using swap to assign all variables.. if (func.type == Function::eOperatorEqual && Token::Match(ftok, "[;{}] %var% (") && Token::Match(ftok->linkAt(2), ") . %var% ( *| 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); break; } } 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 "return" if (ftok->str() == "return") 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::Match(ftok, "::| memset ( this ,")) { assignAllVar(usage); return; } // Clearing array.. else if (Token::Match(ftok, "::| memset ( %var% ,")) { if (ftok->str() == "::") ftok = ftok->next(); assignVar(ftok->strAt(2), scope, usage); ftok = ftok->linkAt(1); continue; } // Calling member function? else if (Token::simpleMatch(ftok, "operator= (") && ftok->previous()->str() != "::") { // recursive call / calling overloaded function // assume that all variables are initialized if (std::find(callstack.begin(), callstack.end(), ftok->str()) != callstack.end()) { /** @todo false negative: just bail */ assignAllVar(usage); return; } /** @todo check function parameters for overloaded function so we check the right one */ // check if member function exists 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); } } // using default operator =, assume everything initialized else { assignAllVar(usage); } } else if (Token::Match(ftok, "::| %var% (") && ftok->str() != "if") { if (ftok->str() == "::") ftok = ftok->next(); // 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(); // Assume that variables that are passed to it are initialized.. for (const Token *tok2 = ftok; tok2; tok2 = tok2->next()) { if (Token::Match(tok2, "[;{}]")) break; if (Token::Match(tok2, "[(,] &| %var% [,)]")) { tok2 = tok2->next(); if (tok2->str() == "&") tok2 = tok2->next(); assignVar(tok2->str(), scope, usage); } } } // 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 { for (const Token *tok = ftok->tokAt(2); tok && tok != ftok->next()->link(); tok = tok->next()) { 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% [|.")) { const Token *tok2 = ftok; while (tok2) { if (Token::simpleMatch(tok2->next(), "[")) tok2 = tok2->next()->link(); else if (Token::Match(tok2->next(), ". %var%")) tok2 = tok2->tokAt(2); else break; } if (tok2 && tok2->strAt(1) == "=") assignVar(ftok->str(), scope, usage); } // Assignment of array item of member variable? else if (Token::Match(ftok, "* %var% =")) { assignVar(ftok->next()->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 although it has private member variables. " "Member variables of builtin types are left uninitialized when the class is " "instanciated. That may cause bugs or undefined behavior."); } void CheckClass::uninitVarError(const Token *tok, const std::string &classname, const std::string &varname, bool inconclusive) { reportError(tok, Severity::warning, "uninitMemberVar", "Member variable '" + classname + "::" + varname + "' is not initialized in the constructor.", inconclusive); } void CheckClass::operatorEqVarError(const Token *tok, const std::string &classname, const std::string &varname, bool inconclusive) { reportError(tok, Severity::warning, "operatorEqVarError", "Member variable '" + classname + "::" + varname + "' is not assigned a value in '" + classname + "::operator='.", inconclusive); } //--------------------------------------------------------------------------- // ClassCheck: Use initialization list instead of assignment //--------------------------------------------------------------------------- void CheckClass::initializationListUsage() { if (!_settings->isEnabled("performance")) return; const std::size_t functions = symbolDatabase->functionScopes.size(); for (std::size_t i = 0; i < functions; ++i) { const Scope * scope = symbolDatabase->functionScopes[i]; // Check every constructor if (!scope->function || (scope->function->type != Function::eConstructor && scope->function->type != Function::eCopyConstructor)) continue; Scope* owner = scope->functionOf; for (const Token* tok = scope->classStart; tok != scope->classEnd; tok = tok->next()) { if (Token::Match(tok, "%var% (")) // Assignments might depend on this function call or if/for/while/switch statement from now on. break; if (Token::Match(tok, "try|do {")) break; if (tok->varId() && Token::Match(tok, "%var% = %any%")) { const Variable* var = symbolDatabase->getVariableFromVarId(tok->varId()); if (var && var->scope() == owner && !var->isStatic()) { bool allowed = true; for (const Token* tok2 = tok->tokAt(2); tok2->str() != ";"; tok2 = tok2->next()) { if (tok2->varId()) { const Variable* var2 = symbolDatabase->getVariableFromVarId(tok2->varId()); if (var2 && var2->scope() == owner) { // Is there a dependency between two member variables? allowed = false; break; } } else if (tok2->str() == "this") { // 'this' instance is not completely constructed in initialization list allowed = false; break; } else if (Token::Match(tok2, "%var% (") && tok2->strAt(-1) != "." && isMemberFunc(owner, tok2)) { // Member function called? allowed = false; break; } } if (!allowed) continue; if (!var->isPointer() && (var->type() || Token::Match(var->typeStartToken(), "std :: string|wstring !!::") || (Token::Match(var->typeStartToken(), "std :: %type% <") && !Token::simpleMatch(var->typeStartToken()->linkAt(3), "> ::")) || symbolDatabase->isClassOrStruct(var->typeStartToken()->str()))) suggestInitializationList(tok, tok->str()); } } } } } void CheckClass::suggestInitializationList(const Token* tok, const std::string& varname) { reportError(tok, Severity::performance, "useInitializationList", "Variable '" + varname + "' is assigned in constructor body. Consider performing initialization in initialization list.\n" "When an object of a class is created, the constructors of all member variables are called consecutively " "in the order the variables are declared, even if you don't explicitly write them to the initialization list. You " "could avoid assigning '" + varname + "' a value by passing the value to the constructor in the initialization list."); } //--------------------------------------------------------------------------- // ClassCheck: Unused private functions //--------------------------------------------------------------------------- static bool checkFunctionUsage(const std::string& name, const Scope* scope) { if (!scope) return true; // Assume its used, if scope is not seen for (std::list::const_iterator func = scope->functionList.begin(); func != scope->functionList.end(); ++func) { if (func->functionScope) { for (const Token *ftok = func->functionScope->classStart; ftok != func->functionScope->classEnd; ftok = ftok->next()) { if (ftok->str() == name && ftok->next()->str() == "(") // Function called. TODO: Handle overloads return true; } } } return false; // Unused in this scope } void CheckClass::privateFunctions() { if (!_settings->isEnabled("style")) return; const std::size_t classes = symbolDatabase->classAndStructScopes.size(); for (std::size_t i = 0; i < classes; ++i) { const Scope * scope = symbolDatabase->classAndStructScopes[i]; // dont check borland classes with properties.. if (Token::findsimplematch(scope->classStart, "; __property ;", scope->classEnd)) continue; // check that the whole class implementation is seen bool whole = true; for (std::list::const_iterator 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; std::list FuncList; /** @todo embedded class have access to private functions */ if (!scope->getNestedNonFunctions()) { for (std::list::const_iterator func = scope->functionList.begin(); func != scope->functionList.end(); ++func) { // Get private functions.. if (func->type == Function::eFunction && func->access == Private) FuncList.push_back(&*func); } } // Bailout for overriden virtual functions of base classes if (!scope->derivedFrom.empty()) { // Check virtual functions for (std::list::iterator it = FuncList.begin(); it != FuncList.end();) { if ((*it)->isImplicitlyVirtual(true)) // Give true as default value to be returned if we don't see all base classes FuncList.erase(it++); else ++it; } } while (!FuncList.empty()) { const std::string& funcName = FuncList.front()->tokenDef->str(); // Check that all private functions are used bool used = checkFunctionUsage(funcName, &*scope); // Usage in this class // Check in friend classes for (std::list::const_iterator it = scope->friendList.begin(); !used && it != scope->friendList.end(); ++it) used = checkFunctionUsage(funcName, it->scope); if (!used) { // Final check; check if the function pointer is used somewhere.. const std::string _pattern("return|throw|(|)|,|= &|" + funcName); // or if the function address is used somewhere... // eg. sigc::mem_fun(this, &className::classFunction) const std::string _pattern2("& " + scope->className + " :: " + funcName); const std::string methodAsArgument("(|, " + scope->className + " :: " + funcName + " ,|)"); const std::string methodAssigned("%var% = &| " + scope->className + " :: " + funcName); if (!Token::findmatch(_tokenizer->tokens(), _pattern.c_str()) && !Token::findmatch(_tokenizer->tokens(), _pattern2.c_str()) && !Token::findmatch(_tokenizer->tokens(), methodAsArgument.c_str()) && !Token::findmatch(_tokenizer->tokens(), methodAssigned.c_str())) { unusedPrivateFunctionError(FuncList.front()->tokenDef, scope->className, funcName); } } 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() { const std::size_t functions = symbolDatabase->functionScopes.size(); for (std::size_t i = 0; i < functions; ++i) { const Scope * scope = symbolDatabase->functionScopes[i]; for (const Token *tok = scope->classStart; tok && tok != scope->classEnd; tok = tok->next()) { if (!Token::Match(tok, "memset|memcpy|memmove ( %any%")) continue; const Token* arg1 = tok->tokAt(2); const Token* arg3 = arg1; arg3 = arg3->nextArgument(); if (arg3) arg3 = arg3->nextArgument(); const Token *typeTok = 0; if (Token::Match(arg3, "sizeof ( %type% ) )")) typeTok = arg3->tokAt(2); else if (Token::Match(arg3, "sizeof ( %type% :: %type% ) )")) typeTok = arg3->tokAt(4); else if (Token::Match(arg3, "sizeof ( struct %type% ) )")) typeTok = arg3->tokAt(3); else if (Token::Match(arg1, "&| %var% ,")) { unsigned int varid = arg1->str() == "&" ? arg1->next()->varId() : arg1->varId(); const Variable *var = symbolDatabase->getVariableFromVarId(varid); if (var && (var->typeStartToken() == var->typeEndToken() || Token::Match(var->typeStartToken(), "%type% :: %type%"))) typeTok = var->typeEndToken(); } // No type defined => The tokens didn't match if (!typeTok) continue; if (typeTok->str() == "(") typeTok = typeTok->next(); const Scope *type = symbolDatabase->findVariableType(&(*scope), typeTok); if (type) checkMemsetType(&(*scope), tok, type); } } } void CheckClass::checkMemsetType(const Scope *start, const Token *tok, const Scope *type) { // recursively check all parent classes for (std::size_t i = 0; i < type->derivedFrom.size(); i++) { if (type->derivedFrom[i].scope) checkMemsetType(start, tok, type->derivedFrom[i].scope); } // Warn if type is a class that contains any virtual functions std::list::const_iterator func; for (func = type->functionList.begin(); func != type->functionList.end(); ++func) { if (func->isVirtual) memsetError(tok, tok->str(), "virtual method", type->classDef->str()); } // Warn if type is a class or struct that contains any std::* variables std::list::const_iterator var; for (var = type->varlist.begin(); var != type->varlist.end(); ++var) { // don't warn if variable static or const, pointer or reference if (!var->isStatic() && !var->isConst() && !var->isPointer() && !var->isReference()) { const Token *tok1 = var->typeStartToken(); // check for std:: type if (Token::simpleMatch(tok1, "std ::")) memsetError(tok, tok->str(), "'std::" + tok1->strAt(2) + "'", type->classDef->str()); // check for known type else if (var->type()) checkMemsetType(start, tok, var->type()); } } } void CheckClass::memsetError(const Token *tok, const std::string &memfunc, const std::string &classname, const std::string &type) { reportError(tok, Severity::error, "memsetClass", "Using '" + memfunc + "' on " + type + " that contains a " + classname + "."); } //--------------------------------------------------------------------------- // ClassCheck: "void operator=(" and "const type & operator=(" //--------------------------------------------------------------------------- void CheckClass::operatorEq() { if (!_settings->isEnabled("style")) return; const std::size_t classes = symbolDatabase->classAndStructScopes.size(); for (std::size_t i = 0; i < classes; ++i) { const Scope * scope = symbolDatabase->classAndStructScopes[i]; std::list::const_iterator func; for (func = scope->functionList.begin(); func != scope->functionList.end(); ++func) { if (func->type == Function::eOperatorEqual && func->access != Private) { // use definition for check so we don't have to deal with qualification if (!(Token::Match(func->tokenDef->tokAt(-3), ";|}|{|public:|protected:|private:|virtual %type% &") && func->tokenDef->strAt(-2) == scope->className)) { // make sure we really have a copy assignment operator if (Token::Match(func->tokenDef->tokAt(2), "const| %var% &")) { if (func->tokenDef->strAt(2) == "const" && func->tokenDef->strAt(3) == scope->className) operatorEqReturnError(func->tokenDef->previous(), scope->className); else if (func->tokenDef->strAt(2) == scope->className) operatorEqReturnError(func->tokenDef->previous(), scope->className); } } } } } } void CheckClass::operatorEqReturnError(const Token *tok, const std::string &className) { reportError(tok, Severity::style, "operatorEq", "'" + className + "::operator=' should return '" + className + " &'."); } //--------------------------------------------------------------------------- // ClassCheck: "C& operator=(const C&) { ... return *this; }" // operator= should return a reference to *this //--------------------------------------------------------------------------- void CheckClass::operatorEqRetRefThis() { if (!_settings->isEnabled("style")) return; const std::size_t classes = symbolDatabase->classAndStructScopes.size(); for (std::size_t i = 0; i < classes; ++i) { const Scope * scope = symbolDatabase->classAndStructScopes[i]; 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(-3), ";|}|{|public:|protected:|private:|virtual %type% &") && func->tokenDef->strAt(-2) == scope->className) { checkReturnPtrThis(&(*scope), &(*func), func->functionScope->classStart, func->functionScope->classEnd); } } } } } 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::simpleMatch(tok->next(), cast.c_str())) tok = tok->tokAt(4); // check if a function is called if (tok->strAt(2) == "(" && tok->linkAt(2)->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->isConst) { /** @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 if *this is returned else if (!(Token::Match(tok->next(), "(| * this ;|=") || Token::simpleMatch(tok->next(), "operator= (") || Token::simpleMatch(tok->next(), "this . operator= (") || (Token::Match(tok->next(), "%type% :: operator= (") && tok->next()->str() == scope->className))) operatorEqRetRefThisError(func->token); } } if (!foundReturn) operatorEqRetRefThisError(func->token); } void CheckClass::operatorEqRetRefThisError(const Token *tok) { reportError(tok, Severity::style, "operatorEqRetRefThis", "'operator=' should return reference to 'this' instance."); } //--------------------------------------------------------------------------- // 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->isEnabled("style")) return; const std::size_t classes = symbolDatabase->classAndStructScopes.size(); for (std::size_t i = 0; i < classes; ++i) { const Scope * scope = symbolDatabase->classAndStructScopes[i]; std::list::const_iterator func; // skip classes with multiple inheritance if (scope->derivedFrom.size() > 1) continue; for (func = scope->functionList.begin(); func != scope->functionList.end(); ++func) { if (func->type == Function::eOperatorEqual && func->hasBody) { // make sure that the operator takes an object of the same type as *this, otherwise we can't detect self-assignment checks if (func->argumentList.empty()) continue; const Token* typeTok = func->argumentList.front().typeEndToken(); while (typeTok->str() == "const" || typeTok->str() == "&" || typeTok->str() == "*") typeTok = typeTok->previous(); if (typeTok->str() != scope->className) continue; // make sure return signature is correct if (Token::Match(func->tokenDef->tokAt(-3), ";|}|{|public:|protected:|private: %type% &") && func->tokenDef->strAt(-2) == scope->className) { // find the parameter name const Token *rhs = func->argumentList.begin()->nameToken(); if (!hasAssignSelf(&(*func), rhs)) { if (hasAllocation(&(*func), &*scope)) operatorEqToSelfError(func->token); } } } } } } bool CheckClass::hasAllocation(const Function *func, const Scope* scope) { // This function is called when no simple check was found for assignment // to self. We are currently looking for: // - deallocate member ; ... member = // - alloc member // That is not ideal because it can cause false negatives but its currently // necessary to prevent false positives. const Token *last = func->functionScope->classEnd; for (const Token *tok = func->functionScope->classStart; tok && (tok != last); tok = tok->next()) { if (Token::Match(tok, "%var% = malloc|realloc|calloc|new") && isMemberVar(scope, tok)) return true; // check for deallocating memory const Token *var = 0; if (Token::Match(tok, "free ( %var%")) var = tok->tokAt(2); else if (Token::Match(tok, "delete [ ] %var%")) var = tok->tokAt(3); else if (Token::Match(tok, "delete %var%")) var = tok->next(); // Check for assignement to the deleted pointer (only if its a member of the class) if (var && isMemberVar(scope, var)) { for (const Token *tok1 = var->next(); tok1 && (tok1 != last); tok1 = tok1->next()) { if (Token::Match(tok1, "%var% =")) { if (tok1->str() == var->str()) return true; } } } } return false; } bool CheckClass::hasAssignSelf(const Function *func, const Token *rhs) { const Token *last = func->functionScope->classEnd; for (const Token *tok = func->functionScope->classStart; tok && tok != last; tok = tok->next()) { if (Token::simpleMatch(tok, "if (")) { const Token *tok1 = tok->tokAt(2); const Token *tok2 = tok->next()->link(); if (tok1 && tok2) { for (; tok1 && tok1 != tok2; tok1 = tok1->next()) { if (Token::Match(tok1, "this ==|!= & %var%")) { if (tok1->strAt(3) == rhs->str()) return true; } else if (Token::Match(tok1, "& %var% ==|!= this")) { if (tok1->strAt(1) == rhs->str()) return true; } } } } } return false; } void CheckClass::operatorEqToSelfError(const Token *tok) { reportError(tok, Severity::warning, "operatorEqToSelf", "'operator=' should check for assignment to self to avoid problems with dynamic memory.\n" "'operator=' should check for assignment to self to ensure that each block of dynamically " "allocated memory is owned and managed by only one instance of the class."); } //--------------------------------------------------------------------------- // 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 const std::size_t classes = symbolDatabase->classAndStructScopes.size(); for (std::size_t i = 0; i < classes; ++i) { const Scope * scope = symbolDatabase->classAndStructScopes[i]; // Skip base classes 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->linkAt(3) == destructor->token->tokAt(4)) continue; const Token *derived = scope->classDef; const Token *derivedClass = derived->next(); // 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; // Check for this pattern: // 1. Base class pointer is given the address of derived class instance // 2. Base class pointer is deleted // // If this pattern is not seen then bailout the checking of these base/derived classes { // pointer variables of type 'Base *' std::set basepointer; // pointer variables of type 'Base *' that should not be deleted std::set dontDelete; // No deletion of derived class instance through base class pointer found => the code is ok bool ok = true; for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) { // Declaring base class pointer if (Token::simpleMatch(tok, baseName.c_str())) { if (Token::Match(tok->previous(), ("[;{}] " + baseName + " * %var% ;").c_str())) basepointer.insert(tok->tokAt(2)->varId()); } // Assign base class pointer with pointer to derived class instance else if (Token::Match(tok, "[;{}] %var% =") && tok->next()->varId() > 0 && basepointer.find(tok->next()->varId()) != basepointer.end()) { // new derived class.. if (Token::simpleMatch(tok->tokAt(3), ("new " + derivedClass->str()).c_str())) { dontDelete.insert(tok->next()->varId()); } } // Delete base class pointer that might point at derived class else if (Token::Match(tok, "delete %var% ;") && tok->next()->varId() && dontDelete.find(tok->next()->varId()) != dontDelete.end()) { ok = false; break; } } // No base class pointer that points at a derived class is deleted if (ok) continue; } // 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.\n" "Class '" + Base + "' which is inherited by class '" + Derived + "' does not have a virtual destructor. " "If you destroy instances of the derived class by deleting a pointer that points to the base class, only " "the destructor of the base class is executed. Thus, dynamic memory that is managed by the derived class " "could leak. This can be avoided by adding a virtual destructor to the base class."); } //--------------------------------------------------------------------------- // warn for "this-x". The indented code may be "this->x" //--------------------------------------------------------------------------- void CheckClass::thisSubtraction() { if (!_settings->isEnabled("style")) return; const Token *tok = _tokenizer->tokens(); for (;;) { tok = Token::findmatch(tok, "this - %var%"); if (!tok) break; if (tok->strAt(-1) != "*") thisSubtractionError(tok); tok = tok->next(); } } void CheckClass::thisSubtractionError(const Token *tok) { reportError(tok, Severity::warning, "thisSubtraction", "Suspicious pointer subtraction. Did you intend to write '->'?"); } //--------------------------------------------------------------------------- // can member function be const? //--------------------------------------------------------------------------- void CheckClass::checkConst() { // This is an inconclusive check. False positives: #3322. if (!_settings->inconclusive) return; if (!_settings->isEnabled("style")) return; const std::size_t classes = symbolDatabase->classAndStructScopes.size(); for (std::size_t i = 0; i < classes; ++i) { const Scope * scope = symbolDatabase->classAndStructScopes[i]; 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->isVirtual) { // get last token of return type const Token *previous = func->tokenDef->previous(); // does the function return a pointer or reference? if (Token::Match(previous, "*|&")) { const Token *temp = 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 = previous; while (!Token::Match(temp->previous(), ";|}|{|public:|protected:|private:")) { temp = temp->previous(); if (temp->str() == "const") break; } if (temp->str() != "const") continue; } else if (func->isOperator && Token::Match(previous, ";|{|}|public:|private:|protected:")) { // Operator without return type: conversion operator const std::string& opName = func->tokenDef->str(); if (opName.compare(8, 5, "const") != 0 && opName[opName.size()-1] == '&') continue; } else { // don't warn for unknown types.. // LPVOID, HDC, etc if (previous->isUpperCaseName() && previous->str().size() > 2 && !symbolDatabase->isClassOrStruct(previous->str())) continue; } // check if base class function is virtual if (!scope->derivedFrom.empty()) { if (func->isImplicitlyVirtual(true)) continue; } bool memberAccessed = false; // if nothing non-const was found. write error.. if (checkConstFunc(&(*scope), &*func, memberAccessed)) { 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->isConst || (!memberAccessed && !func->isOperator)) { if (func->isInline) checkConstError(func->token, classname, functionName, !memberAccessed && !func->isOperator); else // not inline checkConstError2(func->token, func->tokenDef, classname, functionName, !memberAccessed && !func->isOperator); } } } } } } bool CheckClass::isMemberVar(const Scope *scope, const Token *tok) { bool again = false; // try to find the member variable do { again = false; if (tok->str() == "this") { return true; } else if (Token::simpleMatch(tok->tokAt(-3), "( * this )")) { return true; } else if (Token::Match(tok->tokAt(-2), "%var% . %var%")) { tok = tok->tokAt(-2); again = true; } else if (Token::Match(tok->tokAt(-2), "] . %var%")) { tok = tok->linkAt(-2)->previous(); again = true; } else if (tok->str() == "]") { tok = tok->link()->previous(); again = true; } } while (again); std::list::const_iterator var; for (var = scope->varlist.begin(); var != scope->varlist.end(); ++var) { if (var->name() == tok->str()) { if (tok->varId() == 0) symbolDatabase->debugMessage(tok, "CheckClass::isMemberVar found used member variable \'" + tok->str() + "\' with varid 0"); return !var->isStatic(); } } // 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; } static unsigned int countParameters(const Token *tok) { tok = tok->tokAt(2); if (tok->str() == ")") return 0; unsigned int numpar = 1; while (NULL != (tok = tok->nextArgument())) numpar++; return numpar; } static unsigned int countMinArgs(const Token* argList) { if (!argList) return 0; argList = argList->next(); if (argList->str() == ")") return 0; unsigned int count = 1; for (; argList; argList = argList->next()) { if (argList->link() && Token::Match(argList, "(|[|{|<")) argList = argList->link(); else if (argList->str() == ",") count++; else if (argList->str() == "=") return count-1; else if (argList->str() == ")") break; } return count; } bool CheckClass::isMemberFunc(const Scope *scope, const Token *tok) { unsigned int args = countParameters(tok); for (std::list::const_iterator func = scope->functionList.begin(); func != scope->functionList.end(); ++func) { /** @todo we need to look at the argument types when there are overloaded functions * with the same number of arguments */ if (func->tokenDef->str() == tok->str() && (func->argCount() == args || (func->argCount() > args && countMinArgs(func->argDef) <= args))) { return !func->isStatic; } } // 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 (isMemberFunc(derivedFrom, tok)) return true; } } } return false; } bool CheckClass::isConstMemberFunc(const Scope *scope, const Token *tok) { unsigned int args = countParameters(tok); std::list::const_iterator func; unsigned int matches = 0; unsigned int consts = 0; for (func = scope->functionList.begin(); func != scope->functionList.end(); ++func) { /** @todo we need to look at the argument types when there are overloaded functions * with the same number of arguments */ if (func->tokenDef->str() == tok->str() && (func->argCount() == args || (func->argCount() > args && countMinArgs(func->argDef) <= args))) { matches++; if (func->isConst) consts++; } } // if there are multiple matches that are all const, return const if (matches > 0 && matches == consts) 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 Function *func, bool& memberAccessed) { // if the function doesn't have any assignment nor function call, // it can be a const function.. for (const Token *tok1 = func->functionScope->classStart; tok1 && tok1 != func->functionScope->classEnd; tok1 = tok1->next()) { if (tok1->isName() && isMemberVar(scope, tok1)) { memberAccessed = true; const Variable* v = symbolDatabase->getVariableFromVarId(tok1->varId()); if (v && v->isMutable()) continue; if (tok1->str() == "this" && tok1->previous()->isAssignmentOp()) return(false); unsigned int lastVarId = tok1->varId(); const Token* end = tok1; for (;;) { if (Token::Match(end->next(), ". %var%")) { end = end->tokAt(2); if (end->varId()) lastVarId = end->varId(); } else if (end->strAt(1) == "[") { if (end->varId()) { const Variable *var = symbolDatabase->getVariableFromVarId(end->varId()); if (var && Token::simpleMatch(var->typeStartToken(), "std :: map")) // operator[] changes a map return(false); } end = end->linkAt(1); } else if (end->strAt(1) == ")") end = end->next(); else break; } if (end->strAt(1) == "(") { const Variable *var = symbolDatabase->getVariableFromVarId(lastVarId); if (!var) return(false); if (Token::simpleMatch(var->typeStartToken(), "std ::") // assume all std::*::size() and std::*::empty() are const && (Token::Match(end, "size|empty|cend|crend|cbegin|crbegin|max_size|length|count|capacity|get_allocator|c_str|str ( )") || Token::Match(end, "rfind|copy"))) ; else if (!var->type() || !isConstMemberFunc(var->type(), end)) return(false); } // Assignment else if (end->next()->type() == Token::eAssignmentOp) return(false); // Streaming else if (end->strAt(1) == "<<" && tok1->strAt(-1) != "<<") return(false); else if (tok1->strAt(-1) == ">>") return(false); // ++/-- else if (end->next()->type() == Token::eIncDecOp || tok1->previous()->type() == Token::eIncDecOp) return(false); const Token* start = tok1; while (tok1->strAt(-1) == ")") tok1 = tok1->linkAt(-1); if (start->strAt(-1) == "delete") return(false); tok1 = end; } // streaming: << else if (Token::simpleMatch(tok1->previous(), ") <<") && isMemberVar(scope, tok1->tokAt(-2))) { const Variable* var = symbolDatabase->getVariableFromVarId(tok1->tokAt(-2)->varId()); if (!var || !var->isMutable()) return(false); } // function call.. else if (Token::Match(tok1, "%var% (") && !tok1->isStandardType() && !Token::Match(tok1, "return|if|string|switch|while|catch|for")) { if (isMemberFunc(scope, tok1) && tok1->strAt(-1) != ".") { if (!isConstMemberFunc(scope, tok1)) return(false); memberAccessed = true; } // Member variable given as parameter for (const Token* tok2 = tok1->tokAt(2); tok2 && tok2 != tok1->next()->link(); tok2 = tok2->next()) { if (tok2->str() == "(") tok2 = tok2->link(); else if (tok2->isName() && isMemberVar(scope, tok2)) { const Variable* var = symbolDatabase->getVariableFromVarId(tok2->varId()); if (!var || !var->isMutable()) return(false); // TODO: Only bailout if function takes argument as non-const reference } } } else if (Token::simpleMatch(tok1, "> (") && (!tok1->link() || !Token::Match(tok1->link()->previous(), "static_cast|const_cast|dynamic_cast|reinterpret_cast"))) { return(false); } } return(true); } void CheckClass::checkConstError(const Token *tok, const std::string &classname, const std::string &funcname, bool suggestStatic) { checkConstError2(tok, 0, classname, funcname, suggestStatic); } void CheckClass::checkConstError2(const Token *tok1, const Token *tok2, const std::string &classname, const std::string &funcname, bool suggestStatic) { std::list toks; toks.push_back(tok1); if (tok2) toks.push_back(tok2); if (!suggestStatic) reportError(toks, Severity::style, "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' 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 the task of the function first - is " "it a function that must not change object internal state?", true); else reportError(toks, Severity::performance, "functionStatic", "Technically the member function '" + classname + "::" + funcname + "' can be static.\n" "The member function '" + classname + "::" + funcname + "' can be made a static " "function. Making a function static can bring a performance benefit since no 'this' instance is " "passed to the function. This change should not cause compiler errors but it does not " "necessarily make sense conceptually. Think about your design and the task of the function first - " "is it a function that must not access members of class instances?", true); } //--------------------------------------------------------------------------- // ClassCheck: Check that initializer list is in declared order. //--------------------------------------------------------------------------- struct VarInfo { VarInfo(const Variable *_var, const Token *_tok) : var(_var), tok(_tok) { } const Variable *var; const Token *tok; }; void CheckClass::initializerListOrder() { if (!_settings->isEnabled("style")) return; // This check is not inconclusive. However it only determines if the initialization // order is incorrect. It does not determine if being out of order causes // a real error. Out of order is not necessarily an error but you can never // have an error if the list is in order so this enforces defensive programming. if (!_settings->inconclusive) return; const std::size_t classes = symbolDatabase->classAndStructScopes.size(); for (std::size_t i = 0; i < classes; ++i) { const Scope * info = symbolDatabase->classAndStructScopes[i]; std::list::const_iterator func; // iterate through all member functions looking for constructors for (func = info->functionList.begin(); func != info->functionList.end(); ++func) { if ((func->type == Function::eConstructor || func->type == Function::eCopyConstructor) && func->hasBody) { // check for initializer list const Token *tok = func->arg->link()->next(); if (tok->str() == ":") { std::vector vars; tok = tok->next(); // find all variable initializations in list while (tok && tok->str() != "{") { if (Token::Match(tok, "%var% (")) { const Variable *var = info->getVariable(tok->str()); if (var) vars.push_back(VarInfo(var, tok)); if (Token::Match(tok->tokAt(2), "%var% =")) { var = info->getVariable(tok->strAt(2)); if (var) vars.push_back(VarInfo(var, tok->tokAt(2))); } tok = tok->next()->link()->next(); } else tok = tok->next(); } // need at least 2 members to have out of order initialization for (std::size_t j = 1; j < vars.size(); j++) { // check for out of order initialization if (vars[j].var->index() < vars[j - 1].var->index()) initializerListError(vars[j].tok,vars[j].var->nameToken(), info->className, vars[j].var->name()); } } } } } } void CheckClass::initializerListError(const Token *tok1, const Token *tok2, const std::string &classname, const std::string &varname) { std::list toks; toks.push_back(tok1); toks.push_back(tok2); reportError(toks, Severity::style, "initializerList", "Member variable '" + classname + "::" + varname + "' is in the wrong place in the initializer list.\n" "Member variable '" + classname + "::" + varname + "' is in the wrong place in the initializer list. " "Members are initialized in the order they are declared, not in the " "order they are in the initializer list. Keeping the initializer list " "in the same order that the members were declared prevents order dependent " "initialization errors.", true); }