Skip to content

Commit 804e055

Browse files
PKEuSPKEuS
committed
New check: initialization by itself in initializer list (danmar#3982)
Refactorizations: - Rearranged code in checkclass.cpp to increase readability - Several fixes for testclass.cpp tests.
1 parent 9eb28cb commit 804e055

File tree

3 files changed

+176
-79
lines changed

3 files changed

+176
-79
lines changed

‎lib/checkclass.cpp‎

Lines changed: 103 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -2039,6 +2039,42 @@ void CheckClass::initializerListError(const Token *tok1, const Token *tok2, cons
20392039
"initialization errors.", true);
20402040
}
20412041

2042+
2043+
//---------------------------------------------------------------------------
2044+
// Check for self initialization in initialization list
2045+
//---------------------------------------------------------------------------
2046+
2047+
void CheckClass::checkSelfInitialization()
2048+
{
2049+
const SymbolDatabase *symbolDatabase = _tokenizer->getSymbolDatabase();
2050+
for (std::size_t i = 0; i < symbolDatabase->functionScopes.size(); ++i) {
2051+
const Scope* scope = symbolDatabase->functionScopes[i];
2052+
const Function* function = scope->function;
2053+
if (!function || !function->isConstructor())
2054+
continue;
2055+
2056+
const Token* tok = function->arg->link()->next();
2057+
if (tok->str() != ":")
2058+
continue;
2059+
2060+
for (; tok != scope->classStart; tok = tok->next()) {
2061+
if (Token::Match(tok, "[:,] %var% ( %var% )") && tok->next()->varId() && tok->next()->varId() == tok->tokAt(3)->varId()) {
2062+
selfInitializationError(tok, tok->strAt(1));
2063+
}
2064+
}
2065+
}
2066+
}
2067+
2068+
void CheckClass::selfInitializationError(const Token* tok, const std::string& name)
2069+
{
2070+
reportError(tok, Severity::error, "selfInitialization", "Member variable '" + name + "' is initialized by itself.\n");
2071+
}
2072+
2073+
2074+
//---------------------------------------------------------------------------
2075+
// Check for pure virtual function calls
2076+
//---------------------------------------------------------------------------
2077+
20422078
void CheckClass::checkPureVirtualFunctionCall()
20432079
{
20442080
const std::size_t functions = symbolDatabase->functionScopes.size();
@@ -2064,61 +2100,12 @@ void CheckClass::checkPureVirtualFunctionCall()
20642100
}
20652101
}
20662102

