walkero
/
cppcheck
1
0
Fork 0
Code Issues Pull Requests Projects Releases 1 Wiki Activity

New check: checking for copy ctor and eq operator co-existence

This commit is contained in:
Alexander Alekseev 2017-03-24 12:00:20 +01:00 committed by Daniel Marjamäki
parent a60d588cbe
commit abba762d42
3 changed files with 122 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

48
lib/checkclass.cpp
View File

@ -2333,3 +2333,51 @@ void CheckClass::duplInheritedMembersError(const Token *tok1, const Token* tok2,
std::string(baseIsStruct ? "struct" : "class") + " '" + basename + "'.";
reportError(toks, Severity::warning, "duplInheritedMember", message, CWE398, false);
}
//---------------------------------------------------------------------------
// Check that copy constructor and operator defined together
//---------------------------------------------------------------------------
void CheckClass::checkCopyCtorAndEqOperator()
{
if (!_settings->isEnabled("warning"))
return;
const std::size_t classes = symbolDatabase->classAndStructScopes.size();
for (std::size_t i = 0; i < classes; ++i) {
const Scope * scope = symbolDatabase->classAndStructScopes[i];
if (scope->varlist.empty())
continue;
int hasCopyCtor = 0;
int hasAssignmentOperator = 0;
std::list<Function>::const_iterator func;
for (func = scope->functionList.begin(); func != scope->functionList.end(); ++func) {
if (!hasCopyCtor && func->type == Function::eCopyConstructor) {
hasCopyCtor = func->hasBody() ? 2 : 1;
}
if (!hasAssignmentOperator && func->type == Function::eOperatorEqual) {
const Variable * variable = func->getArgumentVar(0);
if (variable && variable->type() && variable->type()->classScope == scope) {
hasAssignmentOperator = func->hasBody() ? 2 : 1;
}
}
}
if (std::abs(hasCopyCtor - hasAssignmentOperator) == 2)
copyCtorAndEqOperatorError(scope->classDef, scope->className, scope->type == Scope::eStruct, hasCopyCtor);
}
}
void CheckClass::copyCtorAndEqOperatorError(const Token *tok, const std::string &classname, bool isStruct, bool hasCopyCtor)
{
const std::string message = "The " + std::string(isStruct ? "struct" : "class") + " '" + classname +
"' has '" + getFunctionTypeName(hasCopyCtor ? Function::eCopyConstructor : Function::eOperatorEqual) +
"' but lack of '" + getFunctionTypeName(hasCopyCtor ? Function::eOperatorEqual : Function::eCopyConstructor) +
"'.";
reportError(tok, Severity::warning, "copyCtorAndEqOperator", message);
}

9
lib/checkclass.h
View File

@ -77,6 +77,7 @@ public:
checkClass.checkDuplInheritedMembers();
checkClass.checkExplicitConstructors();
checkClass.checkCopyCtorAndEqOperator();
}
@ -136,6 +137,9 @@ public:
/** @brief Check duplicated inherited members */
void checkDuplInheritedMembers();
/** @brief Check that copy constructor and operator defined together */
void checkCopyCtorAndEqOperator();
private:
const SymbolDatabase *symbolDatabase;
@ -167,6 +171,7 @@ private:
void selfInitializationError(const Token* tok, const std::string& varname);
void callsPureVirtualFunctionError(const Function & scopeFunction, const std::list<const Token *> & tokStack, const std::string &purefuncname);
void duplInheritedMembersError(const Token* tok1, const Token* tok2, const std::string &derivedname, const std::string &basename, const std::string &variablename, bool derivedIsStruct, bool baseIsStruct);
void copyCtorAndEqOperatorError(const Token *tok, const std::string &classname, bool isStruct, bool hasCopyCtor);
void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const {
CheckClass c(nullptr, settings, errorLogger);
@ -196,6 +201,7 @@ private:
c.suggestInitializationList(nullptr, "variable");
c.selfInitializationError(nullptr, "var");
c.duplInheritedMembersError(nullptr, 0, "class", "class", "variable", false, false);
c.copyCtorAndEqOperatorError(nullptr, "class", false, false);
}
static std::string myName() {
@ -221,7 +227,8 @@ private:
"- Initialization of a member with itself\n"
"- Suspicious subtraction from 'this'\n"
"- Call of pure virtual function in constructor/destructor\n"
"- Duplicated inherited data members\n";
"- Duplicated inherited data members\n"
"- If 'copy constructor' defined, 'operator=' also should be defined and vice versa\n";
}
// operatorEqRetRefThis helper functions

66
test/testclass.cpp
View File

@ -184,8 +184,74 @@ private:
TEST_CASE(duplInheritedMembers);
TEST_CASE(explicitConstructors);
TEST_CASE(copyCtorAndEqOperator);
}
void checkCopyCtorAndEqOperator(const char code[]) {
// Clear the error log
errout.str("");
Settings settings;
settings.addEnabled("warning");
// Tokenize..
Tokenizer tokenizer(&settings, this);
std::istringstream istr(code);
tokenizer.tokenize(istr, "test.cpp");
tokenizer.simplifyTokenList2();
// Check..
CheckClass checkClass(&tokenizer, &settings, this);
checkClass.checkCopyCtorAndEqOperator();
}
void copyCtorAndEqOperator() {
checkCopyCtorAndEqOperator("class A \n"
"{ \n"
" A(const A& other) { } \n"
" A& operator=(const A& other) { return *this; }\n"
"};");
ASSERT_EQUALS("", errout.str());
checkCopyCtorAndEqOperator("class A \n"
"{ \n"
"};");
ASSERT_EQUALS("", errout.str());
checkCopyCtorAndEqOperator("class A \n"
"{ \n"
" A(const A& other) { } \n"
"};");
ASSERT_EQUALS("", errout.str());
checkCopyCtorAndEqOperator("class A \n"
"{ \n"
" A& operator=(const A& other) { return *this; }\n"
"};");
ASSERT_EQUALS("", errout.str());
checkCopyCtorAndEqOperator("class A \n"
"{ \n"
" A(const A& other) { } \n"
" int x;\n"
"};");
ASSERT_EQUALS("[test.cpp:1]: (warning) The class 'A' has 'copy constructor' but lack of 'operator='.\n", errout.str());
checkCopyCtorAndEqOperator("class A \n"
"{ \n"
" A& operator=(const A& other) { return *this; }\n"
" int x;\n"
"};");
ASSERT_EQUALS("[test.cpp:1]: (warning) The class 'A' has 'operator=' but lack of 'copy constructor'.\n", errout.str());
checkCopyCtorAndEqOperator("class A \n"
"{ \n"
" A& operator=(const int &x) { this->x = x; return *this; }\n"
" int x;\n"
"};");
ASSERT_EQUALS("", errout.str());
}
void checkExplicitConstructors(const char code[]) {
// Clear the error log