2067-
void CheckClass::checkDuplInheritedMembers()
2068-
{
2069-
if (!_settings->isEnabled("warning"))
2070-
return;
2071-
2072-
// Iterate over all classes
2073-
for (std::list<Type>::const_iterator classIt = symbolDatabase->typeList.begin();
2074-
classIt != symbolDatabase->typeList.end();
2075-
++classIt) {
2076-
// Iterate over the parent classes
2077-
for (std::vector<Type::BaseInfo>::const_iterator parentClassIt = classIt->derivedFrom.begin();
2078-
parentClassIt != classIt->derivedFrom.end();
2079-
++parentClassIt) {
2080-
// Check if there is info about the 'Base' class
2081-
if (!parentClassIt->type || !parentClassIt->type->classScope)
2082-
continue;
2083-
// Check if they have a member variable in common
2084-
for (std::list<Variable>::const_iterator classVarIt = classIt->classScope->varlist.begin();
2085-
classVarIt != classIt->classScope->varlist.end();
2086-
++classVarIt) {
2087-
for (std::list<Variable>::const_iterator parentClassVarIt = parentClassIt->type->classScope->varlist.begin();
2088-
parentClassVarIt != parentClassIt->type->classScope->varlist.end();
2089-
++parentClassVarIt) {
2090-
if (classVarIt->name() == parentClassVarIt->name() && !parentClassVarIt->isPrivate()) { // Check if the class and its parent have a common variable
2091-
duplInheritedMembersError(classVarIt->nameToken(), parentClassVarIt->nameToken(),
2092-
classIt->name(), parentClassIt->type->name(), classVarIt->name(),
2093-
classIt->classScope->type == Scope::eStruct,
2094-
parentClassIt->type->classScope->type == Scope::eStruct);
2095-
}
2096-
}
2097-
}
2098-
}
2099-
}
2100-
}
2101-
2102-
void CheckClass::duplInheritedMembersError(const Token *tok1, const Token* tok2,
2103-
const std::string &derivedname, const std::string &basename,
2104-
const std::string &variablename, bool derivedIsStruct, bool baseIsStruct)
2105-
{
2106-
std::list<const Token *> toks;
2107-
toks.push_back(tok1);
2108-
toks.push_back(tok2);
2109-
2110-
const std::string message = "The " + std::string(derivedIsStruct ? "struct" : "class") + " '" + derivedname +
2111-
"' defines member variable with name '" + variablename + "' also defined in its parent " +
2112-
std::string(baseIsStruct ? "struct" : "class") + " '" + basename + "'.";
2113-
reportError(toks, Severity::warning, "duplInheritedMember", message);
2114-
}
2115-
21162103
const std::list<const Token *> & CheckClass::callsPureVirtualFunction(const Function & function,
21172104
std::map<const Function *, std::list<const Token *> > & callsPureVirtualFunctionMap)
21182105
{
2119-
std::pair<std::map<const Function *, std::list<const Token *> >::iterator , bool > found=
2120-
callsPureVirtualFunctionMap.insert(std::pair<const Function *, std::list< const Token *> >(&function,std::list<const Token *>()));
2121-
std::list<const Token *> & pureFunctionCalls=found.first->second;
2106+
std::pair<std::map<const Function *, std::list<const Token *> >::iterator, bool > found =
2107+
callsPureVirtualFunctionMap.insert(std::pair<const Function *, std::list< const Token *> >(&function, std::list<const Token *>()));
2108+
std::list<const Token *> & pureFunctionCalls = found.first->second;
21222109
if (found.second) {
21232110
if (function.hasBody) {
21242111
for (const Token *tok = function.arg->link();
@@ -2128,24 +2115,24 @@ const std::list<const Token *> & CheckClass::callsPureVirtualFunction(const Func
21282115
function.type != Function::eCopyConstructor &&
21292116
function.type != Function::eMoveConstructor &&
21302117
function.type != Function::eDestructor) {
2131-
if ((Token::simpleMatch(tok,") {") &&
2118+
if ((Token::simpleMatch(tok, ") {") &&
21322119
tok->link() &&
2133-
Token::Match(tok->link()->previous(),"if|switch")) ||
2134-
Token::simpleMatch(tok,"else {")
2120+
Token::Match(tok->link()->previous(), "if|switch")) ||
2121+
Token::simpleMatch(tok, "else {")
21352122
) {
21362123
// Assume pure virtual function call is prevented by "if|else|switch" condition
21372124
tok = tok->linkAt(1);
21382125
continue;
21392126
}
21402127
}
2141-
const Function * callFunction=tok->function();
2128+
const Function * callFunction = tok->function();
21422129
if (!callFunction ||
21432130
function.nestedIn != callFunction->nestedIn ||
2144-
(tok->previous() && tok->previous()->str()=="."))
2131+
(tok->previous() && tok->previous()->str() == "."))
21452132
continue;
21462133

21472134
if (tok->previous() &&
2148-
tok->previous()->str()=="(") {
2135+
tok->previous()->str() == "(") {
21492136
const Token * prev = tok->previous();
21502137
if (prev->previous() &&
21512138
(_settings->library.ignorefunction(tok->str())
@@ -2158,7 +2145,7 @@ const std::list<const Token *> & CheckClass::callsPureVirtualFunction(const Func
21582145
continue;
21592146
}
21602147

2161-
const std::list<const Token *> & pureFunctionCallsOfTok=callsPureVirtualFunction(*callFunction,
2148+
const std::list<const Token *> & pureFunctionCallsOfTok = callsPureVirtualFunction(*callFunction,
21622149
callsPureVirtualFunctionMap);
21632150
if (!pureFunctionCallsOfTok.empty()) {
21642151
pureFunctionCalls.push_back(tok);
@@ -2179,13 +2166,13 @@ void CheckClass::getFirstPureVirtualFunctionCallStack(
21792166
pureFuncStack.push_back(pureCall.function()->token);
21802167
return;
21812168
}
2182-
std::map<const Function *, std::list<const Token *> >::const_iterator found=callsPureVirtualFunctionMap.find(pureCall.function());
2183-
if (found==callsPureVirtualFunctionMap.end() ||
2169+
std::map<const Function *, std::list<const Token *> >::const_iterator found = callsPureVirtualFunctionMap.find(pureCall.function());
2170+
if (found == callsPureVirtualFunctionMap.end() ||
21842171
found->second.empty()) {
21852172
pureFuncStack.clear();
21862173
return;
21872174
}
2188-
const Token & firstPureCall=**found->second.begin();
2175+
const Token & firstPureCall = **found->second.begin();
21892176
pureFuncStack.push_back(&firstPureCall);
21902177
getFirstPureVirtualFunctionCallStack(callsPureVirtualFunctionMap, firstPureCall, pureFuncStack);
21912178
}
@@ -2195,8 +2182,61 @@ void CheckClass::callsPureVirtualFunctionError(
21952182
const std::list<const Token *> & tokStack,
21962183
const std::string &purefuncname)
21972184
{
2198-
const char * scopeFunctionTypeName=getFunctionTypeName(scopeFunction.type);
2185+
const char * scopeFunctionTypeName = getFunctionTypeName(scopeFunction.type);
21992186
reportError(tokStack, Severity::warning, "pureVirtualCall", "Call of pure virtual function '" + purefuncname + "' in " + scopeFunctionTypeName + ".\n"
22002187
"Call of pure virtual function '" + purefuncname + "' in " + scopeFunctionTypeName + ". The call will fail during runtime.");
22012188
}
22022189

2190+
2191+
//---------------------------------------------------------------------------
2192+
// Check for members hiding inherited members with the same name
2193+
//---------------------------------------------------------------------------
2194+
2195+
void CheckClass::checkDuplInheritedMembers()
2196+
{
2197+
if (!_settings->isEnabled("warning"))
2198+
return;
2199+
2200+
// Iterate over all classes
2201+
for (std::list<Type>::const_iterator classIt = symbolDatabase->typeList.begin();
2202+
classIt != symbolDatabase->typeList.end();
2203+
++classIt) {
2204+
// Iterate over the parent classes
2205+
for (std::vector<Type::BaseInfo>::const_iterator parentClassIt = classIt->derivedFrom.begin();
2206+
parentClassIt != classIt->derivedFrom.end();
2207+
++parentClassIt) {
2208+
// Check if there is info about the 'Base' class
2209+
if (!parentClassIt->type || !parentClassIt->type->classScope)
2210+
continue;
2211+
// Check if they have a member variable in common
2212+
for (std::list<Variable>::const_iterator classVarIt = classIt->classScope->varlist.begin();
2213+
classVarIt != classIt->classScope->varlist.end();
2214+
++classVarIt) {
2215+
for (std::list<Variable>::const_iterator parentClassVarIt = parentClassIt->type->classScope->varlist.begin();
2216+
parentClassVarIt != parentClassIt->type->classScope->varlist.end();
2217+
++parentClassVarIt) {
2218+
if (classVarIt->name() == parentClassVarIt->name() && !parentClassVarIt->isPrivate()) { // Check if the class and its parent have a common variable
2219+
duplInheritedMembersError(classVarIt->nameToken(), parentClassVarIt->nameToken(),
2220+
classIt->name(), parentClassIt->type->name(), classVarIt->name(),
2221+
classIt->classScope->type == Scope::eStruct,
2222+
parentClassIt->type->classScope->type == Scope::eStruct);
2223+
}
2224+
}
2225+
}
2226+
}
2227+
}
2228+
}
2229+
2230+
void CheckClass::duplInheritedMembersError(const Token *tok1, const Token* tok2,
2231+
const std::string &derivedname, const std::string &basename,
2232+
const std::string &variablename, bool derivedIsStruct, bool baseIsStruct)
2233+
{
2234+
std::list<const Token *> toks;
2235+
toks.push_back(tok1);
2236+
toks.push_back(tok2);
2237+
2238+
const std::string message = "The " + std::string(derivedIsStruct ? "struct" : "class") + " '" + derivedname +
2239+
"' defines member variable with name '" + variablename + "' also defined in its parent " +
2240+
std::string(baseIsStruct ? "struct" : "class") + " '" + basename + "'.";
2241+
reportError(toks, Severity::warning, "duplInheritedMember", message);
2242+
}

‎lib/checkclass.h‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@ class CPPCHECKLIB CheckClass : public Check {
6868
checkClass.operatorEqToSelf();
6969
checkClass.initializerListOrder();
7070
checkClass.initializationListUsage();
71+
checkClass.checkSelfInitialization();
7172

7273
checkClass.virtualDestructor();
7374
checkClass.checkConst();
@@ -116,8 +117,12 @@ class CPPCHECKLIB CheckClass : public Check {
116117
/** @brief Check initializer list order */
117118
void initializerListOrder();
118119

120+
/** @brief Suggest using initialization list */
119121
void initializationListUsage();
120122

123+
/** @brief Check for initialization of a member with itself */
124+
void checkSelfInitialization();
125+
121126
void copyconstructors();
122127

123128
/** @brief call of pure virtual funcion */
@@ -150,6 +155,7 @@ class CPPCHECKLIB CheckClass : public Check {
150155
void checkConstError2(const Token *tok1, const Token *tok2, const std::string &classname, const std::string &funcname, bool suggestStatic);
151156
void initializerListError(const Token *tok1,const Token *tok2, const std::string & classname, const std::string &varname);
152157
void suggestInitializationList(const Token *tok, const std::string& varname);
158+
void selfInitializationError(const Token* tok, const std::string& name);
153159
void callsPureVirtualFunctionError(const Function & scopeFunction, const std::list<const Token *> & tokStack, const std::string &purefuncname);
154160
void duplInheritedMembersError(const Token* tok1, const Token* tok2, const std::string &derivedname, const std::string &basename, const std::string &variablename, bool derivedIsStruct, bool baseIsStruct);
155161

@@ -174,6 +180,7 @@ class CPPCHECKLIB CheckClass : public Check {
174180
c.checkConstError(0, "class", "function", true);
175181
c.initializerListError(0, 0, "class", "variable");
176182
c.suggestInitializationList(0, "variable");
183+
c.selfInitializationError(0, "var");
177184
c.duplInheritedMembersError(0, 0, "class", "class", "variable", false, false);
178185
}
179186

@@ -196,6 +203,7 @@ class CPPCHECKLIB CheckClass : public Check {
196203
"* Constness for member functions\n"
197204
"* Order of initializations\n"
198205
"* Suggest usage of initialization list\n"
206+
"* Initialization of a member with itself\n"
199207
"* Suspicious subtraction from 'this'\n"
200208
"* Call of pure virtual function in constructor/destructor\n"
201209
"* Duplicated inherited data members\n";

0 commit comments

Comments
 (0)